Page MenuHomePhabricator
Create Task
Maniphest T176453

VisualEditor IME test failing in CI on pull-through, but not locally or in the source repo?
Closed, ResolvedPublic8 Estimated Story Points
Actions

Description

The "Setup slugs in TreeModifier" patch in VE-core passed unit tests locally and in CI, and was merged. However, the pull-through into VE-MW is failing mediawiki-extensions-qunit-jessie in one of the fake IME tests that's also tested in VE-core.

https://gerrit.wikimedia.org/r/#/c/379557/ and also https://gerrit.wikimedia.org/r/#/c/379421/ for just this patch alone.

Local testing shows this passing in Chrome and Chromium 58, 60, and 61, so it might be a CI config issue?

This currently blocks development (and deployment of an important fix). :-(

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Make IME tests more consistent across test environmentsVisualEditor/VisualEditorREL1_30+16 -17
Make IME tests more consistent across test environmentsVisualEditor/VisualEditormaster+16 -17
Update VE core submodule to master (0ec5fe55f)mediawiki/extensions/VisualEditormaster+1 -1
IME tests: Temporarily disable 'backspace-chromium-ubuntu-none.js'VisualEditor/VisualEditormaster+0 -2
Customize query in gerrit

Related Objects

Event Timeline

Jdforrester-WMF created this task.Sep 22 2017, 12:28 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 22 2017, 12:28 AM
gerritbot subscribed.Sep 22 2017, 4:15 PM

Change 379781 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[VisualEditor/VisualEditor@master] IME tests: Temporarily disable 'backspace-chromium-ubuntu-none.js'

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

gerritbot added a project: Patch-For-Review.Sep 22 2017, 4:15 PM
gerritbot added a comment.Sep 22 2017, 4:26 PM

Change 379781 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] IME tests: Temporarily disable 'backspace-chromium-ubuntu-none.js'

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

gerritbot added a comment.Sep 22 2017, 5:28 PM

Change 379557 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (7d38cf7ee)

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

Jdforrester-WMF added a comment.Sep 22 2017, 5:57 PM

OK, it's actually failing all IME tests; this was just the first one. Commenting it out just made the next fail, and the next…

dchan added a comment.Sep 25 2017, 3:23 AM

As I've noted in gerrit, the Filibuster dump at patchset https://gerrit.wikimedia.org/r/#/c/379557/21/ on the CI system differs from the one I get on my local machine. At the very start of the first IME test, when "गा" is inserted, the CI system has

<p class='ve-ce-branchNode ve-ce-contentBranchNode ve-ce-paragraphNode'><span class='ve-ce-branchNode-slug ve-ce-branchNode-inlineSlug'><#text>गा</#text><img class='ve-ce-chimera ve-ce-chimera-webkit'></img></span></p>

whereas my local machine has

<p class='ve-ce-branchNode ve-ce-contentBranchNode ve-ce-paragraphNode'><#text>गा</#text><span class='ve-ce-branchNode-slug ve-ce-branchNode-inlineSlug'><img class='ve-ce-chimera ve-ce-chimera-webkit'></img></span></p>

I.e. the CI system has [text],[inlineSlug] whereas my local machine has [inlineSlug],[text].

Perhaps this looks like some browser- or environment-specific difference in how the DOM functions work, when called from ve.ce.TestRunner#changeText .

dchan added a comment.Sep 25 2017, 9:14 AM

OK, I have tested a rewritten version of ve.ce.TestRunner#changeText on the CI environment with Filibuster ( https://gerrit.wikimedia.org/r/#/c/379557/23/ ), and it looks like it works. So now we just need to put it into VE standalone then pull that through into VE-MW. And then see whether I'm completely wrong or not :)

gerritbot added a comment.Sep 25 2017, 9:16 AM

Change 380451 had a related patch set uploaded (by Divec; owner: Divec):
[VisualEditor/VisualEditor@master] Make IME tests more consistent across test environments

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

dchan claimed this task.Sep 25 2017, 9:37 AM
gerritbot added a comment.Sep 25 2017, 2:40 PM

Change 380451 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Make IME tests more consistent across test environments

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

gerritbot added a comment.Sep 25 2017, 2:43 PM

Change 379557 had a related patch set uploaded (by Jforrester; owner: DLynch):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (0ec5fe55f)

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

Jdforrester-WMF closed this task as Resolved.Sep 25 2017, 3:10 PM
Jdforrester-WMF set the point value for this task to 8.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptSep 25 2017, 3:10 PM
gerritbot added a comment.Sep 25 2017, 3:16 PM

Change 379557 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (0ec5fe55f)

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

gerritbot added a comment.Sep 25 2017, 3:52 PM

Change 380534 had a related patch set uploaded (by Jforrester; owner: Divec):
[VisualEditor/VisualEditor@REL1_30] Make IME tests more consistent across test environments

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

gerritbot added a comment.Sep 25 2017, 3:55 PM

Change 380534 merged by jenkins-bot:
[VisualEditor/VisualEditor@REL1_30] Make IME tests more consistent across test environments

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2017-09-26 (1.31.0-wmf.1)).Sep 25 2017, 4:00 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits