From 858b583c881ef1f2efc5742580a497368c0f11ee Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Fri, 19 Jul 2024 14:56:35 -0400
Subject: [PATCH] test_runner: defer inheriting hooks until run()

This commit updates the way the test runner computes inherited
hooks. Instead of computing them when the Test/Suite is
constructed, they are now computed just prior to running the
Test/Suite. The reason is because when multiple test files are
run in the same process, it is possible for the inherited hooks
to change as more files are loaded.

PR-URL: https://github.com/nodejs/node/pull/53927
Fixes: https://github.com/nodejs/node/issues/51548
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 lib/internal/test_runner/test.js | 42 +++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index b79ff7a049ea6c..8d07d273653721 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -408,14 +408,6 @@ class Test extends AsyncResource {
       this.childNumber = 0;
       this.timeout = kDefaultTimeout;
       this.entryFile = entryFile;
-      this.hooks = {
-        __proto__: null,
-        before: [],
-        after: [],
-        beforeEach: [],
-        afterEach: [],
-        ownAfterEachCount: 0,
-      };
     } else {
       const nesting = parent.parent === null ? parent.nesting :
         parent.nesting + 1;
@@ -431,14 +423,6 @@ class Test extends AsyncResource {
       this.childNumber = parent.subtests.length + 1;
       this.timeout = parent.timeout;
       this.entryFile = parent.entryFile;
-      this.hooks = {
-        __proto__: null,
-        before: [],
-        after: [],
-        beforeEach: ArrayPrototypeSlice(parent.hooks.beforeEach),
-        afterEach: ArrayPrototypeSlice(parent.hooks.afterEach),
-        ownAfterEachCount: 0,
-      };
 
       if (this.willBeFiltered()) {
         this.filtered = true;
@@ -514,6 +498,14 @@ class Test extends AsyncResource {
     this.subtests = [];
     this.waitingOn = 0;
     this.finished = false;
+    this.hooks = {
+      __proto__: null,
+      before: [],
+      after: [],
+      beforeEach: [],
+      afterEach: [],
+      ownAfterEachCount: 0,
+    };
 
     if (!this.config.only && (only || this.parent?.runOnlySubtests)) {
       const warning =
@@ -691,6 +683,21 @@ class Test extends AsyncResource {
     this.abortController.abort();
   }
 
+  computeInheritedHooks() {
+    if (this.parent.hooks.beforeEach.length > 0) {
+      ArrayPrototypeUnshift(
+        this.hooks.beforeEach,
+        ...ArrayPrototypeSlice(this.parent.hooks.beforeEach),
+      );
+    }
+
+    if (this.parent.hooks.afterEach.length > 0) {
+      ArrayPrototypePushApply(
+        this.hooks.afterEach, ArrayPrototypeSlice(this.parent.hooks.afterEach),
+      );
+    }
+  }
+
   createHook(name, fn, options) {
     validateOneOf(name, 'hook name', kHookNames);
     // eslint-disable-next-line no-use-before-define
@@ -715,7 +722,6 @@ class Test extends AsyncResource {
     } else {
       ArrayPrototypePush(this.hooks[name], hook);
     }
-    return hook;
   }
 
   fail(err) {
@@ -817,6 +823,7 @@ class Test extends AsyncResource {
   async run() {
     if (this.parent !== null) {
       this.parent.activeSubtests++;
+      this.computeInheritedHooks();
     }
     this.startTime ??= hrtime();
 
@@ -1211,6 +1218,7 @@ class Suite extends Test {
   }
 
   async run() {
+    this.computeInheritedHooks();
     const hookArgs = this.getRunArgs();
 
     let stopPromise;