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

fix: fix automatic capabilities conversion from JWP to W3C #277

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gkalpak
Copy link

@gkalpak gkalpak commented Oct 19, 2022

A couple of fixes mainly related to the automatic conversion of capabilities from the legacy JWP format to the new W3C.
See the individual commit messages for details.

@gkalpak gkalpak marked this pull request as draft October 24, 2022 15:05
The `processConfig()` function attempts to convert capabilities from the
deprecated JWP format to the new standard W3C one.

Fix the logic for detecting when capabilites are in JWP format.
(Previously, the logic was inverted.)

Also, make sure that the `sauce:options` property is correctly populated
in both cases.
Update the automatic conversion of capabilities from JWP to W3C format
to exclude Internet Explorer 9. Although according to the
[SauceLabs docs][1] all versions of Internet Explorer support the new
W3C capabilities format, in reality only versions >=10 do.

SauceLabs support has confirmed this and said that until the docs are
fixed, _"the [Platform Configurator][2] would be the most reliable
reference for compliant capabilities"_. Indeed, in the Platform
Configurator, IE 9 only offers the option to select `JWP (legacy)` as
the capabilities format.

[1]: https://docs.saucelabs.com/dev/w3c-webdriver-capabilities/index.html#browser-compatibility
[2]: https://saucelabs.com/platform/platform-configurator
Use the value of `appium:platformVersion` (if present) in `browserName`.

Since the [capabilities config when using Appium][1] does not include a
browser version, but only the platform version, including the value of
`appium:platformVersion` in `browserName` is essential in order to be
able to identify which platform the logs refer to.

[1]: https://docs.saucelabs.com/mobile-apps/automated-testing/appium/virtual-devices/#ios-code-examples
@gkalpak gkalpak changed the title fix: correctly convert JWP capabilities to W3C fix: fix automatic capabilities conversion from JWP to W3C Oct 24, 2022
@gkalpak gkalpak marked this pull request as ready for review October 25, 2022 09:00
Copy link

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

I don't doubt that this fixes the issue, but I have some questions about aligning the comments and deletion of capabilities with the isW3C check.

@@ -57,17 +57,19 @@ export function processConfig(config: any = {}, args: any = {}) {
};

// transform JWP capabilities into W3C capabilities for backward compatibility
if (isW3C(args)) {
if (!isW3C(args)) {

Choose a reason for hiding this comment

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

This change doesn't seem to align with the comments above or below.

Why would you "delete JWP capabilities" if you are in JWP mode?

Copy link
Author

@gkalpak gkalpak Oct 30, 2022

Choose a reason for hiding this comment

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

If the capabilities are in the old JWP format, then they are converted to the new W3C format (inside the if block). Essentially, JWP's version and platform properties are converted to W3C's browserVersion and platformName properties. At the end, the JWP properties, version and platform are deleted from the args object (since they are not needed any more).

Does that make sense?

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.

2 participants