Page MenuHomePhabricator

Reading web extensions are incompatible with QUnit 2
Closed, ResolvedPublic

Description

I'm seeing issues when I install any extension or skin with unit tests
e.g. RelatedArticles(fixed), MobileFrontend, Minerva(fixed)
Core tests seem to be working completely fine.

TypeError: test.callback.call is not a function
    at runTest (http://localhost:8888/w/load.php?debug=false&lang=en&modules=jquery.qunit&only=scripts&raw=1&skin=vector&sync=1:20:162)
    at Test.run (http://localhost:8888/w/load.php?debug=false&lang=en&modules=jquery.qunit&only=scripts&raw=1&skin=vector&sync=1:19:914)
    at http://localhost:8888/w/load.php?debug=false&lang=en&modules=jquery.qunit&only=scripts&raw=1&skin=vector&sync=1:23:875
    at Object.advance (http://localhost:8888/w/load.php?debug=false&lang=en&modules=jquery.qunit&only=scripts&raw=1&skin=vector&sync=1:14:956)
    at begin (http://localhost:8888/w/load.php?debug=false&lang=en&modules=jquery.qunit&only=scripts&raw=1&skin=vector&sync=1:44:336)
    at http://localhost:8888/w/load.php?debug=false&lang=en&modules=jquery.qunit&only=scripts&raw=1&skin=vector&sync=1:43:855

The reason is there is an upgrade to QUnit 2 going on.
Right now QUnit 1 is used for Jenkins, but that's going to go away soon.
See https://phabricator.wikimedia.org/T174598#3568353 for an explanation.
We should update our tests or we will suffer tremendous pain with merging very soon!

Sign off steps (developer only)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 30 2017, 6:20 PM

(I'm using grunt karma in mean time but other people might be depending on that page to run tests)

Jdlrobson added a comment.EditedAug 30 2017, 9:39 PM

I'm guessing this is because of usages of QUnit.expect and async....

Jdlrobson edited subscribers, added: Krinkle; removed: Aklapper.Aug 30 2017, 9:40 PM
Krinkle closed this task as Declined.EditedAug 30 2017, 10:21 PM
Krinkle triaged this task as High priority.

I'm guessing this is because of usages of QUnit.expect and async....

Indeed. These presumably come from one of the installed extensions (RelatedArticles, MobileFrontend).

Those deprecated methods were removed in QUnit 2.

The Special-page runner was upgraded to QUnit 2 last month. There are still some issues left unaddressed in Wikibase that forced us to leave the grunt-cli runner on QUnit 1.x at the moment (used by Jenkins). The Special-page runner can be used to debug any incompatibilities with QUnit 2 in preparation for the grunt-cli upgrade.

See:

Krinkle moved this task from Inbox to QUnit on the MediaWiki-Core-Testing board.Aug 30 2017, 10:21 PM
Jdlrobson reopened this task as Open.Aug 30 2017, 10:36 PM

Sounds like these need fixing for MobileFrontend etc though?

I assume we don't want to support them... but we'd still like to run them .

Change 374908 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] QUnit: Drop assertions for expected number of tests

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

Jdlrobson renamed this task from Special:JavaScript/qunit/test unusable to Reading web extensions are incompatible with QUnit 2.Aug 31 2017, 1:44 PM
Jdlrobson added a project: Readers-Web-Backlog.

Change 374992 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Make Minerva QUnit tests v2 compatible

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

Jdlrobson updated the task description. (Show Details)Aug 31 2017, 1:46 PM
Jdlrobson moved this task from Incoming to Upcoming on the Readers-Web-Backlog board.

Change 374992 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Make Minerva QUnit tests v2 compatible

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

Jdlrobson updated the task description. (Show Details)

Change 375390 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/RelatedArticles@master] Make RelatedArticles tests QUnit 2 compatible

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

Can someone please sanity check https://gerrit.wikimedia.org/r/#/c/374908/ since I'm touching lots of files. Then we can call this task done.

Jdlrobson claimed this task.Sep 1 2017, 3:25 PM
Jdlrobson removed a project: RelatedArticles.
Jdlrobson updated the task description. (Show Details)

Change 375390 merged by jenkins-bot:
[mediawiki/extensions/RelatedArticles@master] Make RelatedArticles tests QUnit 2 compatible

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

@Jdlrobson, I left a comment in gerrit, in case you missed it.

https://gerrit.wikimedia.org/r/374908 has a +1.
I am going on vacation later today.

Change 374908 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] QUnit: Drop assertions for expected number of tests

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

Jdlrobson updated the task description. (Show Details)Sep 8 2017, 6:46 PM
Jdlrobson removed a project: Patch-For-Review.
Jdlrobson reassigned this task from Jdlrobson to bmansurov.Sep 8 2017, 7:05 PM

@bmansurov on irc said he can have a go at signing this off

bmansurov closed this task as Resolved.Sep 8 2017, 7:41 PM
bmansurov removed bmansurov as the assignee of this task.

Tests are passing locally.