test_runner: refactor hook creation

cjihrig · RafaelGSS · commit 961cbf0be0b2 · 2024-08-21T17:49:20.000-03:00
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: #54353 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git 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;
       }
