Tests: Fix race-condition broken mw.loader unit test

* testloader.php was resolving dependencies when it shouldn't
  the server *never* does that. That's handled by the client, which
  knows the state of what is and isn't module.

  Due to a race condition between:
  - handlePending() call from loader.state() call for
    load.phpmodules?=testMissing
  - asynchronous callback from mw.loader.using as dependencies
    are being resolved.

  It didn't expose any actual problem, the unit test was simply
  assuming that they would arrive in a certain order, when that isn't
  the case by design.

  Random failures are seen here:
  - https://integration.mediawiki.org/testswarm/user/mediawiki/
  - http://integration.wmflabs.org/testswarm/job/4

  > Module "test.missing" must have state "missing"
  > - "missing"
  > + "loading"
  -> Because callback was triggered from mw.loader.state() already
     it didn't get to the implement() call yet.

  > Module "test.use_missing" must have state "error"
  > - "error"
  > + "loading"
  -> (same reason)

  This is now fixed.

* Greatly simplified the mock load.php

Change-Id: I000ee726a062f6c6d630ad6c07cfc0b48d145d35

diff --git a/tests/qunit/data/load.mock.php b/tests/qunit/data/load.mock.php
new file mode 100644 (file)
index 0000000..1c18970
--- /dev/null
+++ b/tests/qunit/data/load.mock.php
@@ -0,0 +1,58 @@
+<?php
+/**
+ * Mock load.php with pre-defined test modules.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @package MediaWiki
+ * @author Lupo
+ * @since 1.20
+ */
+header( 'Content-Type: text/javascript; charset=utf-8' );
+
+require_once '../../../includes/Xml.php';
+
+$moduleImplementations = array(
+       'testUsesMissing' => "
+mw.loader.implement( 'testUsesMissing', function () {
+       QUnit.ok( false, 'Module test.usesMissing script should not run.');
+       QUnit.start();
+}, {}, {});
+",
+
+       'testUsesNestedMissing' => "
+mw.loader.implement( 'testUsesNestedMissing', function () {
+       QUnit.ok( false, 'Module testUsesNestedMissing script should not run.');
+}, {}, {});
+",
+);
+
+$response = '';
+
+// Only support for non-encoded module names, full module names expected
+if ( isset( $_GET['modules'] ) ) {
+       $modules = explode( ',', $_GET['modules'] );
+       foreach ( $modules as $module ) {
+               if ( isset( $moduleImplementations[$module] ) ) {
+                       $response .= $moduleImplementations[$module];
+               } else {
+                       $response .= Xml::encodeJsCall( 'mw.loader.state', array( $module, 'missing' ) );
+               }
+       }
+}
+
+echo $response;

diff --git a/tests/qunit/data/testloader.php b/tests/qunit/data/testloader.php
deleted file mode 100644 (file)
index 967983e..0000000
--- a/tests/qunit/data/testloader.php
+++ /dev/null
@@ -1,69 +0,0 @@
-<?php
-/**
- * ResourceLoader stub working with pre-defined test modules.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- * http://www.gnu.org/copyleft/gpl.html
- *
- * @file
- * @package MediaWiki
- * @author Lupo
- * @since 1.20
- */
-header( 'Content-Type: text/javascript; charset=utf-8' );\r
-
-require_once dirname( __FILE__ ) . "/../../../includes/Xml.php";
-require_once dirname( __FILE__ ) . "/../../../includes/resourceloader/ResourceLoaderContext.php";
-
-$modules = array (
-       'test.use_missing' => array (
-               'src' => 'mw.loader.implement("test.use_missing", function () {start(); ok(false, "Module test.use_missing should not run.");}, {}, {});',
-               'deps' => array ( 'test.missing' )
-       ),
-       'test.use_missing2' => array (
-               'src' => 'mw.loader.implement("test.use_missing2", function () {start(); ok(false, "Module test.use_missing2 should not run.");}, {}, {});',
-               'deps' => array ( 'test.missing2' )
-       )
-);
-
-function addModule ( $modules, $moduleName, &$gotten ) {
-       $result = "";
-       if ( isset( $gotten[$moduleName] ) ) {
-               return $result;
-       }
-       $gotten[$moduleName] = true;
-       if ( isset( $modules[$moduleName] ) ) {
-               $deps = $modules[$moduleName]['deps'];
-               foreach ( $deps as $depName ) {
-                       $result .= addModule( $depName, $gotten ) . "\n";
-               }
-               $result .= $modules[$moduleName]['src'] . "\n";
-       } else {
-               $result .= 'mw.loader.state( ' . Xml::encodeJsVar( $moduleName ) . ', "missing" );' . "\n";
-       }
-       return $result . "\n";
-}
-
-$result = "";
-
-if ( isset( $_GET['modules'] ) ) {
-       $toGet = ResourceLoaderContext::expandModuleNames( $_GET['modules'] );
-       $gotten = array();
-       foreach ( $toGet as $moduleName ) {
-               $result .= addModule( $modules, $moduleName, $gotten );
-       }
-}
-
-echo $result;

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 2f11521..00fcf38 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -343,43 +343,50 @@ test( 'mw.loader missing dependency', function() {
        );
 } );
 
-test( 'mw.loader real missing dependency', function() {
-       expect( 6 );
+test( 'mw.loader dependency handling', function () {
+       expect( 5 );
 
        mw.loader.addSource(
-               'test',
-               {'loadScript' : QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/testloader.php' )}
+               'testloader',
+               {
+                       loadScript: QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' )
+               }
        );
+
        mw.loader.register( [
-               ['test.missing', '0', [], null, 'test'], ['test.missing2', '0', [], null, 'test'],
-               ['test.use_missing', '0', ['test.missing'], null, 'test'],
-               ['test.use_missing2', '0', ['test.missing2'], null, 'test']
+               // [module, version, dependencies, group, source]
+               ['testMissing', '1', [], null, 'testloader'],
+               ['testUsesMissing', '1', ['testMissing'], null, 'testloader'],
+               ['testUsesNestedMissing', '1', ['testUsesMissing'], null, 'testloader']
        ] );
 
+       function verifyModuleStates() {
+               equal( mw.loader.getState( 'testMissing' ), 'missing', 'Module not known to server must have state "missing"' );
+               equal( mw.loader.getState( 'testUsesMissing' ), 'error', 'Module with missing dependency must have state "error"' );
+               equal( mw.loader.getState( 'testUsesNestedMissing' ), 'error', 'Module with indirect missing dependency must have state "error"' );
+       }
+
        stop();
-       // Asynch ahead
 
-       mw.loader.load( ['test.use_missing'] );
+       mw.loader.using( ['testUsesNestedMissing'],
+               function () {
+                       ok( false, 'Error handler should be invoked.' );
+                       ok( true ); // Dummy to reach QUnit expect()
 
-       function verifyModuleStates() {
-               strictEqual( mw.loader.getState( 'test.missing' ), 'missing', 'Module "test.missing" must have state "missing"' );
-               strictEqual( mw.loader.getState( 'test.missing2' ), 'missing', 'Module "test.missing2" must have state "missing"' );
-               strictEqual( mw.loader.getState( 'test.use_missing' ), 'error', 'Module "test.use_missing" must have state "error"' );
-               strictEqual( mw.loader.getState( 'test.use_missing2' ), 'error', 'Module "test.use_missing2" must have state "error"' );
-       }
+                       verifyModuleStates();
 
-       mw.loader.using( ['test.use_missing2'],
-               function() {
                        start();
-                       ok( false, "Success called wrongly." );
-                       ok( true , "QUnit expected() count dummy" );
-                       verifyModuleStates();
                },
-               function( e, dependencies ) {
-                       start();
-                       ok( true, "Error handler called correctly." );
-                       deepEqual( dependencies, ['test.missing2'], "Dependencies correct." );
+               function ( e, badmodules ) {
+                       ok( true, 'Error handler should be invoked.' );
+                       // As soon as server spits out state('testMissing', 'missing');
+                       // it will bubble up and trigger the error callback.
+                       // Therefor the badmodules array is not testUsesMissing or testUsesNestedMissing.
+                       deepEqual( badmodules, ['testMissing'], 'Bad modules as expected.' );
+
                        verifyModuleStates();
+
+                       start();
                }
        );
 } );