Skip to content

Commit

Permalink
test_runner: fix nested hooks
Browse files Browse the repository at this point in the history
PR-URL: nodejs#47648
Fixes: nodejs#47643
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
MoLow committed Jul 6, 2023
1 parent ffcff68 commit 0ea6371
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 27 deletions.
31 changes: 14 additions & 17 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ class Test extends AsyncResource {
this.testNumber = 0;
this.timeout = kDefaultTimeout;
this.root = this;
this.hooks = {
__proto__: null,
before: [],
after: [],
beforeEach: [],
afterEach: [],
};
} else {
const nesting = parent.parent === null ? parent.nesting :
parent.nesting + 1;
Expand All @@ -204,6 +211,13 @@ class Test extends AsyncResource {
this.testNumber = parent.subtests.length + 1;
this.timeout = parent.timeout;
this.root = parent.root;
this.hooks = {
__proto__: null,
before: [],
after: [],
beforeEach: ArrayPrototypeSlice(parent.hooks.beforeEach),
afterEach: ArrayPrototypeSlice(parent.hooks.afterEach),
};
}

switch (typeof concurrency) {
Expand Down Expand Up @@ -277,12 +291,6 @@ class Test extends AsyncResource {
this.pendingSubtests = [];
this.readySubtests = new SafeMap();
this.subtests = [];
this.hooks = {
before: [],
after: [],
beforeEach: [],
afterEach: [],
};
this.waitingOn = 0;
this.finished = false;

Expand Down Expand Up @@ -772,11 +780,6 @@ class Suite extends Test {

async run() {
const hookArgs = this.getRunArgs();
const afterEach = runOnce(async () => {
if (this.parent?.hooks.afterEach.length > 0) {
await this.parent.runHook('afterEach', hookArgs);
}
});

try {
this.parent.activeSubtests++;
Expand All @@ -789,10 +792,6 @@ class Suite extends Test {
return;
}

if (this.parent?.hooks.beforeEach.length > 0) {
await this.parent.runHook('beforeEach', hookArgs);
}

await this.runHook('before', hookArgs);

const stopPromise = stopTest(this.timeout, this.signal);
Expand All @@ -801,11 +800,9 @@ class Suite extends Test {

await SafePromiseRace([promise, stopPromise]);
await this.runHook('after', hookArgs);
await afterEach();

this.pass();
} catch (err) {
try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
if (isTestFailureError(err)) {
this.fail(err);
} else {
Expand Down
14 changes: 6 additions & 8 deletions test/fixtures/test-runner/output/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ describe('describe hooks', () => {
'before describe hooks',
'beforeEach 1', '1', 'afterEach 1',
'beforeEach 2', '2', 'afterEach 2',
'beforeEach nested',
'before nested',
'beforeEach nested 1', 'nested 1', 'afterEach nested 1',
'beforeEach nested 2', 'nested 2', 'afterEach nested 2',
'beforeEach nested 1', '+beforeEach nested 1', 'nested 1', 'afterEach nested 1', '+afterEach nested 1',
'beforeEach nested 2', '+beforeEach nested 2', 'nested 2', 'afterEach nested 2', '+afterEach nested 2',
'after nested',
'afterEach nested',
'after describe hooks',
]);
});
Expand All @@ -44,10 +42,10 @@ describe('describe hooks', () => {
testArr.push('after ' + this.name);
});
beforeEach(function() {
testArr.push('beforeEach ' + this.name);
testArr.push('+beforeEach ' + this.name);
});
afterEach(function() {
testArr.push('afterEach ' + this.name);
testArr.push('+afterEach ' + this.name);
});
it('nested 1', () => testArr.push('nested 1'));
test('nested 2', () => testArr.push('nested 2'));
Expand Down Expand Up @@ -111,8 +109,8 @@ test('test hooks', async (t) => {
'beforeEach 1', 'before test hooks', '1', 'afterEach 1',
'beforeEach 2', '2', 'afterEach 2',
'beforeEach nested',
'nested beforeEach nested 1', 'nested1', 'nested afterEach nested 1',
'nested beforeEach nested 2', 'nested 2', 'nested afterEach nested 2',
'beforeEach nested 1', 'nested beforeEach nested 1', 'nested1', 'afterEach nested 1', 'nested afterEach nested 1',
'beforeEach nested 2', 'nested beforeEach nested 2', 'nested 2', 'afterEach nested 2', 'nested afterEach nested 2',
'afterEach nested',
]);
});
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/test-runner/output/name_pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ test('top level test enabled', common.mustCall(async (t) => {

describe('top level describe enabled', () => {
before(common.mustCall());
beforeEach(common.mustCall(4));
afterEach(common.mustCall(4));
beforeEach(common.mustCall(3));
afterEach(common.mustCall(3));
after(common.mustCall());

it('nested it disabled', common.mustNotCall());
Expand Down

0 comments on commit 0ea6371

Please sign in to comment.