Page MenuHomePhabricator

Unknown asyncTest failures
Closed, ResolvedPublic

Description

I think this is incorrectly set up as a synchronous test when it's not.
This causes Jenkins to fail intermittently

16:49:03 >> MobileFrontend TalkOverlay - global failure
16:49:03 >> Message: Error: Called start() outside of a test context while already started
16:49:03 >> Actual: null
16:49:03 >> Expected: undefined
16:49:03 >> http://localhost:9412/jenkins-mwext-MobileFrontend-qunit-mobile-11552/load.php?debug=false&lang=en&modules=jquery.qunit%7Cjquery.qunit.completenessTest&skin=minerva&version=20150506T164844Z&*:4
16:49:03 
16:49:03 >> MobileFrontend TalkOverlay - global failure
16:49:03 >> Message: Expected number of assertions to be defined, but expect() was not called.
16:49:03 >> Actual: null
16:49:03 >> Expected: undefined
16:49:03 >> at http://localhost:9412/jenkins-mwext-MobileFrontend-qunit-mobile-11552/load.php?debug=false&lang=en&modules=jquery.qunit%7Cjquery.qunit.completenessTest&skin=minerva&version=20150506T164844Z&*:4
16:49:03 >>   at http://localhost:9412/jenkins-mwext-MobileFrontend-qunit-mobile-11552/load.php?debug=false&lang=en&modules=jquery.qunit%7Cjquery.qunit.completenessTest&skin=minerva&version=20150506T164844Z&*:7
16:49:03 
16:49:03 Warning: 2/456 assertions failed (2182ms) Use --force to continue.
16:49:03

Event Timeline

Jdlrobson created this task.May 6 2015, 5:36 PM
Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Readers-Web-Backlog.
Jdlrobson moved this task to To Triage on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Jdlrobson.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 6 2015, 5:36 PM
phuedx claimed this task.May 8 2015, 10:31 AM
phuedx set Security to None.

Gonna take a look at this.

phuedx triaged this task as High priority.May 8 2015, 10:31 AM
phuedx renamed this task from Intermediate talk overlay failures to Unknown asyncTest failures.May 8 2015, 3:43 PM

I've been unable to reproduce the TalkOverlay failure but have instead seen MobileFrontend photo failures, which are synchronous tests. One of the async tests is bleeding, maybe?

phuedx added a comment.May 8 2015, 3:46 PM

I also regularly see Router test failures.

Very possible that a test is not setup as an asyncTest
We should probably remove the uploads code.. what do people think?

toggle code also seems to fail a lot more than it should...

We don't develop uploads actively, but we should holf some basic tests, so that a change to the basics of mobilefrontend doesn't break with uploads enabled? :)

If we want to check tests, if an async one is missing, we could convert our QUnit.asyncTest ro use the non-deprecated assert.async: https://api.qunitjs.com/QUnit.asyncTest/

Another job failed randomly, on watchstar tests:

https://integration.wikimedia.org/ci/job/mwext-MobileFrontend-qunit-mobile/11650/consoleFull

21:32:57 >> MobileFrontend: Watchstar.js - Logged in user watches article
21:32:57 >> Message: Called start() while already started (test's semaphore was 0 already)
21:32:57 >> Actual: null
21:32:57 >> Expected: undefined
21:32:57 >> at http://localhost:9412/jenkins-mwext-MobileFrontend-qunit-mobile-11650/load.php?debug=false&lang=en&modules=jquery.qunit%7Cjquery.qunit.completenessTest&skin=minerva&version=20150508T213232Z&*:5
21:32:57 >> MobileFrontend: Watchstar.js - Logged in user watches article
21:32:57 >> Message: Expected 3 assertions, but 4 were run
21:32:57 >> Actual: null
21:32:57 >> Expected: undefined
21:32:57 >> at http://localhost:9412/jenkins-mwext-MobileFrontend-qunit-mobile-11650/load.php?debug=false&lang=en&modules=jquery.qunit%7Cjquery.qunit.completenessTest&skin=minerva&version=20150508T213232Z&*:4

From here: https://gerrit.wikimedia.org/r/#/c/209544/

Change 210015 had a related patch set uploaded (by Phuedx):
Make the InfiniteScroll test synchronous

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

There are a three modules tested with asynchronous tests: mobile.infiniteScroll (test_InfiniteScroll.js, mobile.startup (test_Router.js), and mobile.watchlist (test_WatchListApi.js).

Change 210015 abandoned by Phuedx:
Make the InfiniteScroll test synchronous

Reason:
We're going to see if assert.async fixes these issues.

Also, manipulating internal state isn't the greatest idea. We might have to trigger scroll events manually.

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

Change 210018 had a related patch set uploaded (by Phuedx):
Make WatchListApi tests synchronous

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

210018 is really just a hygiene patch. It seems like the problem is in test_Router.js.

Change 210022 had a related patch set uploaded (by Jhernandez):
Disable router tests until proper fix

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

The culprit is QUnit.start and QUnit.stop not working as before on teardown of the test_Router.js. ⬆️ Above patch disables the router tests until we can figure out a way of fixing those.

From what I've been investigating, there is now way we can stop and start QUnit in setup & teardown, and stop and start are being deprecated. The test suite for the router needs to be re-thought and the Router itself should be decoupled from the browser (it is completely coupled to window/window.location/etc making it super hard to test without messing with the browser).

I've created T98731 for fixing the tests and improving the router. We really need to stabilize jenkins/qunit since it is affecting lots of patches.

I've rebased a patch I was having trouble merging https://gerrit.wikimedia.org/r/#/c/209544/ on top of the router-tests-disabled one and it verifies correctly now, so I'm fairly certain that the router tests were the problem.

Merge https://gerrit.wikimedia.org/r/210022 and rebase your problematic patches on top of it if you want to continue working.

Change 210022 merged by jenkins-bot:
Disable router tests until proper fix

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

phuedx added a comment.EditedMay 11 2015, 1:50 PM

I'm going to resolve this as closed as @Jhernandez's patch is merged and we know what the problem is.

Edit

… and @Jhernandez has written a task targeting the specific issue.

Change 210018 merged by jenkins-bot:
Hygiene: Make WatchListApi tests synchronous

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