From 853a291903dc6d21c3d1e92400c2d205a5537958 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Wed, 27 Mar 2019 20:23:02 +0000 Subject: [PATCH] resourceloader: Improve test cases for MessageBlobStore Move source code to includes/resourceloader to match test case. This is part of ResourceLoader and not meant to be used elsewhere. Merge two similar test cases for getting blobs and fetching messages which were doing the same thing. Rewrite the test names to be a better reflection of the stories they test, add comments for why, and re-order them to put related tests together. Move test-utilities to the bottom and make them actually private. Change-Id: I7a437eebf3ba6a722e286dfe77c2f9fe49ad222f --- autoload.php | 2 +- .../MessageBlobStore.php | 0 .../resourceloader/MessageBlobStoreTest.php | 214 ++++++++---------- 3 files changed, 91 insertions(+), 125 deletions(-) rename includes/{cache => resourceloader}/MessageBlobStore.php (100%) diff --git a/autoload.php b/autoload.php index 528b7fe372..16236ed2a5 100644 --- a/autoload.php +++ b/autoload.php @@ -974,7 +974,7 @@ $wgAutoloadLocalClasses = [ 'MergeMessageFileList' => __DIR__ . '/maintenance/mergeMessageFileList.php', 'MergeableUpdate' => __DIR__ . '/includes/deferred/MergeableUpdate.php', 'Message' => __DIR__ . '/includes/Message.php', - 'MessageBlobStore' => __DIR__ . '/includes/cache/MessageBlobStore.php', + 'MessageBlobStore' => __DIR__ . '/includes/resourceloader/MessageBlobStore.php', 'MessageCache' => __DIR__ . '/includes/cache/MessageCache.php', 'MessageCacheUpdate' => __DIR__ . '/includes/deferred/MessageCacheUpdate.php', 'MessageContent' => __DIR__ . '/includes/content/MessageContent.php', diff --git a/includes/cache/MessageBlobStore.php b/includes/resourceloader/MessageBlobStore.php similarity index 100% rename from includes/cache/MessageBlobStore.php rename to includes/resourceloader/MessageBlobStore.php diff --git a/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php b/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php index 70bf39f75d..e57764306e 100644 --- a/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php +++ b/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php @@ -3,7 +3,7 @@ use Wikimedia\TestingAccessWrapper; /** - * @group Cache + * @group ResourceLoader * @covers MessageBlobStore */ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase { @@ -13,64 +13,17 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase { protected function setUp() { parent::setUp(); - // MediaWiki tests defaults $wgMainWANCache to CACHE_NONE. - // Use hash instead so that caching is observed - $this->wanCache = $this->getMockBuilder( WANObjectCache::class ) - ->setConstructorArgs( [ [ - 'cache' => new HashBagOStuff(), - 'pool' => 'test', - 'relayer' => new EventRelayerNull( [] ) - ] ] ) - ->setMethods( [ 'makePurgeValue' ] ) - ->getMock(); - - $this->wanCache->expects( $this->any() ) - ->method( 'makePurgeValue' ) - ->will( $this->returnCallback( function ( $timestamp, $holdoff ) { - // Disable holdoff as it messes with testing. Aside from a 0-second holdoff, - // make sure that "time" passes between getMulti() check init and the set() - // in recacheMessageBlob(). This especially matters for Windows clocks. - $ts = (float)$timestamp - 0.0001; - - return WANObjectCache::PURGE_VAL_PREFIX . $ts . ':0'; - } ) ); - } - - protected function makeBlobStore( $methods = null, $rl = null ) { - $blobStore = $this->getMockBuilder( MessageBlobStore::class ) - ->setConstructorArgs( [ $rl ?? $this->createMock( ResourceLoader::class ) ] ) - ->setMethods( $methods ) - ->getMock(); - - $access = TestingAccessWrapper::newFromObject( $blobStore ); - $access->wanCache = $this->wanCache; - return $blobStore; - } - - protected function makeModule( array $messages ) { - $module = new ResourceLoaderTestModule( [ 'messages' => $messages ] ); - $module->setName( 'test.blobstore' ); - return $module; - } - - /** @covers MessageBlobStore::setLogger */ - public function testSetLogger() { - $blobStore = $this->makeBlobStore(); - $this->assertSame( null, $blobStore->setLogger( new Psr\Log\NullLogger() ) ); + // MediaWiki's test wrapper sets $wgMainWANCache to CACHE_NONE. + // Use HashBagOStuff here so that we can observe caching. + $this->wanCache = new WANObjectCache( [ + 'cache' => new HashBagOStuff() + ] ); + + $this->clock = 1301655600.000; + $this->wanCache->setMockTime( $this->clock ); } - /** @covers MessageBlobStore::getResourceLoader */ - public function testGetResourceLoader() { - // Call protected method - $blobStore = TestingAccessWrapper::newFromObject( $this->makeBlobStore() ); - $this->assertInstanceOf( - ResourceLoader::class, - $blobStore->getResourceLoader() - ); - } - - /** @covers MessageBlobStore::fetchMessage */ - public function testFetchMessage() { + public function testBlobCreation() { $module = $this->makeModule( [ 'mainpage' ] ); $rl = new ResourceLoader(); $rl->register( $module->getName(), $module ); @@ -81,140 +34,153 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase { $this->assertEquals( '{"mainpage":"Main Page"}', $blob, 'Generated blob' ); } - /** @covers MessageBlobStore::fetchMessage */ - public function testFetchMessageFail() { + public function testBlobCreation_unknownMessage() { $module = $this->makeModule( [ 'i-dont-exist' ] ); $rl = new ResourceLoader(); $rl->register( $module->getName(), $module ); - $blobStore = $this->makeBlobStore( null, $rl ); - $blob = $blobStore->getBlob( $module, 'en' ); + // Generating a blob should succeed without errors, + // even if a message is unknown. + $blob = $blobStore->getBlob( $module, 'en' ); $this->assertEquals( '{"i-dont-exist":"\u29fci-dont-exist\u29fd"}', $blob, 'Generated blob' ); } - public function testGetBlob() { - $module = $this->makeModule( [ 'foo' ] ); + public function testMessageCachingAndPurging() { + $module = $this->makeModule( [ 'example' ] ); $rl = new ResourceLoader(); $rl->register( $module->getName(), $module ); - $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); + + // Advance this new WANObjectCache instance to a normal state, + // by doing one "get" and letting its hold off period expire. + // Without this, the first real "get" would lazy-initialise the + // checkKey and thus reject the first "set". + $blobStore->getBlob( $module, 'en' ); + $this->clock += 20; + + // Arrange version 1 of a message $blobStore->expects( $this->once() ) ->method( 'fetchMessage' ) - ->will( $this->returnValue( 'Example' ) ); + ->will( $this->returnValue( 'First version' ) ); + // Assert $blob = $blobStore->getBlob( $module, 'en' ); + $this->assertEquals( '{"example":"First version"}', $blob, 'Blob for v1' ); - $this->assertEquals( '{"foo":"Example"}', $blob, 'Generated blob' ); - } - - public function testGetBlobCached() { - $module = $this->makeModule( [ 'example' ] ); - $rl = new ResourceLoader(); - $rl->register( $module->getName(), $module ); - + // Arrange version 2 $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); $blobStore->expects( $this->once() ) ->method( 'fetchMessage' ) - ->will( $this->returnValue( 'First' ) ); + ->will( $this->returnValue( 'Second version' ) ); + $this->clock += 20; - $module = $this->makeModule( [ 'example' ] ); + // Assert + // We do not validate whether a cached message is up-to-date. + // Instead, changes to messages will send us a purge. + // When cache is not purged or expired, it must be used. $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"First"}', $blob, 'Generated blob' ); + $this->assertEquals( '{"example":"First version"}', $blob, 'Reuse cached v1 blob' ); - $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); - $blobStore->expects( $this->never() ) - ->method( 'fetchMessage' ) - ->will( $this->returnValue( 'Second' ) ); + // Purge cache + $blobStore->updateMessage( 'example' ); + $this->clock += 20; - $module = $this->makeModule( [ 'example' ] ); + // Assert $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"First"}', $blob, 'Cache hit' ); + $this->assertEquals( '{"example":"Second version"}', $blob, 'Updated blob for v2' ); } - public function testUpdateMessage() { + public function testPurgeEverything() { $module = $this->makeModule( [ 'example' ] ); $rl = new ResourceLoader(); $rl->register( $module->getName(), $module ); $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); - $blobStore->expects( $this->once() ) + // Advance this new WANObjectCache instance to a normal state. + $blobStore->getBlob( $module, 'en' ); + $this->clock += 20; + + // Arrange version 1 and 2 + $blobStore->expects( $this->exactly( 2 ) ) ->method( 'fetchMessage' ) - ->will( $this->returnValue( 'First' ) ); + ->will( $this->onConsecutiveCalls( 'First', 'Second' ) ); + // Assert $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"First"}', $blob, 'Generated blob' ); + $this->assertEquals( '{"example":"First"}', $blob, 'Blob for v1' ); - $blobStore->updateMessage( 'example' ); + $this->clock += 20; - $module = $this->makeModule( [ 'example' ] ); - $rl = new ResourceLoader(); - $rl->register( $module->getName(), $module ); - $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); - $blobStore->expects( $this->once() ) - ->method( 'fetchMessage' ) - ->will( $this->returnValue( 'Second' ) ); + // Assert + $blob = $blobStore->getBlob( $module, 'en' ); + $this->assertEquals( '{"example":"First"}', $blob, 'Blob for v1 again' ); + + // Purge everything + $blobStore->clear(); + $this->clock += 20; + // Assert $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"Second"}', $blob, 'Updated blob' ); + $this->assertEquals( '{"example":"Second"}', $blob, 'Blob for v2' ); } - public function testValidation() { + public function testValidateAgainstModuleRegistry() { + // Arrange version 1 of a module $module = $this->makeModule( [ 'foo' ] ); $rl = new ResourceLoader(); $rl->register( $module->getName(), $module ); - $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); $blobStore->expects( $this->once() ) ->method( 'fetchMessage' ) ->will( $this->returnValueMap( [ + // message key, language code, message value [ 'foo', 'en', 'Hello' ], ] ) ); + // Assert $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"foo":"Hello"}', $blob, 'Generated blob' ); + $this->assertEquals( '{"foo":"Hello"}', $blob, 'Blob for v1' ); - // Now, imagine a change to the module is deployed. The module now contains - // message 'foo' and 'bar'. While updateMessage() was not called (since no - // message values were changed) it should detect the change in list of - // message keys. + // Arrange version 2 of module + // While message values may be out of date, the set of messages returned + // must always match the set of message keys required by the module. + // We do not receive purges for this because no messages were changed. $module = $this->makeModule( [ 'foo', 'bar' ] ); $rl = new ResourceLoader(); $rl->register( $module->getName(), $module ); - $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); $blobStore->expects( $this->exactly( 2 ) ) ->method( 'fetchMessage' ) ->will( $this->returnValueMap( [ + // message key, language code, message value [ 'foo', 'en', 'Hello' ], [ 'bar', 'en', 'World' ], ] ) ); + // Assert $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"foo":"Hello","bar":"World"}', $blob, 'Updated blob' ); + $this->assertEquals( '{"foo":"Hello","bar":"World"}', $blob, 'Blob for v2' ); } - public function testClear() { - $module = $this->makeModule( [ 'example' ] ); - $rl = new ResourceLoader(); - $rl->register( $module->getName(), $module ); - $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl ); - $blobStore->expects( $this->exactly( 2 ) ) - ->method( 'fetchMessage' ) - ->will( $this->onConsecutiveCalls( 'First', 'Second' ) ); - - $now = microtime( true ); - $this->wanCache->setMockTime( $now ); - - $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"First"}', $blob, 'Generated blob' ); + public function testSetLoggedIsVoid() { + $blobStore = $this->makeBlobStore(); + $this->assertSame( null, $blobStore->setLogger( new Psr\Log\NullLogger() ) ); + } - $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"First"}', $blob, 'Cache-hit' ); + private function makeBlobStore( $methods = null, $rl = null ) { + $blobStore = $this->getMockBuilder( MessageBlobStore::class ) + ->setConstructorArgs( [ $rl ?? $this->createMock( ResourceLoader::class ) ] ) + ->setMethods( $methods ) + ->getMock(); - $now += 1; - $blobStore->clear(); + $access = TestingAccessWrapper::newFromObject( $blobStore ); + $access->wanCache = $this->wanCache; + return $blobStore; + } - $blob = $blobStore->getBlob( $module, 'en' ); - $this->assertEquals( '{"example":"Second"}', $blob, 'Updated blob' ); + private function makeModule( array $messages ) { + $module = new ResourceLoaderTestModule( [ 'messages' => $messages ] ); + $module->setName( 'test.blobstore' ); + return $module; } } -- 2.20.1