Skip to content

Commit

Permalink
test_runner: fix ordering of test hooks
Browse files Browse the repository at this point in the history
For tests with subtests the before hook was being run after the
beforeEach hook, which is the opposite to test suites and expectations.

Also, a function was being used to close over the after hooks, but at
the point it was being run the after hooks were not yet set up.

Fixes nodejs#47915

PR-URL: nodejs#47931
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
philnash authored and MoLow committed Jul 6, 2023
1 parent 4bece05 commit 4737314
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
8 changes: 4 additions & 4 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,12 @@ class Test extends AsyncResource {
});

try {
if (this.parent?.hooks.beforeEach.length > 0) {
await this.parent.runHook('beforeEach', { args, ctx });
}
if (this.parent?.hooks.before.length > 0) {
await this.parent.runHook('before', this.parent.getRunArgs());
}
if (this.parent?.hooks.beforeEach.length > 0) {
await this.parent.runHook('beforeEach', { args, ctx });
}
const stopPromise = stopTest(this.timeout, this.signal);
const runArgs = ArrayPrototypeSlice(args);
ArrayPrototypeUnshift(runArgs, this.fn, ctx);
Expand Down Expand Up @@ -574,8 +574,8 @@ class Test extends AsyncResource {
return;
}

await after();
await afterEach();
await after();
this.pass();
} catch (err) {
try { await after(); } catch { /* Ignore error. */ }
Expand Down
7 changes: 6 additions & 1 deletion test/fixtures/test-runner/output/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,24 @@ test('test hooks', async (t) => {
await t.test('2', () => testArr.push('2'));

await t.test('nested', async (t) => {
t.before((t) => testArr.push('nested before ' + t.name));
t.after((t) => testArr.push('nested after ' + t.name));
t.beforeEach((t) => testArr.push('nested beforeEach ' + t.name));
t.afterEach((t) => testArr.push('nested afterEach ' + t.name));
await t.test('nested 1', () => testArr.push('nested1'));
await t.test('nested 2', () => testArr.push('nested 2'));
});

assert.deepStrictEqual(testArr, [
'beforeEach 1', 'before test hooks', '1', 'afterEach 1',
'before test hooks',
'beforeEach 1', '1', 'afterEach 1',
'beforeEach 2', '2', 'afterEach 2',
'beforeEach nested',
'nested before nested',
'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',
'nested after nested',
]);
});

Expand Down

0 comments on commit 4737314

Please sign in to comment.