Page MenuHomePhabricator
Create Task
Maniphest T205397

mwselenium-quibble-docker job is broken on MobileFrontend master
Closed, InvalidPublic
Actions

Description

mwselenium-quibble-docker job is broken on MobileFrontend master, it always fails, as seen here: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/462657

(I originally noticed this when tests failed on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/461846)

It looks like the job started failing on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/462026, and yet somehow that change was allowed to merge? Apparently this is my fault, but I have no idea what I'm doing here.

Event Timeline

matmarex triaged this task as Unbreak Now! priority.Sep 25 2018, 9:34 AM
matmarex created this task.
Restricted Application added subscribers: Liuxinyu970226, TerraCodes, Aklapper. · View Herald TranscriptSep 25 2018, 9:34 AM
• Niedzielski added subscribers: pmiazga, Jdlrobson, • Niedzielski.Sep 25 2018, 2:23 PM

Thanks for flagging this.

I've reverted this change temporarily since we're not sure whether this test failure will cause issues in production. Sorry for the churn. Unfortunately, we missed the train but @pmiazga and @Jdlrobson may swat the revert if we can't fix the real issue before then.

Jdlrobson added a comment.Sep 25 2018, 2:43 PM

Looks like the test was added in the wrong place.. in the includes folder. I should have caught that in code review.

Jdlrobson added a project: Web-Team-Backlog.Sep 25 2018, 2:47 PM

Should be harmless in production, but there is autodiscovery of PHP code modules. There is no harm in SWATing it.

matmarex added a comment.Sep 25 2018, 2:59 PM

Thanks, and sorry for breaking it. The change only touched code under tests/, so it should indeed be harmless in production.

Since we probably want the test back, can you help me figure out what I need to do to make it pass?

As a side note, the fact that this could be merged seems like CI misconfiguration. The 'mwselenium-quibble-docker' job only runs in the test build, but not in the gate-and-submit build. Assuming you want Selenium test failures to prevent commits from being merged, this should probably be changed.

matmarex added a comment.Sep 25 2018, 4:08 PM
In T205397#4615255, @matmarex wrote:

Since we probably want the test back, can you help me figure out what I need to do to make it pass?

I guess we can just move it back to Minerva, where it worked fine… https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/462742

matmarex closed this task as Resolved.Sep 25 2018, 4:08 PM

(Job is no longer broken after the changes were reverted.)

Jdlrobson changed the task status from Resolved to Declined.Sep 25 2018, 4:20 PM

So looks like there's 2 problems here:

  1. Didn't get caught in CI before the merge
  2. the test is broken - I have explained why on https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/462026/

Given the patch has been reverted and this is not quibble relating declining as invalid.

Restricted Application removed a subscriber: Liuxinyu970226. · View Herald TranscriptSep 25 2018, 4:20 PM
Jdlrobson added a comment.Sep 25 2018, 4:21 PM

((I debugged this using the video on https://integration.wikimedia.org/ci/job/mwselenium-quibble-docker/1164/)

Jdlrobson changed the task status from Declined to Invalid.Sep 25 2018, 4:21 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