Core: keeping count of tests not ran to ensure moduleDone is called

Gruntfile.js

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

src/core/config.js

@@ -46,7 +46,7 @@ const config = {
 		tests: [],
 		childModules: [],
 		testsRun: 0,
-		unskippedTestsRun: 0,
+		testsIgnored: 0,
 		hooks: {
 			before: [],
 			beforeEach: [],

src/module.js

@@ -23,7 +23,7 @@ function createModule( name, testEnvironment, modifiers ) {
 		tests: [],
 		moduleId: generateHash( moduleName ),
 		testsRun: 0,
-		unskippedTestsRun: 0,
+		testsIgnored: 0,
 		childModules: [],
 		suiteReport: new SuiteReport( name, parentSuite ),
 

src/test.js

@@ -208,7 +208,7 @@ Test.prototype = {
 
 		const runHook = () => {
 			if ( hookName === "before" ) {
-				if ( hookOwner.unskippedTestsRun !== 0 ) {
+				if ( hookOwner.testsRun !== 0 ) {
 					return;
 				}
 
@@ -217,8 +217,7 @@ Test.prototype = {
 
 			// The 'after' hook should only execute when there are not tests left and
 			// when the 'after' and 'finish' tasks are the only tasks left to process
-			if ( hookName === "after" &&
-				hookOwner.unskippedTestsRun !== numberOfUnskippedTests( hookOwner ) - 1 &&
+			if ( hookName === "after" && !lastTestWithinModuleExecuted( hookOwner ) &&
 				( config.queue.length > 0 || ProcessingQueue.taskCount() > 2 ) ) {
 				return;
 			}
@@ -309,7 +308,11 @@ Test.prototype = {
 			}
 		}
 
-		notifyTestsRan( module, skipped );
+		if ( skipped ) {
+			updateTestsIgnored( module );
+		} else {
+			updateTestsRun( module );
+		}
 
 		// Store result when possible
 		if ( storage ) {
@@ -344,13 +347,13 @@ Test.prototype = {
 			// generating stack trace is expensive, so using a getter will help defer this until we need it
 			get source() { return test.stack; }
 		} ).then( function() {
-			if ( module.testsRun === numberOfTests( module ) ) {
+			if ( allTestsExecuted( module ) ) {
 				const completedModules = [ module ];
 
 				// Check if the parent modules, iteratively, are done. If that the case,
 				// we emit the `suiteEnd` event and trigger `moduleDone` callback.
 				let parent = module.parentModule;
-				while ( parent && parent.testsRun === numberOfTests( parent ) ) {
+				while (	parent &&	allTestsExecuted( parent ) ) {
 					completedModules.push( parent );
 					parent = parent.parentModule;
 				}
@@ -394,6 +397,7 @@ Test.prototype = {
 		const test = this;
 
 		if ( !this.valid() ) {
+			updateTestsIgnored( this.module );
 			return;
 		}
 
@@ -551,10 +555,7 @@ Test.prototype = {
 	},
 
 	valid: function() {
-		var filter = config.filter,
-			regexFilter = /^(!?)\/([\w\W]*)\/(i?$)/.exec( filter ),
-			module = config.module && config.module.toLowerCase(),
-			fullName = ( this.module.name + ": " + this.testName );
+		var module = config.module && config.module.toLowerCase();
 
 		function moduleChainNameMatch( testModule ) {
 			var testModuleName = testModule.name ? testModule.name.toLowerCase() : null;
@@ -593,6 +594,14 @@ Test.prototype = {
 			return false;
 		}
 
+		return this.filterMatch();
+	},
+
+	filterMatch: function() {
+		var filter = config.filter,
+			regexFilter = /^(!?)\/([\w\W]*)\/(i?$)/.exec( filter ),
+			fullName = ( this.module.name + ": " + this.testName );
+
 		if ( !filter ) {
 			return true;
 		}
@@ -857,23 +866,24 @@ function collectTests( module ) {
 	return tests;
 }
 
-function numberOfTests( module ) {
-	return collectTests( module ).length;
+function allTestsExecuted( module ) {
+	return module.testsRun + module.testsIgnored === collectTests( module ).length;
 }
 
-function numberOfUnskippedTests( module ) {
-	return collectTests( module ).filter( test => !test.skip ).length;
+function lastTestWithinModuleExecuted( module ) {
+	return module.testsRun + module.testsIgnored === collectTests( module ).length - 1;
 }
 
-function notifyTestsRan( module, skipped ) {
+function updateTestsRun( module ) {
 	module.testsRun++;
-	if ( !skipped ) {
-		module.unskippedTestsRun++;
-	}
 	while ( ( module = module.parentModule ) ) {
 		module.testsRun++;
-		if ( !skipped ) {
-			module.unskippedTestsRun++;
-		}
+	}
+}
+
+function updateTestsIgnored( module ) {
+	module.testsIgnored++;
+	while ( ( module = module.parentModule ) ) {
+		module.testsIgnored++;
 	}
 }

test/callbacks-filters.html

@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta charset="UTF-8">
+	<title>QUnit Callbacks Filters Test Suite</title>
+	<link rel="stylesheet" href="../dist/qunit.css">
+	<script src="../dist/qunit.js"></script>
+	<script src="callbacks-filters.js"></script>
+</head>
+<body>
+	<div id="qunit"></div>
+	<div id="qunit-fixture"></div>
+</body>
+</html>

test/callbacks-filters.js

@@ -0,0 +1,171 @@
+var invokedHooks = [];
+var done = false;
+
+function callback( name ) {
+	return function() {
+		if ( done ) {
+			return;
+		}
+
+		invokedHooks.push( name );
+	};
+}
+
+QUnit.begin( callback( "begin" ) );
+QUnit.moduleStart( callback( "moduleStart" ) );
+QUnit.testStart( callback( "testStart" ) );
+QUnit.testDone( callback( "testDone" ) );
+QUnit.moduleDone( callback( "moduleDone" ) );
+QUnit.done( callback( "done" ) );
+
+QUnit.done( function() {
+	if ( done ) {
+		return;
+	}
+
+	done = true;
+
+	QUnit.test( "verify callback order", function( assert ) {
+		assert.deepEqual( invokedHooks, [
+			"begin",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"testStart",
+			"testDone",
+			"moduleDone",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"moduleDone",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"moduleDone",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"moduleDone",
+			"moduleDone",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"moduleStart",
+			"testStart",
+			"testDone",
+			"testStart",
+			"after",
+			"testDone",
+			"moduleDone",
+			"testStart",
+			"after",
+			"testDone",
+			"moduleDone",
+			"done"
+		] );
+	} );
+} );
+
+QUnit.config.moduleId = [ "1cf055b9" ];
+
+// match for module Id
+QUnit.module( "module1", function() {
+	QUnit.test( "test should run 1.1", function( assert ) {
+		assert.ok( true );
+	} );
+
+	QUnit.test( "test should run 1.2", function( assert ) {
+		assert.ok( true );
+	} );
+} );
+
+// no match module Id
+QUnit.module( "module2", function() {
+	QUnit.test( "test should NOT run 2.1", function( assert ) {
+		assert.ok( false );
+	} );
+
+	QUnit.test( "test should NOT run 2.2", function( assert ) {
+		assert.ok( false );
+	} );
+} );
+
+QUnit.config.moduleId = [];
+QUnit.config.testId = [ "3b3c4e75" ];
+
+QUnit.module( "module3", function() {
+
+	// match test Id
+	QUnit.test( "ABCtest should run 3.1", function( assert ) {
+		assert.ok( true );
+	} );
+
+	QUnit.test( "test should NOT run 3.2", function( assert ) {
+		assert.ok( false );
+	} );
+} );
+
+QUnit.config.testId = [];
+QUnit.config.filter = "!.2";
+
+QUnit.module( "module4", function() {
+
+	// match string filter
+	QUnit.test( "test should run 4.1", function( assert ) {
+		assert.ok( true );
+	} );
+
+	QUnit.test( "test should NOT run 4.2", function( assert ) {
+		assert.ok( false );
+	} );
+} );
+
+// Make sure nested module scenarios are correct
+QUnit.module( "module5", function() {
+	QUnit.test( "test should run 5", function( assert ) {
+		assert.ok( true );
+	} );
+
+	QUnit.module( "nestedModule5A", function() {
+		QUnit.test( "test should run 5.A.1", function( assert ) {
+			assert.ok( true );
+		} );
+	} );
+} );
+
+QUnit.module( "module6", {
+	after: function( ) {
+		invokedHooks.push( "after" );
+	}
+}, function() {
+	QUnit.test( "test should run 6.1", function( assert ) {
+		assert.ok( true );
+	} );
+
+	QUnit.test( "test should NOT run 6.2", function( assert ) {
+		assert.ok( true );
+	} );
+
+	QUnit.module( "nestedModule6A", {
+		after: function( ) {
+			invokedHooks.push( "after" );
+		}
+	}, function() {
+		QUnit.test( "test should run 6.A.1", function( assert ) {
+			assert.ok( true );
+		} );
+		QUnit.test( "test should run 6.A.2", function( assert ) {
+			assert.ok( true );
+		} );
+		QUnit.test( "test should run 6.A.3", function( assert ) {
+			assert.ok( true );
+		} );
+	} );
+
+	QUnit.test( "test should run 6.3", function( assert ) {
+		assert.ok( true );
+	} );
+} );

test/main/modules.js

@@ -81,14 +81,10 @@ QUnit.module( "after (skip)", {
 	}
 } );
 
-QUnit.test( "does not run after initial tests", function( assert ) {
+QUnit.test( "does not run after final non-skipped tests", function( assert ) {
 	assert.expect( 0 );
 } );
 
-QUnit.test( "runs after final unskipped test", function( assert ) {
-	assert.expect( 1 );
-} );
-
 QUnit.skip( "last test in module is skipped" );
 
 QUnit.module( "before/after with all tests skipped", {