Skip to content

[py] Update WPEWebKit support code - #13278

Merged
titusfortner merged 5 commits into
SeleniumHQ:trunkfrom
lauromoura:wpewebkit-update
Jan 21, 2024
Merged

[py] Update WPEWebKit support code#13278
titusfortner merged 5 commits into
SeleniumHQ:trunkfrom
lauromoura:wpewebkit-update

Conversation

@lauromoura

Copy link
Copy Markdown
Contributor

Description

This PR changes the support code around WPEWebKit helper modules after the last update, a couple of years ago.

Motivation and Context

In the recent Selenium versions, some changes in the way the driver classes are named and the move towards options instead of raw capabilities dictionaries, alongside other minor issues, made the existing WPEWebkit module to fail to create the driver correctly. For example, failing either to find the right service executable or failing the capability verification.

The WPEWebKit is a WebKit port focused on embedded devices. Developers creating WebApps for these devices can take advantage of the remote webdriver (used by these helper classes) to automate testing directly on the device.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
    • There's a change in a wpewebkit.Service parameter, but they're not yet covered by the API docs. Should I add them in a separate PR?
  • I have updated the documentation accordingly.
    • Ditto
  • I have added tests to cover my changes.
    • The changes should be covered by the existing tests, so no new tests added.
  • All new and existing tests passed.
    • I didn't see any failure in the unit tests and most of the //py:test-wpewebkit passes, with some specific failures that will be dealt in a separate PR.

Comment thread py/selenium/webdriver/common/driver_finder.py

@titusfortner titusfortner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I think these classes should be in a separate library from Selenium. You should be able to subclass what is there and have it work properly. (If you can't, we should fix that). If it's in Selenium, it should match what the other drivers are doing.

Comment thread py/selenium/webdriver/wpewebkit/webdriver.py Outdated
Comment thread py/selenium/webdriver/common/driver_finder.py Outdated
Comment thread py/selenium/webdriver/wpewebkit/webdriver.py Outdated
@lauromoura

Copy link
Copy Markdown
Contributor Author

(Updated after missign a tox fix and rebase)

@codecov-commenter

codecov-commenter commented Dec 18, 2023

Copy link
Copy Markdown

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (b556ac3) 58.07% compared to head (d1e955b) 58.18%.
Report is 1 commits behind head on trunk.

❗ Current head d1e955b differs from pull request most recent head f1bd50e. Consider uploading reports for the commit f1bd50e to get more accurate results

Files Patch % Lines
py/selenium/webdriver/wpewebkit/webdriver.py 50.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #13278      +/-   ##
==========================================
+ Coverage   58.07%   58.18%   +0.10%     
==========================================
  Files          88       88              
  Lines        5340     5330      -10     
  Branches      224      224              
==========================================
  Hits         3101     3101              
+ Misses       2015     2005      -10     
  Partials      224      224              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Naming conventions, as WPEWebKit does not follow the Simple
  capitalized name scheme
- (TODO): Driver location finding
- Remove non-standard default capabilities, like in 497cde3
- Add legacy desired_capabilities to new style Options, like in 9f5801c
- To match parent Service class

Amend into Update WPEWebKit service parameter
Receiving a Service instance instead of service details spread out
@lauromoura

Copy link
Copy Markdown
Contributor Author

Pinging @titusfortner for review

@titusfortner titusfortner added this to the 4.17 milestone Jan 16, 2024
@titusfortner
titusfortner merged commit 24d88d7 into SeleniumHQ:trunk Jan 21, 2024
lauromoura added a commit to lauromoura/WebKit that referenced this pull request Mar 29, 2024
https://bugs.webkit.org/show_bug.cgi?id=267836

Reviewed by NOBODY (OOPS!).

Bump selenium tests to upstream 907b2197dada02, which includes the fixes
for WPE from SeleniumHQ/selenium#13278

The changes in our runner code were minimal, basically moving the driver
name to lowercase.

* Tools/Scripts/webkitpy/webdriver_tests/webdriver_driver_wpe.py:
(WebDriverWPE.selenium_name):
* WebDriverTests/imported/selenium/common/: Updated to new revision
* WebDriverTests/imported/selenium/importer.json: Update hash to be
  imported
* WebDriverTests/imported/selenium/py: Updated to new revision
webkit-commit-queue pushed a commit to lauromoura/WebKit that referenced this pull request Apr 1, 2024
https://bugs.webkit.org/show_bug.cgi?id=267836

Reviewed by Carlos Garcia Campos.

Bump selenium tests to upstream 907b2197dada02, which includes the fixes
for WPE from SeleniumHQ/selenium#13278

The changes in our runner code were minimal, basically moving the driver
name to lowercase.

* Tools/Scripts/webkitpy/webdriver_tests/webdriver_driver_wpe.py:
(WebDriverWPE.selenium_name):
* WebDriverTests/imported/selenium/common/: Updated to new revision
* WebDriverTests/imported/selenium/importer.json: Update hash to be
  imported
* WebDriverTests/imported/selenium/py: Updated to new revision

Canonical link: https://commits.webkit.org/276878@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants