Core: Run before and after hooks with module context

The before and after hooks run once per module as long as there is at least one test in the module. Using environment inheritance allows us to use the module context in those hooks, which allows reading the expected changes to the context from a before hook inside nested modules. Fixes qunitjs#1328. Ref qunitjs#869.
raycohen · Mar 20, 2021 · d0958a6 · d0958a6
1 parent 93bd4b6
commit d0958a6
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 39 deletions.
diff --git a/src/module.js b/src/module.js
@@ -39,10 +39,10 @@ function createModule( name, testEnvironment, modifiers ) {
 		ignored: modifiers.ignored || false
 	};
 
-	const env = {};
+	let env = {};
 	if ( parentModule ) {
 		parentModule.childModules.push( module );
-		extend( env, parentModule.testEnvironment );
+		env = Object.create( parentModule.testEnvironment || {} );
 	}
 	extend( env, testEnvironment );
 	module.testEnvironment = env;

diff --git a/src/test.js b/src/test.js
@@ -132,7 +132,7 @@ Test.prototype = {
 		return callbackPromises.then( () => {
 			config.current = this;
 
-			this.testEnvironment = extend( {}, module.testEnvironment );
+			this.testEnvironment = Object.create( module.testEnvironment || {} );
 
 			this.started = now();
 			emit( "testStart", this.testReport.start( true ) );
@@ -196,17 +196,18 @@ Test.prototype = {
 
 	queueHook( hook, hookName, hookOwner ) {
 		const callHook = () => {
-			const promise = hook.call( this.testEnvironment, this.assert );
+			let promise;
+			if ( hookName === "before" || hookName === "after" ) {
+				promise = hook.call( this.module.testEnvironment, this.assert );
+			} else {
+				promise = hook.call( this.testEnvironment, this.assert );
+			}
 			this.resolvePromise( promise, hookName );
 		};
 
 		const runHook = () => {
-			if ( hookName === "before" ) {
-				if ( hookOwner.testsRun !== 0 ) {
-					return;
-				}
-
-				this.preserveEnvironment = true;
+			if ( hookName === "before" && hookOwner.testsRun !== 0 ) {
+				return;
 			}
 
 			// The 'after' hook should only execute when there are not tests left and
@@ -381,13 +382,6 @@ Test.prototype = {
 		}
 	},
 
-	preserveTestEnvironment: function() {
-		if ( this.preserveEnvironment ) {
-			this.module.testEnvironment = this.testEnvironment;
-			this.testEnvironment = extend( {}, this.module.testEnvironment );
-		}
-	},
-
 	queue() {
 		const test = this;
 
@@ -403,11 +397,6 @@ Test.prototype = {
 				},
 
 				...test.hooks( "before" ),
-
-				function() {
-					test.preserveTestEnvironment();
-				},
-
 				...test.hooks( "beforeEach" ),
 
 				function() {

diff --git a/test/main/async.js b/test/main/async.js
@@ -314,18 +314,18 @@ QUnit.module( "assert.async", function() {
 		} );
 	} );
 
+	var inAfterHookModuleState;
 	QUnit.module( "in after hook", {
 		after: function( assert ) {
-			assert.equal( this.state, "done", "called after test callback" );
+			assert.equal( inAfterHookModuleState, "done", "called after test callback" );
 			assert.true( true, "called before expected assert count is validated" );
 		}
 	}, function() {
 		QUnit.test( "call order", function( assert ) {
 			assert.expect( 2 );
-			var done = assert.async(),
-				testContext = this;
+			var done = assert.async();
 			setTimeout( function() {
-				testContext.state = "done";
+				inAfterHookModuleState = "done";
 				done();
 			} );
 		} );

diff --git a/test/main/modules.js b/test/main/modules.js
@@ -1,31 +1,37 @@
 QUnit.module( "QUnit.module", function() {
-
+	var hooksOrder = [];
 	QUnit.module( "before/beforeEach/afterEach/after", {
-		before: function() {
+		before: function( assert ) {
+			assert.deepEqual( hooksOrder, [], "No hooks run before before" );
 			this.lastHook = "module-before";
+			hooksOrder.push( "module-before" );
 		},
 		beforeEach: function( assert ) {
-			assert.strictEqual( this.lastHook, "module-before",
+			assert.deepEqual( hooksOrder, [ "module-before" ],
 				"Module's beforeEach runs after before" );
 			this.lastHook = "module-beforeEach";
+			hooksOrder.push( "module-beforeEach" );
 		},
 		afterEach: function( assert ) {
-			assert.strictEqual( this.lastHook, "test-block",
+			assert.deepEqual( hooksOrder, [ "module-before", "module-beforeEach", "test-block" ],
 				"Module's afterEach runs after current test block" );
 			this.lastHook = "module-afterEach";
+			hooksOrder.push( "module-afterEach" );
 		},
 		after: function( assert ) {
-			assert.strictEqual( this.lastHook, "module-afterEach",
+			assert.deepEqual( hooksOrder, [ "module-before", "module-beforeEach", "test-block", "module-afterEach" ],
 				"Module's afterEach runs before after" );
 			this.lastHook = "module-after";
+			hooksOrder.push( "module-after" );
 		}
 	} );
 
 	QUnit.test( "hooks order", function( assert ) {
-		assert.expect( 4 );
+		assert.expect( 5 );
 
-		assert.strictEqual( this.lastHook, "module-beforeEach",
+		assert.deepEqual( hooksOrder, [ "module-before", "module-beforeEach" ],
 			"Module's beforeEach runs before current test block" );
+		hooksOrder.push( "test-block" );
 		this.lastHook = "test-block";
 	} );
 
@@ -285,6 +291,10 @@ QUnit.module( "QUnit.module", function() {
 	QUnit.module( "contained suite `this`", function( hooks ) {
 		this.outer = 1;
 
+		hooks.before( function() {
+			this.outerBefore = 10;
+		} );
+
 		hooks.beforeEach( function() {
 			this.outer++;
 		} );
@@ -309,7 +319,13 @@ QUnit.module( "QUnit.module", function() {
 		QUnit.module( "nested suite `this`", function( hooks ) {
 			this.inner = true;
 
+			hooks.before( function() {
+				this.innerBefore = 20;
+			} );
+
 			hooks.beforeEach( function( assert ) {
+				assert.strictEqual( this.outerBefore, 10 );
+				assert.strictEqual( this.innerBefore, 20 );
 				assert.strictEqual( this.outer, 2 );
 				assert.true( this.inner );
 			} );
@@ -334,13 +350,15 @@ QUnit.module( "QUnit.module", function() {
 		} );
 	} );
 
+	var lastRunHook;
 	QUnit.module( "nested modules before/after", {
 		before: function( assert ) {
 			assert.true( true, "before hook ran" );
 			this.lastHook = "before";
+			lastRunHook = "before";
 		},
 		after: function( assert ) {
-			assert.strictEqual( this.lastHook, "outer-after" );
+			assert.strictEqual( lastRunHook, "outer-after" );
 		}
 	}, function() {
 		QUnit.test( "should run before", function( assert ) {
@@ -352,10 +370,11 @@ QUnit.module( "QUnit.module", function() {
 			before: function( assert ) {
 				assert.true( true, "outer before hook ran" );
 				this.lastHook = "outer-before";
+				lastRunHook = "outer-before";
 			},
 			after: function( assert ) {
-				assert.strictEqual( this.lastHook, "outer-test" );
-				this.lastHook = "outer-after";
+				assert.strictEqual( lastRunHook, "outer-test" );
+				lastRunHook = "outer-after";
 			}
 		}, function() {
 			QUnit.module( "inner", {
@@ -364,7 +383,7 @@ QUnit.module( "QUnit.module", function() {
 					this.lastHook = "inner-before";
 				},
 				after: function( assert ) {
-					assert.strictEqual( this.lastHook, "inner-test" );
+					assert.strictEqual( lastRunHook, "inner-test" );
 				}
 			}, function() {
 				QUnit.test( "should run outer-before and inner-before", function( assert ) {
@@ -374,13 +393,13 @@ QUnit.module( "QUnit.module", function() {
 
 				QUnit.test( "should run inner-after", function( assert ) {
 					assert.expect( 1 );
-					this.lastHook = "inner-test";
+					lastRunHook = "inner-test";
 				} );
 			} );
 
 			QUnit.test( "should run outer-after and after", function( assert ) {
 				assert.expect( 2 );
-				this.lastHook = "outer-test";
+				lastRunHook = "outer-test";
 			} );
 		} );
 	} );