Page MenuHomePhabricator

Failing CI test: Special:OAuthConsumerRegistration and Special:OAuthManageConsumers fatal on construction since ActorNormalization was made a proper service
Closed, ResolvedPublic

Description

Seen in https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-noselenium-docker/65004/console.

17:59:21 There were 2 errors:
17:59:21 
17:59:21 1) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set #201 ('OAuthConsumerRegistration')
17:59:21 TypeError: Argument 1 passed to SpecialPageExecutor::executeSpecialPage() must be an instance of SpecialPage, null given, called in /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php on line 46
17:59:21 
17:59:21 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:28
17:59:21 /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php:46
17:59:21 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:449
17:59:21 /workspace/src/maintenance/doMaintenance.php:106
17:59:21 === Logs generated by test case
17:59:21 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
17:59:21 [localisation] [debug] LocalisationCache using store LCStoreNull []
17:59:21 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
17:59:21 ===
17:59:21 
17:59:21 2) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set #202 ('OAuthManageConsumers')
17:59:21 TypeError: Argument 1 passed to SpecialPageExecutor::executeSpecialPage() must be an instance of SpecialPage, null given, called in /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php on line 46
17:59:21 
17:59:21 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:28
17:59:21 /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php:46
17:59:21 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:449
17:59:21 /workspace/src/maintenance/doMaintenance.php:106
17:59:21 === Logs generated by test case
17:59:21 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
17:59:21 [localisation] [debug] LocalisationCache using store LCStoreNull []
17:59:21 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
17:59:21 ===
17:59:21 
17:59:21 ERRORS!

git bisect blames Make ActorNormalization a proper service, which changed the way SpecialPageFatalTest constructs its special pages.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

So far I’ve only seen this in the WikibaseManifest extension, which seems a bit strange.

(I can reproduce it locally without having WikibaseManifest installed.)

Hm... This patch indeed touches that test, the idea there is to decrease the amount of MWServices initialized in dataProvider - that's executed before the tests are setup, so that's no good..

Okay, so what happens is that OAuth only registers those two special pages if ( Utils::isCentralWiki() ), and that function looks like this:

public static function isCentralWiki() {
	global $wgMWOAuthCentralWiki;

	return ( wfWikiId() === $wgMWOAuthCentralWiki );
}

And wfWikiId(), in turn, is:

function wfWikiID() {
	global $wgDBprefix, $wgDBname;

	if ( $wgDBprefix ) {
		return "$wgDBname-$wgDBprefix";
	} else {
		return $wgDBname;
	}
}

In the data provider, wfWikiId() is whatever your normal wiki ID is, and so it’ll probably equal $wgMWOAuthCentralWiki. But in the test proper, the $wgDBprefix will have been customized by MediaWikiIntegrationTest, and so the wiki ID will now be different, and no longer equal $wgMWOAuthCentralWiki. This wasn’t a problem before – I assume it meant that those two special pages silently were never tested by SpecialPageFatalTest – but now it’s a problem because they were registered at the time the data provider ran, but not afterwards, and so the test will try to create them but can’t.

I’m not sure if anything should be fixed in OAuth, but I suppose a good fix in MediaWiki core in the meantime is to make SpecialPageFatalTest check if it was able to create the special page, and otherwise mark the test skipped. (The test is meant to check that the page executes without failure, so I think failure to create the special page can be considered acceptable.)

Change 666718 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/core@master] SpecialPageFatalTest: tolerate failure to create page

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

So far I’ve only seen this in the WikibaseManifest extension, which seems a bit strange.

The explanation for this must be that WikibaseManifest is one of few extensions that pulls in OAuth in CI. (The others are OAuth itself and OAuthRateLimiter, as far as I can tell. Both haven’t had many changes recently where this issue would’ve come up – last l10n-bot change two weeks ago, last non-l10n-bot change (LibUp) four weeks ago.)

Change 666718 merged by jenkins-bot:
[mediawiki/core@master] SpecialPageFatalTest: tolerate failure to create page

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

Jdforrester-WMF renamed this task from Failing CI test: SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set #201 ('OAuthConsumerRegistration') – Argument 1 passed to SpecialPageExecutor::executeSpecialPage() must be an instance of SpecialPage, null given to Failing CI test: Special:OAuthConsumerRegistration and Special:OAuthManageConsumers fatal on construction since ActorNormalization was made a proper service.Feb 24 2021, 9:33 PM