From: Timo Tijhof Date: Sat, 13 Jul 2019 18:27:33 +0000 (+0100) Subject: resourceloader: Clean up ResourceLoaderWikiModuleTest X-Git-Tag: 1.34.0-rc.0~993^2 X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/journal.php?a=commitdiff_plain;h=397a3ee0bdf98abdba253e4a66949f83daa97172;p=lhc%2Fweb%2Fwiklou.git resourceloader: Clean up ResourceLoaderWikiModuleTest * Remove redundant any() calls. * Use willReturn() instead of will(returnValue()). * Use yield for providers and add missing test case descriptions. * Use createMock() instead of wfGetDB() where the DB isn't needed. * For provideIsKnownEmpty, re-order the cases and add a few extra cases, and document why they behave the way they do. Change-Id: Iba42325a55bb3dfc50a8d2af46e1ddba8dda885a --- diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php index e8a0884b63..6ac037795c 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php @@ -4,10 +4,12 @@ use MediaWiki\MediaWikiServices; use Wikimedia\Rdbms\IDatabase; use Wikimedia\TestingAccessWrapper; +/** + * @covers ResourceLoaderWikiModule + */ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { /** - * @covers ResourceLoaderWikiModule::__construct * @dataProvider provideConstructor */ public function testConstructor( $params ) { @@ -15,6 +17,13 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { $this->assertInstanceOf( ResourceLoaderWikiModule::class, $module ); } + public static function provideConstructor() { + yield 'null' => [ null ]; + yield 'empty' => [ [] ]; + yield 'unknown settings' => [ [ 'foo' => 'baz' ] ]; + yield 'real settings' => [ [ 'MediaWiki:Common.js' ] ]; + } + private function prepareTitleInfo( array $mockInfo ) { $module = TestingAccessWrapper::newFromClass( ResourceLoaderWikiModule::class ); $info = []; @@ -24,21 +33,8 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { return $info; } - public static function provideConstructor() { - return [ - // Nothing - [ null ], - [ [] ], - // Unrecognized settings - [ [ 'foo' => 'baz' ] ], - // Real settings - [ [ 'scripts' => [ 'MediaWiki:Common.js' ] ] ], - ]; - } - /** * @dataProvider provideGetPages - * @covers ResourceLoaderWikiModule::getPages */ public function testGetPages( $params, Config $config, $expected ) { $module = new ResourceLoaderWikiModule( $params ); @@ -48,7 +44,7 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { $getPages = new ReflectionMethod( $module, 'getPages' ); $getPages->setAccessible( true ); $out = $getPages->invoke( $module, ResourceLoaderContext::newDummyContext() ); - $this->assertEquals( $expected, $out ); + $this->assertSame( $expected, $out ); } public static function provideGetPages() { @@ -84,98 +80,104 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { } /** - * @covers ResourceLoaderWikiModule::getGroup * @dataProvider provideGetGroup */ public function testGetGroup( $params, $expected ) { $module = new ResourceLoaderWikiModule( $params ); - $this->assertEquals( $expected, $module->getGroup() ); + $this->assertSame( $expected, $module->getGroup() ); } public static function provideGetGroup() { - return [ - // No group specified - [ [], null ], - // A random group - [ [ 'group' => 'foobar' ], 'foobar' ], - ]; + yield 'no group' => [ [], null ]; + yield 'some group' => [ [ 'group' => 'foobar' ], 'foobar' ]; } /** - * @covers ResourceLoaderWikiModule::isKnownEmpty * @dataProvider provideIsKnownEmpty */ public function testIsKnownEmpty( $titleInfo, $group, $dependencies, $expected ) { $module = $this->getMockBuilder( ResourceLoaderWikiModule::class ) - ->setMethods( [ 'getTitleInfo', 'getGroup', 'getDependencies' ] ) - ->getMock(); - $module->expects( $this->any() ) - ->method( 'getTitleInfo' ) - ->will( $this->returnValue( $this->prepareTitleInfo( $titleInfo ) ) ); - $module->expects( $this->any() ) - ->method( 'getGroup' ) - ->will( $this->returnValue( $group ) ); - $module->expects( $this->any() ) - ->method( 'getDependencies' ) - ->will( $this->returnValue( $dependencies ) ); - $context = $this->getMockBuilder( ResourceLoaderContext::class ) ->disableOriginalConstructor() + ->setMethods( [ 'getTitleInfo', 'getGroup', 'getDependencies' ] ) ->getMock(); - $this->assertEquals( $expected, $module->isKnownEmpty( $context ) ); + $module->method( 'getTitleInfo' ) + ->willReturn( $this->prepareTitleInfo( $titleInfo ) ); + $module->method( 'getGroup' ) + ->willReturn( $group ); + $module->method( 'getDependencies' ) + ->willReturn( $dependencies ); + $context = $this->createMock( ResourceLoaderContext::class ); + $this->assertSame( $expected, $module->isKnownEmpty( $context ) ); } public static function provideIsKnownEmpty() { - return [ - // No valid pages - [ [], 'test1', [], true ], - // 'site' module with a non-empty page - [ - [ 'MediaWiki:Common.js' => [ 'page_len' => 1234 ] ], - 'site', - [], - false, - ], - // 'site' module without existing pages but dependencies - [ - [], - 'site', - [ 'mobile.css' ], - false, - ], - // 'site' module which is empty but has dependencies - [ - [ 'MediaWiki:Common.js' => [ 'page_len' => 0 ] ], - 'site', - [ 'mobile.css' ], - false, - ], - // 'site' module with an empty page - [ - [ 'MediaWiki:Foo.js' => [ 'page_len' => 0 ] ], - 'site', - [], - false, - ], - // 'user' module with a non-empty page - [ - [ 'User:Example/common.js' => [ 'page_len' => 25 ] ], - 'user', - [], - false, - ], - // 'user' module with an empty page - [ - [ 'User:Example/foo.js' => [ 'page_len' => 0 ] ], - 'user', - [], - true, - ], + yield 'nothing' => [ + [], + null, + [], + // No pages exist, considered empty. + true, + ]; + + yield 'an empty page exists (no group)' => [ + [ 'Project:Example/foo.js' => [ 'page_len' => 0 ] ], + null, + [], + // There is an existing page, so we should let the module be queued. + // Its emptiness might be temporary, hence considered non-empty (T70488). + false, + ]; + yield 'an empty page exists (site group)' => [ + [ 'MediaWiki:Foo.js' => [ 'page_len' => 0 ] ], + 'site', + [], + // There is an existing page, hence considered non-empty. + false, + ]; + yield 'an empty page exists (user group)' => [ + [ 'User:Example/foo.js' => [ 'page_len' => 0 ] ], + 'user', + [], + // There is an existing page, but it is empty. + // For user-specific modules, don't bother loading a known-empty module. + // Given user-specific HTML output, this will vary and re-appear if/when + // the page becomes non-empty again. + true, + ]; + + yield 'no pages but having dependencies (no group)' => [ + [], + null, + [ 'another-module' ], + false, + ]; + yield 'no pages but having dependencies (site group)' => [ + [], + 'site', + [ 'another-module' ], + false, + ]; + yield 'no pages but having dependencies (user group)' => [ + [], + 'user', + [ 'another-module' ], + false, + ]; + + yield 'a non-empty page exists (user group)' => [ + [ 'User:Example/foo.js' => [ 'page_len' => 25 ] ], + 'user', + [], + false, + ]; + yield 'a non-empty page exists (site group)' => [ + [ 'MediaWiki:Foo.js' => [ 'page_len' => 25 ] ], + 'site', + [], + false, ]; } - /** - * @covers ResourceLoaderWikiModule::getTitleInfo - */ public function testGetTitleInfo() { $pages = [ 'MediaWiki:Common.css' => [ 'type' => 'styles' ], @@ -187,26 +189,20 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { ] ); $expected = $titleInfo; - $module = $this->getMockBuilder( TestResourceLoaderWikiModule::class ) - ->setMethods( [ 'getPages' ] ) + $module = $this->getMockBuilder( ResourceLoaderWikiModule::class ) + ->setMethods( [ 'getPages', 'getTitleInfo' ] ) ->getMock(); $module->method( 'getPages' )->willReturn( $pages ); - // Can't mock static methods - $module::$returnFetchTitleInfo = $titleInfo; + $module->method( 'getTitleInfo' )->willReturn( $titleInfo ); $context = $this->getMockBuilder( ResourceLoaderContext::class ) ->disableOriginalConstructor() ->getMock(); $module = TestingAccessWrapper::newFromObject( $module ); - $this->assertEquals( $expected, $module->getTitleInfo( $context ), 'Title info' ); + $this->assertSame( $expected, $module->getTitleInfo( $context ), 'Title info' ); } - /** - * @covers ResourceLoaderWikiModule::getTitleInfo - * @covers ResourceLoaderWikiModule::setTitleInfo - * @covers ResourceLoaderWikiModule::preloadTitleInfo - */ public function testGetPreloadedTitleInfo() { $pages = [ 'MediaWiki:Common.css' => [ 'type' => 'styles' ], @@ -240,17 +236,14 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { ); TestResourceLoaderWikiModule::preloadTitleInfo( $context, - wfGetDB( DB_REPLICA ), + $this->createMock( IDatabase::class ), [ 'testmodule' ] ); $module = TestingAccessWrapper::newFromObject( $module ); - $this->assertEquals( $expected, $module->getTitleInfo( $context ), 'Title info' ); + $this->assertSame( $expected, $module->getTitleInfo( $context ), 'Title info' ); } - /** - * @covers ResourceLoaderWikiModule::preloadTitleInfo - */ public function testGetPreloadedBadTitle() { // Set up TestResourceLoaderWikiModule::$returnFetchTitleInfo = []; @@ -267,7 +260,7 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { // Act TestResourceLoaderWikiModule::preloadTitleInfo( $context, - wfGetDB( DB_REPLICA ), + $this->createMock( IDatabase::class ), [ 'testmodule' ] ); @@ -276,58 +269,52 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { $this->assertSame( [], $module->getTitleInfo( $context ), 'Title info' ); } - /** - * @covers ResourceLoaderWikiModule::preloadTitleInfo - */ public function testGetPreloadedTitleInfoEmpty() { $context = new ResourceLoaderContext( new EmptyResourceLoader(), new FauxRequest() ); - // Covers early return + // This covers the early return case $this->assertSame( null, ResourceLoaderWikiModule::preloadTitleInfo( $context, - wfGetDB( DB_REPLICA ), + $this->createMock( IDatabase::class ), [] ) ); } public static function provideGetContent() { - return [ - 'Bad title' => [ null, '[x]' ], - 'Dead redirect' => [ null, [ - 'text' => 'Dead redirect', - 'title' => 'Dead_redirect', - 'redirect' => 1, - ] ], - 'Bad content model' => [ null, [ - 'text' => 'MediaWiki:Wikitext', - 'ns' => NS_MEDIAWIKI, - 'title' => 'Wikitext', - ] ], - 'No JS content found' => [ null, [ - 'text' => 'MediaWiki:Script.js', - 'ns' => NS_MEDIAWIKI, - 'title' => 'Script.js', - ] ], - 'No CSS content found' => [ null, [ - 'text' => 'MediaWiki:Styles.css', - 'ns' => NS_MEDIAWIKI, - 'title' => 'Script.css', - ] ], - ]; + yield 'Bad title' => [ null, '[x]' ]; + yield 'Dead redirect' => [ null, [ + 'text' => 'Dead redirect', + 'title' => 'Dead_redirect', + 'redirect' => 1, + ] ]; + yield 'Bad content model' => [ null, [ + 'text' => 'MediaWiki:Wikitext', + 'ns' => NS_MEDIAWIKI, + 'title' => 'Wikitext', + ] ]; + yield 'No JS content found' => [ null, [ + 'text' => 'MediaWiki:Script.js', + 'ns' => NS_MEDIAWIKI, + 'title' => 'Script.js', + ] ]; + yield 'No CSS content found' => [ null, [ + 'text' => 'MediaWiki:Styles.css', + 'ns' => NS_MEDIAWIKI, + 'title' => 'Script.css', + ] ]; } /** - * @covers ResourceLoaderWikiModule::getContent * @dataProvider provideGetContent */ public function testGetContent( $expected, $title ) { $context = $this->getResourceLoaderContext( [], new EmptyResourceLoader ); $module = $this->getMockBuilder( ResourceLoaderWikiModule::class ) ->setMethods( [ 'getContentObj' ] )->getMock(); - $module->expects( $this->any() ) - ->method( 'getContentObj' )->willReturn( null ); + $module->method( 'getContentObj' ) + ->willReturn( null ); if ( is_array( $title ) ) { $title += [ 'ns' => NS_MAIN, 'id' => 1, 'len' => 1, 'redirect' => 0 ]; @@ -344,23 +331,18 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { } $module = TestingAccessWrapper::newFromObject( $module ); - $this->assertEquals( + $this->assertSame( $expected, $module->getContent( $titleText, $context ) ); } - /** - * @covers ResourceLoaderWikiModule::getContent - * @covers ResourceLoaderWikiModule::getContentObj - * @covers ResourceLoaderWikiModule::shouldEmbedModule - */ public function testContentOverrides() { $pages = [ 'MediaWiki:Common.css' => [ 'type' => 'style' ], ]; - $module = $this->getMockBuilder( TestResourceLoaderWikiModule::class ) + $module = $this->getMockBuilder( ResourceLoaderWikiModule::class ) ->setMethods( [ 'getPages' ] ) ->getMock(); $module->method( 'getPages' )->willReturn( $pages ); @@ -377,7 +359,7 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { } ); $this->assertTrue( $module->shouldEmbedModule( $context ) ); - $this->assertEquals( [ + $this->assertSame( [ 'all' => [ "/*\nMediaWiki:Common.css\n*/\n.override{}" ] @@ -392,10 +374,6 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { $this->assertFalse( $module->shouldEmbedModule( $context ) ); } - /** - * @covers ResourceLoaderWikiModule::getContent - * @covers ResourceLoaderWikiModule::getContentObj - */ public function testGetContentForRedirects() { // Set up context and module object $context = new DerivativeResourceLoaderContext( @@ -404,11 +382,10 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { $module = $this->getMockBuilder( ResourceLoaderWikiModule::class ) ->setMethods( [ 'getPages' ] ) ->getMock(); - $module->expects( $this->any() ) - ->method( 'getPages' ) - ->will( $this->returnValue( [ + $module->method( 'getPages' ) + ->willReturn( [ 'MediaWiki:Redirect.js' => [ 'type' => 'script' ] - ] ) ); + ] ); $context->setContentOverrideCallback( function ( Title $title ) { if ( $title->getPrefixedText() === 'MediaWiki:Redirect.js' ) { $handler = new JavaScriptContentHandler(); @@ -430,14 +407,14 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { 1 // redirect ); - $this->assertEquals( + $this->assertSame( "/*\nMediaWiki:Redirect.js\n*/\ntarget;\n", $module->getScript( $context ), 'Redirect resolved by getContent' ); } - function tearDown() { + public function tearDown() { Title::clearCaches(); parent::tearDown(); }