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

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 7e623b5..2336a37 100644 (file)
--- 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 <code>LocalSettings.php</code> 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 3e9676c..8c137aa 100644 (file)
--- 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,