saucebrowser: use an environment variable to specify the appium version

[lpinca]

Appium 1.5 does not work on OS X 10.9:

/Users/luigi/repos/eventemitter3/node_modules/zuul/bin/zuul:328
            throw err.message;
            ^
iphone@7.0: [init({"name":"eventemitter3","tags":[],"browserName":"iphone","version":"7.0","platform":"Mac 10.9","appium-version":"1.5","record-screenshots":false,"record-video":false})] The environment you requested was unavailable.: Appium 1.5.3 does not support Mac 10.9. Please check our platform configurator (https://saucelabs.com/docs/platforms)

This patch fixes the issue by not enforcing a specific Appium version.

[mention-bot]

@lpinca, thanks for your PR! By analyzing the annotation information on this pull request, we identified @vvo, @defunctzombie and @tmcw to be potential reviewers

[feross]

@lpinca This will bring back some bugs that were fixed in earlier releases.

[lpinca]

The enforced Appium version was originally added to fix issues with iOS (https://github.com/defunctzombie/zuul/pull/272, https://github.com/defunctzombie/zuul/issues/270).

Now iOS works fine also when an Appium version is not specified.

If we still want to keep a fixed Appium version we should revert https://github.com/defunctzombie/zuul/pull/281 and go back to 1.4 to fix the issue posted above.

[feross]

I'll defer to the other maintainers on this issue.

[JakeChampion]

I can confirm this works with every browser that SauceLabs has, here is a PR where I used this branch -- https://github.com/rowanmanning/proclaim/pull/32/files

[3rd-Eden]

Could this be looked at? I would be great to have working testing again.

[defunctzombie]

What is the correct appium version to use here? Or is it best to leave it unspecified?

Can we make this PR accept an env var to set the version or leave unspecified if no env var defined? My concern is that we remove this and then someone else has an issue where we need to specify the version. I don't know the full context here so if someone can just tell me what the correct course of action here is that would be :+1:

[JakeChampion]

An env-var makes sense so long as if it is not specified we default to the latest version supplied.

[lpinca]

@defunctzombie according to the docs the best version to use is the latest version (1.5.x). Unfortunately this doesn't work with iOS 7.0.

When a version is not specified the default one is used.

A specific version (1.4) was added to zuul to "fix" an issue with iOS 9.2. This is no longer required as tests against iOS 9.2 now run fine also when no version is specified.

I think it is fine to leave the option unspecified but having an env var to enforce a specific version is also a good idea. What name would you use? Is process.env.APPIUM_VESION ok?

@JakeChampion IMHO it would be better to use the default appium version when the env var is not specified, that is enforce a specific version only when the env var is set.

Edit: iOS 7.0 will reach EOL in ~20 days, see https://wiki.saucelabs.com/display/DOCS/2016/08/08/30-day+EOL+Notice+for+iOS+7.0.

[defunctzombie]

process.env.APPIUM_VESION

Sounds good to me.

Thank you for the detailed explanation.

[defunctzombie]

maybe SAUCE_APPIUM_VERSION if we want to be a bit more pedantic about what this affects.

[lpinca]

Ok, I will update the pr in a couple of hours.

[lpinca]

Updated.

[defunctzombie]

LGTM

[defunctzombie]

released as 3.11.0

[lpinca]

Thank you.