From 961cbf0be0b21cb01906a6feb50f35f458466597 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 12 Aug 2024 23:15:38 -0400 Subject: [PATCH] test_runner: refactor hook creation This commit makes hook creation more consistent by always passing in a reference to the test that owns the hook. It also removes some unnecessary validation on internal API. PR-URL: https://github.com/nodejs/node/pull/54353 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina --- lib/internal/test_runner/test.js | 59 ++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9fe5cf9ccd0e9c..d043d507a3d6ce 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -311,23 +311,43 @@ class TestContext { } before(fn, options) { - this.#test - .createHook('before', fn, { __proto__: null, ...options, hookType: 'before', loc: getCallerLocation() }); + this.#test.createHook('before', fn, { + __proto__: null, + ...options, + parent: this.#test, + hookType: 'before', + loc: getCallerLocation(), + }); } after(fn, options) { - this.#test - .createHook('after', fn, { __proto__: null, ...options, hookType: 'after', loc: getCallerLocation() }); + this.#test.createHook('after', fn, { + __proto__: null, + ...options, + parent: this.#test, + hookType: 'after', + loc: getCallerLocation(), + }); } beforeEach(fn, options) { - this.#test - .createHook('beforeEach', fn, { __proto__: null, ...options, hookType: 'beforeEach', loc: getCallerLocation() }); + this.#test.createHook('beforeEach', fn, { + __proto__: null, + ...options, + parent: this.#test, + hookType: 'beforeEach', + loc: getCallerLocation(), + }); } afterEach(fn, options) { - this.#test - .createHook('afterEach', fn, { __proto__: null, ...options, hookType: 'afterEach', loc: getCallerLocation() }); + this.#test.createHook('afterEach', fn, { + __proto__: null, + ...options, + parent: this.#test, + hookType: 'afterEach', + loc: getCallerLocation(), + }); } } @@ -1089,14 +1109,17 @@ class Test extends AsyncResource { class TestHook extends Test { #args; constructor(fn, options) { - if (options === null || typeof options !== 'object') { - options = kEmptyObject; - } - const { loc, timeout, signal } = options; - super({ __proto__: null, fn, loc, timeout, signal }); - - this.parentTest = options.parent ?? null; - this.hookType = options.hookType; + const { hookType, loc, parent, timeout, signal } = options; + super({ + __proto__: null, + fn, + loc, + timeout, + signal, + harness: parent.root.harness, + }); + this.parentTest = parent; + this.hookType = hookType; } run(args) { if (this.error && !this.outerSignal?.aborted) { @@ -1120,9 +1143,7 @@ class TestHook extends Test { const { error, loc, parentTest: parent } = this; // Report failures in the root test's after() hook. - if (error && parent !== null && - parent === parent.root && this.hookType === 'after') { - + if (error && parent === parent.root && this.hookType === 'after') { if (isTestFailureError(error)) { error.failureType = kHookFailure; }