Page MenuHomePhabricator

Run browser tests as part of "npm test" for wikidata/query/gui in CI
Closed, ResolvedPublic5 Estimated Story Points

Description

We want to have the browser tests running in CI so that we don't miss tests breaking.
T267393 (also being tackled right now) makes sure that these tests pass locally.

Note: Lots of the discussion in the comments are discussing issues that are long gone, only comments in 2021+ are relevant now
Also: The current attached Gerrit changes may be addressing issues that are no longer relevant

Link to the repository: https://gerrit.wikimedia.org/r/admin/repos/wikidata/query/gui
Link to the repository of the jenkins config: https://gerrit.wikimedia.org/g/integration/config/

Acceptance Criteria

  • Browser tests run in CI for the wikidata/query/gui repository

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@hashar @zeljkofilipin this seems to beyond our (WMDE) realm. We're happy to provide more context/information, but we'd appreciate advise/help with getting this actually resolved.

zeljkofilipin moved this task from Backlog 🔙 to Next 🔜 on the User-zeljkofilipin board.
  • I will be at Wikimedia-Hackathon-2019 next week, if anybody wants to pair on this.
  • The convention is to have Selenium tests (including wdio.conf.js) in tests/selenium, if possible. See Selenium/Node.js#write-tests for examples.
  • I don't think CI supports running Selenium tests in Firefox (wdio.conf.js#33). All other repositories use Chrome.
  • There is no need for selenium-standalone (Gruntfile.js#273). Use Chromedriver instead. It should be available in CI.

@zeljkofilipin I will be at the Hackathon and would gladly pair on this. I arrive on Fri 17.05.

Change 511308 had a related patch set uploaded (by Noa wmde; owner: Noa wmde):
[integration/config@master] Change Dockerfile and wikidata.yml to support running browser tests for the query service gui

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

Change 511309 had a related patch set uploaded (by Noa wmde; owner: Noa wmde):
[wikidata/query/gui@master] Rename separate grunt task from 'browser_test' to 'test_all'

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

Here is an explanation of the issue @Smalyshev encountered on T219749 as I posted it on https://gerrit.wikimedia.org/r/#/c/integration/config/+/511308/

The failure comes from the CI tests for wikidata/query/rdf which are run by the Maven test suite. The job doing that is wikidata-query-rdf-maven-java8-docker which really just ship java8 and then runs maven. There is no chromedriver being spawned.

The wikidata/query/rdf test suite then run the GUI test suite using npm, but since the gui code uses wdio/chromedriver and the maven image does not have it ... The suite fails to connect to chromedriver ( ECONNREFUSED 127.0.0.1:4444 ).

I am not sure whether the GUI test should be run as part of testing wikidata/query/rdf . Probably only a subset of them should?


For wikidata/query/gui, the CI job runs npm test which in turns seems to invoke grunt test. So I guess it is all about adding wdio task to the list of tasks executed when running grunt test. Specially, revert https://gerrit.wikimedia.org/r/#/c/wikidata/query/gui/+/506666/

BUT, in wikidata/query/rdf, as soon as the GUI submodule is updated, the test will fail since they are run with maven and the container does not have chromedriver nor is it intended to run wdio test. My big question is: do you actually need to run the GUI npm test when testing wikidata/query/rdf ? That seems to just rerun tests that already ran perfectly fine in the upstream repo wikidata/query/gui.

This might also be useful:
T199116: Quibble should run `npm install` and `npm run selenium-test` for each extension/skin that has Selenium tests
I have a patch for review which will run browser tests for each extension separately, which makes it possible to run custom setup scripts and install local npm dependencies. This seems like a simpler solution than spinning up the headless browser in another phase of testing. Or maybe I'm misunderstanding why you want the workaround here--would you mind explaining why it's desirable to run browser tests along with npm test?

Change 511309 abandoned by Hashar:
Rename separate grunt task from 'browser_test' to 'test_all'

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

Change 511309 restored by Hashar:
Rename separate grunt task from 'browser_test' to 'test_all'

Reason:
Oops, I wanted to abandon another change sorry.

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

Addshore added a subscriber: Addshore.

The failure comes from the CI tests for wikidata/query/rdf which are run by the Maven test suite. The job doing that is wikidata-query-rdf-maven-java8-docker which really just ship java8 and then runs maven. There is no chromedriver being spawned

Soon the UI will not be bundled as part of the wdqs backend java stuff.
does that make all of this easier / eliminate some problems?
See T241291

If not it'd be good to get a summary / overview of what the state of everything is here!

Soon the UI will not be bundled as part of the wdqs backend java stuff.

This is now true.
The gui is no longer bundled with the java stuff.

Addshore renamed this task from Run browser tests as part of "npm test" of wikidata/query/gui to Run browser tests as part of "npm test" of wikidata/query/gui in CI.Jan 26 2021, 1:18 PM
Addshore renamed this task from Run browser tests as part of "npm test" of wikidata/query/gui in CI to Run browser tests as part of "npm test" for wikidata/query/gui in CI.Jan 26 2021, 1:44 PM
Addshore updated the task description. (Show Details)
Addshore updated the task description. (Show Details)
Addshore removed a subscriber: alaa_wmde.
Addshore updated the task description. (Show Details)

Task inspection note: we probably want to start with adding the browser_test to the test script in package.jsonand see what happens

Change 665124 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[wikidata/query/gui@master] Add wdio to grunt test task

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

Task inspection note: we probably want to start with adding the browser_test to the test script in package.jsonand see what happens

Tried it, doesn’t work (no Java installed in the container). We’ll probably want to switch the CI job from node10-test to node10-test-browser (or add a separate CI job using that container which only runs the browser tests?), and then either remove selenium-standalone completely or disable it in CI, in order to use the chromedriver provided by the container, as recommended in T222200#5172117.

Wait, nevermind, the job already runs on node10-test-browser. Not sure why wdio fails with an ECONNREFUSED error against a random port, then…

I’m at a loss. I tried removing selenium-standalone completely, but wdio still fails with the same error, “connection refused” on a seemingly random port number. Even with --logLevel=trace, wdio prints just a handful of messages, not enough for me to figure out why it’s trying to connect to that port. (The port doesn’t seem to match any environment variable.) I tried bringing our wdio.conf.js closer to other repositories, but to no avail. I don’t see any other significant differences between how we launch wdio and how other repositories do it.

Unassigning myself since I’ll be working on Wikibase Service Migration starting next week. I hope someone else can figure out what’s going on here… you can see all the things I tried on the Gerrit change linked above.

Change 666094 had a related patch set uploaded (by Tonina Zhelyazkova; owner: Tonina Zhelyazkova):
[wikidata/query/gui@master] Make browser tests run in CI

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

Quibble runs some services in the background, which you would also need in order to run browser tests. Specifically I think you're missing:

  • xvfb in order to run headless, like xvfb :94 -screen 0 1280x1024x24 -nolisten tcp -nolisten unix
  • chromedriver, either the npm driver or standalone: chromedriver --port=4444 --url-base=/wd/hub

It might also be possible to use Quibble as the test runner, like quibble --run selenium, for example in this template, but I'm not sure whether anyone has tried this outside of mediawiki-extensions-* yet?

Task inspection note: we probably want to start with adding the browser_test to the test script in package.json and see what happens

This sounds slightly nasty to me, don't you sometimes want to run just the simple tests locally? It's fine to have a separate script target and your CI configuration can call both npm run test and npm run selenium-test. But if you decide you really do want to merge the test suites, that should work too.

Change 666138 had a related patch set uploaded (by Tonina Zhelyazkova; owner: Tonina Zhelyazkova):
[wikidata/query/gui@master] Make browser tests run in CI

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

We could also look at just running chromedriver in a docker container as part of the CI job that is defined for query-gui.
So the job would be 1) run chromedrive 2) run tests 3) stop chromedriver?
Would need to see how possible @hashar thinks this would be but I don't see reasons that it shouldnt work?

Change 666368 had a related patch set uploaded (by Awight; owner: Awight):
[wikidata/query/gui@master] [WIP] experiment with browser testing

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

I mashed together some bits laying around and got the above patch to pass. The only big issue is the grunt-merge-i18n package coming from github, which breaks due to missing ssh. Otherwise, I think this is usable.

Moving to Review, so people can take a look at Adam W's patch. Thanks @awight

Notes:

  • The wdio-chromedriver-service seems better from a distance, but the antagonist seems to be in the details. See this commit for some notes about why the approach was abandoned in mw-core.
  • The container already includes a /run-with-xvfb.sh, ideally we can drop ./selenium.sh and set the container entrypoint to start the services, then run npm run browser_test directly.
  • I guess the simplest fix for the grunt-merge-i18n dependency is to upload it to npm?

Looks like @awight's latest patch makes the wdio tests run in CI.

I find some of my comments from 2019 still stand (T222200#5172117). I'll quote relevant parts:

  • The convention is to have Selenium tests (including wdio.conf.js) in tests/selenium, if possible. See Selenium/Node.js#write-tests for examples.
  • There is no need for selenium-standalone (Gruntfile.js#273). Use Chromedriver instead. It should be available in CI.
  • Is there a reason this repository doesn't have Selenium tests in tests/selenium?
  • Is there a reason this repository uses selenium-standalone instead of chromedriver?
  • Is there a reason this repository uses selenium-standalone instead of chromedriver?

I think we can remove that dependency if we merge my patch using standalone chromedriver from the container.

The latest patchset drops selenium-standalone.

Change 666138 abandoned by Tonina Zhelyazkova:
[wikidata/query/gui@master] Make browser tests run in CI

Reason:

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

Change 667623 had a related patch set uploaded (by Addshore; owner: Addshore):
[integration/config@master] Add {name}-node10-browser-webdriver-docker

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

Change 511308 abandoned by Addshore:
[integration/config@master] Change Dockerfile and wikidata.yml to support running browser tests for the query service gui

Reason:

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

Change 668040 had a related patch set uploaded (by Addshore; owner: Addshore):
[integration/config@master] Add {name}-node10-browser-webdriver-docker

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

Change 668040 merged by jenkins-bot:
[integration/config@master] Add {name}-node10-browser-webdriver-docker

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

Change 667623 merged by jenkins-bot:
[integration/config@master] Use {name}-node10-browser-webdriver-docker for wikidata/query/gui

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

Change 666368 had a related patch set uploaded (by Addshore; owner: Awight):
[wikidata/query/gui@master] [WIP] experiment with browser testing

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

Change 666368 merged by jenkins-bot:
[wikidata/query/gui@master] Run browser tests in npm test, but don't run webdriver

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

Change 665124 abandoned by Addshore:
[wikidata/query/gui@master] WIP: Run wdio without selenium-standalone

Reason:
Task is now resolved

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

Change 511309 abandoned by Addshore:
[wikidata/query/gui@master] Rename separate grunt task from 'browser_test' to 'test_all'

Reason:
Task is now resolved

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