From 63d7f2ad1345453dc16f16493fc7e5524219cfb8 Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Wed, 1 Aug 2018 15:40:47 +0300 Subject: [PATCH] Automatically reset namespace caches when needed This avoids error-prone code written separately in every test. In addition to no existing tests resetting the TitleFormatter (more services probably need to be reset as well), they mostly reset only the namespace cache on $wgContLang, which wouldn't help for any other language. The parser test runner still doesn't do this, but maybe it should. Change-Id: I44b7a1aec48f14b0950907fa14bd0df80f674296 --- languages/Language.php | 18 +++++++ tests/phpunit/MediaWikiTestCase.php | 26 ++++++++++ tests/phpunit/includes/EditPageTest.php | 33 ++++--------- tests/phpunit/includes/MessageTest.php | 4 +- tests/phpunit/includes/PagePropsTest.php | 35 +++++--------- tests/phpunit/includes/PrefixSearchTest.php | 9 +--- tests/phpunit/includes/RevisionDbTestBase.php | 16 ------- tests/phpunit/includes/TitleMethodsTest.php | 14 ------ .../phpunit/includes/api/ApiEditPageTest.php | 47 ++++++------------- .../phpunit/includes/auth/AuthManagerTest.php | 31 ++++++++++-- .../includes/content/ContentHandlerTest.php | 13 +---- tests/phpunit/languages/LanguageTest.php | 41 ++++++++++++++++ 12 files changed, 153 insertions(+), 134 deletions(-) diff --git a/languages/Language.php b/languages/Language.php index 7f04a6874e..453a610d42 100644 --- a/languages/Language.php +++ b/languages/Language.php @@ -244,6 +244,24 @@ class Language { throw new MWException( "Invalid fallback sequence for language '$code'" ); } + /** + * Intended for tests that may change configuration in a way that invalidates caches. + * + * @since 1.32 + */ + public static function clearCaches() { + if ( !defined( 'MW_PHPUNIT_TEST' ) ) { + throw new MWException( __METHOD__ . ' must not be used outside tests' ); + } + self::$dataCache = null; + // Reinitialize $dataCache, since it's expected to always be available + self::getLocalisationCache(); + self::$mLangObjCache = []; + self::$fallbackLanguageCache = []; + self::$grammarTransformations = null; + self::$languageNameCache = null; + } + /** * Checks whether any localisation is available for that language tag * in MediaWiki (MessagesXx.php exists). diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index 0b7481b2a4..8aacdee51e 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -628,6 +628,12 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { foreach ( $this->mwGlobalsToUnset as $value ) { unset( $GLOBALS[$value] ); } + if ( + array_key_exists( 'wgExtraNamespaces', $this->mwGlobals ) || + in_array( 'wgExtraNamespaces', $this->mwGlobalsToUnset ) + ) { + $this->resetNamespaces(); + } $this->mwGlobals = []; $this->mwGlobalsToUnset = []; $this->restoreLoggers(); @@ -745,6 +751,26 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { foreach ( $pairs as $key => $value ) { $GLOBALS[$key] = $value; } + + if ( array_key_exists( 'wgExtraNamespaces', $pairs ) ) { + $this->resetNamespaces(); + } + } + + /** + * Must be called whenever namespaces are changed, e.g., $wgExtraNamespaces is altered. + * Otherwise old namespace data will lurk and cause bugs. + */ + private function resetNamespaces() { + MWNamespace::clearCaches(); + Language::clearCaches(); + + // We can't have the TitleFormatter holding on to an old Language object either + // @todo We shouldn't need to reset all the aliases here. + $services = MediaWikiServices::getInstance(); + $services->resetServiceForTesting( 'TitleFormatter' ); + $services->resetServiceForTesting( 'TitleParser' ); + $services->resetServiceForTesting( '_MediaWikiTitleCodec' ); } /** diff --git a/tests/phpunit/includes/EditPageTest.php b/tests/phpunit/includes/EditPageTest.php index 036b61826e..216d92c01e 100644 --- a/tests/phpunit/includes/EditPageTest.php +++ b/tests/phpunit/includes/EditPageTest.php @@ -12,34 +12,19 @@ class EditPageTest extends MediaWikiLangTestCase { protected function setUp() { - global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang; - parent::setUp(); - $this->setContentLang( $wgContLang ); - $this->setMwGlobals( [ - 'wgExtraNamespaces' => $wgExtraNamespaces, - 'wgNamespaceContentModels' => $wgNamespaceContentModels, - 'wgContentHandlers' => $wgContentHandlers, + 'wgExtraNamespaces' => [ + 12312 => 'Dummy', + 12313 => 'Dummy_talk', + ], + 'wgNamespaceContentModels' => [ 12312 => 'testing' ], ] ); - - $wgExtraNamespaces[12312] = 'Dummy'; - $wgExtraNamespaces[12313] = 'Dummy_talk'; - - $wgNamespaceContentModels[12312] = "testing"; - $wgContentHandlers["testing"] = 'DummyContentHandlerForTesting'; - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache - } - - protected function tearDown() { - global $wgContLang; - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache - parent::tearDown(); + $this->mergeMwGlobalArrayValue( + 'wgContentHandlers', + [ 'testing' => 'DummyContentHandlerForTesting' ] + ); } /** diff --git a/tests/phpunit/includes/MessageTest.php b/tests/phpunit/includes/MessageTest.php index 70f4af9e78..3e3d04aeed 100644 --- a/tests/phpunit/includes/MessageTest.php +++ b/tests/phpunit/includes/MessageTest.php @@ -26,7 +26,7 @@ class MessageTest extends MediaWikiLangTestCase { $this->assertSame( $key, $message->getKey() ); $this->assertSame( $params, $message->getParams() ); - $this->assertEquals( $expectedLang, $message->getLanguage() ); + $this->assertSame( $expectedLang->getCode(), $message->getLanguage()->getCode() ); $messageSpecifier = $this->getMockForAbstractClass( MessageSpecifier::class ); $messageSpecifier->expects( $this->any() ) @@ -37,7 +37,7 @@ class MessageTest extends MediaWikiLangTestCase { $this->assertSame( $key, $message->getKey() ); $this->assertSame( $params, $message->getParams() ); - $this->assertEquals( $expectedLang, $message->getLanguage() ); + $this->assertSame( $expectedLang->getCode(), $message->getLanguage()->getCode() ); } public static function provideConstructor() { diff --git a/tests/phpunit/includes/PagePropsTest.php b/tests/phpunit/includes/PagePropsTest.php index f602cdabcf..646b487691 100644 --- a/tests/phpunit/includes/PagePropsTest.php +++ b/tests/phpunit/includes/PagePropsTest.php @@ -27,18 +27,20 @@ class PagePropsTest extends MediaWikiLangTestCase { private $the_properties; protected function setUp() { - global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang; - parent::setUp(); - $wgExtraNamespaces[12312] = 'Dummy'; - $wgExtraNamespaces[12313] = 'Dummy_talk'; - - $wgNamespaceContentModels[12312] = 'DUMMY'; - $wgContentHandlers['DUMMY'] = 'DummyContentHandlerForTesting'; + $this->setMwGlobals( [ + 'wgExtraNamespaces' => [ + 12312 => 'Dummy', + 12313 => 'Dummy_talk', + ], + 'wgNamespaceContentModels' => [ 12312 => 'DUMMY' ], + ] ); - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache + $this->mergeMwGlobalArrayValue( + 'wgContentHandlers', + [ 'DUMMY' => 'DummyContentHandlerForTesting' ] + ); if ( !$this->the_properties ) { $this->the_properties = [ @@ -72,21 +74,6 @@ class PagePropsTest extends MediaWikiLangTestCase { } } - protected function tearDown() { - global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang; - - parent::tearDown(); - - unset( $wgExtraNamespaces[12312] ); - unset( $wgExtraNamespaces[12313] ); - - unset( $wgNamespaceContentModels[12312] ); - unset( $wgContentHandlers['DUMMY'] ); - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache - } - /** * Test getting a single property from a single page. The property was * set in setUp(). diff --git a/tests/phpunit/includes/PrefixSearchTest.php b/tests/phpunit/includes/PrefixSearchTest.php index 3bde5de41f..98ec7e027c 100644 --- a/tests/phpunit/includes/PrefixSearchTest.php +++ b/tests/phpunit/includes/PrefixSearchTest.php @@ -50,7 +50,7 @@ class PrefixSearchTest extends MediaWikiLangTestCase { $this->markTestSkipped( 'Main namespace does not support wikitext.' ); } - // Avoid special pages from extensions interferring with the tests + // Avoid special pages from extensions interfering with the tests $this->setMwGlobals( [ 'wgSpecialPages' => [], 'wgHooks' => [], @@ -61,17 +61,10 @@ class PrefixSearchTest extends MediaWikiLangTestCase { $this->originalHandlers = TestingAccessWrapper::newFromClass( Hooks::class )->handlers; TestingAccessWrapper::newFromClass( Hooks::class )->handlers = []; - // Clear caches so that our new namespace appears - MWNamespace::clearCaches(); - Language::factory( 'en' )->resetNamespaces(); - SpecialPageFactory::resetList(); } public function tearDown() { - MWNamespace::clearCaches(); - Language::factory( 'en' )->resetNamespaces(); - parent::tearDown(); TestingAccessWrapper::newFromClass( Hooks::class )->handlers = $this->originalHandlers; diff --git a/tests/phpunit/includes/RevisionDbTestBase.php b/tests/phpunit/includes/RevisionDbTestBase.php index e17f855fee..57b42f6a4a 100644 --- a/tests/phpunit/includes/RevisionDbTestBase.php +++ b/tests/phpunit/includes/RevisionDbTestBase.php @@ -58,8 +58,6 @@ abstract class RevisionDbTestBase extends MediaWikiTestCase { abstract protected function getMcrTablesToReset(); protected function setUp() { - global $wgContLang; - $this->tablesUsed += $this->getMcrTablesToReset(); parent::setUp(); @@ -93,10 +91,6 @@ abstract class RevisionDbTestBase extends MediaWikiTestCase { $this->getMcrMigrationStage() ); - MWNamespace::clearCaches(); - // Reset namespace cache - $wgContLang->resetNamespaces(); - $this->overrideMwServices(); if ( !$this->testPage ) { @@ -108,16 +102,6 @@ abstract class RevisionDbTestBase extends MediaWikiTestCase { } } - protected function tearDown() { - global $wgContLang; - - parent::tearDown(); - - MWNamespace::clearCaches(); - // Reset namespace cache - $wgContLang->resetNamespaces(); - } - abstract protected function getContentHandlerUseDB(); private function makeRevisionWithProps( $props = null ) { diff --git a/tests/phpunit/includes/TitleMethodsTest.php b/tests/phpunit/includes/TitleMethodsTest.php index e898c63124..715d46914a 100644 --- a/tests/phpunit/includes/TitleMethodsTest.php +++ b/tests/phpunit/includes/TitleMethodsTest.php @@ -12,8 +12,6 @@ use MediaWiki\MediaWikiServices; class TitleMethodsTest extends MediaWikiLangTestCase { protected function setUp() { - global $wgContLang; - parent::setUp(); $this->mergeMwGlobalArrayValue( @@ -30,18 +28,6 @@ class TitleMethodsTest extends MediaWikiLangTestCase { 12302 => CONTENT_MODEL_JAVASCRIPT, ] ); - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache - } - - protected function tearDown() { - global $wgContLang; - - parent::tearDown(); - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache } public static function provideEquals() { diff --git a/tests/phpunit/includes/api/ApiEditPageTest.php b/tests/phpunit/includes/api/ApiEditPageTest.php index 4febb46f1c..4c276d6273 100644 --- a/tests/phpunit/includes/api/ApiEditPageTest.php +++ b/tests/phpunit/includes/api/ApiEditPageTest.php @@ -14,42 +14,25 @@ class ApiEditPageTest extends ApiTestCase { protected function setUp() { - global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang; - parent::setUp(); - $this->setContentLang( $wgContLang ); - $this->setMwGlobals( [ - 'wgExtraNamespaces' => $wgExtraNamespaces, - 'wgNamespaceContentModels' => $wgNamespaceContentModels, - 'wgContentHandlers' => $wgContentHandlers, + 'wgExtraNamespaces' => [ + 12312 => 'Dummy', + 12313 => 'Dummy_talk', + 12314 => 'DummyNonText', + 12315 => 'DummyNonText_talk', + ], + 'wgNamespaceContentModels' => [ + 12312 => 'testing', + 12314 => 'testing-nontext', + ], + ] ); + $this->mergeMwGlobalArrayValue( 'wgContentHandlers', [ + 'testing' => 'DummyContentHandlerForTesting', + 'testing-nontext' => 'DummyNonTextContentHandler', + 'testing-serialize-error' => 'DummySerializeErrorContentHandler', ] ); - - $wgExtraNamespaces[12312] = 'Dummy'; - $wgExtraNamespaces[12313] = 'Dummy_talk'; - $wgExtraNamespaces[12314] = 'DummyNonText'; - $wgExtraNamespaces[12315] = 'DummyNonText_talk'; - - $wgNamespaceContentModels[12312] = "testing"; - $wgNamespaceContentModels[12314] = "testing-nontext"; - - $wgContentHandlers["testing"] = 'DummyContentHandlerForTesting'; - $wgContentHandlers["testing-nontext"] = 'DummyNonTextContentHandler'; - $wgContentHandlers["testing-serialize-error"] = - 'DummySerializeErrorContentHandler'; - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache - } - - protected function tearDown() { - global $wgContLang; - - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); # reset namespace cache - - parent::tearDown(); } public function testEdit() { diff --git a/tests/phpunit/includes/auth/AuthManagerTest.php b/tests/phpunit/includes/auth/AuthManagerTest.php index d14ad59e92..7170e557fe 100644 --- a/tests/phpunit/includes/auth/AuthManagerTest.php +++ b/tests/phpunit/includes/auth/AuthManagerTest.php @@ -82,6 +82,31 @@ class AuthManagerTest extends \MediaWikiTestCase { return new \Message( $key, $params, \Language::factory( 'en' ) ); } + /** + * Test two AuthenticationResponses for equality. We don't want to use regular assertEquals + * because that recursively compares members, which leads to false negatives if e.g. Language + * caches are reset. + * + * @param AuthenticationResponse $response1 + * @param AuthenticationResponse $response2 + * @param string $msg + * @return bool + */ + private function assertResponseEquals( + AuthenticationResponse $expected, AuthenticationResponse $actual, $msg = '' + ) { + foreach ( ( new \ReflectionClass( $expected ) )->getProperties() as $prop ) { + $name = $prop->getName(); + $usedMsg = ltrim( "$msg ($name)" ); + if ( $name === 'message' && $expected->message ) { + $this->assertSame( $expected->message->serialize(), $actual->message->serialize(), + $usedMsg ); + } else { + $this->assertEquals( $expected->$name, $actual->$name, $usedMsg ); + } + } + } + /** * Initialize the AuthManagerConfig variable in $this->config * @@ -1030,7 +1055,7 @@ class AuthManagerTest extends \MediaWikiTestCase { $this->assertSame( 'http://localhost/', $req->returnToUrl ); $ret->message = $this->message( $ret->message ); - $this->assertEquals( $response, $ret, "Response $i, response" ); + $this->assertResponseEquals( $response, $ret, "Response $i, response" ); if ( $success ) { $this->assertSame( $id, $session->getUser()->getId(), "Response $i, authn" ); @@ -2082,7 +2107,7 @@ class AuthManagerTest extends \MediaWikiTestCase { "Response $i, login marker" ); } $ret->message = $this->message( $ret->message ); - $this->assertEquals( $response, $ret, "Response $i, response" ); + $this->assertResponseEquals( $response, $ret, "Response $i, response" ); if ( $success || $response->status === AuthenticationResponse::FAIL ) { $this->assertNull( $this->request->getSession()->getSecret( 'AuthManager::accountCreationState' ), @@ -3517,7 +3542,7 @@ class AuthManagerTest extends \MediaWikiTestCase { $this->assertSame( 'http://localhost/', $req->returnToUrl ); $ret->message = $this->message( $ret->message ); - $this->assertEquals( $response, $ret, "Response $i, response" ); + $this->assertResponseEquals( $response, $ret, "Response $i, response" ); if ( $response->status === AuthenticationResponse::PASS || $response->status === AuthenticationResponse::FAIL ) { diff --git a/tests/phpunit/includes/content/ContentHandlerTest.php b/tests/phpunit/includes/content/ContentHandlerTest.php index 7c63105b97..323a63d788 100644 --- a/tests/phpunit/includes/content/ContentHandlerTest.php +++ b/tests/phpunit/includes/content/ContentHandlerTest.php @@ -8,7 +8,6 @@ use MediaWiki\MediaWikiServices; class ContentHandlerTest extends MediaWikiTestCase { protected function setUp() { - global $wgContLang; parent::setUp(); $this->setMwGlobals( [ @@ -34,20 +33,12 @@ class ContentHandlerTest extends MediaWikiTestCase { ], ] ); - // Reset namespace cache - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); - // And LinkCache + // Reset LinkCache MediaWikiServices::getInstance()->resetServiceForTesting( 'LinkCache' ); } protected function tearDown() { - global $wgContLang; - - // Reset namespace cache - MWNamespace::clearCaches(); - $wgContLang->resetNamespaces(); - // And LinkCache + // Reset LinkCache MediaWikiServices::getInstance()->resetServiceForTesting( 'LinkCache' ); parent::tearDown(); diff --git a/tests/phpunit/languages/LanguageTest.php b/tests/phpunit/languages/LanguageTest.php index 35bb1f057f..f99bc70a8d 100644 --- a/tests/phpunit/languages/LanguageTest.php +++ b/tests/phpunit/languages/LanguageTest.php @@ -1,5 +1,7 @@ assertEquals( "a{$c}b{$c}c{$and}{$s}d", $lang->listToText( [ 'a', 'b', 'c', 'd' ] ) ); } + /** + * @covers Language::clearCaches + */ + public function testClearCaches() { + $languageClass = TestingAccessWrapper::newFromClass( Language::class ); + + // Populate $dataCache + Language::getLocalisationCache()->getItem( 'zh', 'mainpage' ); + $oldCacheObj = Language::$dataCache; + $this->assertNotCount( 0, + TestingAccessWrapper::newFromObject( Language::$dataCache )->loadedItems ); + + // Populate $mLangObjCache + $lang = Language::factory( 'en' ); + $this->assertNotCount( 0, Language::$mLangObjCache ); + + // Populate $fallbackLanguageCache + Language::getFallbacksIncludingSiteLanguage( 'en' ); + $this->assertNotCount( 0, $languageClass->fallbackLanguageCache ); + + // Populate $grammarTransformations + $lang->getGrammarTransformations(); + $this->assertNotNull( $languageClass->grammarTransformations ); + + // Populate $languageNameCache + Language::fetchLanguageNames(); + $this->assertNotNull( $languageClass->languageNameCache ); + + Language::clearCaches(); + + $this->assertNotSame( $oldCacheObj, Language::$dataCache ); + $this->assertCount( 0, + TestingAccessWrapper::newFromObject( Language::$dataCache )->loadedItems ); + $this->assertCount( 0, Language::$mLangObjCache ); + $this->assertCount( 0, $languageClass->fallbackLanguageCache ); + $this->assertNull( $languageClass->grammarTransformations ); + $this->assertNull( $languageClass->languageNameCache ); + } + /** * @dataProvider provideIsSupportedLanguage * @covers Language::isSupportedLanguage -- 2.20.1