Page MenuHomePhabricator

Intermittent failure of mwext-qunit-jessie for Popups
Closed, ResolvedPublic

Event Timeline

phuedx created this task.Apr 13 2017, 11:22 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 13 2017, 11:22 AM

I'll add more detail as I investigate.

phuedx updated the task description. (Show Details)Apr 13 2017, 11:25 AM
Paladox triaged this task as High priority.Apr 13 2017, 12:02 PM
Paladox added a subscriber: Paladox.

may be related to jQuery being updated to 3.x in mediawiki core. https://gerrit.wikimedia.org/r/#/c/322812/35/includes/DefaultSettings.php

phuedx added a comment.EditedApr 13 2017, 12:16 PM

With wikimedia/mediawiki@bf69459d:

Running the Wikibase QUnit tests in Chrome (57.0.2987.133) doesn't yield animation warnings. Nor does running the VE QUnit tests.

phuedx removed phuedx as the assignee of this task.Apr 13 2017, 1:15 PM

Alright, I've got to unlick this cookie for a while. I think the easiest change to make would be to change the default value of $wgUsejQueryThree to false.

^ Sorry. Missing context: reverting rMWbf69459d5011: Upgrade to jQuery v3 causes the tests to go green again (see rMW6e9fb743590a: WIP Revert "Upgrade to jQuery v3").

hashar added a subscriber: hashar.Apr 13 2017, 1:24 PM

The MediaWiki core change that enables it is https://gerrit.wikimedia.org/r/#/c/322812/ and it shows that the mediawiki-extensions-qunit job is falling from time to time. Probably there is a race condition somewhere.

Looks like it has been merged too fast and should be reverted.

These are actually many separate unrelated issues. I think James and I fixed most of them yesterday. Any new reports?

@Krinkle: I'm having a hard time tracking down the root cause of the "Unfinished animations" errors in https://integration.wikimedia.org/ci/job/mediawiki-extensions-qunit-jessie/28266/consoleFull for example. Any pointers would be appreciated.

@Krinkle: I'm having a hard time tracking down the root cause of the "Unfinished animations" errors in https://integration.wikimedia.org/ci/job/mediawiki-extensions-qunit-jessie/28266/consoleFull for example. Any pointers would be appreciated.

We spent a few hours trying to find them in MF and couldn't work out exactly where. It seemed to be triggered via Toast, but only rarely. Very irritating.

@Krinkle: I'm having a hard time tracking down the root cause of the "Unfinished animations" errors in https://integration.wikimedia.org/ci/job/mediawiki-extensions-qunit-jessie/28266/consoleFull for example. Any pointers would be appreciated.

We spent a few hours trying to find them in MF and couldn't work out exactly where. It seemed to be triggered via Toast, but only rarely. Very irritating.

Indeed. I can try and help, but note that these are not new. They've been bugging tests intermittently for at least 6 months. Basically, a unit test (presumably one from MobileFrontend) triggers a code path that in turn triggers mobile.Toast which calls mw.notification which then animates on-screen.

The triggering test is unaware of this animation and eventually tries to move on to the next test which QUnit reports as an error. Any of various patterns could resolve this, for example:

  • Add test element(s) to qunit-fixture (which is automatically clean up by the default teardown).
  • Mock animation speed to duration=0 and wait 1 tick for jQuery's fx promise.
  • Mock mw.notify().
  • Mock Toast.
  • Prevent the unrelated UI code from running.
  • Remove the element from the DOM manually.

There are also these two elements left behind in the DOM:

<div class="view-border-box"><p>undefined</p></div>

<div class="drawer position-fixed view-border-box">
   <div class="mw-ui-icon mw-ui-icon-arrow mw-ui-icon-element  cancel" title=""></div>
   <p>Keep track of this page and all changes to it.</p>
   <a href="/w/index.php?title=Special:UserLogin&amp;returnto=Special%3AJavaScriptTest%2Fqunit%2Fexport&amp;warning=mobile-frontend-watchlist-purpose&amp;campaign=mobile_watchPageActionCta&amp;returntoquery=article_action%3Dwatch" class="mw-ui-button mw-ui-progressive">Log in</a>
   <div>
      <a href="/w/index.php?title=Special:UserLogin&amp;returnto=Special%3AJavaScriptTest%2Fqunit%2Fexport&amp;warning=mobile-frontend-watchlist-signup-action&amp;campaign=mobile_watchPageActionCta&amp;returntoquery=article_action%3Dwatch&amp;type=signup" class="mw-ui-anchor mw-ui-progressive ">Sign up</a>
   </div>
</div>

Jdlrobson raised the priority of this task from High to Unbreak Now!.Apr 13 2017, 9:11 PM
Jdlrobson added a subscriber: Jdlrobson.

We can't merge anything while this is the case.
What got merged since yesterday that could have made this error so prominent?

Restricted Application added subscribers: Jay8g, TerraCodes. · View Herald TranscriptApr 13 2017, 9:11 PM
greg added a subscriber: greg.

(Doesn't seem to be anything wrong with CI infra nor config, afaict)

Krinkle renamed this task from mwext- and mediawiki-extensions-qunit-jessie builds failing in core and other extensions to Intermittent failure of mwext-qunit-jessie and mediawiki-extensions-qunit-jessie.Apr 13 2017, 11:39 PM
Krinkle lowered the priority of this task from Unbreak Now! to Normal.
Krinkle removed a project: MediaWiki-ResourceLoader.

Okay, so a few different things are going on.

  • jQuery 3 upgrade required a few minor adjustments to unit tests. This was a consistent failure of mediawiki-extensions-qunit-jessie for extensions part of the shared gate. This was already fixed yesterday (may've still been visible on some commits with no updates/rechecks since then). This did not affect Page-Previews as it does not run the shared extension gate job.
  • In general, unrelated to jQuery 3, unfinished animations have been causing intermittent failures. Typically ending up somewhere in VisualEditor (which happens to be the one in the test suite queue after MobileFrontend). In most cases, however, they pass fine and this is not a new issue. – Also did not affect Page-Previews commits.
  • The mwext-qunit-jessie job is for individual extensions, it runs only tests registered by that extension job. Commit https://gerrit.wikimedia.org/r/347523 which triggered this task via T160081#3178322 is failing with an unfinished animation error from jquery.color in mediawiki/core. Most likely another yet unrelated rare intermittent failure that is not recently introduced.

I suggest either re-purposing it for the lower priority issue mentioned T162876#3180702, or closing it and filing a separate one for that. I'll see if I can figure out how to make that Page-Previews commit pass.

NOTE: If something else goes wrong and we need to temporarily disable jQuery 3 in Jenkins, please do so by setting $wgUsejQueryThree = false; in integration/jenkins.git:/mediawiki.
Krinkle renamed this task from Intermittent failure of mwext-qunit-jessie and mediawiki-extensions-qunit-jessie to Intermittent failure of mwext-qunit-jessie for Popups.Apr 14 2017, 12:54 AM
Krinkle added a comment.EditedApr 14 2017, 12:57 AM

I've narrowed down the culprit using https://gerrit.wikimedia.org/r/#/c/348185/, it was caused by the mw.loader.using( 'ext.eventLogging.Schema' ) inside the Popups src code.

That init code is presumably triggered by one of Popups' unit tests, which then triggers this RL request. Meanwhile, the unit tests assumes things are done, makes its assertions and moves on to the next test. At a later point, the promise is resolved and this function suddenly starts running more init code in the middle of an unrelated test.

This part either needs to become part of that Popups/qunit integration test (in that the test at least waits for it to complete), or be mocked/disabled/bypassed in some way.

While it seems consistent in Jenkins, I couldn't reproduce it locally with the same extensions and version of MediaWiki core so it's not directly tied to a logic change in jQuery 3.

I'm unsure as to why this would be happening more often now than yesterday. I imagine it might be a timing thing specific to Chrome and our Nodepool VMs that somehow made this race condition more likely when jQuery 3 is loaded.

Change 348200 had a related patch set uploaded (by Phuedx):
[mediawiki/extensions/Popups@master] Don't load entire codebase in QUnit tests

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

I just had the same issue ("Unfinished animations") at https://gerrit.wikimedia.org/r/#/c/344811/ (which is mediawiki/core). I've hit "recheck" until it worked ¯\_(ツ)_/¯

phuedx claimed this task.Apr 16 2017, 5:28 AM
phuedx moved this task from Needs Analysis to Needs Code Review on the Reading-Web-Sprint-95 board.

Thanks for looking into this @Krinkle/@Jdforrester-WMF! With your knowledge dumps this was actually a simple fix.

Change 348200 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Don't load entire codebase in QUnit tests

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

Jdforrester-WMF closed this task as Resolved.Apr 17 2017, 6:27 PM

Thanks for fixing this in Popups; now we need to find the next repo that's broken.

Change 348641 had a related patch set uploaded (by Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Avoid loading toasts in tests that are not testing toasts

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

Change 348641 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Avoid loading toasts in tests that are not testing toasts

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