Note, as I wrote in the other task: interestingly, codehealth jobs are not affected, in that these tests fail correctly there, see https://integration.wikimedia.org/ci/job/mwext-codehealth-patch/25987/console.

In T266441#6577664, @Daimona wrote:

Note, as I wrote in the other task: interestingly, codehealth jobs are not affected, in that these tests fail correctly there, see https://integration.wikimedia.org/ci/job/mwext-codehealth-patch/25987/console.

Yes, good addition, thanks for flagging. The differences with that one are:

1. it's using mwext-phpunit-coverage script, which in turn executes:

php -d zend_extension=xdebug.so \
       vendor/bin/phpunit \
       --testsuite=extensions:unit \
       --exclude-group Dump,Broken,ParserFuzz,Stub \
       --coverage-clover "$LOG_DIR"/clover.xml \
       --log-junit "$LOG_DIR"/junit.xml &

That should be just about equivalent to composer phpunit:unit, since the composer script evaluates to vendor/bin/phpunit --colors=always --testsuite=core:unit,extensions:unit,skins:unit.

2. In the codehealth job, only the AbuseFilter repository is cloned.

The lack of errors in the description could make sense if the AbuseFilter repository wasn't cloned at the time the tests were run, but it doesn't look like that is the case.

Hmm, so:

vendor/bin/phpunit --testsuite=core:unit,extensions:unit -- no errors

vendor/bin/phpunit --testsuite=skins:unit,extensions:unit -- errors

vendor/bin/phpunit --testsuite=extensions:unit -- errors

`vendor/bin/phpunit --testsuite=core:unit,skins:unit,extensions:unit -- no errors

OK, it looks like if you remove tests/phpunit/unit/includes/language/SpecialPageAliasTest.php then the AbuseFilter unit tests fail as they should.

I don't yet know why that test is bootstrapping MediaWiki nor why calls to MediaWikiServices are allowed inside a test that extends MediaWikiUnitTestCase.

One idea is to adjust quibble so that LocalSettings.php is moved out of the way before the unit tests run.

Change 636430 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[integration/quibble@master] Move LocalSettings.php out of the way before running unit tests

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

In T266441#6578243, @kostajh wrote:

OK, it looks like if you remove tests/phpunit/unit/includes/language/SpecialPageAliasTest.php then the AbuseFilter unit tests fail as they should.

I don't yet know why that test is bootstrapping MediaWiki nor why calls to MediaWikiServices are allowed inside a test that extends MediaWikiUnitTestCase.

validSpecialPageAliasesProvider, which is a data provider, is calling Language::fetchLanguageNames(), which calls MediaWikiServices::getInstance() etc. Might this be the cause? It seems a bit weak though, that a single test doing that will bootstrap MediaWiki and prevent further failures later.

In T266441#6578491, @Daimona wrote:

In T266441#6578243, @kostajh wrote:

OK, it looks like if you remove tests/phpunit/unit/includes/language/SpecialPageAliasTest.php then the AbuseFilter unit tests fail as they should.

I don't yet know why that test is bootstrapping MediaWiki nor why calls to MediaWikiServices are allowed inside a test that extends MediaWikiUnitTestCase.

validSpecialPageAliasesProvider, which is a data provider, is calling Language::fetchLanguageNames(), which calls MediaWikiServices::getInstance() etc. Might this be the cause? It seems a bit weak though, that a single test doing that will bootstrap MediaWiki and prevent further failures later.

Yeah, apparently that's all you need. :(

I think when this went live, we explicitly ran composer phpunit:unit without LocalSettings.php in place. It seems that this got revised at some point (possibly in the plan execution patches from a few months back?) since the MediaWiki install is happening before the unit tests are run in https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-noselenium-docker/42693/consoleFull#console-section-6 . cc @awight

I think we may want to avoid having to move the file in CI, so that out-of–the-box things are locally more similar to CI.

If use of MediaWikiServices is the issue, can we plant some kind of explosive there during the unit test run? It's not clear to me how calling MediaWikiServices would lead to inclusion of LocalSettings.php. I assume this isn't what's happening, but that it is included ahead of time, in which case we may want to fix that so that the composer entry point bootstrapping isn't including any LocalSettings.php file, regardless of whether it exists or not.

In T266441#6581966, @Krinkle wrote:

I think we may want to avoid having to move the file in CI, so that out-of–the-box things are locally more similar to CI.

Yeah. Even so, I think it would still be useful to have in quibble, but yes we want this to work locally.

If use of MediaWikiServices is the issue, can we plant some kind of explosive there during the unit test run? It's not clear to me how calling MediaWikiServices would lead to inclusion of LocalSettings.php. I assume this isn't what's happening, but that it is included ahead of time, in which case we may want to fix that so that the composer entry point bootstrapping isn't including any LocalSettings.php file, regardless of whether it exists or not.

Having looked at this more closely, it doesn't seem like LocalSettings.php is the culprit after all. I'm not sure why I thought that 😕

I think the main problem is that we have too much magic going on in our data providers, which are harder to intercept since they are evaluated independently of the MediaWikiUnitTestCase setup process.

Change 636913 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] [WIP] Prevent access to MediaWikiServices in unit tests

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

Change 636915 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] [WIP] Prevent access to MediaWikiServices in integration and unit tests

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

Change 637280 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] phpunit: Block calls to MW services in data providers and in unit tests

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

Change 636915 abandoned by Kosta Harlan:
[mediawiki/core@master] [WIP] Prevent access to MediaWikiServices in integration and unit tests

Reason:
See I5913839d049916cf569efddb250644a47f3206d7

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

Change 636913 abandoned by Kosta Harlan:
[mediawiki/core@master] [WIP] Prevent access to MediaWikiServices in unit tests

Reason:
See I5913839d049916cf569efddb250644a47f3206d7

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

Change 641941 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] phpunit: Use a closure for services to enable unit testing

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

Change 641944 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] phpunit: Switch SpecialPageAliasTest to integration tests

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

Change 641941 merged by jenkins-bot:
[mediawiki/core@master] phpunit: Use a closure for services to enable unit testing

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

Change 641944 merged by jenkins-bot:
[mediawiki/core@master] phpunit: Switch SpecialPageAliasTest to integration tests

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

Change 644174 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[integration/quibble@master] Run phpunit-unit before MediaWiki install

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

Change 636430 abandoned by Kosta Harlan:
[integration/quibble@master] Move LocalSettings.php out of the way before running unit tests

Reason:
We need to fix the bug referenced here in core; that said I took the idea to run phpunit-unit earlier and have a patch for that in I935004d8ed51d1c4a11c6db863ac182bd4d14b34

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

Change 644174 merged by jenkins-bot:
[integration/quibble@master] Run phpunit-unit before MediaWiki install

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

Change 654804 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/quibble@master] Release Quibble 0.0.46

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

Change 654804 merged by jenkins-bot:
[integration/quibble@master] Release Quibble 0.0.46

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

Mentioned in SAL (#wikimedia-releng) [2021-01-07T16:01:18Z] <hashar> Tag Quibble 0.0.46 @ df9e75329ab # T225218 T266441 T263500