Page MenuHomePhabricator

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

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

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.

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

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

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.

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