Page MenuHomePhabricator

Static service container reference in ServiceWiring.php triggers test failures
Closed, ResolvedPublic

Description

Problem

The UserGroupManagerFactory service definition in ServiceWiring.php captures a static reference to the service Container in the cache invalidation callback function:

	'UserGroupManagerFactory' => static function ( MediaWikiServices $services ): UserGroupManagerFactory {
		return new UserGroupManagerFactory(
			new ServiceOptions(
				UserGroupManager::CONSTRUCTOR_OPTIONS, $services->getMainConfig()
			),
			$services->getReadOnlyMode(),
			$services->getDBLoadBalancerFactory(),
			$services->getHookContainer(),
			$services->getUserEditTracker(),
			$services->getGroupPermissionsLookup(),
			$services->getJobQueueGroupFactory(),
			LoggerFactory::getInstance( 'UserGroupManager' ),
			$services->getTempUserConfig(),
			[ static function ( UserIdentity $user ) use ( $services ) {
				if ( $user->getWikiId() === UserIdentity::LOCAL ) {
					$services->getPermissionManager()->invalidateUsersRightsCache( $user );
				}
				$services->getUserFactory()->invalidateCache( $user );
			} ]
		);
	},

During test suites that destroy and recreate the service container, this saved reference results in a ContainerDisabledException during some test runs:

Wikimedia\Services\ContainerDisabledException: Container disabled!

Steps to reproduce
In a Mediawiki checkout with the Flow extension installed.

  1. Copy phpunit.dist.xml to phpunit.xml
  2. Add a test suite with the following three tests:
<testsuite name="failing_group">
  <file>./tests/phpunit/structure/SpecialPageFatalTest.php</file>
  <file>./extensions/Flow/tests/phpunit/Api/ApiFlowReplyTest.php</file>
  <file>./extensions/Flow/tests/phpunit/PermissionsTest.php</file>
</testsuite>
  1. Run the named test suite:
mw docker mediawiki exec -- composer run phpunit:entrypoint -- --testsuite failing_group

Observed behaviour
The test run fails:

$ mw docker mediawiki exec -- composer run phpunit:entrypoint -- --testsuite failing_group
> phpunit '--testsuite' 'failing_group'
Using PHP 8.1.18
Running with MediaWiki settings because there might be integration tests
PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

..E............................................................  63 / 109 ( 57%)
..............................................                  109 / 109 (100%)

Time: 00:14.287, Memory: 131.00 MB

There was 1 error:

1) Flow\Tests\Api\ApiFlowReplyTest::testTopLevelReply
Wikimedia\Services\ContainerDisabledException: Container disabled!

/var/www/html/w/vendor/wikimedia/services/src/ServiceContainer.php:403
/var/www/html/w/includes/MediaWikiServices.php:356
/var/www/html/w/includes/MediaWikiServices.php:1662

Expected Behaviour
The tests should pass.

Event Timeline

I wasn't sure if this belongs as a Mediawiki issue or a Flow issue, but the "easy" fix I have for it is a patch to Mediawiki.

Change #1024339 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/core@master] Remove UserGroupManagerFactory static service container reference

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

Change #1024453 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/Flow@master] Consistently reset container in ApiTestCase::setUp

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

Change #1024339 abandoned by Arthur taylor:

[mediawiki/core@master] Remove UserGroupManagerFactory static service container reference

Reason:

Wrong approach to fix this issue

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

Change #1024453 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Consistently reset container in ApiTestCase::setUp

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

Change #1069283 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/Flow@REL1_42] Consistently reset container in ApiTestCase::setUp

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

Change #1069283 merged by jenkins-bot:

[mediawiki/extensions/Flow@REL1_42] Consistently reset container in ApiTestCase::setUp

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