Page MenuHomePhabricator

Merge blocker: MediaWiki\MassMessage\MassMessageJobTest::testLQTMessageSending
Open, HighPublic

Description

This is currently blocking merges to ContentTranslation (in addition to T216689: Merge blocker: quibble-vendor-mysql-hhvm-docker in gate fails for most merges (exit status -11))

13:25:58 1) MediaWiki\MassMessage\MassMessageJobTest::testLQTMessageSending
13:25:58 === Logs generated by test case
13:25:58 [PHPUnitCommand] [info] End test LessFileCompilationTest::testLessFileCompilation {"private":false}
13:25:58 ===
13:25:58 Failed asserting that false is true.
13:25:58
13:25:58 /workspace/src/extensions/MassMessage/tests/phpunit/job/MassMessageJobTest.php:72
13:25:58 /workspace/src/tests/phpunit/MediaWikiTestCase.php:425
13:25:58 /workspace/src/maintenance/doMaintenance.php:94
13:25:58

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 4 2019, 2:00 PM
Nikerabbit triaged this task as Unbreak Now! priority.Mar 4 2019, 2:00 PM
Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptMar 4 2019, 2:00 PM

Change 494277 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/MassMessage@master] Skip LiquidThreads test

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

I tried to investigate this, but didn’t get very far – MassMessageJob::addLQTThread() tries to make an API request, LQT’s ApiThreadAction::execute() doesn’t seem to be called, and MassMessageJob::addLQTThread() doesn’t check the return value of the API call. I hope someone else can figure out what’s going on here, but unless that happens soon, I propose to skip the test in the meantime to unblock ContentTranslation, Wikidata -related extensions and probably others.

Change 494277 merged by jenkins-bot:
[mediawiki/extensions/MassMessage@master] Skip LiquidThreads test

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

Lucas_Werkmeister_WMDE lowered the priority of this task from Unbreak Now! to High.Mar 4 2019, 6:34 PM

Looks like this is no longer blocking merges (now I’m back to seeing T216689 instead of this problem), so lowering priority. Thanks @D3r1ck01 for the quick merge!

Legoktm added a subscriber: Legoktm.Mar 5 2019, 8:16 AM

I didn't realize MassMessage was now a dependency of ContentTranslation...in any case, did anyone test whether the failure was real? Does MM + LQT posting still work?

I didn't realize MassMessage was now a dependency of ContentTranslation...

It isn't. I believe this job pulls in a long list of extensions, but I don't know where that list is defined.

CI dependencies (i. e., also install and test those other extensions for any change in this extension) are defined in integration/config, and this specific dependency was introduced in Ib70c64d047 (already reverted and then restored once, see T217487; cc @Daimona, @Reedy) – ContentTranslation requires AbuseFilter, which requires CentralAuth, which requires MassMessage, which requires LiquidThreads.

(Side note: since MassMessage also includes tests for sending messages to Structured Discussions, shouldn’t it require Flow too?)

Reedy added a comment.Mar 5 2019, 10:39 AM

(Side note: since MassMessage also includes tests for sending messages to Structured Discussions, shouldn’t it require Flow too?)

If people care about those tests being run, then yeah, it should be in zuul/parameter_functions.py

If people care about those tests being run, then yeah, it should be in zuul/parameter_functions.py

Adding in Iaaa845a42b.