resourceloader: Clean up ResourceLoaderWikiModuleTest
authorTimo Tijhof <krinklemail@gmail.com>
Sat, 13 Jul 2019 18:27:33 +0000 (19:27 +0100)
committerRoan Kattouw <roan.kattouw@gmail.com>
Mon, 15 Jul 2019 22:38:46 +0000 (15:38 -0700)
* 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

tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php

index e8a0884..6ac0377 100644 (file)
@@ -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();
        }