From 03b764d30ed33f5b23a9dbf2c6e2e81a847729d4 Mon Sep 17 00:00:00 2001 From: Ray Cohen Date: Mon, 24 Jun 2024 21:14:27 -0400 Subject: [PATCH] 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. Once before hooks have run, create a flattened deep copy of the module testEnvironment and assign that to test testEnvironment. At this point nothing should use test.testEnvironment until the before hooks have run. Fixes https://github.com/qunitjs/qunit/issues/1328. Ref https://github.com/qunitjs/qunit/issues/869. Closes https://github.com/qunitjs/qunit/pull/1762. --- src/core/utilities.js | 4 +- src/module.js | 4 +- src/test.js | 27 ++++------ test/main/async.js | 16 +++--- test/main/modules.js | 117 ++++++++++++++++++++---------------------- 5 files changed, 79 insertions(+), 89 deletions(-) diff --git a/src/core/utilities.js b/src/core/utilities.js index 19285a256..a9dc85999 100644 --- a/src/core/utilities.js +++ b/src/core/utilities.js @@ -75,9 +75,9 @@ export function objectValuesSubset (obj, model) { } // Support: IE 11, iOS 7-8 -export function extend (a, b, undefOnly) { +export function extend (a, b, undefOnly, allProperties) { for (const prop in b) { - if (hasOwn.call(b, prop)) { + if (hasOwn.call(b, prop) || allProperties) { if (b[prop] === undefined) { delete a[prop]; } else if (!(undefOnly && typeof a[prop] !== 'undefined')) { diff --git a/src/module.js b/src/module.js index bd3614c72..8990ca25c 100644 --- a/src/module.js +++ b/src/module.js @@ -21,9 +21,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 index 300feedf8..e75f57110 100644 --- a/src/test.js +++ b/src/test.js @@ -158,8 +158,6 @@ Test.prototype = { return moduleStartChain.then(() => { config.current = this; - this.testEnvironment = extend({}, module.testEnvironment); - this.started = performance.now(); emit('testStart', this.testReport.start(true)); return runLoggingCallbacks('testStart', { @@ -251,17 +249,19 @@ Test.prototype = { queueHook (hook, hookName, hookOwner) { const callHook = () => { - const promise = hook.call(this.testEnvironment, this.assert); + let promise; + if (hookName === 'before' || hookName === 'after') { + // before and after hooks are called with the owning module's testEnvironment + promise = hook.call(hookOwner.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 @@ -483,13 +483,6 @@ Test.prototype = { } }, - preserveTestEnvironment: function () { - if (this.preserveEnvironment) { - this.module.testEnvironment = this.testEnvironment; - this.testEnvironment = extend({}, this.module.testEnvironment); - } - }, - queue () { const test = this; @@ -507,7 +500,7 @@ Test.prototype = { ...test.hooks('before'), function () { - test.preserveTestEnvironment(); + test.testEnvironment = extend({}, test.module.testEnvironment, false, true); }, ...test.hooks('beforeEach'), diff --git a/test/main/async.js b/test/main/async.js index fff0292d3..dbf597dcf 100644 --- 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 index 053d6b889..1d6ce05a7 100644 --- a/test/main/modules.js +++ b/test/main/modules.js @@ -16,42 +16,36 @@ QUnit.module('QUnit.module', function () { // parent > child > one // parent > child > two 'parent-before: (empty)', - 'child-before: beforeP=1', - 'parent-beforeEach: beforeP=1 beforeC=1', - 'child-beforeEach: beforeP=1 beforeC=1 beforeEach=P', - 'child-test: beforeP=1 beforeC=1 beforeEach=PC', - 'child-afterEach: beforeP=1 beforeC=1 beforeEach=PC tester=1', - 'parent-afterEach: beforeP=1 beforeC=1 beforeEach=PC tester=1 afterEach=C', - 'parent-beforeEach: beforeP=1 beforeC=1', - 'child-beforeEach: beforeP=1 beforeC=1 beforeEach=P', - 'child-test: beforeP=1 beforeC=1 beforeEach=PC', - 'child-afterEach: beforeP=1 beforeC=1 beforeEach=PC tester=2', - 'parent-afterEach: beforeP=1 beforeC=1 beforeEach=PC tester=2 afterEach=C', - 'child-after: beforeP=1 beforeC=1 beforeEach=PC tester=2 afterEach=CP', - 'parent-after: beforeP=1 beforeC=1 beforeEach=PC tester=2 afterEach=CP afterC=1' + 'child-before: %beforeP=1', + 'parent-beforeEach: beforeC=1 beforeP=1', + 'child-beforeEach: beforeC=1 beforeP=1 beforeEach=P', + 'child-test: beforeC=1 beforeP=1 beforeEach=PC', + 'child-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=1', + 'parent-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=1 afterEach=C', + 'parent-beforeEach: beforeC=1 beforeP=1', + 'child-beforeEach: beforeC=1 beforeP=1 beforeEach=P', + 'child-test: beforeC=1 beforeP=1 beforeEach=PC', + 'child-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=2', + 'parent-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=2 afterEach=C', + 'child-after: beforeC=1 %beforeP=1', + 'parent-after: beforeP=1' ], - // FIXME: https://github.com/qunitjs/qunit/issues/1328 - // - parent test missing own state if there is a child module before the test. - // - last test state presists into after() 'parent with trailing test': [ // parent > child > one // parent > two 'parent-before: (empty)', - 'child-before: beforeP=1', - 'parent-beforeEach: beforeP=1 beforeC=1', - 'child-beforeEach: beforeP=1 beforeC=1 beforeEach=P', - 'child-test: beforeP=1 beforeC=1 beforeEach=PC', - 'child-afterEach: beforeP=1 beforeC=1 beforeEach=PC tester=1', - 'parent-afterEach: beforeP=1 beforeC=1 beforeEach=PC tester=1 afterEach=C', - 'child-after: beforeP=1 beforeC=1 beforeEach=PC tester=1 afterEach=CP', - 'parent-beforeEach: (empty)', - 'parent-test: beforeEach=P', - 'parent-afterEach: beforeEach=P tester=2', - 'parent-after: beforeEach=P tester=2 afterEach=P' + 'child-before: %beforeP=1', + 'parent-beforeEach: beforeC=1 beforeP=1', + 'child-beforeEach: beforeC=1 beforeP=1 beforeEach=P', + 'child-test: beforeC=1 beforeP=1 beforeEach=PC', + 'child-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=1', + 'parent-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=1 afterEach=C', + 'child-after: beforeC=1 %beforeP=1', + 'parent-beforeEach: beforeP=1', + 'parent-test: beforeP=1 beforeEach=P', + 'parent-afterEach: beforeP=1 beforeEach=P tester=2', + 'parent-after: beforeP=1' ], - - // FIXME: https://github.com/qunitjs/qunit/issues/1328 - // child is missing parent state if there is an initial test before the child module. 'parent with initial test': [ // parent > one // parent > child > two @@ -59,14 +53,14 @@ QUnit.module('QUnit.module', function () { 'parent-beforeEach: beforeP=1', 'parent-test: beforeP=1 beforeEach=P', 'parent-afterEach: beforeP=1 beforeEach=P tester=1', - 'child-before: (empty)', - 'parent-beforeEach: beforeC=1', - 'child-beforeEach: beforeC=1 beforeEach=P', - 'child-test: beforeC=1 beforeEach=PC', - 'child-afterEach: beforeC=1 beforeEach=PC tester=2', - 'parent-afterEach: beforeC=1 beforeEach=PC tester=2 afterEach=C', - 'child-after: beforeC=1 beforeEach=PC tester=2 afterEach=CP', - 'parent-after: beforeC=1 beforeEach=PC tester=2 afterEach=CP afterC=1' + 'child-before: %beforeP=1', + 'parent-beforeEach: beforeC=1 beforeP=1', + 'child-beforeEach: beforeC=1 beforeP=1 beforeEach=P', + 'child-test: beforeC=1 beforeP=1 beforeEach=PC', + 'child-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=2', + 'parent-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=2 afterEach=C', + 'child-after: beforeC=1 %beforeP=1', + 'parent-after: beforeP=1' ], // Confirm each step waits for the previous before restoring/saving testEnvironment @@ -78,34 +72,33 @@ QUnit.module('QUnit.module', function () { 'parent-beforeEach: beforeP=1', 'parent-test: beforeP=1 beforeEach=P', 'parent-afterEach: beforeP=1 beforeEach=P tester=1', - 'child-before: (empty)', - 'parent-beforeEach: beforeC=1', - 'child-beforeEach: beforeC=1 beforeEach=P', - 'child-test: beforeC=1 beforeEach=PC', - 'child-afterEach: beforeC=1 beforeEach=PC tester=2', - 'parent-afterEach: beforeC=1 beforeEach=PC tester=2 afterEach=C', - 'child-after: beforeC=1 beforeEach=PC tester=2 afterEach=CP', - 'parent-after: beforeC=1 beforeEach=PC tester=2 afterEach=CP afterC=1' + 'child-before: %beforeP=1', + 'parent-beforeEach: beforeC=1 beforeP=1', + 'child-beforeEach: beforeC=1 beforeP=1 beforeEach=P', + 'child-test: beforeC=1 beforeP=1 beforeEach=PC', + 'child-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=2', + 'parent-afterEach: beforeC=1 beforeP=1 beforeEach=PC tester=2 afterEach=C', + 'child-after: beforeC=1 %beforeP=1', + 'parent-after: beforeP=1' ], 'multiple hooks': [ - 'parent-before: (empty)', 'parent-before: beforeP=1', - 'child-before: beforeP=12', - 'child-before: beforeP=12 beforeC=1', - 'parent-beforeEach: beforeP=12 beforeC=12', - 'parent-beforeEach: beforeP=12 beforeC=12 beforeEach=P1', - 'child-beforeEach: beforeP=12 beforeC=12 beforeEach=P1P2', - 'child-beforeEach: beforeP=12 beforeC=12 beforeEach=P1P2C1', - 'child-test: beforeP=12 beforeC=12 beforeEach=P1P2C1C2', - 'child-afterEach: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2', - 'child-afterEach: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2', - 'parent-afterEach: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1', - 'parent-afterEach: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1P2', - 'child-after: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1P2P1', - 'child-after: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1P2P1 afterC=2', - 'parent-after: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1P2P1 afterC=21', - 'parent-after: beforeP=12 beforeC=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1P2P1 afterC=21 afterP=2' + 'child-before: %beforeP=12', + 'child-before: beforeC=1 %beforeP=12', + 'parent-beforeEach: beforeC=12 beforeP=12', + 'parent-beforeEach: beforeC=12 beforeP=12 beforeEach=P1', + 'child-beforeEach: beforeC=12 beforeP=12 beforeEach=P1P2', + 'child-beforeEach: beforeC=12 beforeP=12 beforeEach=P1P2C1', + 'child-test: beforeC=12 beforeP=12 beforeEach=P1P2C1C2', + 'child-afterEach: beforeC=12 beforeP=12 beforeEach=P1P2C1C2 tester=2', + 'child-afterEach: beforeC=12 beforeP=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2', + 'parent-afterEach: beforeC=12 beforeP=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1', + 'parent-afterEach: beforeC=12 beforeP=12 beforeEach=P1P2C1C2 tester=2 afterEach=C2C1P2', + 'child-after: beforeC=12 %beforeP=12', + 'child-after: beforeC=12 afterC=2 %beforeP=12', + 'parent-after: beforeP=12', + 'parent-after: beforeP=12 afterP=2' ] };