From 5f47d994bc72ad13c5483237d39d891e1591f57b Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 30 Jul 2019 16:06:35 +0100 Subject: [PATCH] resourceloader: Don't explicitly enqueue test libs on SpecialJavaScriptTest The test-only modules registered by QUnitTestResources.php are currently were previously caught by the array_keys() catch-all in registerTestModules() which meant that modules like 'test.sinonjs' would be requested on SpecialJavaScriptTest despite not doing anything by itself, nor executing at the "right" time per se through this means. In order for it to execute at the right time, the testrunner has to depend on it (which it does, already). But, that also means it doesn't need to be requested separately. Doing so could be confusing. This is neccecary in order to move 'jquery.qunit' from Resources.php to QUnitTestResources.php as otherwise, listing in QUnitTestResources.php, would implicitly mean SpecialJavaScriptTest.php thinks it's a test suite and load it. That is a problem, because when we run the tests headless from the command-line with Karma, the environment already has a QUnit interface defined, and should not be loaded a second time by MW. Change-Id: I08b31cd1dee516cf0d26bafdb8cc7c1223633bad --- includes/resourceloader/ResourceLoader.php | 63 ++++++++------------- includes/specials/SpecialJavaScriptTest.php | 4 +- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 7e623b53a7..2336a37498 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -58,11 +58,10 @@ class ResourceLoader implements LoggerAwareInterface { protected $config; /** - * Associative array mapping framework ids to a list of names of test suite modules - * like [ 'qunit' => [ 'mediawiki.tests.qunit.suites', 'ext.foo.tests', ... ], ... ] - * @var array + * List of module names that contain QUnit test suites + * @var string[] */ - protected $testModuleNames = []; + protected $testSuiteModuleNames = []; /** * E.g. [ 'source-id' => 'http://.../load.php' ] @@ -374,6 +373,7 @@ class ResourceLoader implements LoggerAwareInterface { /** * @internal For use by ServiceWiring only + * @codeCoverageIgnore */ public function registerTestModules() { global $IP; @@ -384,39 +384,37 @@ class ResourceLoader implements LoggerAwareInterface { . 'Edit your LocalSettings.php to enable it.' ); } - $testModules = [ - 'qunit' => [], - ]; + // This has a 'qunit' key for compat with the below hook. + $testModulesMeta = [ 'qunit' => [] ]; // Get test suites from extensions // Avoid PHP 7.1 warning from passing $this by reference $rl = $this; - Hooks::run( 'ResourceLoaderTestModules', [ &$testModules, &$rl ] ); + Hooks::run( 'ResourceLoaderTestModules', [ &$testModulesMeta, &$rl ] ); $extRegistry = ExtensionRegistry::getInstance(); // In case of conflict, the deprecated hook has precedence. - $testModules['qunit'] += $extRegistry->getAttribute( 'QUnitTestModules' ); + $testModules = $testModulesMeta['qunit'] + $extRegistry->getAttribute( 'QUnitTestModules' ); - // Add the QUnit testrunner as implicit dependency to extension test suites. - foreach ( $testModules['qunit'] as &$module ) { - // Shuck any single-module dependency as an array + $testSuiteModuleNames = []; + foreach ( $testModules as $name => &$module ) { + // Turn any single-module dependency into an array if ( isset( $module['dependencies'] ) && is_string( $module['dependencies'] ) ) { $module['dependencies'] = [ $module['dependencies'] ]; } + // Ensure the testrunner loads before any test suites $module['dependencies'][] = 'test.mediawiki.qunit.testrunner'; - } - // Get core test suites - $testModules['qunit'] = - ( include "$IP/tests/qunit/QUnitTestResources.php" ) + $testModules['qunit']; + // Keep track of the test suites to load on SpecialJavaScriptTest + $testSuiteModuleNames[] = $name; + } - foreach ( $testModules as $id => $names ) { - // Register test modules - $this->register( $testModules[$id] ); + // Core test suites (their names have further precedence). + $testModules = ( include "$IP/tests/qunit/QUnitTestResources.php" ) + $testModules; + $testSuiteModuleNames[] = 'test.mediawiki.qunit.suites'; - // Keep track of their names so that they can be loaded together - $this->testModuleNames[$id] = array_keys( $testModules[$id] ); - } + $this->register( $testModules ); + $this->testSuiteModuleNames = $testSuiteModuleNames; } /** @@ -470,25 +468,14 @@ class ResourceLoader implements LoggerAwareInterface { } /** - * Get a list of test module names for one (or all) frameworks. + * Get a list of module names with QUnit test suites. * - * If the given framework id is unknkown, or if the in-object variable is not an array, - * then it will return an empty array. - * - * @param string $framework Get only the test module names for one - * particular framework (optional) + * @internal For use by SpecialJavaScriptTest only * @return array + * @codeCoverageIgnore */ - public function getTestModuleNames( $framework = 'all' ) { - if ( $framework == 'all' ) { - return $this->testModuleNames; - } elseif ( isset( $this->testModuleNames[$framework] ) - && is_array( $this->testModuleNames[$framework] ) - ) { - return $this->testModuleNames[$framework]; - } else { - return []; - } + public function getTestSuiteModuleNames() { + return $this->testSuiteModuleNames; } /** diff --git a/includes/specials/SpecialJavaScriptTest.php b/includes/specials/SpecialJavaScriptTest.php index 3e9676c64e..8c137aa175 100644 --- a/includes/specials/SpecialJavaScriptTest.php +++ b/includes/specials/SpecialJavaScriptTest.php @@ -103,7 +103,7 @@ class SpecialJavaScriptTest extends SpecialPage { $query['only'] = 'scripts'; $startupContext = new ResourceLoaderContext( $rl, new FauxRequest( $query ) ); - $modules = $rl->getTestModuleNames( 'qunit' ); + $modules = $rl->getTestSuiteModuleNames(); // Disable autostart because we load modules asynchronously. By default, QUnit would start // at domready when there are no tests loaded and also fire 'QUnit.done' which then instructs @@ -170,7 +170,7 @@ JAVASCRIPT ); // Use 'raw' because QUnit loads before ResourceLoader initialises (omit mw.loader.state call) - // Use 'test' to ensure OutputPage doesn't use the "async" attribute because QUnit must + // Use 'sync' to ensure OutputPage doesn't use the "async" attribute because QUnit must // load before qunit/export. $scripts = $out->makeResourceLoaderLink( 'jquery.qunit', ResourceLoaderModule::TYPE_SCRIPTS, -- 2.20.1