Page MenuHomePhabricator

ERROR webdriver: Request failed due to Error: session not created: Chrome version must be between 70 and 73
Open, LowPublic

Description

In 539441, we've added chromedriver to package.json. It's v73, I'm assuming because that's the version we need in CI (because of Chromium version there probably not being up to date).

Unfortunately, now chromedriver doesn't work on a machine with a recent Chrome (v77), and my assumption is that would be the vast majority of developers.

Steps to reproduce on a macos with chromedriver previously installed via homebrew:

brew cask uninstall chromedriver
cd mediawiki
rm -rf node_modules
npm install
npm run selenium
...
2019-10-04T10:57:19.124Z ERROR @wdio/runner: Error: session not created: Chrome version must be between 70 and 73
  (Driver info: chromedriver=73.0.3683.20 (8e2b610813e167eee3619ac4ce6e42e3ec622017),platform=Mac OS X 10.13.6 x86_64)
...

Details

Related Gerrit Patches:
mediawiki/core : masterselenium: Replace wdio-chromedriver with ./selenium.sh script
mediawiki/core : masterUpgrade chromedriver to v77
mediawiki/skins/MinervaNeue : masterUpgraded chromedriver to v77

Event Timeline

zeljkofilipin triaged this task as High priority.Oct 4 2019, 10:58 AM
zeljkofilipin created this task.
zeljkofilipin edited projects, added User-zeljkofilipin; removed User-Zoranzoki21.
zeljkofilipin moved this task from Inbox to Selenium on the MediaWiki-Core-Testing board.
zeljkofilipin moved this task from INBOX to Doing on the Release-Engineering-Team-TODO (201910) board.
zeljkofilipin moved this task from Backlog 🔙 to In Progress 🔨 on the User-zeljkofilipin board.
zeljkofilipin updated the task description. (Show Details)
zeljkofilipin updated the task description. (Show Details)

Change 540842 had a related patch set uploaded (by Zfilipin; owner: Zfilipin):
[mediawiki/core@master] WIP Upgrade chromedriver

https://gerrit.wikimedia.org/r/540842

Yeah, running selenium via the CI images is probably the best choice for long-term developer sanity.

Change 543692 had a related patch set uploaded (by Edward Tadros; owner: Edward Tadros):
[mediawiki/skins/MinervaNeue@master] Upgraded chromedriver to v77

https://gerrit.wikimedia.org/r/543692

Change 543692 abandoned by Nray:
Upgraded chromedriver to v77

https://gerrit.wikimedia.org/r/543692

Change 540842 abandoned by Zfilipin:
Upgrade chromedriver to v77

https://gerrit.wikimedia.org/r/540842

zeljkofilipin closed this task as Invalid.Oct 24 2019, 12:33 PM
zeljkofilipin updated the task description. (Show Details)

I have updated the documentation (Selenium/Node.js/Target MediaWiki-Vagrant):

The workaround is to install the latest version of chromedriver locally:

brew cask install chromedriver

Then, in two terminal windows or tabs:

$ chromedriver --url-base=wd/hub --port=4444
$ npm run selenium-test
hashar reopened this task as Open.Oct 24 2019, 12:42 PM
hashar added a subscriber: hashar.

In 539441, we've added chromedriver to package.json. It's v73, I'm assuming because that's the version we need in CI (because of Chromium version there probably not being up to date).

That sounds prone to lot of failures. It means that package.json pins chromedriver to some arbitrary version (in this case v73) which would not match the Chromium / Chrome version available in the environment. So package.json should either:

  • also pin the Chrome/Chromium version like puppeteer does

Or

  • drop the chromedriver entirely from package.json and rely on the environment having compatibles chromium/chromedriver version (as Debian does)
zeljkofilipin removed zeljkofilipin as the assignee of this task.Oct 24 2019, 1:21 PM
zeljkofilipin removed a project: User-zeljkofilipin.
zeljkofilipin moved this task from Doing to INBOX on the Release-Engineering-Team-TODO (201910) board.

I added to package.json for local use. For CI, we set --skip-chromedriver which means this isn't involved there at all.

The reason I added it is so that users don't have to manually start chrome driver which should make it significantly more user-friendly. And indeed made the instructions a lot simpler as well.

So the package isn't about installing chromedriver (the binary) but about wdio being able to start it automatically. For that to work reliably it also has to control the version as the way to start it could change subtly over time.

It is however entirely optional. Nothing has changed compared to before in that CI still installs and starts its own. For local use this can be done as well, by setting --skip-driver or by using npm run selenium-test instead of npm run selenium which is exactly that (wdio + skip-chromedriver), and is what Quibble uses.

If we remove chromedriver from package.json this means we need our shell script again to start chromedriver. I guess that would be fine. Although we would want to fix it so that it supports arguments as well so that we don't have to bring back the old instructions of "if you want to use --filter you have to start chromedriver manually" which is imho too inaccessible.

Adding chromium itself to package.json isn't really an option. That would require a lot of work on our end to maintain over time which seems undesirable (there is also no official npm package providing that). It also means that users can't test against their own graphic version of Chrome/Chromium out of the box unless they also install their own chromedriver. Which is fine, but at that point, might as well go all-in and embrace that upfront. I don't think we need to worry about installing Chromium locally as developers can be expected to have this already (in addition to Firefox). So installing it only makes the installer slower and adds more maintenance cost without a clear benefit.

@hashar I still think this task should be resolved. I've opened it since I've misunderstood what adding chromedriver to package.json means. I've updated the documentation to work with the current setup. There's nothing to do here. If you think changes to package.json should be made, that's probably a separate task.

zeljkofilipin lowered the priority of this task from High to Low.Oct 30 2019, 2:06 PM

For now at least decreasing priority.

There are two issues:

  • CI installs the chromedriver and I have noticed the binary is downloaded. Maybe because the installer does not manage to find the chromedriver shipped in our containers.
  • Locally anyone having a more recent chromium and running npm run selenium would end facing the issue described on that task. Though potentially that one can be addressed by removing the version pinning. Assuming locally people have the latest Chromium

There are two issues:

  • CI installs the chromedriver and I have noticed the binary is downloaded. Maybe because the installer does not manage to find the chromedriver shipped in our containers.

It is downloaded because npm always downloads all possible dependencies. There is no thinking or checking there. I declared it as a dependency for convenience during local development so that users can use it against their local Chrome install without needing to also learn about chromedriver.

It isn't used in CI because we explicitly tell CI not to use it:

package.json
"selenium-test": "wdio ./tests/selenium/wdio.conf.js --skip-chromedriver"

The reason we do that is because Quibble already starts the Chromedriver server. I forgot why, but maybe to save time when we run wdio over multiple repositories?

Anyway, because it cannot be started twice (conflicting port), we have two choices with the WDIO plugin: A) Install your own chromedriver and start it yourself, or B) use the bundled chromedriver and it will be started automatically for you. There is not a middle ground where it can start your own install.

I think it is best for usability that when developers run tests locally that they only have to run npm run selenium and that it works, including for --filter and --mochaOpts.grep options. Never having to start or stop some background process.

The easiest way to do that seemed to be to use the chromedriver package.

The downside of course is that this means we download an unused package in CI (maybe not an issue?), and that for development locally the version of Chrome has to match the version of Chromedriver specified in package.json. This might be an issue depending on what we expect that developer environment to be. If that environment is local Fresh-node or local Quibble, then that is fine because they have the same versions pinned and we maintain that. If that environment is unprotected npm execution on local machine with auto-upgrading Chrome, then maybe we have bigger problems :)

Anyway, I think there is definitely other environments as well that are secure and also not easy to pin a Chrome version. So we should definitely fix this. I think we can maybe fix this by bring back the selenium.sh script that automatically starts chromedriver. For users of Quibble and Fresh-node that will still be easy because both of those install chromedriver already. People with custom environments will need to install Chromedriver but that might be okay? (Same as before I did this).

The main tricky bit is supporting arguments such as --filter to run specific tests. That did not work with the selenium.sh script, which is why the README had long documentation about starting chromedriver servers etc which I think made it too difficult to use. But, I have an abandoned patch that fixes this bug (Patch Set 5 of https://gerrit.wikimedia.org/r/538307).

Krinkle claimed this task.Dec 17 2019, 5:47 PM
Krinkle added a project: Performance-Team.

Change 558617 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] selenium: Replace wdio-chromedriver with ./selenium.sh script

https://gerrit.wikimedia.org/r/558617

Krinkle removed Krinkle as the assignee of this task.Mon, Jan 6, 10:28 PM
Krinkle moved this task from Inbox to Blocked or Needs-CR on the Performance-Team board.

Awaiting code review, @zeljkofilipin or @hashar.

thcipriani added a subscriber: thcipriani.

Awaiting code review, @zeljkofilipin or @hashar.

reassigning based on this comment

Change 558617 merged by jenkins-bot:
[mediawiki/core@master] selenium: Replace wdio-chromedriver with ./selenium.sh script

https://gerrit.wikimedia.org/r/558617