Core: Advance the processQueue when errors are thrown in callbacks

Author: step2yeung

Gruntfile.js

@@ -151,6 +151,7 @@ module.exports = function( grunt ) {
 						"test/reorderError2.html",
 						"test/callbacks.html",
 						"test/callbacks-promises.html",
+						"test/callbacks-rejected-promises.html",
 						"test/events.html",
 						"test/events-in-test.html",
 						"test/logs.html",

reporter/html.js

@@ -858,16 +858,15 @@ export function escapeText( s ) {
 	} );
 
 	QUnit.testDone( function( details ) {
-		var testTitle, time, testItem, assertList, status,
+		var testTitle, time, assertList, status,
 			good, bad, testCounts, skipped, sourceName,
-			tests = id( "qunit-tests" );
+			tests = id( "qunit-tests" ),
+			testItem = id( "qunit-test-output-" + details.testId );
 
-		if ( !tests ) {
+		if ( !tests || !testItem ) {
 			return;
 		}
 
-		testItem = id( "qunit-test-output-" + details.testId );
-
 		removeClass( testItem, "running" );
 
 		if ( details.failed > 0 ) {

src/core.js

@@ -27,8 +27,8 @@ export const globalSuite = new SuiteReport();
 // it since each module has a suiteReport associated with it.
 config.currentModule.suiteReport = globalSuite;
 
-var globalStartCalled = false;
-var runStarted = false;
+let globalStartCalled = false;
+let runStarted = false;
 
 // Figure out if we're running the tests from a server or not
 QUnit.isLocal = !( defined.document && window.location.protocol !== "file:" );
@@ -50,7 +50,7 @@ extend( QUnit, {
 	only: only,
 
 	start: function( count ) {
-		var globalStartAlreadyCalled = globalStartCalled;
+		const globalStartAlreadyCalled = globalStartCalled;
 
 		if ( !config.current ) {
 			globalStartCalled = true;
@@ -149,8 +149,7 @@ function unblockAndAdvanceQueue() {
 }
 
 export function begin() {
-	var i, l,
-		modulesLog = [];
+	const modulesLog = [];
 
 	// If the test run hasn't officially begun yet
 	if ( !config.started ) {
@@ -164,19 +163,22 @@ export function begin() {
 		}
 
 		// Avoid unnecessary information by not logging modules' test environments
-		for ( i = 0, l = config.modules.length; i < l; i++ ) {
+		config.modules.forEach( configModule => {
 			modulesLog.push( {
-				name: config.modules[ i ].name,
-				tests: config.modules[ i ].tests
+				name: configModule.name,
+				tests: configModule.tests
 			} );
-		}
+		} );
 
 		// The test run is officially beginning now
 		emit( "runStart", globalSuite.start( true ) );
 		runLoggingCallbacks( "begin", {
 			totalTests: Test.count,
 			modules: modulesLog
-		} ).then( unblockAndAdvanceQueue );
+		} ).then( unblockAndAdvanceQueue, function( err ) {
+			setTimeout( unblockAndAdvanceQueue );
+			throw err;
+		} );
 	} else {
 		unblockAndAdvanceQueue();
 	}

src/core/logging.js

@@ -1,41 +1,73 @@
 import config from "./config";
 import { objectType } from "./utilities";
 import Promise from "../promise";
+import {
+	setTimeout
+} from "../globals";
 
-// Register logging callbacks
-export function registerLoggingCallbacks( obj ) {
-	var i, l, key,
-		callbackNames = [ "begin", "done", "log", "testStart", "testDone",
-			"moduleStart", "moduleDone" ];
-
-	function registerLoggingCallback( key ) {
-		var loggingCallback = function( callback ) {
-			if ( objectType( callback ) !== "function" ) {
-				throw new Error(
-					"QUnit logging methods require a callback function as their first parameters."
-				);
+const	callbackNames = [
+	"begin",
+	"done",
+	"log",
+	"testStart",
+	"testDone",
+	"moduleStart",
+	"moduleDone"
+];
+
+function _promisfyCallbacksSequentially( callbacks, args ) {
+	return callbacks.reduce( ( promiseChain, callback ) => {
+		return promiseChain.then(
+			() => callback( args ),
+			( err ) => {
+				setTimeout( callback( args ) );
+				throw err;
 			}
+		);
+	}, Promise.resolve( [] ) );
+}
 
-			config.callbacks[ key ].push( callback );
-		};
+function _registerLoggingCallback( key ) {
+	const loggingCallback = ( callback ) => {
+		if ( objectType( callback ) !== "function" ) {
+			throw new Error(
+				"QUnit logging methods require a callback function as their first parameters."
+			);
+		}
 
-		return loggingCallback;
-	}
+		config.callbacks[ key ].push( callback );
+	};
 
-	for ( i = 0, l = callbackNames.length; i < l; i++ ) {
-		key = callbackNames[ i ];
+	return loggingCallback;
+}
+
+/**
+ * Register logging callbacks to qunit
+ *
+ * @export
+ * @param {Object} qunit
+ */
+export function registerLoggingCallbacks( qunit ) {
+	callbackNames.forEach( key => {
 
 		// Initialize key collection of logging callback
 		if ( objectType( config.callbacks[ key ] ) === "undefined" ) {
 			config.callbacks[ key ] = [];
 		}
-
-		obj[ key ] = registerLoggingCallback( key );
-	}
+		qunit[ key ] = _registerLoggingCallback( key );
+	} );
 }
 
+/**
+ * execute all callbacks from the given key as a sequential list of promises
+ *
+ * @export
+ * @param {String} key
+ * @param {Object} args
+ * @returns {Promise}
+ */
 export function runLoggingCallbacks( key, args ) {
-	var callbacks = config.callbacks[ key ];
+	const callbacks = config.callbacks[ key ];
 
 	// Handling 'log' callbacks separately. Unlike the other callbacks,
 	// the log callback is not controlled by the processing queue,
@@ -46,10 +78,5 @@ export function runLoggingCallbacks( key, args ) {
 		return;
 	}
 
-	// ensure that each callback is executed serially
-	return callbacks.reduce( ( promiseChain, callback ) => {
-		return promiseChain.then( () => {
-			return Promise.resolve( callback( args ) );
-		} );
-	}, Promise.resolve( [] ) );
+	return _promisfyCallbacksSequentially( callbacks, args );
 }

src/core/processing-queue.js

@@ -61,13 +61,19 @@ function processTaskQueue( start ) {
 
 		if ( !defined.setTimeout || config.updateRate <= 0 || elapsedTime < config.updateRate ) {
 			const task = taskQueue.shift();
-			Promise.resolve( task() ).then( function() {
+			const processNextTaskOrAdvance = () => {
 				if ( !taskQueue.length ) {
 					advance();
 				} else {
 					processTaskQueue( start );
 				}
-			} );
+			};
+			const throwAndAdvance = ( err ) => {
+				setTimeout( advance );
+				throw err;
+			};
+
+			Promise.resolve( task() ).then( processNextTaskOrAdvance, throwAndAdvance );
 		} else {
 			setTimeout( advance );
 		}
@@ -153,20 +159,9 @@ function unitSamplerGenerator( seed ) {
 	};
 }
 
-/**
- * This function is called when the ProcessingQueue is done processing all
- * items. It handles emitting the final run events.
- */
-function done() {
-	const storage = config.storage;
-
-	ProcessingQueue.finished = true;
-
-	const runtime = now() - config.started;
-	const passed = config.stats.all - config.stats.bad;
-
+// Throws corresponding error if no tests were executed
+function throwErrorIfNoTestExecuted() {
 	if ( config.stats.all === 0 ) {
-
 		if ( config.filter && config.filter.length ) {
 			throw new Error( `No tests matched the filter "${config.filter}".` );
 		}
@@ -182,29 +177,46 @@ function done() {
 		if ( config.testId && config.testId.length ) {
 			throw new Error( `No tests matched the testId "${config.testId}".` );
 		}
-
 		throw new Error( "No tests were run." );
+	}
+}
+
+// Clear own storage items when tests completes
+function cleanStorage() {
+	const storage = config.storage;
+	if ( storage && config.stats.bad === 0 ) {
+		for ( let i = storage.length - 1; i >= 0; i-- ) {
+			const key = storage.key( i );
 
+			if ( key.indexOf( "qunit-test-" ) === 0 ) {
+				storage.removeItem( key );
+			}
+		}
+	}
+}
+
+/**
+ * This function is called when the ProcessingQueue is done processing all
+ * items. It handles emitting the final run events.
+ */
+function done() {
+	ProcessingQueue.finished = true;
+
+	try {
+		throwErrorIfNoTestExecuted();
+	} catch ( err ) {
+		throw err;
 	}
 
 	emit( "runEnd", globalSuite.end( true ) );
 	runLoggingCallbacks( "done", {
-		passed,
+		passed: config.stats.all - config.stats.bad,
 		failed: config.stats.bad,
 		total: config.stats.all,
-		runtime
-	} ).then( () => {
-
-		// Clear own storage items if all tests passed
-		if ( storage && config.stats.bad === 0 ) {
-			for ( let i = storage.length - 1; i >= 0; i-- ) {
-				const key = storage.key( i );
-
-				if ( key.indexOf( "qunit-test-" ) === 0 ) {
-					storage.removeItem( key );
-				}
-			}
-		}
+		runtime: now() - config.started
+	} ).then( cleanStorage, function( err ) {
+		cleanStorage();
+		throw err;
 	} );
 }