Maniphest T203137

Dev: Standardising our use of headless node qunit tests across Popups and MobileFrontend
Closed, ResolvedPublic5 Estimated Story Points
Actions

Description

As headless QUnit tests are adopted throughout the WMF we are creating multiple bespoke test runners. Before this gets out of control let's fold the Node.js QUnit functionality into the mw-node-qunit library and share the maintenance burden.

During code review of the Hygiene: introduce headless QUnit tests a couple of us were surprised to see qunit listed in package.json of MobileFrontend, given that Popups uses node-qunit which provides a wrapper around qunit and some of the scaffolding e.g. MockMediaWiki that now lives inside MobileFrontend.

Stephen later explained mw-node-qunit adds jsdom, sinon, and wraps QUnit and since @Jhernandez wrote it and the team is not familiar with it we may want to revisit it.

Possible options down the road include

Spinning out wikimedia/mock-mediawiki for handling MediaWiki dependencies
Use qunit in all extensions using webpack
Expanding/adapting mw-node-qunit

Related tasks: T212521

Acceptance criteria

Use of headless qunit runner is consistent between Popups and MobileFrontend
Tests should gate merging. This is currently the case for Popups but not headless tests in MobileFrontend.

Details

Subject	Repo	Branch	Lines +/-
Use 6.1.0 of mw-node-qunit	mediawiki/extensions/MobileFrontend	master	+133 -131
Use 6.1.0 of @wikimedia/mw-node-qunit	mediawiki/extensions/Popups	master	+170 -350
Use mw-node-qunit@6.0.0	mediawiki/extensions/MobileFrontend	master	+33 -207
Upgrade Popups mw-node-qunit version	mediawiki/extensions/Popups	master	+445 -164

Customize query in gerrit

Related Objects

Mentioned In: T212521: RFC: Reconsider how we run QUnit unit tests
Mentioned Here: T212521: RFC: Reconsider how we run QUnit unit tests

Event Timeline

Jdlrobson created this task.Aug 30 2018, 1:06 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 30 2018, 1:06 AM

Jdlrobson edited projects, added MobileFrontend (MobileFrontend and MinervaNeue architecture); removed MobileFrontend.Aug 30 2018, 1:07 AM

Jdlrobson triaged this task as Medium priority.Aug 30 2018, 1:12 AM

Jdlrobson moved this task from Backlog to PHP architecture on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.

Jdlrobson raised the priority of this task from Medium to High.Aug 30 2018, 1:31 AM

Jdlrobson moved this task from PHP architecture to Code coverage on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.

Jdlrobson moved this task from Code coverage to PHP architecture on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.

Move scaffolding to mw-node-qunit

I would prefer to create a new NPM package, @wikimedia/mock-mediawiki, that we include as a devDependency where needed (perhaps in mw-node-qunit or MobileFrontend). Tests that need it can import the mock. The reason is that other repos don't care about QUnit but do care about mocking out MediaWiki.

Update MobileFrontend to use node-qunit

Can we wait until this is required? mw-node-qunit adds jsdom, sinon, and wraps QUnit. @Jhernandez is quite familiar with this code having written it but I don't think the current team is and I would prefer to reevaluate which parts are still necessary and which aren't to avoid unnecessary complexity.

Thanks for the background!
It feels like we might need to use mw-node-qunit (given we are likely to need sinon and jsdom) but I also see where you are coming from that we don't want to use it prematurely.
I like the idea of a new NPM package
As long as we are aiming to be consistent for both Popups and MobileFrontend I think this can indeed wait.

Should I make this task about "Standardising our use of headless node qunit tests across Popups and MobileFrontend"?

sounds good to me.

Jdlrobson renamed this task from Discussion: Why does MobileFrontend use qunit and Popups wikimedia/node-qunit to Standardising our use of headless node qunit tests across Popups and MobileFrontend.Sep 5 2018, 6:21 PM

Jdlrobson updated the task description. (Show Details)

Jdlrobson moved this task from PHP architecture to Code coverage on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.Sep 7 2018, 5:13 PM

Tracking for time being (will pull in from MobileFrontend.js when we are further along)

Jdlrobson lowered the priority of this task from High to Low.Sep 18 2018, 11:53 PM

Jdlrobson moved this task from Code coverage to Post-initial migration on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.

I was just re-reading the task and I find these comments funny 😃:

and since @Jhernandez wrote it and the team is not familiar with it we may want to revisit it.

@Jhernandez is quite familiar with this code having written it but I don't think the current team is and I would prefer to reevaluate which parts are still necessary and which aren't to avoid unnecessary complexity.

mw-node-qunit/wikimedia-setup.js and mw-node-qunit/index.js are a total of only 72 LOC 😂 I'm fairly certain the team can get familiar with the runner code and understand its complexity in about 10 minutes tops.

The rest of the API surface (jQuery, qunit, sinon) are already present independently of using mw-node-qunit or not.

BTW it has been 5 months since the last release, it is probably a good idea to do a dependency upgrade and a new release 👍

When we get round to doing this, we may find that what we've created is near identical or the existing mw-node-qunit is a subset.
I suspect we'll repurpose mw-node-qunit regardless (either replacing it or adding to it)
If the library can handle MobileFrontend I think it's safe to say it can handle any extension.

A good time to do this is probably just before we start thinking about porting code in Minerva to webpack.

• Niedzielski updated the task description. (Show Details)Dec 21 2018, 6:08 PM

Jdlrobson mentioned this in T212521: RFC: Reconsider how we run QUnit unit tests.Dec 21 2018, 6:10 PM

• Niedzielski updated the task description. (Show Details)Dec 21 2018, 6:11 PM

• Niedzielski updated the task description. (Show Details)Dec 21 2018, 6:15 PM

Jdlrobson raised the priority of this task from Low to Medium.Mar 8 2019, 8:04 PM

Jdlrobson moved this task from Post-initial migration to Misc on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.

Change 521303 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Use mw-node-qunit@6.0.0

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

Change 523793 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Upgrade Popups mw-node-qunit version

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

This has been sitting around for a while now. It needs code review which dropped off when Stephen went on sabbatical.

Jdlrobson renamed this task from Standardising our use of headless node qunit tests across Popups and MobileFrontend to Dev: Standardising our use of headless node qunit tests across Popups and MobileFrontend.Oct 7 2019, 4:26 PM

Jdlrobson updated the task description. (Show Details)

ovasileva set the point value for this task to 5.Oct 9 2019, 4:21 PM

Jdlrobson moved this task from Upcoming to Needs Code Review on the Web-Team-Backlog (Kanbanana-2019-20-Q2) board.Oct 15 2019, 10:21 PM

Jdrewniak claimed this task.Oct 16 2019, 5:06 PM

Change 523793 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Upgrade Popups mw-node-qunit version

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

Change 521303 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Use mw-node-qunit@6.0.0

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

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Web-Team-Backlog (Kanbanana-2019-20-Q2) board.Oct 21 2019, 4:46 PM

Jdlrobson moved this task from Ready for Signoff to Needs More Work on the Web-Team-Backlog (Kanbanana-2019-20-Q2) board.Oct 21 2019, 5:07 PM

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.3; 2019-10-22).Oct 21 2019, 6:00 PM

Jdlrobson moved this task from Needs More Work to Needs Code Review on the Web-Team-Backlog (Kanbanana-2019-20-Q2) board.Oct 21 2019, 9:53 PM

Change 545069 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Use 6.1.0 of @wikimedia/mw-node-qunit

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

Change 545070 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Use 6.1.0 of mw-node-qunit

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

Once https://github.com/wikimedia/mw-node-qunit/pull/8 is reviewed I will do a new release and that will unblock the above 2 patches and finish up this task.

Change 545069 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Use 6.1.0 of @wikimedia/mw-node-qunit

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

ReleaseTaggerBot edited projects, added MW-1.35-notes (1.35.0-wmf.4; 2019-10-29); removed MW-1.35-notes (1.35.0-wmf.3; 2019-10-22).Oct 23 2019, 4:00 PM

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Web-Team-Backlog (Kanbanana-2019-20-Q2) board.Oct 23 2019, 5:57 PM

Change 545070 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Use 6.1.0 of mw-node-qunit