resourceloader: Fix broken code in load.php mock used to make a test fail

The load.mock.php has some module implementation stubs that call global
QUnit methods. This has been broken since the upgrade to QUnit 2, which removed
support for these (deprecated) methods, in favour of context-based assert.

However, this went unnoticed because these stubs are only there to make
our test fail if code is behaving incorrectly. Naturally, this isn't the
case normally. In theory, any unmerged code between QUnit 2 upgrade and now
would've failed Jenkins for something like 'QUnit.ok undefined' instead of
the more descriptive error message the stub supplies.

Fix this by instead statically exposing the contextual assert object
to the stubs.

Also centralise the clean up (teardown) for this new exposure, and also
clean up the other static exposure we have (testCallback) in the same way,
and document the cases where we intentionally clean it up inline.

Change-Id: I00b71e7bc9aa97275dfabf1070c4141fa76adb05

diff --git a/tests/qunit/data/load.mock.php b/tests/qunit/data/load.mock.php
index 23fb4c9..fed5746 100644 (file)
--- a/tests/qunit/data/load.mock.php
+++ b/tests/qunit/data/load.mock.php
@@ -27,21 +27,19 @@ header( 'Content-Type: text/javascript; charset=utf-8' );
 $moduleImplementations = [
        'testUsesMissing' => "
 mw.loader.implement( 'testUsesMissing', function () {
-       QUnit.ok( false, 'Module usesMissing script should not run.' );
-       QUnit.start();
+       mw.loader.testFail( 'Module usesMissing script should not run.' );
 }, {}, {});
 ",
 
        'testUsesNestedMissing' => "
 mw.loader.implement( 'testUsesNestedMissing', function () {
-       QUnit.ok( false, 'Module testUsesNestedMissing script should not run.' );
-       QUnit.start();
+       mw.loader.testFail('Module testUsesNestedMissing script should not run.' );
 }, {}, {});
 ",
 
        'testSkipped' => "
 mw.loader.implement( 'testSkipped', function () {
-       QUnit.ok( false, 'Module testSkipped was supposed to be skipped.' );
+       mw.loader.testFail( false, 'Module testSkipped was supposed to be skipped.' );
 }, {}, {});
 ",
 

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
index 64415e0..e774112 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js
@@ -1,7 +1,12 @@
 ( function ( mw, $ ) {
        QUnit.module( 'mediawiki (mw.loader)', QUnit.newMwEnvironment( {
-               setup: function () {
+               setup: function ( assert ) {
                        mw.loader.store.enabled = false;
+
+                       // Expose for load.mock.php
+                       mw.loader.testFail = function ( reason ) {
+                               assert.ok( false, reason );
+                       };
                },
                teardown: function () {
                        mw.loader.store.enabled = false;
@@ -10,6 +15,14 @@
                                window.Set = this.nativeSet;
                                mw.redefineFallbacksForTest();
                        }
+                       // Remove any remaining temporary statics
+                       // exposed for cross-file mocks.
+                       if ( 'testCallback' in mw.loader ) {
+                               delete mw.loader.testCallback;
+                       }
+                       if ( 'testFail' in mw.loader ) {
+                               delete mw.loader.testFail;
+                       }
                }
        } ) );
 
@@ -94,8 +107,6 @@
 
                return mw.loader.using( 'test.callback', function () {
                        assert.strictEqual( isAwesomeDone, true, 'test.callback module should\'ve caused isAwesomeDone to be true' );
-                       delete mw.loader.testCallback;
-
                }, function () {
                        assert.ok( false, 'Error callback fired while loader.using "test.callback" module' );
                } );
@@ -113,8 +124,6 @@
 
                return mw.loader.using( 'hasOwnProperty', function () {
                        assert.strictEqual( isAwesomeDone, true, 'hasOwnProperty module should\'ve caused isAwesomeDone to be true' );
-                       delete mw.loader.testCallback;
-
                }, function () {
                        assert.ok( false, 'Error callback fired while loader.using "hasOwnProperty" module' );
                } );
@@ -133,7 +142,6 @@
                return mw.loader.using( 'test.promise' )
                        .done( function () {
                                assert.strictEqual( isAwesomeDone, true, 'test.promise module should\'ve caused isAwesomeDone to be true' );
-                               delete mw.loader.testCallback;
                        } )
                        .fail( function () {
                                assert.ok( false, 'Error callback fired while loader.using "test.promise" module' );
@@ -710,6 +718,7 @@
                assert.equal( target.slice( 0, 2 ), '//', 'URL is protocol-relative' );
 
                mw.loader.testCallback = function () {
+                       // Ensure once, delete now
                        delete mw.loader.testCallback;
                        assert.ok( true, 'callback' );
                        done();
@@ -728,6 +737,7 @@
                assert.equal( target.slice( 0, 1 ), '/', 'URL is relative to document root' );
 
                mw.loader.testCallback = function () {
+                       // Ensure once, delete now
                        delete mw.loader.testCallback;
                        assert.ok( true, 'callback' );
                        done();