From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 25 Aug 2016 01:50:30 +0000 (-0700)
Subject: resourceloader: Improve coverage in ResourceLoaderTest.php
X-Git-Tag: 1.31.0-rc.0~5901^2
X-Git-Url: http://git.cyclocoop.org/%24action?a=commitdiff_plain;h=7dbad8562c68d587244c1663279b602028511a92;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Improve coverage in ResourceLoaderTest.php

* Fix signature of makeLoaderSourcesScript() to match
  the change in behaviour since e103ba265.

* Consistently order providers before the test.

* Simplify testRegisterValid() and remove needless @depends.

* Remove unused private method stripNoflip().

Coverage:

* Expand test coverage for register().

* Add tests for getModuleNames().

* Add tests for getModule().

* Expand test coverage for addSource().
  (case of invalid array)

* Expand test coverage for makeLoaderImplementScript().
  (case of unwrapped user script, and case of invalid scripts)

* Add tests for makeLoaderSourcesScript().

Change-Id: Ibca3e486fcd3664f171f135327a0f340ee6da9ee
---

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 6426feacaf..9ce2c5a6f3 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -56,7 +56,7 @@ class ResourceLoader implements LoggerAwareInterface {
 	protected $moduleInfos = [];
 
 	/** @var Config $config */
-	private $config;
+	protected $config;
 
 	/**
 	 * Associative array mapping framework ids to a list of names of test suite modules
@@ -1333,10 +1333,10 @@ MESSAGE;
 	 *       Register sources with the given IDs and properties.
 	 *
 	 * @param string $id Source ID
-	 * @param array $properties Source properties (see addSource())
+	 * @param string $loadUrl load.php url
 	 * @return string
 	 */
-	public static function makeLoaderSourcesScript( $id, $properties = null ) {
+	public static function makeLoaderSourcesScript( $id, $loadUrl = null ) {
 		if ( is_array( $id ) ) {
 			return Xml::encodeJsCall(
 				'mw.loader.addSource',
@@ -1346,7 +1346,7 @@ MESSAGE;
 		} else {
 			return Xml::encodeJsCall(
 				'mw.loader.addSource',
-				[ $id, $properties ],
+				[ $id, $loadUrl ],
 				ResourceLoader::inDebugMode()
 			);
 		}
diff --git a/tests/TestsAutoLoader.php b/tests/TestsAutoLoader.php
index ef540a8f48..54882802cb 100644
--- a/tests/TestsAutoLoader.php
+++ b/tests/TestsAutoLoader.php
@@ -45,6 +45,7 @@ $wgAutoloadClasses += [
 	'ResourceLoaderTestCase' => "$testDir/phpunit/ResourceLoaderTestCase.php",
 	'ResourceLoaderTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php",
 	'ResourceLoaderFileModuleTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php",
+	'EmptyResourceLoader' => "$testDir/phpunit/ResourceLoaderTestCase.php",
 	'TestUser' => "$testDir/phpunit/includes/TestUser.php",
 	'TestUserRegistry' => "$testDir/phpunit/includes/TestUserRegistry.php",
 	'LessFileCompilationTest' => "$testDir/phpunit/LessFileCompilationTest.php",
diff --git a/tests/phpunit/ResourceLoaderTestCase.php b/tests/phpunit/ResourceLoaderTestCase.php
index 84bf2fdc12..18a49f60bd 100644
--- a/tests/phpunit/ResourceLoaderTestCase.php
+++ b/tests/phpunit/ResourceLoaderTestCase.php
@@ -1,5 +1,8 @@
 <?php
 
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
 	/**
 	 * @param string $lang
@@ -128,3 +131,13 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
 
 class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {
 }
+
+class EmptyResourceLoader extends ResourceLoader {
+	// TODO: This won't be needed once ResourceLoader is empty by default
+	// and default registrations are done from ServiceWiring instead.
+	public function __construct( Config $config = null, LoggerInterface $logger = null ) {
+		$this->setLogger( $logger ?: new NullLogger() );
+		$this->config = $config ?: ConfigFactory::getDefaultInstance()->makeConfig( 'main' );
+		$this->setMessageBlobStore( new MessageBlobStore( $this, $this->getLogger() ) );
+	}
+}
diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
index 9b62b82dee..ab1323ecdc 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
@@ -310,7 +310,6 @@ mw.loader.register( [
 	 * @dataProvider provideGetModuleRegistrations
 	 * @covers ResourceLoaderStartUpModule::compileUnresolvedDependencies
 	 * @covers ResourceLoaderStartUpModule::getModuleRegistrations
-	 * @covers ResourceLoader::makeLoaderSourcesScript
 	 * @covers ResourceLoader::makeLoaderRegisterScript
 	 */
 	public function testGetModuleRegistrations( $case ) {
diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index 65cd6edaf9..53c99265f8 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -17,18 +17,12 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
 		] );
 	}
 
-	public static function provideValidModules() {
-		return [
-			[ 'TEST.validModule1', new ResourceLoaderTestModule() ],
-		];
-	}
-
 	/**
-	 * Ensures that the ResourceLoaderRegisterModules hook is called when a new
-	 * ResourceLoader object is constructed.
+	 * Ensure the ResourceLoaderRegisterModules hook is called.
+	 *
 	 * @covers ResourceLoader::__construct
 	 */
-	public function testCreatingNewResourceLoaderCallsRegistrationHook() {
+	public function testConstructRegistrationHook() {
 		$resourceLoaderRegisterModulesHook = false;
 
 		$this->setMwGlobals( 'wgHooks', [
@@ -39,66 +33,112 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
 			]
 		] );
 
-		$resourceLoader = new ResourceLoader();
+		$unused = new ResourceLoader();
 		$this->assertTrue(
 			$resourceLoaderRegisterModulesHook,
 			'Hook ResourceLoaderRegisterModules called'
 		);
-
-		return $resourceLoader;
 	}
 
 	/**
-	 * @dataProvider provideValidModules
-	 * @depends testCreatingNewResourceLoaderCallsRegistrationHook
 	 * @covers ResourceLoader::register
 	 * @covers ResourceLoader::getModule
 	 */
-	public function testRegisteredValidModulesAreAccessible(
-		$name, ResourceLoaderModule $module, ResourceLoader $resourceLoader
-	) {
-		$resourceLoader->register( $name, $module );
-		$this->assertEquals( $module, $resourceLoader->getModule( $name ) );
+	public function testRegisterValid() {
+		$module = new ResourceLoaderTestModule();
+		$resourceLoader = new EmptyResourceLoader();
+		$resourceLoader->register( 'test', $module );
+		$this->assertEquals( $module, $resourceLoader->getModule( 'test' ) );
 	}
 
 	/**
-	 * @covers ResourceLoaderFileModule::compileLessFile
+	 * @covers ResourceLoader::register
 	 */
-	public function testLessFileCompilation() {
-		$context = $this->getResourceLoaderContext();
-		$basePath = __DIR__ . '/../../data/less/module';
-		$module = new ResourceLoaderFileModule( [
-			'localBasePath' => $basePath,
-			'styles' => [ 'styles.less' ],
-		] );
-		$module->setName( 'test.less' );
-		$styles = $module->getStyles( $context );
-		$this->assertStringEqualsFile( $basePath . '/styles.css', $styles['all'] );
+	public function testRegisterInvalidName() {
+		$resourceLoader = new EmptyResourceLoader();
+		$this->setExpectedException( 'MWException', "name 'test!invalid' is invalid" );
+		$resourceLoader->register( 'test!invalid', new ResourceLoaderTestModule() );
 	}
 
 	/**
-	 * Strip @noflip annotations from CSS code.
-	 * @param string $css
-	 * @return string
+	 * @covers ResourceLoader::register
 	 */
-	private static function stripNoflip( $css ) {
-		return str_replace( '/*@noflip*/ ', '', $css );
+	public function testRegisterInvalidType() {
+		$resourceLoader = new EmptyResourceLoader();
+		$this->setExpectedException( 'MWException', 'ResourceLoader module info type error' );
+		$resourceLoader->register( 'test', new stdClass() );
 	}
 
 	/**
-	 * @dataProvider providePackedModules
-	 * @covers ResourceLoader::makePackedModulesString
+	 * @covers ResourceLoader::getModuleNames
 	 */
-	public function testMakePackedModulesString( $desc, $modules, $packed ) {
-		$this->assertEquals( $packed, ResourceLoader::makePackedModulesString( $modules ), $desc );
+	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() );
+		$this->assertEquals(
+			[ 'test.foo', 'test.bar' ],
+			$resourceLoader->getModuleNames()
+		);
 	}
 
 	/**
-	 * @dataProvider providePackedModules
-	 * @covers ResourceLoaderContext::expandModuleNames
+	 * @covers ResourceLoader::isModuleRegistered
 	 */
-	public function testexpandModuleNames( $desc, $modules, $packed ) {
-		$this->assertEquals( $modules, ResourceLoaderContext::expandModuleNames( $packed ), $desc );
+	public function testIsModuleRegistered() {
+		$rl = new EmptyResourceLoader();
+		$rl->register( 'test', new ResourceLoaderTestModule() );
+		$this->assertTrue( $rl->isModuleRegistered( 'test' ) );
+		$this->assertFalse( $rl->isModuleRegistered( 'test.unknown' ) );
+	}
+
+	/**
+	 * @covers ResourceLoader::getModule
+	 */
+	public function testGetModuleUnknown() {
+		$rl = new EmptyResourceLoader();
+		$this->assertSame( null, $rl->getModule( 'test' ) );
+	}
+
+	/**
+	 * @covers ResourceLoader::getModule
+	 */
+	public function testGetModuleClass() {
+		$rl = new EmptyResourceLoader();
+		$rl->register( 'test', [ 'class' => ResourceLoaderTestModule::class ] );
+		$this->assertInstanceOf(
+			ResourceLoaderTestModule::class,
+			$rl->getModule( 'test' )
+		);
+	}
+
+	/**
+	 * @covers ResourceLoader::getModule
+	 */
+	public function testGetModuleClassDefault() {
+		$rl = new EmptyResourceLoader();
+		$rl->register( 'test', [] );
+		$this->assertInstanceOf(
+			ResourceLoaderFileModule::class,
+			$rl->getModule( 'test' ),
+			'Array-style module registrations default to FileModule'
+		);
+	}
+
+	/**
+	 * @covers ResourceLoaderFileModule::compileLessFile
+	 */
+	public function testLessFileCompilation() {
+		$context = $this->getResourceLoaderContext();
+		$basePath = __DIR__ . '/../../data/less/module';
+		$module = new ResourceLoaderFileModule( [
+			'localBasePath' => $basePath,
+			'styles' => [ 'styles.less' ],
+		] );
+		$module->setName( 'test.less' );
+		$styles = $module->getStyles( $context );
+		$this->assertStringEqualsFile( $basePath . '/styles.css', $styles['all'] );
 	}
 
 	public static function providePackedModules() {
@@ -126,20 +166,34 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
 		];
 	}
 
+	/**
+	 * @dataProvider providePackedModules
+	 * @covers ResourceLoader::makePackedModulesString
+	 */
+	public function testMakePackedModulesString( $desc, $modules, $packed ) {
+		$this->assertEquals( $packed, ResourceLoader::makePackedModulesString( $modules ), $desc );
+	}
+
+	/**
+	 * @dataProvider providePackedModules
+	 * @covers ResourceLoaderContext::expandModuleNames
+	 */
+	public function testexpandModuleNames( $desc, $modules, $packed ) {
+		$this->assertEquals( $modules, ResourceLoaderContext::expandModuleNames( $packed ), $desc );
+	}
+
 	public static function provideAddSource() {
 		return [
-			[ 'examplewiki', '//example.org/w/load.php', 'examplewiki' ],
-			[ 'example2wiki', [ 'loadScript' => '//example.com/w/load.php' ], 'example2wiki' ],
+			[ 'foowiki', 'https://example.org/w/load.php', 'foowiki' ],
+			[ 'foowiki', [ 'loadScript' => 'https://example.org/w/load.php' ], 'foowiki' ],
 			[
-				[ 'foowiki' => '//foo.org/w/load.php', 'bazwiki' => '//baz.org/w/load.php' ],
+				[
+					'foowiki' => 'https://example.org/w/load.php',
+					'bazwiki' => 'https://example.com/w/load.php',
+				],
 				null,
 				[ 'foowiki', 'bazwiki' ]
-			],
-			[
-				[ 'foowiki' => '//foo.org/w/load.php' ],
-				null,
-				false,
-			],
+			]
 		];
 	}
 
@@ -150,10 +204,6 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
 	 */
 	public function testAddSource( $name, $info, $expected ) {
 		$rl = new ResourceLoader;
-		if ( $expected === false ) {
-			$this->setExpectedException( 'MWException', 'ResourceLoader duplicate source addition error' );
-			$rl->addSource( $name, $info );
-		}
 		$rl->addSource( $name, $info );
 		if ( is_array( $expected ) ) {
 			foreach ( $expected as $source ) {
@@ -164,17 +214,23 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
 		}
 	}
 
-	public static function fakeSources() {
-		return [
-			'examplewiki' => [
-				'loadScript' => '//example.org/w/load.php',
-				'apiScript' => '//example.org/w/api.php',
-			],
-			'example2wiki' => [
-				'loadScript' => '//example.com/w/load.php',
-				'apiScript' => '//example.com/w/api.php',
-			],
-		];
+	/**
+	 * @covers ResourceLoader::addSource
+	 */
+	public function testAddSourceDupe() {
+		$rl = new ResourceLoader;
+		$this->setExpectedException( 'MWException', 'ResourceLoader duplicate source addition error' );
+		$rl->addSource( 'foo', 'https://example.org/w/load.php' );
+		$rl->addSource( 'foo', 'https://example.com/w/load.php' );
+	}
+
+	/**
+	 * @covers ResourceLoader::addSource
+	 */
+	public function testAddSourceInvalid() {
+		$rl = new ResourceLoader;
+		$this->setExpectedException( 'MWException', 'with no "loadScript" key' );
+		$rl->addSource( 'foo',  [ 'x' => 'https://example.org/w/load.php' ] );
 	}
 
 	public static function provideLoaderImplement() {
@@ -204,8 +260,6 @@ mw.example();
 				'name' => 'test.example',
 				'scripts' => 'mw.example();',
 				'styles' => [],
-				'messages' => new XmlJsCode( '{}' ),
-				'templates' => [],
 
 				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) {
 mw.example();
@@ -218,7 +272,6 @@ mw.example();
 				'scripts' => [],
 				'styles' => [ 'css' => [ '.mw-example {}' ] ],
 				'messages' => new XmlJsCode( '{}' ),
-				'templates' => [],
 
 				'expected' => 'mw.loader.implement( "test.example", [], {
     "css": [
@@ -231,9 +284,7 @@ mw.example();
 
 				'name' => 'test.example',
 				'scripts' => 'mw.example();',
-				'styles' => [],
 				'messages' => [ 'example' => '' ],
-				'templates' => [],
 
 				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) {
 mw.example();
@@ -246,8 +297,6 @@ mw.example();
 
 				'name' => 'test.example',
 				'scripts' => 'mw.example();',
-				'styles' => [],
-				'messages' => new XmlJsCode( '{}' ),
 				'templates' => [ 'example.html' => '' ],
 
 				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery, require, module ) {
@@ -256,14 +305,39 @@ mw.example();
     "example.html": ""
 } );',
 			] ],
+			[ [
+				'title' => 'Implement unwrapped user script',
+
+				'name' => 'user',
+				'scripts' => 'mw.example( 1 );',
+
+				'expected' => 'mw.loader.implement( "user", "mw.example( 1 );" );',
+			] ],
+			[ [
+				'title' => 'Implement unwrapped user script',
+				'debug' => false,
+
+				'name' => 'user',
+				'scripts' => 'mw.example( 1 );',
+
+				'expected' => 'mw.loader.implement("user","mw.example(1);");',
+			] ],
 		];
 	}
 
 	/**
 	 * @dataProvider provideLoaderImplement
 	 * @covers ResourceLoader::makeLoaderImplementScript
+	 * @covers ResourceLoader::trimArray
 	 */
 	public function testMakeLoaderImplementScript( $case ) {
+		$case += [
+			'styles' => [], 'templates' => [], 'messages' => new XmlJsCode( '{}' ),
+			'debug' => true
+		];
+		ResourceLoader::clearCache();
+		$this->setMwGlobals( 'wgResourceLoaderDebug', $case['debug'] );
+
 		$this->assertEquals(
 			$case['expected'],
 			ResourceLoader::makeLoaderImplementScript(
@@ -276,6 +350,63 @@ mw.example();
 		);
 	}
 
+	/**
+	 * @covers ResourceLoader::makeLoaderImplementScript
+	 */
+	public function testMakeLoaderImplementScriptInvalid() {
+		$this->setExpectedException( 'MWException', 'Invalid scripts error' );
+		ResourceLoader::makeLoaderImplementScript(
+			'test', // name
+			123, // scripts
+			null, // styles
+			null, // messages
+			null // templates
+		);
+	}
+
+	/**
+	 * @covers ResourceLoader::makeLoaderSourcesScript
+	 */
+	public function testMakeLoaderSourcesScript() {
+		$this->assertEquals(
+			'mw.loader.addSource( "local", "/w/load.php" );',
+			ResourceLoader::makeLoaderSourcesScript( 'local', '/w/load.php' )
+		);
+		$this->assertEquals(
+			'mw.loader.addSource( {
+    "local": "/w/load.php"
+} );',
+			ResourceLoader::makeLoaderSourcesScript( [ 'local' => '/w/load.php' ] )
+		);
+		$this->assertEquals(
+			'mw.loader.addSource( {
+    "local": "/w/load.php",
+    "example": "https://example.org/w/load.php"
+} );',
+			ResourceLoader::makeLoaderSourcesScript( [
+				'local' => '/w/load.php',
+				'example' => 'https://example.org/w/load.php'
+			] )
+		);
+		$this->assertEquals(
+			'mw.loader.addSource( [] );',
+			ResourceLoader::makeLoaderSourcesScript( [] )
+		);
+	}
+
+	private static function fakeSources() {
+		return [
+			'examplewiki' => [
+				'loadScript' => '//example.org/w/load.php',
+				'apiScript' => '//example.org/w/api.php',
+			],
+			'example2wiki' => [
+				'loadScript' => '//example.com/w/load.php',
+				'apiScript' => '//example.com/w/api.php',
+			],
+		];
+	}
+
 	/**
 	 * @covers ResourceLoader::getLoadScript
 	 */
@@ -295,14 +426,4 @@ mw.example();
 			$this->assertTrue( true );
 		}
 	}
-
-	/**
-	 * @covers ResourceLoader::isModuleRegistered
-	 */
-	public function testIsModuleRegistered() {
-		$rl = new ResourceLoader();
-		$rl->register( 'test.module', new ResourceLoaderTestModule() );
-		$this->assertTrue( $rl->isModuleRegistered( 'test.module' ) );
-		$this->assertFalse( $rl->isModuleRegistered( 'test.modulenotregistered' ) );
-	}
 }