Page MenuHomePhabricator

Add selenium tests for GlobalWatchlist
Closed, ResolvedPublic10 Estimated Story Points

Description

Would have helped to catch the regression in T284198: Restore/fix vue version of Special:GlobalWatchlist for wvui 0.2.0 earlier - we should test that the non-vue version of the display produces a good output (can check for existence of, eg, the refresh ooui button) and that the vue version of the display produces a good output using wvui (check for a button with the wvui-button class)

Roadmap:

  • Figure out how selenium works and what it takes to set up
  • Add a placeholder test to make sure the tests are running, eg checking Special:Version for the extension having been loaded properly (cf initial patch at T224903)
  • Add tests for OOUI version of the display
  • Add tests for Vue version of display (temporarily default $wgGlobalWatchlistDevMode to true to allow overriding which version to use)
  • Figure out a better way to allow testing the Vue version without needing to enable dev mode by default, and restore the default to false

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 set the point value for this task to 10.
DannyS712 added a subscriber: zeljkofilipin.

@zeljkofilipin I've seen that you're active with the selenium tests in multiple extensions - would you be willing to help review?

@zeljkofilipin I've seen that you're active with the selenium tests in multiple extensions - would you be willing to help review?

Sure! I can write the first test too, to get you started. What would be the simplest and the most useful thing to test? I'm not familiar with GlobalWatchlist, so you'll probably have to explain very carefully what to do. 😁 Feel free to add screenshots or video, if that's easier to you.

@zeljkofilipin I've seen that you're active with the selenium tests in multiple extensions - would you be willing to help review?

Sure! I can write the first test too, to get you started. What would be the simplest and the most useful thing to test? I'm not familiar with GlobalWatchlist, so you'll probably have to explain very carefully what to do. 😁 Feel free to add screenshots or video, if that's easier to you.

I'm happy to write the tests, always interested in learning a new aspect of the code and Selenium tests look interesting, but thanks for the offer - I should get a chance to start in the next few days

Change 698854 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Set up selenium for testing, add a first basic test

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

It took me a while to figure out generating the package-lock file - note for the future: don't try to generate it from within vagrant
I was unable to run the tests locally using vagrant, getting the following error:

vagrant@vagrant:/vagrant/mediawiki/extensions/GlobalWatchlist$ npm run selenium-test

> @ selenium-test /vagrant/mediawiki/extensions/GlobalWatchlist
> wdio tests/selenium/wdio.conf.js

/vagrant/mediawiki/extensions/GlobalWatchlist/node_modules/@wdio/cli/build/index.js:35
const run = async () => {
                  ^

SyntaxError: Unexpected token (
    at createScript (vm.js:56:10)
    at Object.runInThisContext (vm.js:97:10)
    at Module._compile (module.js:549:28)
    at Object.Module._extensions..js (module.js:586:10)
    at Module.load (module.js:494:32)
    at tryModuleLoad (module.js:453:12)
    at Function.Module._load (module.js:445:3)
    at Module.require (module.js:504:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/vagrant/mediawiki/extensions/GlobalWatchlist/node_modules/@wdio/cli/bin/wdio.js:11:1)

npm ERR! Linux 4.9.0-12-amd64
npm ERR! argv "/usr/bin/node" "/usr/bin/npm" "run" "selenium-test"
npm ERR! node v6.17.1
npm ERR! npm  v2.15.12
npm ERR! code ELIFECYCLE
npm ERR! @ selenium-test: `wdio tests/selenium/wdio.conf.js`
npm ERR! Exit status 1

But, they work in Jenkins, so I guess its fine? (Though the quibble-vendor-mysql-php72-selenium-docker test does include some odd outputs:

Selenium extensions/GlobalWatchlist
01:45:50 INFO:quibble.commands:Running webdriver test in /workspace/src/extensions/GlobalWatchlist
01:45:50 WARNING:backend.ChromeWebDriver:[1623228346.319][SEVERE]: bind() failed: Cannot assign requested address (99)
01:45:56 
01:45:56 > core-js-pure@3.14.0 postinstall /workspace/src/extensions/GlobalWatchlist/node_modules/core-js-pure
01:45:56 > node -e "try{require('./postinstall')}catch(e){}"
01:45:56 
01:45:57 
01:45:57 > core-js@3.10.1 postinstall /workspace/src/extensions/GlobalWatchlist/node_modules/core-js
01:45:57 > node -e "try{require('./postinstall')}catch(e){}"
01:45:57 
01:45:57 
01:45:57 > fibers@5.0.0 install /workspace/src/extensions/GlobalWatchlist/node_modules/fibers
01:45:57 > node build.js || nodejs build.js
01:45:57 
01:45:59 make: Entering directory '/workspace/src/extensions/GlobalWatchlist/node_modules/fibers/build'
01:45:59   CXX(target) Release/obj.target/fibers/src/fibers.o
01:45:59 ../src/fibers.cc: In function ‘void uni::SetAccessor(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::String>, uni::FunctionType (*)(v8::Local<v8::String>, const GetterCallbackInfo&), void (*)(v8::Local<v8::String>, v8::Local<v8::Value>, const SetterCallbackInfo&))’:
01:45:59 ../src/fibers.cc:355:87: warning: cast between incompatible function types from ‘uni::FunctionType (*)(v8::Local<v8::String>, const GetterCallbackInfo&)’ {aka ‘void (*)(v8::Local<v8::String>, const v8::PropertyCallbackInfo<v8::Value>&)’} to ‘v8::AccessorNameGetterCallback’ {aka ‘void (*)(v8::Local<v8::Name>, const v8::PropertyCallbackInfo<v8::Value>&)’} [-Wcast-function-type]
01:45:59    object->SetAccessor(isolate->GetCurrentContext(), name, (AccessorNameGetterCallback)getter, (AccessorNameSetterCallback)setter).ToChecked();
01:45:59                                                                                        ^~~~~~
01:45:59 ../src/fibers.cc:355:123: warning: cast between incompatible function types from ‘void (*)(v8::Local<v8::String>, v8::Local<v8::Value>, const SetterCallbackInfo&)’ {aka ‘void (*)(v8::Local<v8::String>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<void>&)’} to ‘v8::AccessorNameSetterCallback’ {aka ‘void (*)(v8::Local<v8::Name>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<void>&)’} [-Wcast-function-type]
01:45:59    object->SetAccessor(isolate->GetCurrentContext(), name, (AccessorNameGetterCallback)getter, (AccessorNameSetterCallback)setter).ToChecked();
01:45:59                                                                                                                            ^~~~~~
01:45:59 In file included from ../src/coroutine.h:1,
01:45:59                  from ../src/fibers.cc:1:
01:45:59 ../src/fibers.cc: At global scope:
01:45:59 /cache/node-gyp/10.24.0/include/node/node.h:573:43: warning: cast between incompatible function types from ‘void (*)(v8::Local<v8::Object>)’ to ‘node::addon_register_func’ {aka ‘void (*)(v8::Local<v8::Object>, v8::Local<v8::Value>, void*)’} [-Wcast-function-type]
01:45:59        (node::addon_register_func) (regfunc),                          \
01:45:59                                            ^
01:45:59 /cache/node-gyp/10.24.0/include/node/node.h:607:3: note: in expansion of macro ‘NODE_MODULE_X’
01:45:59    NODE_MODULE_X(modname, regfunc, NULL, 0)  // NOLINT (readability/null_usage)
01:45:59    ^~~~~~~~~~~~~
01:45:59 ../src/fibers.cc:930:1: note: in expansion of macro ‘NODE_MODULE’
01:45:59  NODE_MODULE(fibers, init)
01:45:59  ^~~~~~~~~~~
01:45:59   CXX(target) Release/obj.target/fibers/src/coroutine.o
01:46:00   CC(target) Release/obj.target/fibers/src/libcoro/coro.o
01:46:00   SOLINK_MODULE(target) Release/obj.target/fibers.node
01:46:00   COPY Release/fibers.node
01:46:00 make: Leaving directory '/workspace/src/extensions/GlobalWatchlist/node_modules/fibers/build'
01:46:00 Installed in `/workspace/src/extensions/GlobalWatchlist/node_modules/fibers/bin/linux-x64-64-glibc/fibers.node`
01:46:00 added 975 packages in 13.548s
01:46:01 
01:46:01 > @ selenium-test /workspace/src/extensions/GlobalWatchlist
01:46:01 > wdio tests/selenium/wdio.conf.js
01:46:01 
01:46:02 
01:46:02 Execution of 1 spec files started at 2021-06-09T08:46:02.916Z
01:46:02 
01:46:04 [0-0] RUNNING in chrome - /tests/selenium/specs/version.js
01:46:07 INFO:backend.PhpWebserver:[Wed Jun  9 08:46:07 2021] 127.0.0.1:57806 [200]: //index.php?title=Special%3AVersion
01:46:07 INFO:backend.PhpWebserver:[Wed Jun  9 08:46:07 2021] 127.0.0.1:57808 [200]: /load.php?lang=en&modules=mediawiki.special.version%7Cskins.vector.styles.legacy&only=styles&skin=vector
01:46:08 INFO:backend.PhpWebserver:[Wed Jun  9 08:46:08 2021] 127.0.0.1:57810 [200]: /load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector
01:46:08 INFO:backend.PhpWebserver:[Wed Jun  9 08:46:08 2021] 127.0.0.1:57820 [200]: /load.php?lang=en&modules=jquery%2Csite%7Cjquery.client%7Cmediawiki.String%2CTitle%2Capi%2Cbase%2Ccldr%2CjqueryMsg%2Clanguage%2Cutil%7Cmediawiki.libs.pluralruleparser%7Cmediawiki.page.ready%7Cskins.vector.legacy.js%7Cuser.defaults&skin=vector&version=1azh6
01:46:08 INFO:backend.PhpWebserver:[Wed Jun  9 08:46:08 2021] 127.0.0.1:57822 [404]: /favicon.ico - No such file or directory
01:46:09 [0-0] PASSED in chrome - /tests/selenium/specs/version.js
01:46:09 
01:46:09  "dot" Reporter:
01:46:09 .
01:46:09 
01:46:09 Spec Files:	 1 passed, 1 total (100% completed) in 00:00:06

@zeljkofilipin do those Jenkins warnings matter? The overall job still passes

(documenting here for when I get confused later) the $ and $$ functions do not appear to be jquery, but rather document.querySelector and document.querySelectorAll (respectively) and return HTMLElement or a NodeList (respectively, if there are no matching elements querySelector returns null and querySelectorAll returns an empty NodeList)

Change 698861 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Add selenium tests to Special:GlobalWatchlist

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

It took me a while to figure out generating the package-lock file - note for the future: don't try to generate it from within vagrant
I was unable to run the tests locally using vagrant, getting the following error:

npm ERR! node v6.17.1
npm ERR! npm  v2.15.12

Vagrant is using a very old version of node (v6). CI is currently using v10 and v12. I would recommend using Fresh to generate package-lock.json file.

But, they work in Jenkins, so I guess its fine? (Though the quibble-vendor-mysql-php72-selenium-docker test does include some odd outputs:
@zeljkofilipin do those Jenkins warnings matter? The overall job still passes

Those seem to be warnings generated during npm package installation. If tests run fine you can ignore them. But, feel free to report a bug. Maybe it's easy to fix, or a known problem.

(documenting here for when I get confused later) the $ and $$ functions do not appear to be jquery, but rather document.querySelector and document.querySelectorAll (respectively) and return HTMLElement or a NodeList (respectively, if there are no matching elements querySelector returns null and querySelectorAll returns an empty NodeList)

$ and $$ are documented upstream, and correct, it's not jquery.

@zeljkofilipin are you by any chance on IRC? I have a couple questions

@zeljkofilipin are you by any chance on IRC? I have a couple questions

Yes! zeljkof on libera.chat.

Change 698854 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Set up selenium for testing, add a first basic test

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

(documenting here for when I get confused later) the $ and $$ functions do not appear to be jquery, but rather document.querySelector and document.querySelectorAll (respectively) and return HTMLElement or a NodeList (respectively, if there are no matching elements querySelector returns null and querySelectorAll returns an empty NodeList)

$ and $$ are documented upstream, and correct, it's not jquery.

Hmm, okay, but its not document.querySelector(All) either, they return something else (can't find the spec for that object)

Hmm, okay, but its not document.querySelector(All) either, they return something else (can't find the spec for that object)

The best place to ask would be upstream chat: https://gitter.im/webdriverio/webdriverio

Change 699007 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Special:GlobalWatchlist selenium tests - move setup to `before`

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

Hmm, okay, but its not document.querySelector(All) either, they return something else (can't find the spec for that object)

The best place to ask would be upstream chat: https://gitter.im/webdriverio/webdriverio

Thanks - I was just saying that for my own documentation purposes, didn't mean it as a general question - looking at the uses in other tests is enough to figure it out

Change 699009 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Selenium tests for vue version of Special:GlobalWatchlist

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

Change 698861 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Selenium tests for normal version of Special:GlobalWatchlist

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

Change 702170 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Drop started selenium tests for Special:Version

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

Change 702741 had a related patch set uploaded (by Zfilipin; author: Zfilipin):

[mediawiki/extensions/GlobalWatchlist@master] selenium: replace @wdio/spec-reporter with @wdio/dot-reporter

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

Change 702895 had a related patch set uploaded (by Zfilipin; author: Zfilipin):

[mediawiki/extensions/GlobalWatchlist@master] eslint: Do not automatically fix errors

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

Change 702741 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] selenium: Replace @wdio/spec-reporter with @wdio/dot-reporter

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

Change 702895 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] eslint: Do not automatically fix errors

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

Change 699007 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Special:GlobalWatchlist selenium tests - move setup to `before`

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

Change 705471 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Docs: fix reference to core Special:Watchlist

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

Change 705471 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Docs: fix reference to core Special:Watchlist

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

Change 699009 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Selenium tests for vue version of Special:GlobalWatchlist

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

Change 702170 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Drop initial selenium tests for Special:Version

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

Change 705986 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Selenium: use cookies to allow controlling display version

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

DannyS712 updated the task description. (Show Details)

Change 705986 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Selenium: use cookies to allow controlling display version

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

DannyS712 updated the task description. (Show Details)
DannyS712 set Final Story Points to 20.
DannyS712 moved this task from In progress to Done on the MediaWiki-extensions-GlobalWatchlist board.

We can of course keep working on the tests as the code changes, but basic tests for both display versions are now live. Decided to use a cookie to control allowing the displaymode parameter to work even when not in dev mode, so that we could set the default for dev mode back to false.

Thanks so much for the reviews and the help with this @phuedx @zeljkofilipin