Page MenuHomePhabricator

Minerva unit tests failing and causing unrelated CI failures on other repositories
Closed, ResolvedPublic2 Story Points

Description

Unit tests were not running by default in Minerva - only when you run check experimental does Jenkins bother to run them. This is being fixed in T181429.

Some how a unit test got broken and is blocking merges elsewhere.

Developer notes

  • 2 unit tests are breaking and relate to the NotificationBadge due to incorrect usage of NotificationBadge
    • An option used by the test notificationCountRaw is not used.
    • Options names used in test are wrong - notificationCountString and notificationCountRaw NOT notificationCountRawString and notificationCountRawRaw
    • notification-count data attribute is not present on other unit test

Probably broken unwittingly in T172755.

Original report

I submitted a (WIP-ish, unmerged) patchset against RelatedArticles but jerkins is downvoting it consistently; it seemed to me that the failure was entirely unrelated to my patch, which was confirmed my @Legoktm's test patch against RelatedArticle's README file, which obviously in no way could have caused the tests to fail.

Event Timeline

ashley created this task.Nov 25 2017, 10:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 25 2017, 10:26 PM

I suspect there's something wrong with the config setup. That test passes fine locally for me and in Minerva. Maybe it's missing one of Minerva's dependencies e.g. MobileFrontend.

Jdlrobson renamed this task from "Minerva NotificationBadge" JS test causing unrelated CI failures on other repositories to Minerva unit tests failing and causing unrelated CI failures on other repositories.Nov 27 2017, 7:50 PM
Jdlrobson triaged this task as High priority.
Jdlrobson moved this task from Incoming to Upcoming on the Readers-Web-Backlog board.
Jdlrobson updated the task description. (Show Details)Nov 27 2017, 7:56 PM
Jdlrobson updated the task description. (Show Details)Nov 27 2017, 8:18 PM
Jdlrobson updated the task description. (Show Details)Nov 27 2017, 8:25 PM
Jdlrobson moved this task from Backlog to Bugs on the MinervaNeue board.Nov 27 2017, 8:56 PM
ovasileva set the point value for this task to 2.Nov 28 2017, 5:57 PM

Okay after tinkering with my setup I can now replicate this locally. They don't appear to be running on jenkins. I'm on it (T181429)

Change 393833 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Fix broken QUnit test

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

bmansurov added a subscriber: bmansurov.

Skipping QA as check experimental is passing.

Change 393833 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Fix broken QUnit test

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

Jdlrobson closed this task as Resolved.Nov 29 2017, 8:21 PM

Given https://gerrit.wikimedia.org/r/#/c/393425/ is passing and this is a technical task I can sign this off.

Change 394360 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@specialpages] Fix broken QUnit test

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

Change 394360 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@specialpages] Fix broken QUnit test

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