Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] Remove Json Wire Protocol support #11823

Merged
merged 14 commits into from
Apr 5, 2023
Merged

Conversation

shs96c
Copy link
Member

@shs96c shs96c commented Mar 27, 2023

This PR removes JWP support from the Java bindings, though there are undoubtedly places where supporting classes (eg. those that convert capability names from the JWP to W3C formats) have been left over.

Note that it is likely this will break older clients, as it looks like DriverCommand was mis-encoding the capabilities parameter as a Map rather than a collection of Maps. Before this lands, we should check for that.

Best reviewed on a commit-by-commit basis rather than in one go, but this PR is designed to be squashed before merging.

@titusfortner
Copy link
Member

Should we deprecate DesiredCapabilities along with this PR and tell people to use MutableCapabilities if they have to set things dynamically?

@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (ca50360) 54.83% compared to head (26e85e6) 54.83%.

❗ Current head 26e85e6 differs from pull request most recent head 7adb5df. Consider uploading reports for the commit 7adb5df to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #11823   +/-   ##
=======================================
  Coverage   54.83%   54.83%           
=======================================
  Files          85       85           
  Lines        5693     5693           
  Branches      230      230           
=======================================
  Hits         3122     3122           
  Misses       2341     2341           
  Partials      230      230           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@diemol
Copy link
Member

diemol commented Apr 4, 2023

Should we deprecate DesiredCapabilities along with this PR and tell people to use MutableCapabilities if they have to set things dynamically?

I think we should do it apart... do we need to write some reasoning behind it or even check with other TLC members?

@diemol diemol added this to the 4.9 milestone Apr 5, 2023
Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

Failing tests are unrelated to this change.

Thank you, @shs96c!

@diemol diemol merged commit f28f1df into SeleniumHQ:trunk Apr 5, 2023
yashcho pushed a commit to yashcho/selenium that referenced this pull request Apr 7, 2023
* [java] Remove support for the original Json Wire Protocol

* cp: remote jwp from new session new session payload

* cp: make the driver command encode capabilities as a set in all cases

* cp: remove `desiredCapabilities` from encoded new session command

* cp: continuing to remove desired capabilities

* cp: start removing usages of `Dialect.OSS`

* cp: making w3c the default protocol

* cp: start ripping out protocol conversion

* cp: removing protocol conversion

* cp: finish removing `Dialect.OSS` from the tree

* [java] Refining proxyType to fix browser tests

* [java] Fixing proxy tests

* [grid] Removing platform before sending payload to driver

---------

Co-authored-by: Diego Molina <[email protected]>
Co-authored-by: Diego Molina <[email protected]>
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.

4 participants