resourceloader: Implement "skip function" feature
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 30 Apr 2014 21:06:51 +0000 (23:06 +0200)
committerTimo Tijhof <krinklemail@gmail.com>
Thu, 12 Jun 2014 01:48:26 +0000 (03:48 +0200)
A module can be registered with a skip function. Such function,
if provided, will be invoked by the client when a module is
queued for loading. If the function returns true, the client will
bypass any further loading action and mark the module as 'ready'.

This can be used to implement a feature test for a module
providing a shim or polyfill.

* Change visibility of method ResourceLoader::filter to public.

So that it can be invoked by ResourceLoaderStartupModule.

* Add option to suppress the cache key report in ResourceLoader::filter.

We usually only call the minifier once on an entire request
reponse (because it's all concatenated javascript or embedded
javascript in various different closures, still valid as one
large script) and only add a little bottom line for the cache
key. When embedding the skip function we have to run the minifier
on them separately as they're output as strings (not actual
functions). These strings are typically quite small and blowing
up the response with loads of cache keys is not desirable in
production.

* Add method to clear the static cache of ResourceLoader::inDebugMode.

Global static state is evil but, as long as we have it, we at
least need to clear it after switching contexts in the test suite.

Also:
* Remove obsolete setting of 'debug=true' in the FauxRequest in
  ResourceLoaderTestCase. It already sets global wgResourceLoaderDebug
  in the setUp() method.

Bug: 66390
Change-Id: I87a0ea888d791ad39f114380c42e2daeca470961

RELEASE-NOTES-1.24
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderFileModule.php
includes/resourceloader/ResourceLoaderModule.php
includes/resourceloader/ResourceLoaderStartUpModule.php
resources/src/mediawiki/mediawiki.js
tests/phpunit/ResourceLoaderTestCase.php
tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php
tests/qunit/data/load.mock.php
tests/qunit/suites/resources/mediawiki/mediawiki.test.js

index deeb6b8..54e78ac 100644 (file)
@@ -62,6 +62,8 @@ production.
   the Vector skin has gained a label that should make it more discoverable.
 * MWCryptHKDF added for fast, cryptographically secure random number generation
   that won't deplete openssl's entropy pool.
+* ResourceLoader: File modules can now provide a skip function that uses an
+  inline feature test to bypass loading of the module.
 
 === Bug fixes in 1.24 ===
 * (bug 49116) Footer copyright notice is now always displayed in user language
index 7765714..5ac874d 100644 (file)
@@ -35,6 +35,9 @@ class ResourceLoader {
        /** @var array */
        protected static $requiredSourceProperties = array( 'loadScript' );
 
+       /** @var bool */
+       protected static $debugMode = null;
+
        /** @var array Module name/ResourceLoaderModule object pairs */
        protected $modules = array();
 
@@ -145,9 +148,10 @@ class ResourceLoader {
         *
         * @param string $filter Name of filter to run
         * @param string $data Text to filter, such as JavaScript or CSS text
+        * @param string $cacheReport Whether to include the cache key report
         * @return string Filtered data, or a comment containing an error message
         */
-       protected function filter( $filter, $data ) {
+       public function filter( $filter, $data, $cacheReport = true ) {
                global $wgResourceLoaderMinifierStatementsOnOwnLine, $wgResourceLoaderMinifierMaxLineLength;
                wfProfileIn( __METHOD__ );
 
@@ -179,11 +183,15 @@ class ResourceLoader {
                                                $wgResourceLoaderMinifierStatementsOnOwnLine,
                                                $wgResourceLoaderMinifierMaxLineLength
                                        );
-                                       $result .= "\n/* cache key: $key */";
+                                       if ( $cacheReport ) {
+                                               $result .= "\n/* cache key: $key */";
+                                       }
                                        break;
                                case 'minify-css':
                                        $result = CSSMin::minify( $data );
-                                       $result .= "\n/* cache key: $key */";
+                                       if ( $cacheReport ) {
+                                               $result .= "\n/* cache key: $key */";
+                                       }
                                        break;
                        }
 
@@ -1074,15 +1082,15 @@ class ResourceLoader {
         * Returns JS code which calls mw.loader.register with the given
         * parameters. Has three calling conventions:
         *
-        *   - ResourceLoader::makeLoaderRegisterScript( $name, $version, $dependencies, $group, $source ):
+        *   - ResourceLoader::makeLoaderRegisterScript( $name, $version, $dependencies, $group, $source, $skip ):
         *       Register a single module.
         *
         *   - ResourceLoader::makeLoaderRegisterScript( array( $name1, $name2 ) ):
         *       Register modules with the given names.
         *
         *   - ResourceLoader::makeLoaderRegisterScript( array(
-        *        array( $name1, $version1, $dependencies1, $group1, $source1 ),
-        *        array( $name2, $version2, $dependencies1, $group2, $source2 ),
+        *        array( $name1, $version1, $dependencies1, $group1, $source1, $skip1 ),
+        *        array( $name2, $version2, $dependencies1, $group2, $source2, $skip2 ),
         *        ...
         *     ) ):
         *        Registers modules with the given names and parameters.
@@ -1092,10 +1100,11 @@ class ResourceLoader {
         * @param array $dependencies List of module names on which this module depends
         * @param string $group Group which the module is in
         * @param string $source Source of the module, or 'local' if not foreign
+        * @param string $skip Script body of the skip function
         * @return string
         */
        public static function makeLoaderRegisterScript( $name, $version = null,
-               $dependencies = null, $group = null, $source = null
+               $dependencies = null, $group = null, $source = null, $skip = null
        ) {
                if ( is_array( $name ) ) {
                        return Xml::encodeJsCall(
@@ -1107,7 +1116,7 @@ class ResourceLoader {
                        $version = (int)$version > 1 ? (int)$version : 1;
                        return Xml::encodeJsCall(
                                'mw.loader.register',
-                               array( $name, $version, $dependencies, $group, $source ),
+                               array( $name, $version, $dependencies, $group, $source, $skip ),
                                ResourceLoader::inDebugMode()
                        );
                }
@@ -1201,13 +1210,24 @@ class ResourceLoader {
         * @return bool
         */
        public static function inDebugMode() {
-               global $wgRequest, $wgResourceLoaderDebug;
-               static $retval = null;
-               if ( is_null( $retval ) ) {
-                       $retval = $wgRequest->getFuzzyBool( 'debug',
-                               $wgRequest->getCookie( 'resourceLoaderDebug', '', $wgResourceLoaderDebug ) );
+               if ( self::$debugMode === null ) {
+                       global $wgRequest, $wgResourceLoaderDebug;
+                       self::$debugMode = $wgRequest->getFuzzyBool( 'debug',
+                               $wgRequest->getCookie( 'resourceLoaderDebug', '', $wgResourceLoaderDebug )
+                       );
                }
-               return $retval;
+               return self::$debugMode;
+       }
+
+       /**
+        * Reset static members used for caching.
+        *
+        * Global state and $wgRequest are evil, but we're using it right
+        * now and sometimes we need to be able to force ResourceLoader to
+        * re-evaluate the context because it has changed (e.g. in the test suite).
+        */
+       public static function clearCache() {
+               self::$debugMode = null;
        }
 
        /**
index f9ff029..fa9a8f0 100644 (file)
@@ -106,6 +106,11 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
         */
        protected $dependencies = array();
 
+       /**
+        * @var string File name containing the body of the skip function
+        */
+       protected $skipFunction = null;
+
        /**
         * @var array List of message keys used by this module
         * @par Usage:
@@ -204,6 +209,10 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
         *         'group' => [group name string],
         *         // Position on the page to load this module at
         *         'position' => ['bottom' (default) or 'top']
+        *         // Function that, if it returns true, makes the loader skip this module.
+        *         // The file must contain valid JavaScript for execution in a private function.
+        *         // The file must not contain the "function () {" and "}" wrapper though.
+        *         'skipFunction' => [file path]
         *     )
         * @endcode
         */
@@ -267,6 +276,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                                case 'position':
                                case 'localBasePath':
                                case 'remoteBasePath':
+                               case 'skipFunction':
                                        $this->{$member} = (string)$option;
                                        break;
                                // Single booleans
@@ -410,6 +420,28 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                return $this->dependencies;
        }
 
+       /**
+        * Get the skip function.
+        *
+        * @return string|null
+        */
+       public function getSkipFunction() {
+               if ( !$this->skipFunction ) {
+                       return null;
+               }
+
+               global $wgResourceLoaderValidateStaticJS;
+               $localPath = $this->getLocalPath( $this->skipFunction );
+               if ( !file_exists( $localPath ) ) {
+                       throw new MWException( __METHOD__ . ": skip function file not found: \"$localPath\"" );
+               }
+               $contents = file_get_contents( $localPath );
+               if ( $wgResourceLoaderValidateStaticJS ) {
+                       $contents = $this->validateScriptFile( $fileName, $contents );
+               }
+               return $contents;
+       }
+
        /**
         * @return bool
         */
@@ -463,6 +495,9 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        self::tryForKey( $this->skinScripts, $context->getSkin(), 'default' ),
                        $this->loaderScripts
                );
+               if ( $this->skipFunction ) {
+                       $files[] = $this->skipFunction;
+               }
                $files = array_map( array( $this, 'getLocalPath' ), $files );
                // File deps need to be treated separately because they're already prefixed
                $files = array_merge( $files, $this->getFileDependencies( $context->getSkin() ) );
@@ -511,6 +546,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        'targets',
                        'group',
                        'position',
+                       'skipFunction',
                        'localBasePath',
                        'remoteBasePath',
                        'debugRaw',
index 9ddd184..f636105 100644 (file)
@@ -293,6 +293,24 @@ abstract class ResourceLoaderModule {
                return $this->targets;
        }
 
+       /**
+        * Get the skip function.
+        *
+        * Modules that provide fallback functionality can provide a "skip function". This
+        * function, if provided, will be passed along to the module registry on the client.
+        * When this module is loaded (either directly or as a dependency of another module),
+        * then this function is executed first. If the function returns true, the module will
+        * instantly be considered "ready" without requesting the associated module resources.
+        *
+        * The value returned here must be valid javascript for execution in a private function.
+        * It must not contain the "function () {" and "}" wrapper though.
+        *
+        * @return string|null A JavaScript function body returning a boolean value, or null
+        */
+       public function getSkipFunction() {
+               return null;
+       }
+
        /**
         * Get the files this module depends on indirectly for a given skin.
         * Currently these are only image files referenced by the module's CSS.
index 63a444b..8a936c6 100644 (file)
@@ -222,12 +222,24 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
 
                        // FIXME: Convert to numbers, wfTimestamp always gives us stings, even for TS_UNIX
 
+                       $skipFunction = $module->getSkipFunction();
+                       if ( $skipFunction !== null && !ResourceLoader::inDebugMode() ) {
+                               $skipFunction = $resourceLoader->filter( 'minify-js',
+                                       $skipFunction,
+                                       // There will potentially be lots of these little string in the registrations
+                                       // manifest, we don't want to blow up the startup module with
+                                       // "/* cache key: ... */" all over it in non-debug mode.
+                                       /* cacheReport = */ false
+                               );
+                       }
+
                        $registryData[ $name ] = array(
                                'version' => $mtime,
                                'dependencies' => $module->getDependencies(),
                                'group' => $module->getGroup(),
                                'source' => $module->getSource(),
                                'loader' => $module->getLoaderScript(),
+                               'skip' => $skipFunction,
                        );
                }
 
@@ -255,17 +267,25 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        if (
                                !count( $data['dependencies'] ) &&
                                $data['group'] === null &&
-                               $data['source'] === 'local'
+                               $data['source'] === 'local' &&
+                               $data['skip'] === null
                        ) {
-                               // Modules without dependencies, a group or a foreign source;
+                               // Modules with no dependencies, group, foreign source or skip function;
                                // call mw.loader.register(name, timestamp)
                                $registrations[] = array( $name, $data['version'] );
-                       } elseif ( $data['group'] === null && $data['source'] === 'local' ) {
-                               // Modules with dependencies but no group or foreign source;
+                       } elseif (
+                               $data['group'] === null &&
+                               $data['source'] === 'local' &&
+                               $data['skip'] === null
+                       ) {
+                               // Modules with dependencies but no group, foreign source or skip function;
                                // call mw.loader.register(name, timestamp, dependencies)
                                $registrations[] = array( $name, $data['version'], $data['dependencies'] );
-                       } elseif ( $data['source'] === 'local' ) {
-                               // Modules with a group but no foreign source;
+                       } elseif (
+                               $data['source'] === 'local' &&
+                               $data['skip'] === null
+                       ) {
+                               // Modules with a group but no foreign source or skip function;
                                // call mw.loader.register(name, timestamp, dependencies, group)
                                $registrations[] = array(
                                        $name,
@@ -273,8 +293,8 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                                        $data['dependencies'],
                                        $data['group']
                                );
-                       } else {
-                               // Modules with a foreign source;
+                       } elseif ( $data['skip'] === null ) {
+                               // Modules with a foreign source but no skip function;
                                // call mw.loader.register(name, timestamp, dependencies, group, source)
                                $registrations[] = array(
                                        $name,
@@ -283,6 +303,17 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                                        $data['group'],
                                        $data['source']
                                );
+                       } else {
+                               // Modules with a skip function;
+                               // call mw.loader.register(name, timestamp, dependencies, group, source, skip)
+                               $registrations[] = array(
+                                       $name,
+                                       $data['version'],
+                                       $data['dependencies'],
+                                       $data['group'],
+                                       $data['source'],
+                                       $data['skip']
+                               );
                        }
                }
 
index c1815a5..c1f1844 100644 (file)
                         * @throws {Error} If any unregistered module or a dependency loop is encountered
                         */
                        function sortDependencies( module, resolved, unresolved ) {
-                               var n, deps, len;
+                               var n, deps, len, skip;
 
                                if ( registry[module] === undefined ) {
                                        throw new Error( 'Unknown dependency: ' + module );
                                }
+
+                               if ( registry[module].skip !== null ) {
+                                       /*jshint evil:true */
+                                       skip = new Function( registry[module].skip );
+                                       registry[module].skip = null;
+                                       if ( skip() ) {
+                                               registry[module].dependencies = [];
+                                               registry[module].state = 'ready';
+                                               handlePending( module );
+                                               return;
+                                       }
+                               }
+
                                // Resolves dynamic loader function and replaces it with its own results
                                if ( $.isFunction( registry[module].dependencies ) ) {
                                        registry[module].dependencies = registry[module].dependencies();
                                 *  names on which this module depends, or a function that returns that array.
                                 * @param {string} [group=null] Group which the module is in
                                 * @param {string} [source='local'] Name of the source
+                                * @param {string} [skip=null] Script body of the skip function
                                 */
-                               register: function ( module, version, dependencies, group, source ) {
+                               register: function ( module, version, dependencies, group, source, skip ) {
                                        var m;
                                        // Allow multiple registration
                                        if ( typeof module === 'object' ) {
                                                dependencies: [],
                                                group: typeof group === 'string' ? group : null,
                                                source: typeof source === 'string' ? source : 'local',
-                                               state: 'registered'
+                                               state: 'registered',
+                                               skip: typeof skip === 'string' ? skip : null
                                        };
                                        if ( typeof dependencies === 'string' ) {
                                                // Allow dependencies to be given as a single module name
index f8c4c6c..daf4bd9 100644 (file)
@@ -4,7 +4,6 @@ abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
        protected static function getResourceLoaderContext() {
                $resourceLoader = new ResourceLoader();
                $request = new FauxRequest( array(
-                               'debug' => 'true',
                                'lang' => 'en',
                                'modules' => 'startup',
                                'only' => 'scripts',
@@ -17,6 +16,8 @@ abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
        protected function setUp() {
                parent::setUp();
 
+               ResourceLoader::clearCache();
+
                $this->setMwGlobals( array(
                        // For ResourceLoader::inDebugMode since it doesn't have context
                        'wgResourceLoaderDebug' => true,
@@ -42,6 +43,7 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
        protected $dependencies = array();
        protected $group = null;
        protected $source = 'local';
+       protected $skipFunction = null;
        protected $targets = array( 'test' );
 
        public function __construct( $options = array() ) {
@@ -61,6 +63,10 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
        public function getSource() {
                return $this->source;
        }
+
+       public function getSkipFunction() {
+               return $this->skipFunction;
+       }
 }
 
 class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {
index c4412de..df08972 100644 (file)
@@ -114,6 +114,67 @@ mw.loader.addSource( {
         "example"
     ]
 ] );'
+                       ) ),
+                       array( array(
+                               'msg' => 'Conditional dependency function',
+                               'modules' => array(
+                                       'test.x.core' => new ResourceLoaderTestModule(),
+                                       'test.x.polyfill' => new ResourceLoaderTestModule( array(
+                                               'skipFunction' => 'return true;'
+                                       ) ),
+                                       'test.y.polyfill' => new ResourceLoaderTestModule( array(
+                                               'skipFunction' =>
+                                                       'return !!(' .
+                                                       '    window.JSON &&' .
+                                                       '    JSON.parse &&' .
+                                                       '    JSON.stringify' .
+                                                       ');'
+                                       ) ),
+                                       'test.z.foo' => new ResourceLoaderTestModule( array(
+                                               'dependencies' => array(
+                                                       'test.x.core',
+                                                       'test.x.polyfil',
+                                                       'test.y.polyfil',
+                                               ),
+                                       ) ),
+                               ),
+                               'out' => '
+mw.loader.addSource( {
+    "local": {
+        "loadScript": "/w/load.php",
+        "apiScript": "/w/api.php"
+    }
+} );mw.loader.register( [
+    [
+        "test.x.core",
+        "1388534400"
+    ],
+    [
+        "test.x.polyfill",
+        "1388534400",
+        [],
+        null,
+        "local",
+        "return true;"
+    ],
+    [
+        "test.y.polyfill",
+        "1388534400",
+        [],
+        null,
+        "local",
+        "return !!(    window.JSON \u0026\u0026    JSON.parse \u0026\u0026    JSON.stringify);"
+    ],
+    [
+        "test.z.foo",
+        "1388534400",
+        [
+            "test.x.core",
+            "test.x.polyfil",
+            "test.y.polyfil"
+        ]
+    ]
+] );',
                        ) ),
                        array( array(
                                // This may seem like an edge case, but a plain MediaWiki core install
@@ -278,4 +339,77 @@ mw.loader.addSource( {
                );
        }
 
+       public static function provideRegistrations() {
+               return array(
+                       array( array(
+                               'test.blank' => new ResourceLoaderTestModule(),
+                               'test.min' => new ResourceLoaderTestModule( array(
+                                       'skipFunction' =>
+                                               'return !!(' .
+                                               '    window.JSON &&' .
+                                               '    JSON.parse &&' .
+                                               '    JSON.stringify' .
+                                               ');',
+                                       'dependencies' => array(
+                                               'test.blank',
+                                       ),
+                               ) ),
+                       ) )
+               );
+       }
+       /**
+        * @dataProvider provideRegistrations
+        */
+       public function testRegistrationsMinified( $modules ) {
+               $this->setMwGlobals( 'wgResourceLoaderDebug', false );
+
+               $context = self::getResourceLoaderContext();
+               $rl = $context->getResourceLoader();
+               $rl->register( $modules );
+               $this->assertEquals(
+'mw.loader.addSource({"local":{"loadScript":"/w/load.php","apiScript":"/w/api.php"}});'
+. 'mw.loader.register(['
+. '["test.blank","1388534400"],'
+. '["test.min","1388534400",["test.blank"],null,"local",'
+. '"return!!(window.JSON\u0026\u0026JSON.parse\u0026\u0026JSON.stringify);"'
+. ']]);',
+                       ResourceLoaderStartUpModule::getModuleRegistrations( $context ),
+                       'Minified output'
+               );
+       }
+
+       /**
+        * @dataProvider provideRegistrations
+        */
+       public function testRegistrationsUnminified( $modules ) {
+               $context = self::getResourceLoaderContext();
+               $rl = $context->getResourceLoader();
+               $rl->register( $modules );
+               $this->assertEquals(
+'mw.loader.addSource( {
+    "local": {
+        "loadScript": "/w/load.php",
+        "apiScript": "/w/api.php"
+    }
+} );mw.loader.register( [
+    [
+        "test.blank",
+        "1388534400"
+    ],
+    [
+        "test.min",
+        "1388534400",
+        [
+            "test.blank"
+        ],
+        null,
+        "local",
+        "return !!(    window.JSON \u0026\u0026    JSON.parse \u0026\u0026    JSON.stringify);"
+    ]
+] );',
+                       ResourceLoaderStartUpModule::getModuleRegistrations( $context ),
+                       'Unminified output'
+               );
+       }
+
 }
index f6eff77..ee729e6 100644 (file)
@@ -30,15 +30,30 @@ require_once __DIR__ . '/../../../includes/Xml.php';
 $moduleImplementations = array(
        'testUsesMissing' => "
 mw.loader.implement( 'testUsesMissing', function () {
-       QUnit.ok( false, 'Module test.usesMissing script should not run.');
+       QUnit.ok( false, 'Module usesMissing script should not run.' );
        QUnit.start();
 }, {}, {});
 ",
 
        'testUsesNestedMissing' => "
 mw.loader.implement( 'testUsesNestedMissing', function () {
-       QUnit.ok( false, 'Module testUsesNestedMissing script should not run.');
+       QUnit.ok( false, 'Module testUsesNestedMissing script should not run.' );
+       QUnit.start();
+}, {}, {});
+",
+
+       'testSkipped' =>"
+mw.loader.implement( 'testSkipped', function () {
+       QUnit.ok( false, 'Module testSkipped was supposed to be skipped.' );
 }, {}, {});
+",
+
+       'testNotSkipped' =>"
+mw.loader.implement( 'testNotSkipped', function () {}, {}, {});
+",
+
+       'testUsesSkippable' =>"
+mw.loader.implement( 'testUsesSkippable', function () {}, {}, {});
 ",
 );
 
index 0f6b445..9681330 100644 (file)
                }
        } ) );
 
+       mw.loader.addSource(
+               'testloader',
+               {
+                       loadScript: QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' )
+               }
+       );
+
        QUnit.test( 'Initial check', 8, function ( assert ) {
                assert.ok( window.jQuery, 'jQuery defined' );
                assert.ok( window.$, '$ defined' );
        } );
 
        QUnit.asyncTest( 'mw.loader dependency handling', 5, function ( assert ) {
-               mw.loader.addSource(
-                       'testloader',
-                       {
-                               loadScript: QUnit.fixurl( mw.config.get( 'wgScriptPath' ) + '/tests/qunit/data/load.mock.php' )
-                       }
-               );
-
                mw.loader.register( [
                        // [module, version, dependencies, group, source]
                        ['testMissing', '1', [], null, 'testloader'],
                );
        } );
 
+       QUnit.asyncTest( 'mw.loader skin-function handling', 5, function ( assert ) {
+               mw.loader.register( [
+                       // [module, version, dependencies, group, source, skip]
+                       ['testSkipped', '1', [], null, 'testloader', 'return true;'],
+                       ['testNotSkipped', '1', [], null, 'testloader', 'return false;'],
+                       ['testUsesSkippable', '1', ['testSkipped', 'testNotSkipped'], null, 'testloader']
+               ] );
+
+               function verifyModuleStates() {
+                       assert.equal( mw.loader.getState( 'testSkipped' ), 'ready', 'Module is ready when skipped' );
+                       assert.equal( mw.loader.getState( 'testNotSkipped' ), 'ready', 'Module is ready when not skipped but loaded' );
+                       assert.equal( mw.loader.getState( 'testUsesSkippable' ), 'ready', 'Module is ready when skippable dependencies are ready' );
+               }
+
+               mw.loader.using( ['testUsesSkippable'],
+                       function () {
+                               assert.ok( true, 'Success handler should be invoked.' );
+                               assert.ok( true ); // Dummy to match error handler and reach QUnit expect()
+
+                               verifyModuleStates();
+
+                               QUnit.start();
+                       },
+                       function ( e, badmodules ) {
+                               assert.ok( false, 'Error handler should not be invoked.' );
+                               assert.deepEqual( badmodules, [], 'Bad modules as expected.' );
+
+                               verifyModuleStates();
+
+                               QUnit.start();
+                       }
+               );
+       } );
+
        QUnit.asyncTest( 'mw.loader( "//protocol-relative" ) (bug 30825)', 2, function ( assert ) {
                // This bug was actually already fixed in 1.18 and later when discovered in 1.17.
                // Test is for regressions!