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

allow requesting specific platform version #55

Merged
merged 1 commit into from
Dec 26, 2016
Merged

allow requesting specific platform version #55

merged 1 commit into from
Dec 26, 2016

Conversation

marcoow
Copy link
Contributor

@marcoow marcoow commented Dec 23, 2016

This allows running tests on a particular version of the target OS which is especially helpful for mobile devices where there's only one browser version per OS version so being able to request a particular browser version doesn't help much.

Closes #53

@marcoow
Copy link
Contributor Author

marcoow commented Dec 23, 2016

I'm actually not sure how the failing test is related to this as without this change it would just send "platform":"" which should actually be no different from sending it at all (which is the case with the changes in this PR)?

@marcoow
Copy link
Contributor Author

marcoow commented Dec 23, 2016

Argh, sorry for the noise here - just realized Saucelabs takes different options for Selenium and Appium. Actually for Selenium you can specify the OS and version in one string whereas it expects two separate strings for Appium. I'll update this.

@marcoow
Copy link
Contributor Author

marcoow commented Dec 23, 2016

updated - this will now build the "platform" string from the platformName and platformVersion arguments regardless of the platform used on Saucelab's side (which we don't know anyways I think).

@johanneswuerbach
Copy link
Owner

Not sure what advantage this change brings now, align the interface more with Appium?

@marcoow
Copy link
Contributor Author

marcoow commented Dec 23, 2016

This allows running tests on different mobile OS versions which is not possible currently as Appium needs the platformVersion setting.

@johanneswuerbach
Copy link
Owner

johanneswuerbach commented Dec 23, 2016

Sorry, I don't understand. If you say -p Windows -pv XP or -p "Windows XP" you are passing the same platform string Windows XP to sauce. What additional advantage do I gain from using -pv instead of concatenating name and version myself?

Sorry scratch that, I was misreading the code 🤦‍♂️

@johanneswuerbach
Copy link
Owner

Could you rebase? This this is good to go.

@marcoow
Copy link
Contributor Author

marcoow commented Dec 26, 2016

@johanneswuerbach: rebased

@johanneswuerbach johanneswuerbach merged commit 20d62bb into johanneswuerbach:master Dec 26, 2016
@marcoow marcoow deleted the platform-versions branch December 26, 2016 10:45
@marcoow
Copy link
Contributor Author

marcoow commented Dec 26, 2016

🎉

@johanneswuerbach
Copy link
Owner

Published as 2.1.0, sorry for all the back and forth.

@marcoow
Copy link
Contributor Author

marcoow commented Dec 26, 2016

awesome - thanks!

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.

None yet

2 participants