Page MenuHomePhabricator

SetupDynamicConfigTest tries to load MW_CONFIG_FILE in a unit test, which is unsafe (as unit tests are meant to be run without MW installed), so breaks tests for others
Open, Needs TriagePublic

Description

Quibble intentionally runs the unit tests and then MW install for extensions, because the unit tests are by definition never meant to depend on MW being installed.

But https://gerrit.wikimedia.org/r/c/mediawiki/core/+/779853 introduces a read of MW_CONFIG_FILE, which breaks tests e.g. for the WikiLambda extension because it's defined but not created yet: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/778586

00:00:56.020 There were 80 errors:
00:00:56.020 
00:00:56.020 1) SetupDynamicConfigTest::testGlobals with data set "Nothing set" (…)
00:00:56.020 filemtime(): stat failed for /workspace/src/LocalSettings.php
00:00:56.020 
00:00:56.020 /workspace/src/tests/phpunit/unit/includes/SetupDynamicConfigTest.php:193
00:00:56.020 /workspace/src/tests/phpunit/unit/includes/SetupDynamicConfigTest.php:979
00:00:56.020 /workspace/src/tests/phpunit/MediaWikiUnitTestCase.php:111
  • Fix the breakage
  • Add a test to CI / alter its setup so this can't recur

Event Timeline

Change 785891 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/core@master] SetupDynamicConfigTest: Don't fail if MW_CONFIG_FILE isn't created yet

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

Change 785909 had a related patch set uploaded (by Simetrical; author: Simetrical):

[mediawiki/core@master] Don't assume LocalSettings exists in unit tests

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

Change 785891 abandoned by Jforrester:

[mediawiki/core@master] SetupDynamicConfigTest: Don't fail if MW_CONFIG_FILE isn't created yet

Reason:

Going with I14fcfb24d61f8b48161dec843372d3c3a0a0bc11.

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

Change 785909 merged by jenkins-bot:

[mediawiki/core@master] Don't assume LocalSettings exists in unit tests

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

Daniel and I briefly discussed if these tests should be in integration/ rather than unit/, though most of the code probably makes sense to be unit tests.

Also we should think about how we can fix CI so this doesn't happen again.