Skip to content

Commit

Permalink
test_runner: run before hook immediately if test started
Browse files Browse the repository at this point in the history
PR-URL: #52003
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
MoLow authored and marco-ippolito committed May 2, 2024
1 parent f9d9567 commit a223ca4
Show file tree
Hide file tree
Showing 8 changed files with 431 additions and 38 deletions.
6 changes: 5 additions & 1 deletion lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ function setup(root) {
const rejectionHandler =
createProcessEventHandler('unhandledRejection', root);
const coverage = configureCoverage(root, globalOptions);
const exitHandler = () => {
const exitHandler = async () => {
if (root.subtests.length === 0 && (root.hooks.before.length > 0 || root.hooks.after.length > 0)) {
// Run global before/after hooks in case there are no tests
await root.run();
}
root.postRun(new ERR_TEST_FAILURE(
'Promise resolution is still pending but the event loop has already resolved',
kCancelledByParent));
Expand Down
30 changes: 22 additions & 8 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,19 +176,23 @@ class TestContext {
}

before(fn, options) {
this.#test.createHook('before', fn, options);
this.#test
.createHook('before', fn, { __proto__: null, ...options, hookType: 'before', loc: getCallerLocation() });
}

after(fn, options) {
this.#test.createHook('after', fn, options);
this.#test
.createHook('after', fn, { __proto__: null, ...options, hookType: 'after', loc: getCallerLocation() });
}

beforeEach(fn, options) {
this.#test.createHook('beforeEach', fn, options);
this.#test
.createHook('beforeEach', fn, { __proto__: null, ...options, hookType: 'beforeEach', loc: getCallerLocation() });
}

afterEach(fn, options) {
this.#test.createHook('afterEach', fn, options);
this.#test
.createHook('afterEach', fn, { __proto__: null, ...options, hookType: 'afterEach', loc: getCallerLocation() });
}
}

Expand Down Expand Up @@ -496,6 +500,14 @@ class Test extends AsyncResource {
if (name === 'before' || name === 'after') {
hook.run = runOnce(hook.run);
}
if (name === 'before' && this.startTime !== null) {
// Test has already started, run the hook immediately
PromisePrototypeThen(hook.run(this.getRunArgs()), () => {
if (hook.error != null) {
this.fail(hook.error);
}
});
}
ArrayPrototypePush(this.hooks[name], hook);
return hook;
}
Expand Down Expand Up @@ -593,26 +605,28 @@ class Test extends AsyncResource {
return;
}

const { args, ctx } = this.getRunArgs();
const hookArgs = this.getRunArgs();
const { args, ctx } = hookArgs;
const after = async () => {
if (this.hooks.after.length > 0) {
await this.runHook('after', { __proto__: null, args, ctx });
await this.runHook('after', hookArgs);
}
};
const afterEach = runOnce(async () => {
if (this.parent?.hooks.afterEach.length > 0) {
await this.parent.runHook('afterEach', { __proto__: null, args, ctx });
await this.parent.runHook('afterEach', hookArgs);
}
});

let stopPromise;

try {
if (this.parent?.hooks.before.length > 0) {
// This hook usually runs immediately, we need to wait for it to finish
await this.parent.runHook('before', this.parent.getRunArgs());
}
if (this.parent?.hooks.beforeEach.length > 0) {
await this.parent.runHook('beforeEach', { __proto__: null, args, ctx });
await this.parent.runHook('beforeEach', hookArgs);
}
stopPromise = stopTest(this.timeout, this.signal);
const runArgs = ArrayPrototypeSlice(args);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';
const common = require('../../../common');
const { before, after } = require('node:test');

before(common.mustCall(() => console.log('before')));
after(common.mustCall(() => console.log('after')));
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
before
TAP version 13
after
1..0
# tests 0
# suites 0
# pass 0
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
64 changes: 62 additions & 2 deletions test/fixtures/test-runner/output/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('describe hooks', () => {
before(function() {
testArr.push('before ' + this.name);
});
after(function() {
after(common.mustCall(function() {
testArr.push('after ' + this.name);
assert.deepStrictEqual(testArr, [
'before describe hooks',
Expand All @@ -23,7 +23,7 @@ describe('describe hooks', () => {
'after nested',
'after describe hooks',
]);
});
}));
beforeEach(function() {
testArr.push('beforeEach ' + this.name);
});
Expand Down Expand Up @@ -52,18 +52,43 @@ describe('describe hooks', () => {
});
});

describe('describe hooks - no subtests', () => {
const testArr = [];
before(function() {
testArr.push('before ' + this.name);
});
after(common.mustCall(function() {
testArr.push('after ' + this.name);
assert.deepStrictEqual(testArr, [
'before describe hooks - no subtests',
'after describe hooks - no subtests',
]);
}));
beforeEach(common.mustNotCall());
afterEach(common.mustNotCall());
});

describe('before throws', () => {
before(() => { throw new Error('before'); });
it('1', () => {});
test('2', () => {});
});

describe('before throws - no subtests', () => {
before(() => { throw new Error('before'); });
after(common.mustCall());
});

describe('after throws', () => {
after(() => { throw new Error('after'); });
it('1', () => {});
test('2', () => {});
});

describe('after throws - no subtests', () => {
after(() => { throw new Error('after'); });
});

describe('beforeEach throws', () => {
beforeEach(() => { throw new Error('beforeEach'); });
it('1', () => {});
Expand Down Expand Up @@ -123,13 +148,48 @@ test('test hooks', async (t) => {
}));
});


test('test hooks - no subtests', async (t) => {
const testArr = [];

t.before((t) => testArr.push('before ' + t.name));
t.after(common.mustCall((t) => testArr.push('after ' + t.name)));
t.beforeEach(common.mustNotCall());
t.afterEach(common.mustNotCall());

t.after(common.mustCall(() => {
assert.deepStrictEqual(testArr, [
'before test hooks - no subtests',
'after test hooks - no subtests',
]);
}));
});

test('t.before throws', async (t) => {
t.after(common.mustCall());
t.before(() => { throw new Error('before'); });
await t.test('1', () => {});
await t.test('2', () => {});
});

test('t.before throws - no subtests', async (t) => {
t.after(common.mustCall());
t.before(() => { throw new Error('before'); });
});

test('t.after throws', async (t) => {
t.before(common.mustCall());
t.after(() => { throw new Error('after'); });
await t.test('1', () => {});
await t.test('2', () => {});
});

test('t.after throws - no subtests', async (t) => {
t.before(common.mustCall());
t.after(() => { throw new Error('after'); });
});


test('t.beforeEach throws', async (t) => {
t.after(common.mustCall());
t.beforeEach(() => { throw new Error('beforeEach'); });
Expand Down
Loading

0 comments on commit a223ca4

Please sign in to comment.