Skip to content

Commit

Permalink
test_runner: defer inheriting hooks until run()
Browse files Browse the repository at this point in the history
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: #53927
Fixes: #51548
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
cjihrig authored and RafaelGSS committed Aug 25, 2024
1 parent 45b0250 commit 858b583
Showing 1 changed file with 25 additions and 17 deletions.
42 changes: 25 additions & 17 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand All @@ -715,7 +722,6 @@ class Test extends AsyncResource {
} else {
ArrayPrototypePush(this.hooks[name], hook);
}
return hook;
}

fail(err) {
Expand Down Expand Up @@ -817,6 +823,7 @@ class Test extends AsyncResource {
async run() {
if (this.parent !== null) {
this.parent.activeSubtests++;
this.computeInheritedHooks();
}
this.startTime ??= hrtime();

Expand Down Expand Up @@ -1211,6 +1218,7 @@ class Suite extends Test {
}

async run() {
this.computeInheritedHooks();
const hookArgs = this.getRunArgs();

let stopPromise;
Expand Down

0 comments on commit 858b583

Please sign in to comment.