admc / wd

A node.js client for webdriver/selenium 2.
Other
1.53k stars 404 forks source link

Change prefix with 'appium' by default to support w3c #596

Closed khanhdodang closed 4 years ago

khanhdodang commented 4 years ago

Hi @imurchie, This PR changes the default prefix for all unofficial capabilities. If we set the default prefix 'wd', the script will through the error 'deviceName is required' and can't run the test with w3c desired capabilities.

Note: I run the test with Appium 1.13.0.

In addition, I saw the CI failed due to it can't start Sauce Connect. How can I bypass it?

Thanks, Khanh Do

khanhdodang commented 4 years ago

Hi @admc @jlipps Would you have time to take a look on my PR?

admc commented 4 years ago

I pushed your branch, tests are running now: https://travis-ci.org/admc/wd/builds/563345129

admc commented 4 years ago

@imurchie tests are passing, see any reason not to merge this?

imurchie commented 4 years ago

My only problem is that it kind of makes wd default to being an Appium client. I agree that have 'wd' as the default prefix does not make much sense (servers being unlikely to implement any wd-specific functionality).

As a user, I would tend to want to have some sort of notification of this happening (could just be once for all the caps) and how to change it.

khanhdodang commented 4 years ago

Hi @admc and @imurchie any updates on this PR?

admc commented 4 years ago

It seems to make sense to me to have this either configurable in a config file, or set in an environment variable, but changing the default behavior at this point does feel a bit strange, since this isn't an appium specific library. @khanhdodang if you want to do a little extra work and make this easily configurable somehow, I would be happy to take the PR. Sorry for the delays.

khanhdodang commented 4 years ago

Hi @admc I will update the PR and let you know soon.

khanhdodang commented 4 years ago

Hi @admc I found another way to change the prefix when initializing the test session. I add desiredCaps "w3cPrefix":"appium" and can bypass it. So, I will close this PR. Thanks for your time.