resourceloader: Remove use of object registering in test suites
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 11 Jul 2019 19:48:57 +0000 (20:48 +0100)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 12 Jul 2019 01:17:44 +0000 (01:17 +0000)
This was done as a "clever" shortcut to make sure tests a little
but shorter, but also made them less consistent with normal code.

Remove this in favour of 'class' or 'factory' options as needed.
Also remove a bunch of unneeded register() calls.

The tests cover everything affected by this change.

Side fix - isFileModule should reject modules with 'factory'
the same way it rejected raw objects and non-FileModule 'class'
cases already. This is now covered by tests as well.

Bug: T222637
Change-Id: I3996317dbcd780cc6e0f82c84e769c08a3fc42bb

includes/resourceloader/ResourceLoader.php
tests/phpunit/ResourceLoaderTestCase.php
tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderModuleTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php

index ec376e3..6eb9908 100644 (file)
@@ -566,7 +566,7 @@ class ResourceLoader implements LoggerAwareInterface {
                if ( isset( $info['object'] ) ) {
                        return false;
                }
-               return (
+               return !isset( $info['factory'] ) && (
                        // The implied default for 'class' is ResourceLoaderFileModule
                        !isset( $info['class'] ) ||
                        // Explicit default
index 64693b0..2a21351 100644 (file)
@@ -101,6 +101,7 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
        protected $type = ResourceLoaderModule::LOAD_GENERAL;
        protected $targets = [ 'phpunit' ];
        protected $shouldEmbed = null;
+       protected $mayValidateScript = false;
 
        public function __construct( $options = [] ) {
                foreach ( $options as $key => $value ) {
@@ -109,7 +110,14 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
        }
 
        public function getScript( ResourceLoaderContext $context ) {
-               return $this->validateScriptFile( 'input', $this->script );
+               if ( $this->mayValidateScript ) {
+                       // This enables the validation check that replaces invalid
+                       // scripts with a warning message.
+                       // Based on $wgResourceLoaderValidateJS
+                       return $this->validateScriptFile( 'input', $this->script );
+               } else {
+                       return $this->script;
+               }
        }
 
        public function getStyles( ResourceLoaderContext $context ) {
index e094d92..628ddb1 100644 (file)
@@ -11,6 +11,8 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        use MediaWikiCoversValidator;
        use PHPUnit4And6Compat;
 
+       const NAME = 'test.blobstore';
+
        protected function setUp() {
                parent::setUp();
                // MediaWiki's test wrapper sets $wgMainWANCache to CACHE_NONE.
@@ -24,12 +26,15 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        }
 
        public function testBlobCreation() {
-               $module = $this->makeModule( [ 'mainpage' ] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
+               $rl->register( self::NAME, [
+                       'factory' => function () {
+                               return $this->makeModule( [ 'mainpage' ] );
+                       }
+               ] );
 
                $blobStore = $this->makeBlobStore( null, $rl );
-               $blob = $blobStore->getBlob( $module, 'en' );
+               $blob = $blobStore->getBlob( $rl->getModule( self::NAME ), 'en' );
 
                $this->assertEquals( '{"mainpage":"Main Page"}', $blob, 'Generated blob' );
        }
@@ -37,7 +42,6 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        public function testBlobCreation_empty() {
                $module = $this->makeModule( [] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
 
                $blobStore = $this->makeBlobStore( null, $rl );
                $blob = $blobStore->getBlob( $module, 'en' );
@@ -48,7 +52,6 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        public function testBlobCreation_unknownMessage() {
                $module = $this->makeModule( [ 'i-dont-exist', 'mainpage', 'i-dont-exist2' ] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( null, $rl );
 
                // Generating a blob should continue without errors,
@@ -58,9 +61,15 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        }
 
        public function testMessageCachingAndPurging() {
-               $module = $this->makeModule( [ 'example' ] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
+               // Register it so that MessageBlobStore::updateMessage can
+               // discover it from the registry as a module that uses this message.
+               $rl->register( self::NAME, [
+                       'factory' => function () {
+                               return $this->makeModule( [ 'example' ] );
+                       }
+               ] );
+               $module = $rl->getModule( self::NAME );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
 
                // Advance this new WANObjectCache instance to a normal state,
@@ -105,7 +114,6 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
        public function testPurgeEverything() {
                $module = $this->makeModule( [ 'example' ] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
                // Advance this new WANObjectCache instance to a normal state.
                $blobStore->getBlob( $module, 'en' );
@@ -139,7 +147,6 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
                // Arrange version 1 of a module
                $module = $this->makeModule( [ 'foo' ] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
                $blobStore->expects( $this->once() )
                        ->method( 'fetchMessage' )
@@ -158,7 +165,6 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
                // We do not receive purges for this because no messages were changed.
                $module = $this->makeModule( [ 'foo', 'bar' ] );
                $rl = new EmptyResourceLoader();
-               $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( [ 'fetchMessage' ], $rl );
                $blobStore->expects( $this->exactly( 2 ) )
                        ->method( 'fetchMessage' )
@@ -191,7 +197,7 @@ class MessageBlobStoreTest extends PHPUnit\Framework\TestCase {
 
        private function makeModule( array $messages ) {
                $module = new ResourceLoaderTestModule( [ 'messages' => $messages ] );
-               $module->setName( 'test.blobstore' );
+               $module->setName( self::NAME );
                return $module;
        }
 }
index 408a0a2..e1ee324 100644 (file)
@@ -374,7 +374,7 @@ Deprecation message.' ]
        }
 
        private static function makeModule( array $options = [] ) {
-               return new ResourceLoaderTestModule( $options );
+               return $options + [ 'class' => ResourceLoaderTestModule::class ];
        }
 
        private static function makeSampleModules() {
index 3f6e9b0..60b4073 100644 (file)
@@ -86,6 +86,7 @@ class ResourceLoaderModuleTest extends ResourceLoaderTestCase {
                $context = $this->getResourceLoaderContext();
 
                $module = new ResourceLoaderTestModule( [
+                       'mayValidateScript' => true,
                        'script' => "var a = 'this is';\n {\ninvalid"
                ] );
                $this->assertEquals(
index bc7cb69..2691ccc 100644 (file)
@@ -22,7 +22,7 @@ mw.loader.register( [] );'
                        [ [
                                'msg' => 'Basic registry',
                                'modules' => [
-                                       'test.blank' => new ResourceLoaderTestModule(),
+                                       'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -38,10 +38,22 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Optimise the dependency tree (basic case)',
                                'modules' => [
-                                       'a' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'b', 'c', 'd' ] ] ),
-                                       'b' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'c' ] ] ),
-                                       'c' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
-                                       'd' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
+                                       'a' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'b', 'c', 'd' ],
+                                       ],
+                                       'b' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'c' ],
+                                       ],
+                                       'c' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [],
+                                       ],
+                                       'd' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [],
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -76,9 +88,18 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Optimise the dependency tree (tolerate unknown deps)',
                                'modules' => [
-                                       'a' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'b', 'c', 'x' ] ] ),
-                                       'b' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'c', 'x' ] ] ),
-                                       'c' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
+                                       'a' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'b', 'c', 'x' ]
+                                       ],
+                                       'b' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'c', 'x' ]
+                                       ],
+                                       'c' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => []
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -111,11 +132,26 @@ mw.loader.register( [
                                // Regression test for T223402.
                                'msg' => 'Optimise the dependency tree (indirect circular dependency)',
                                'modules' => [
-                                       'top' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'middle1', 'util' ] ] ),
-                                       'middle1' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'middle2', 'util' ] ] ),
-                                       'middle2' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'bottom' ] ] ),
-                                       'bottom' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'top' ] ] ),
-                                       'util' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
+                                       'top' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'middle1', 'util' ],
+                                       ],
+                                       'middle1' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'middle2', 'util' ],
+                                       ],
+                                       'middle2' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'bottom' ],
+                                       ],
+                                       'bottom' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'top' ],
+                                       ],
+                                       'util' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [],
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -162,8 +198,14 @@ mw.loader.register( [
                                // Regression test for T223402.
                                'msg' => 'Optimise the dependency tree (direct circular dependency)',
                                'modules' => [
-                                       'top' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'util', 'top' ] ] ),
-                                       'util' => new ResourceLoaderTestModule( [ 'dependencies' => [] ] ),
+                                       'top' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [ 'util', 'top' ],
+                                       ],
+                                       'util' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'dependencies' => [],
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -187,13 +229,16 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Version falls back gracefully if getVersionHash throws',
                                'modules' => [
-                                       'test.fail' => (
-                                               ( $mock = $this->getMockBuilder( ResourceLoaderTestModule::class )
-                                                       ->setMethods( [ 'getVersionHash' ] )->getMock() )
-                                               && $mock->method( 'getVersionHash' )->will(
-                                                       $this->throwException( new Exception )
-                                               )
-                                       ) ? $mock : $mock
+                                       'test.fail' => [
+                                               'factory' => function () {
+                                                       $mock = $this->getMockBuilder( ResourceLoaderTestModule::class )
+                                                               ->setMethods( [ 'getVersionHash' ] )->getMock();
+                                                       $mock->method( 'getVersionHash' )->will(
+                                                               $this->throwException( new Exception )
+                                                       );
+                                                       return $mock;
+                                               }
+                                       ]
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -212,11 +257,14 @@ mw.loader.state( {
                        [ [
                                'msg' => 'Use version from getVersionHash',
                                'modules' => [
-                                       'test.version' => (
-                                               ( $mock = $this->getMockBuilder( ResourceLoaderTestModule::class )
-                                                       ->setMethods( [ 'getVersionHash' ] )->getMock() )
-                                               && $mock->method( 'getVersionHash' )->willReturn( '1234567' )
-                                       ) ? $mock : $mock
+                                       'test.version' => [
+                                               'factory' => function () {
+                                                       $mock = $this->getMockBuilder( ResourceLoaderTestModule::class )
+                                                               ->setMethods( [ 'getVersionHash' ] )->getMock();
+                                                       $mock->method( 'getVersionHash' )->willReturn( '1234567' );
+                                                       return $mock;
+                                               }
+                                       ]
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -232,11 +280,14 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Re-hash version from getVersionHash if too long',
                                'modules' => [
-                                       'test.version' => (
-                                               ( $mock = $this->getMockBuilder( ResourceLoaderTestModule::class )
-                                                       ->setMethods( [ 'getVersionHash' ] )->getMock() )
-                                               && $mock->method( 'getVersionHash' )->willReturn( '12345678' )
-                                       ) ? $mock : $mock
+                                       'test.version' => [
+                                               'factory' => function () {
+                                                       $mock = $this->getMockBuilder( ResourceLoaderTestModule::class )
+                                                               ->setMethods( [ 'getVersionHash' ] )->getMock();
+                                                       $mock->method( 'getVersionHash' )->willReturn( '12345678' );
+                                                       return $mock;
+                                               }
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -252,9 +303,15 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Group signature',
                                'modules' => [
-                                       'test.blank' => new ResourceLoaderTestModule(),
-                                       'test.group.foo' => new ResourceLoaderTestModule( [ 'group' => 'x-foo' ] ),
-                                       'test.group.bar' => new ResourceLoaderTestModule( [ 'group' => 'x-bar' ] ),
+                                       'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.group.foo' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'group' => 'x-foo',
+                                       ],
+                                       'test.group.bar' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'group' => 'x-bar',
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -282,8 +339,11 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Different target (non-test should not be registered)',
                                'modules' => [
-                                       'test.blank' => new ResourceLoaderTestModule(),
-                                       'test.target.foo' => new ResourceLoaderTestModule( [ 'targets' => [ 'x-foo' ] ] ),
+                                       'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.target.foo' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'targets' => [ 'x-foo' ],
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -300,16 +360,19 @@ mw.loader.register( [
                                'msg' => 'Safemode disabled (default; register all modules)',
                                'modules' => [
                                        // Default origin: ORIGIN_CORE_SITEWIDE
-                                       'test.blank' => new ResourceLoaderTestModule(),
-                                       'test.core-generated' => new ResourceLoaderTestModule( [
+                                       'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.core-generated' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'origin' => ResourceLoaderModule::ORIGIN_CORE_INDIVIDUAL
-                                       ] ),
-                                       'test.sitewide' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.sitewide' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'origin' => ResourceLoaderModule::ORIGIN_USER_SITEWIDE
-                                       ] ),
-                                       'test.user' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.user' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'origin' => ResourceLoaderModule::ORIGIN_USER_INDIVIDUAL
-                                       ] ),
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -339,16 +402,19 @@ mw.loader.register( [
                                'extraQuery' => [ 'safemode' => '1' ],
                                'modules' => [
                                        // Default origin: ORIGIN_CORE_SITEWIDE
-                                       'test.blank' => new ResourceLoaderTestModule(),
-                                       'test.core-generated' => new ResourceLoaderTestModule( [
+                                       'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.core-generated' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'origin' => ResourceLoaderModule::ORIGIN_CORE_INDIVIDUAL
-                                       ] ),
-                                       'test.sitewide' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.sitewide' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'origin' => ResourceLoaderModule::ORIGIN_USER_SITEWIDE
-                                       ] ),
-                                       'test.user' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.user' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'origin' => ResourceLoaderModule::ORIGIN_USER_INDIVIDUAL
-                                       ] ),
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -374,7 +440,10 @@ mw.loader.register( [
                                        ],
                                ],
                                'modules' => [
-                                       'test.blank' => new ResourceLoaderTestModule( [ 'source' => 'example' ] ),
+                                       'test.blank' => [
+                                               'class' => ResourceLoaderTestModule::class,
+                                               'source' => 'example'
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -394,25 +463,28 @@ mw.loader.register( [
                        [ [
                                'msg' => 'Conditional dependency function',
                                'modules' => [
-                                       'test.x.core' => new ResourceLoaderTestModule(),
-                                       'test.x.polyfill' => new ResourceLoaderTestModule( [
+                                       'test.x.core' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.x.polyfill' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'skipFunction' => 'return true;'
-                                       ] ),
-                                       'test.y.polyfill' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.y.polyfill' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'skipFunction' =>
                                                        'return !!(' .
                                                        '    window.JSON &&' .
                                                        '    JSON.parse &&' .
                                                        '    JSON.stringify' .
                                                        ');'
-                                       ] ),
-                                       'test.z.foo' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.z.foo' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'dependencies' => [
                                                        'test.x.core',
                                                        'test.x.polyfill',
                                                        'test.y.polyfill',
                                                ],
-                                       ] ),
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -463,52 +535,62 @@ mw.loader.register( [
                                        ],
                                ],
                                'modules' => [
-                                       'test.blank' => new ResourceLoaderTestModule(),
-                                       'test.x.core' => new ResourceLoaderTestModule(),
-                                       'test.x.util' => new ResourceLoaderTestModule( [
+                                       'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.x.core' => [ 'class' => ResourceLoaderTestModule::class ],
+                                       'test.x.util' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'dependencies' => [
                                                        'test.x.core',
                                                ],
-                                       ] ),
-                                       'test.x.foo' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.x.foo' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'dependencies' => [
                                                        'test.x.core',
                                                ],
-                                       ] ),
-                                       'test.x.bar' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.x.bar' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'dependencies' => [
                                                        'test.x.core',
                                                        'test.x.util',
                                                ],
-                                       ] ),
-                                       'test.x.quux' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.x.quux' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'dependencies' => [
                                                        'test.x.foo',
                                                        'test.x.bar',
                                                        'test.x.util',
                                                        'test.x.unknown',
                                                ],
-                                       ] ),
-                                       'test.group.foo.1' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.group.foo.1' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'group' => 'x-foo',
-                                       ] ),
-                                       'test.group.foo.2' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.group.foo.2' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'group' => 'x-foo',
-                                       ] ),
-                                       'test.group.bar.1' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.group.bar.1' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'group' => 'x-bar',
-                                       ] ),
-                                       'test.group.bar.2' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.group.bar.2' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'group' => 'x-bar',
                                                'source' => 'example',
-                                       ] ),
-                                       'test.target.foo' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.target.foo' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'targets' => [ 'x-foo' ],
-                                       ] ),
-                                       'test.target.bar' => new ResourceLoaderTestModule( [
+                                       ],
+                                       'test.target.bar' => [
+                                               'class' => ResourceLoaderTestModule::class,
                                                'source' => 'example',
                                                'targets' => [ 'x-foo' ],
-                                       ] ),
+                                       ],
                                ],
                                'out' => '
 mw.loader.addSource( {
@@ -614,8 +696,9 @@ mw.loader.register( [
        public static function provideRegistrations() {
                return [
                        [ [
-                               'test.blank' => new ResourceLoaderTestModule(),
-                               'test.min' => new ResourceLoaderTestModule( [
+                               'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
+                               'test.min' => [
+                                       'class' => ResourceLoaderTestModule::class,
                                        'skipFunction' =>
                                                'return !!(' .
                                                '    window.JSON &&' .
@@ -625,7 +708,7 @@ mw.loader.register( [
                                        'dependencies' => [
                                                'test.blank',
                                        ],
-                               ] ),
+                               ],
                        ] ]
                ];
        }
@@ -728,8 +811,8 @@ mw.loader.register( [
                $context1 = $this->getResourceLoaderContext();
                $rl1 = $context1->getResourceLoader();
                $rl1->register( [
-                       'test.a' => new ResourceLoaderTestModule(),
-                       'test.b' => new ResourceLoaderTestModule(),
+                       'test.a' => [ 'class' => ResourceLoaderTestModule::class ],
+                       'test.b' => [ 'class' => ResourceLoaderTestModule::class ],
                ] );
                $module = new ResourceLoaderStartupModule();
                $version1 = $module->getVersionHash( $context1 );
@@ -737,8 +820,8 @@ mw.loader.register( [
                $context2 = $this->getResourceLoaderContext();
                $rl2 = $context2->getResourceLoader();
                $rl2->register( [
-                       'test.b' => new ResourceLoaderTestModule(),
-                       'test.c' => new ResourceLoaderTestModule(),
+                       'test.b' => [ 'class' => ResourceLoaderTestModule::class ],
+                       'test.c' => [ 'class' => ResourceLoaderTestModule::class ],
                ] );
                $module = new ResourceLoaderStartupModule();
                $version2 = $module->getVersionHash( $context2 );
@@ -746,8 +829,11 @@ mw.loader.register( [
                $context3 = $this->getResourceLoaderContext();
                $rl3 = $context3->getResourceLoader();
                $rl3->register( [
-                       'test.a' => new ResourceLoaderTestModule(),
-                       'test.b' => new ResourceLoaderTestModule( [ 'script' => 'different' ] ),
+                       'test.a' => [ 'class' => ResourceLoaderTestModule::class ],
+                       'test.b' => [
+                               'class' => ResourceLoaderTestModule::class,
+                               'script' => 'different',
+                       ],
                ] );
                $module = new ResourceLoaderStartupModule();
                $version3 = $module->getVersionHash( $context3 );
@@ -773,7 +859,10 @@ mw.loader.register( [
                $context = $this->getResourceLoaderContext();
                $rl = $context->getResourceLoader();
                $rl->register( [
-                       'test.a' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'x', 'y' ] ] ),
+                       'test.a' => [
+                               'class' => ResourceLoaderTestModule::class,
+                               'dependencies' => [ 'x', 'y' ],
+                       ],
                ] );
                $module = new ResourceLoaderStartupModule();
                $version1 = $module->getVersionHash( $context );
@@ -781,7 +870,10 @@ mw.loader.register( [
                $context = $this->getResourceLoaderContext();
                $rl = $context->getResourceLoader();
                $rl->register( [
-                       'test.a' => new ResourceLoaderTestModule( [ 'dependencies' => [ 'x', 'z' ] ] ),
+                       'test.a' => [
+                               'class' => ResourceLoaderTestModule::class,
+                               'dependencies' => [ 'x', 'z' ],
+                       ],
                ] );
                $module = new ResourceLoaderStartupModule();
                $version2 = $module->getVersionHash( $context );
index 544afae..f47bdaf 100644 (file)
@@ -70,28 +70,19 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
                $this->assertTrue( ResourceLoader::isValidModuleName( $name ) );
        }
 
-       /**
-        * @covers ResourceLoader::register
-        * @covers ResourceLoader::getModule
-        */
-       public function testRegisterValidObject() {
-               $module = new ResourceLoaderTestModule();
-               $resourceLoader = new EmptyResourceLoader();
-               $resourceLoader->register( 'test', $module );
-               $this->assertEquals( $module, $resourceLoader->getModule( 'test' ) );
-       }
-
        /**
         * @covers ResourceLoader::register
         * @covers ResourceLoader::getModule
         */
        public function testRegisterValidArray() {
-               $module = new ResourceLoaderTestModule();
                $resourceLoader = new EmptyResourceLoader();
                // Covers case of register() setting $rl->moduleInfos,
                // but $rl->modules lazy-populated by getModule()
-               $resourceLoader->register( 'test', [ 'object' => $module ] );
-               $this->assertEquals( $module, $resourceLoader->getModule( 'test' ) );
+               $resourceLoader->register( 'test', [ 'class' => ResourceLoaderTestModule::class ] );
+               $this->assertInstanceOf(
+                       ResourceLoaderTestModule::class,
+                       $resourceLoader->getModule( 'test' )
+               );
        }
 
        /**
@@ -99,10 +90,12 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
         * @group medium
         */
        public function testRegisterEmptyString() {
-               $module = new ResourceLoaderTestModule();
                $resourceLoader = new EmptyResourceLoader();
-               $resourceLoader->register( '', $module );
-               $this->assertEquals( $module, $resourceLoader->getModule( '' ) );
+               $resourceLoader->register( '', [ 'class' => ResourceLoaderTestModule::class ] );
+               $this->assertInstanceOf(
+                       ResourceLoaderTestModule::class,
+                       $resourceLoader->getModule( '' )
+               );
        }
 
        /**
@@ -112,7 +105,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
        public function testRegisterInvalidName() {
                $resourceLoader = new EmptyResourceLoader();
                $this->setExpectedException( MWException::class, "name 'test!invalid' is invalid" );
-               $resourceLoader->register( 'test!invalid', new ResourceLoaderTestModule() );
+               $resourceLoader->register( 'test!invalid', [] );
        }
 
        /**
@@ -133,11 +126,13 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
                        ->method( 'warning' );
                $resourceLoader = new EmptyResourceLoader( null, $logger );
 
-               $module1 = new ResourceLoaderTestModule();
-               $module2 = new ResourceLoaderTestModule();
-               $resourceLoader->register( 'test', $module1 );
-               $resourceLoader->register( 'test', $module2 );
-               $this->assertSame( $module2, $resourceLoader->getModule( 'test' ) );
+               $resourceLoader->register( 'test', [ 'class' => ResourceLoaderSkinModule::class ] );
+               $resourceLoader->register( 'test', [ 'class' => ResourceLoaderStartUpModule::class ] );
+               $this->assertInstanceOf(
+                       ResourceLoaderStartUpModule::class,
+                       $resourceLoader->getModule( 'test' ),
+                       'last one wins'
+               );
        }
 
        /**
@@ -146,8 +141,8 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
        public function testGetModuleNames() {
                // Use an empty one so that core and extension modules don't get in.
                $resourceLoader = new EmptyResourceLoader();
-               $resourceLoader->register( 'test.foo', new ResourceLoaderTestModule() );
-               $resourceLoader->register( 'test.bar', new ResourceLoaderTestModule() );
+               $resourceLoader->register( 'test.foo', [] );
+               $resourceLoader->register( 'test.bar', [] );
                $this->assertEquals(
                        [ 'startup', 'test.foo', 'test.bar' ],
                        $resourceLoader->getModuleNames()
@@ -155,15 +150,21 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
        }
 
        public function provideTestIsFileModule() {
-               $fileModuleObj = $this->getMockBuilder( ResourceLoaderFileModule::class )
-                       ->disableOriginalConstructor()
-                       ->getMock();
+               $fileModuleObj = $this->createMock( ResourceLoaderFileModule::class );
                return [
-                       'object' => [ false,
-                               new ResourceLoaderTestModule()
+                       'factory ignored' => [ false,
+                               [
+                                       'factory' => function () {
+                                               return new ResourceLoaderTestModule();
+                                       }
+                               ]
                        ],
-                       'FileModule object' => [ false,
-                               $fileModuleObj
+                       'factory ignored (actual FileModule)' => [ false,
+                               [
+                                       'factory' => function () use ( $fileModuleObj ) {
+                                               return $fileModuleObj;
+                                       }
+                               ]
                        ],
                        'simple empty' => [ true,
                                []
@@ -214,7 +215,7 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
         */
        public function testIsModuleRegistered() {
                $rl = new EmptyResourceLoader();
-               $rl->register( 'test', new ResourceLoaderTestModule() );
+               $rl->register( 'test', [] );
                $this->assertTrue( $rl->isModuleRegistered( 'test' ) );
                $this->assertFalse( $rl->isModuleRegistered( 'test.unknown' ) );
        }
@@ -709,9 +710,13 @@ END
                        // Disable log from outputErrorAndLog
                        ->setMethods( [ 'outputErrorAndLog' ] )->getMock();
                $rl->register( [
-                       'foo' => self::getSimpleModuleMock(),
-                       'ferry' => self::getFailFerryMock(),
-                       'bar' => self::getSimpleModuleMock(),
+                       'foo' => [ 'class' => ResourceLoaderTestModule::class ],
+                       'ferry' => [
+                               'factory' => function () {
+                                       return self::getFailFerryMock();
+                               }
+                       ],
+                       'bar' => [ 'class' => ResourceLoaderTestModule::class ],
                ] );
                $context = $this->getResourceLoaderContext( [], $rl );
 
@@ -800,7 +805,6 @@ END
                $modules = array_map( function ( $script ) {
                        return self::getSimpleModuleMock( $script );
                }, $scripts );
-               $rl->register( $modules );
 
                $context = $this->getResourceLoaderContext(
                        [
@@ -845,7 +849,6 @@ END
                        'bar' => self::getSimpleModuleMock( 'bar();' ),
                ];
                $rl = new EmptyResourceLoader();
-               $rl->register( $modules );
                $context = $this->getResourceLoaderContext(
                        [
                                'modules' => 'foo|ferry|bar',
@@ -885,7 +888,6 @@ END
                        'bar' => self::getSimpleStyleModuleMock( '.bar{}' ),
                ];
                $rl = new EmptyResourceLoader();
-               $rl->register( $modules );
                $context = $this->getResourceLoaderContext(
                        [
                                'modules' => 'foo|ferry|bar',
@@ -922,9 +924,15 @@ END
                // provide the full Config object here.
                $rl = new EmptyResourceLoader( MediaWikiServices::getInstance()->getMainConfig() );
                $rl->register( [
-                       'foo' => self::getSimpleModuleMock( 'foo();' ),
-                       'ferry' => self::getFailFerryMock(),
-                       'bar' => self::getSimpleModuleMock( 'bar();' ),
+                       'foo' => [ 'factory' => function () {
+                               return self::getSimpleModuleMock( 'foo();' );
+                       } ],
+                       'ferry' => [ 'factory' => function () {
+                               return self::getFailFerryMock();
+                       } ],
+                       'bar' => [ 'factory' => function () {
+                               return self::getSimpleModuleMock( 'bar();' );
+                       } ],
                ] );
                $context = $this->getResourceLoaderContext(
                        [
@@ -981,15 +989,12 @@ END
                ] );
 
                $rl = new EmptyResourceLoader();
-               $rl->register( [
-                       'foo' => $module,
-               ] );
                $context = $this->getResourceLoaderContext(
                        [ 'modules' => 'foo', 'only' => 'scripts' ],
                        $rl
                );
 
-               $modules = [ 'foo' => $rl->getModule( 'foo' ) ];
+               $modules = [ 'foo' => $module ];
                $response = $rl->makeModuleResponse( $context, $modules );
                $extraHeaders = TestingAccessWrapper::newFromObject( $rl )->extraHeaders;
 
@@ -1022,13 +1027,12 @@ END
                ] );
 
                $rl = new EmptyResourceLoader();
-               $rl->register( [ 'foo' => $foo, 'bar' => $bar ] );
                $context = $this->getResourceLoaderContext(
                        [ 'modules' => 'foo|bar', 'only' => 'scripts' ],
                        $rl
                );
 
-               $modules = [ 'foo' => $rl->getModule( 'foo' ), 'bar' => $rl->getModule( 'bar' ) ];
+               $modules = [ 'foo' => $foo, 'bar' => $bar ];
                $response = $rl->makeModuleResponse( $context, $modules );
                $extraHeaders = TestingAccessWrapper::newFromObject( $rl )->extraHeaders;
                $this->assertEquals(
@@ -1073,7 +1077,11 @@ END
                                'makeModuleResponse',
                        ] )
                        ->getMock();
-               $rl->register( 'test', $module );
+               $rl->register( 'test', [
+                       'factory' => function () use ( $module ) {
+                               return $module;
+                       }
+               ] );
                $context = $this->getResourceLoaderContext(
                        [ 'modules' => 'test', 'only' => null ],
                        $rl
@@ -1102,7 +1110,11 @@ END
                                'sendResponseHeaders',
                        ] )
                        ->getMock();
-               $rl->register( 'test', $module );
+               $rl->register( 'test', [
+                       'factory' => function () use ( $module ) {
+                               return $module;
+                       }
+               ] );
                $context = $this->getResourceLoaderContext( [ 'modules' => 'test' ], $rl );
                // Disable logging from outputErrorAndLog
                $this->setLogger( 'exception', new Psr\Log\NullLogger() );
index c1bdebe..e8a0884 100644 (file)
@@ -230,7 +230,6 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                $module::$returnFetchTitleInfo = $titleInfo;
 
                $rl = new EmptyResourceLoader();
-               $rl->register( 'testmodule', $module );
                $context = new ResourceLoaderContext( $rl, new FauxRequest() );
 
                TestResourceLoaderWikiModule::invalidateModuleCache(
@@ -253,20 +252,16 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
         * @covers ResourceLoaderWikiModule::preloadTitleInfo
         */
        public function testGetPreloadedBadTitle() {
-               // Mock values
-               $pages = [
-                       // Covers else branch for invalid page name
-                       '[x]' => [ 'type' => 'styles' ],
-               ];
-               $titleInfo = [];
-
-               // Set up objects
-               $module = $this->getMockBuilder( TestResourceLoaderWikiModule::class )
-                       ->setMethods( [ 'getPages' ] )->getMock();
-               $module->method( 'getPages' )->willReturn( $pages );
-               $module::$returnFetchTitleInfo = $titleInfo;
+               // Set up
+               TestResourceLoaderWikiModule::$returnFetchTitleInfo = [];
                $rl = new EmptyResourceLoader();
-               $rl->register( 'testmodule', $module );
+               $rl->getConfig()->set( 'UseSiteJs', true );
+               $rl->getConfig()->set( 'UseSiteCss', true );
+               $rl->register( 'testmodule', [
+                       'class' => TestResourceLoaderWikiModule::class,
+                       // Covers preloadTitleInfo branch for invalid page name
+                       'styles' => [ '[x]' ],
+               ] );
                $context = new ResourceLoaderContext( $rl, new FauxRequest() );
 
                // Act
@@ -277,8 +272,8 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                );
 
                // Assert
-               $module = TestingAccessWrapper::newFromObject( $module );
-               $this->assertEquals( $titleInfo, $module->getTitleInfo( $context ), 'Title info' );
+               $module = TestingAccessWrapper::newFromObject( $rl->getModule( 'testmodule' ) );
+               $this->assertSame( [], $module->getTitleInfo( $context ), 'Title info' );
        }
 
        /**
@@ -371,7 +366,6 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                $module->method( 'getPages' )->willReturn( $pages );
 
                $rl = new EmptyResourceLoader();
-               $rl->register( 'testmodule', $module );
                $context = new DerivativeResourceLoaderContext(
                        new ResourceLoaderContext( $rl, new FauxRequest() )
                );