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 · Jun 9, 2024 · b8fe4db · b8fe4db
1 parent fd17b66
commit b8fe4db
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 44 deletions.
diff --git a/src/module.js b/src/module.js
@@ -22,9 +22,9 @@ function createModule (name, testEnvironment, modifiers) {
   const skip = (parentModule !== null && parentModule.skip) || modifiers.skip;
   const todo = (parentModule !== null && parentModule.todo) || modifiers.todo;
 
-  const env = {};
+  let env = {};
   if (parentModule) {
-    extend(env, parentModule.testEnvironment);
+    env = Object.create(parentModule.testEnvironment || {});
   }
   extend(env, testEnvironment);
 

diff --git a/src/test.js b/src/test.js
@@ -158,7 +158,7 @@ Test.prototype = {
     return moduleStartChain.then(() => {
       config.current = this;
 
-      this.testEnvironment = extend({}, module.testEnvironment);
+      this.testEnvironment = Object.create(module.testEnvironment || {});
 
       this.started = performance.now();
       emit('testStart', this.testReport.start(true));
@@ -251,17 +251,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
@@ -485,13 +486,6 @@ Test.prototype = {
     }
   },
 
-  preserveTestEnvironment: function () {
-    if (this.preserveEnvironment) {
-      this.module.testEnvironment = this.testEnvironment;
-      this.testEnvironment = extend({}, this.module.testEnvironment);
-    }
-  },
-
   queue () {
     const test = this;
 
@@ -507,11 +501,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
@@ -242,18 +242,22 @@ QUnit.module('assert.async', function () {
     QUnit.test('test', function () {});
   });
 
+  var inBeforeHookModuleState;
   QUnit.module('in before hook', {
     before: function (assert) {
       var done = assert.async();
-      var testContext = this;
       setTimeout(function () {
-        testContext.state = 'before';
+        inBeforeHookModuleState = 'before';
         done();
       });
     }
   }, function () {
     QUnit.test('call order', function (assert) {
-      assert.equal(this.state, 'before', 'called before test callback');
+      assert.equal(
+        inBeforeHookModuleState,
+        'before',
+        'called before test callback'
+      );
     });
   });
 
@@ -289,18 +293,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();
-      var testContext = this;
       setTimeout(function () {
-        testContext.state = 'done';
+        inAfterHookModuleState = 'done';
         done();
       });
     });

diff --git a/test/main/modules.js b/test/main/modules.js
@@ -1,32 +1,35 @@
 /* global setTimeout */
 QUnit.module('QUnit.module', function () {
+  var hooksOrder = [];
   QUnit.module('before/beforeEach/afterEach/after', {
-    before: function () {
+    before: function (assert) {
+      assert.deepEqual(hooksOrder, [], 'before hook runs first');
       this.lastHook = 'module-before';
+      hooksOrder.push('module-before');
     },
     beforeEach: function (assert) {
-      assert.strictEqual(this.lastHook, 'module-before',
-        "Module's beforeEach runs after before");
+      assert.deepEqual(hooksOrder, ['module-before'], 'beforeEach runs after before');
       this.lastHook = 'module-beforeEach';
+      hooksOrder.push('module-beforeEach');
     },
     afterEach: function (assert) {
-      assert.strictEqual(this.lastHook, 'test-block',
-        "Module's afterEach runs after current test block");
+      assert.deepEqual(hooksOrder, ['module-before', 'module-beforeEach', 'test-block'], 'afterEach runs after current test block');
       this.lastHook = 'module-afterEach';
+      hooksOrder.push('module-afterEach');
     },
     after: function (assert) {
-      assert.strictEqual(this.lastHook, 'module-afterEach',
-        "Module's afterEach runs before after");
+      assert.deepEqual(hooksOrder, ['module-before', 'module-beforeEach', 'test-block', 'module-afterEach'], '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',
-      "Module's beforeEach runs before current test block");
+    assert.deepEqual(hooksOrder, ['module-before', 'module-beforeEach'], 'beforeEach runs before current test block');
     this.lastHook = 'test-block';
+    hooksOrder.push('test-block');
   });
 
   QUnit.module('before', {
@@ -285,6 +288,10 @@ QUnit.module('QUnit.module', function () {
   QUnit.module('contained suite `this`', function (hooks) {
     this.outer = 1;
 
+    hooks.before(function () {
+      this.outerBefore = 'outer before value';
+    });
+
     hooks.beforeEach(function () {
       this.outer++;
     });
@@ -309,8 +316,14 @@ QUnit.module('QUnit.module', function () {
     QUnit.module('nested suite `this`', function (hooks) {
       this.inner = true;
 
+      hooks.before(function () {
+        this.innerBefore = 'inner before value';
+      });
+
       hooks.beforeEach(function (assert) {
         assert.strictEqual(this.outer, 2);
+        assert.strictEqual(this.outerBefore, 'outer before value');
+        assert.strictEqual(this.innerBefore, 'inner before value');
         assert.true(this.inner);
       });
 
@@ -334,13 +347,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 +367,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 +380,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 +390,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';
       });
     });
   });