Skip to content

Commit a223ca4

Browse files
MoLowmarco-ippolito
MoLow
authored and
marco-ippolito
committedMay 2, 2024
test_runner: run before hook immediately if test started
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>
·
v20.19.1v20.13.0
1 parent f9d9567 commit a223ca4

File tree

8 files changed

+431
-38
lines changed

8 files changed

+431
-38
lines changed
 

‎lib/internal/test_runner/harness.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,11 @@ function setup(root) {
151151
const rejectionHandler =
152152
createProcessEventHandler('unhandledRejection', root);
153153
const coverage = configureCoverage(root, globalOptions);
154-
const exitHandler = () => {
154+
const exitHandler = async () => {
155+
if (root.subtests.length === 0 && (root.hooks.before.length > 0 || root.hooks.after.length > 0)) {
156+
// Run global before/after hooks in case there are no tests
157+
await root.run();
158+
}
155159
root.postRun(new ERR_TEST_FAILURE(
156160
'Promise resolution is still pending but the event loop has already resolved',
157161
kCancelledByParent));

‎lib/internal/test_runner/test.js

+22-8
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,23 @@ class TestContext {
176176
}
177177

178178
before(fn, options) {
179-
this.#test.createHook('before', fn, options);
179+
this.#test
180+
.createHook('before', fn, { __proto__: null, ...options, hookType: 'before', loc: getCallerLocation() });
180181
}
181182

182183
after(fn, options) {
183-
this.#test.createHook('after', fn, options);
184+
this.#test
185+
.createHook('after', fn, { __proto__: null, ...options, hookType: 'after', loc: getCallerLocation() });
184186
}
185187

186188
beforeEach(fn, options) {
187-
this.#test.createHook('beforeEach', fn, options);
189+
this.#test
190+
.createHook('beforeEach', fn, { __proto__: null, ...options, hookType: 'beforeEach', loc: getCallerLocation() });
188191
}
189192

190193
afterEach(fn, options) {
191-
this.#test.createHook('afterEach', fn, options);
194+
this.#test
195+
.createHook('afterEach', fn, { __proto__: null, ...options, hookType: 'afterEach', loc: getCallerLocation() });
192196
}
193197
}
194198

@@ -496,6 +500,14 @@ class Test extends AsyncResource {
496500
if (name === 'before' || name === 'after') {
497501
hook.run = runOnce(hook.run);
498502
}
503+
if (name === 'before' && this.startTime !== null) {
504+
// Test has already started, run the hook immediately
505+
PromisePrototypeThen(hook.run(this.getRunArgs()), () => {
506+
if (hook.error != null) {
507+
this.fail(hook.error);
508+
}
509+
});
510+
}
499511
ArrayPrototypePush(this.hooks[name], hook);
500512
return hook;
501513
}
@@ -593,26 +605,28 @@ class Test extends AsyncResource {
593605
return;
594606
}
595607

596-
const { args, ctx } = this.getRunArgs();
608+
const hookArgs = this.getRunArgs();
609+
const { args, ctx } = hookArgs;
597610
const after = async () => {
598611
if (this.hooks.after.length > 0) {
599-
await this.runHook('after', { __proto__: null, args, ctx });
612+
await this.runHook('after', hookArgs);
600613
}
601614
};
602615
const afterEach = runOnce(async () => {
603616
if (this.parent?.hooks.afterEach.length > 0) {
604-
await this.parent.runHook('afterEach', { __proto__: null, args, ctx });
617+
await this.parent.runHook('afterEach', hookArgs);
605618
}
606619
});
607620

608621
let stopPromise;
609622

610623
try {
611624
if (this.parent?.hooks.before.length > 0) {
625+
// This hook usually runs immediately, we need to wait for it to finish
612626
await this.parent.runHook('before', this.parent.getRunArgs());
613627
}
614628
if (this.parent?.hooks.beforeEach.length > 0) {
615-
await this.parent.runHook('beforeEach', { __proto__: null, args, ctx });
629+
await this.parent.runHook('beforeEach', hookArgs);
616630
}
617631
stopPromise = stopTest(this.timeout, this.signal);
618632
const runArgs = ArrayPrototypeSlice(args);

‎test/fixtures/test-runner/output/global-hooks-with-no-tests.js

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
'use strict';
2+
const common = require('../../../common');
3+
const { before, after } = require('node:test');
4+
5+
before(common.mustCall(() => console.log('before')));
6+
after(common.mustCall(() => console.log('after')));

‎test/fixtures/test-runner/output/global-hooks-with-no-tests.snapshot

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
before
2+
TAP version 13
3+
after
4+
1..0
5+
# tests 0
6+
# suites 0
7+
# pass 0
8+
# fail 0
9+
# cancelled 0
10+
# skipped 0
11+
# todo 0
12+
# duration_ms *

‎test/fixtures/test-runner/output/hooks.js

+62-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe('describe hooks', () => {
1111
before(function() {
1212
testArr.push('before ' + this.name);
1313
});
14-
after(function() {
14+
after(common.mustCall(function() {
1515
testArr.push('after ' + this.name);
1616
assert.deepStrictEqual(testArr, [
1717
'before describe hooks',
@@ -23,7 +23,7 @@ describe('describe hooks', () => {
2323
'after nested',
2424
'after describe hooks',
2525
]);
26-
});
26+
}));
2727
beforeEach(function() {
2828
testArr.push('beforeEach ' + this.name);
2929
});
@@ -52,18 +52,43 @@ describe('describe hooks', () => {
5252
});
5353
});
5454

55+
describe('describe hooks - no subtests', () => {
56+
const testArr = [];
57+
before(function() {
58+
testArr.push('before ' + this.name);
59+
});
60+
after(common.mustCall(function() {
61+
testArr.push('after ' + this.name);
62+
assert.deepStrictEqual(testArr, [
63+
'before describe hooks - no subtests',
64+
'after describe hooks - no subtests',
65+
]);
66+
}));
67+
beforeEach(common.mustNotCall());
68+
afterEach(common.mustNotCall());
69+
});
70+
5571
describe('before throws', () => {
5672
before(() => { throw new Error('before'); });
5773
it('1', () => {});
5874
test('2', () => {});
5975
});
6076

77+
describe('before throws - no subtests', () => {
78+
before(() => { throw new Error('before'); });
79+
after(common.mustCall());
80+
});
81+
6182
describe('after throws', () => {
6283
after(() => { throw new Error('after'); });
6384
it('1', () => {});
6485
test('2', () => {});
6586
});
6687

88+
describe('after throws - no subtests', () => {
89+
after(() => { throw new Error('after'); });
90+
});
91+
6792
describe('beforeEach throws', () => {
6893
beforeEach(() => { throw new Error('beforeEach'); });
6994
it('1', () => {});
@@ -123,13 +148,48 @@ test('test hooks', async (t) => {
123148
}));
124149
});
125150

151+
152+
test('test hooks - no subtests', async (t) => {
153+
const testArr = [];
154+
155+
t.before((t) => testArr.push('before ' + t.name));
156+
t.after(common.mustCall((t) => testArr.push('after ' + t.name)));
157+
t.beforeEach(common.mustNotCall());
158+
t.afterEach(common.mustNotCall());
159+
160+
t.after(common.mustCall(() => {
161+
assert.deepStrictEqual(testArr, [
162+
'before test hooks - no subtests',
163+
'after test hooks - no subtests',
164+
]);
165+
}));
166+
});
167+
126168
test('t.before throws', async (t) => {
127169
t.after(common.mustCall());
128170
t.before(() => { throw new Error('before'); });
129171
await t.test('1', () => {});
130172
await t.test('2', () => {});
131173
});
132174

175+
test('t.before throws - no subtests', async (t) => {
176+
t.after(common.mustCall());
177+
t.before(() => { throw new Error('before'); });
178+
});
179+
180+
test('t.after throws', async (t) => {
181+
t.before(common.mustCall());
182+
t.after(() => { throw new Error('after'); });
183+
await t.test('1', () => {});
184+
await t.test('2', () => {});
185+
});
186+
187+
test('t.after throws - no subtests', async (t) => {
188+
t.before(common.mustCall());
189+
t.after(() => { throw new Error('after'); });
190+
});
191+
192+
133193
test('t.beforeEach throws', async (t) => {
134194
t.after(common.mustCall());
135195
t.beforeEach(() => { throw new Error('beforeEach'); });

‎test/fixtures/test-runner/output/hooks.snapshot

+156-23
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ ok 1 - describe hooks
3434
duration_ms: *
3535
type: 'suite'
3636
...
37+
# Subtest: describe hooks - no subtests
38+
ok 2 - describe hooks - no subtests
39+
---
40+
duration_ms: *
41+
type: 'suite'
42+
...
3743
# Subtest: before throws
3844
# Subtest: 1
3945
not ok 1 - 1
@@ -54,7 +60,27 @@ ok 1 - describe hooks
5460
code: 'ERR_TEST_FAILURE'
5561
...
5662
1..2
57-
not ok 2 - before throws
63+
not ok 3 - before throws
64+
---
65+
duration_ms: *
66+
type: 'suite'
67+
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
68+
failureType: 'hookFailed'
69+
error: 'before'
70+
code: 'ERR_TEST_FAILURE'
71+
stack: |-
72+
*
73+
*
74+
*
75+
*
76+
*
77+
*
78+
*
79+
*
80+
*
81+
...
82+
# Subtest: before throws - no subtests
83+
not ok 4 - before throws - no subtests
5884
---
5985
duration_ms: *
6086
type: 'suite'
@@ -85,7 +111,27 @@ not ok 2 - before throws
85111
duration_ms: *
86112
...
87113
1..2
88-
not ok 3 - after throws
114+
not ok 5 - after throws
115+
---
116+
duration_ms: *
117+
type: 'suite'
118+
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
119+
failureType: 'hookFailed'
120+
error: 'after'
121+
code: 'ERR_TEST_FAILURE'
122+
stack: |-
123+
*
124+
*
125+
*
126+
*
127+
*
128+
*
129+
*
130+
*
131+
*
132+
...
133+
# Subtest: after throws - no subtests
134+
not ok 6 - after throws - no subtests
89135
---
90136
duration_ms: *
91137
type: 'suite'
@@ -144,7 +190,7 @@ not ok 3 - after throws
144190
*
145191
...
146192
1..2
147-
not ok 4 - beforeEach throws
193+
not ok 7 - beforeEach throws
148194
---
149195
duration_ms: *
150196
type: 'suite'
@@ -194,7 +240,7 @@ not ok 4 - beforeEach throws
194240
*
195241
...
196242
1..2
197-
not ok 5 - afterEach throws
243+
not ok 8 - afterEach throws
198244
---
199245
duration_ms: *
200246
type: 'suite'
@@ -230,7 +276,7 @@ not ok 5 - afterEach throws
230276
duration_ms: *
231277
...
232278
1..2
233-
not ok 6 - afterEach when test fails
279+
not ok 9 - afterEach when test fails
234280
---
235281
duration_ms: *
236282
type: 'suite'
@@ -280,7 +326,7 @@ not ok 6 - afterEach when test fails
280326
*
281327
...
282328
1..2
283-
not ok 7 - afterEach throws and test fails
329+
not ok 10 - afterEach throws and test fails
284330
---
285331
duration_ms: *
286332
type: 'suite'
@@ -317,7 +363,12 @@ not ok 7 - afterEach throws and test fails
317363
duration_ms: *
318364
...
319365
1..3
320-
ok 8 - test hooks
366+
ok 11 - test hooks
367+
---
368+
duration_ms: *
369+
...
370+
# Subtest: test hooks - no subtests
371+
ok 12 - test hooks - no subtests
321372
---
322373
duration_ms: *
323374
...
@@ -363,13 +414,95 @@ ok 8 - test hooks
363414
*
364415
...
365416
1..2
366-
not ok 9 - t.before throws
417+
not ok 13 - t.before throws
367418
---
368419
duration_ms: *
369420
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
370-
failureType: 'subtestsFailed'
371-
error: '2 subtests failed'
421+
failureType: 'testCodeFailure'
422+
error: 'before'
372423
code: 'ERR_TEST_FAILURE'
424+
stack: |-
425+
*
426+
*
427+
*
428+
*
429+
*
430+
*
431+
*
432+
*
433+
*
434+
*
435+
...
436+
# Subtest: t.before throws - no subtests
437+
not ok 14 - t.before throws - no subtests
438+
---
439+
duration_ms: *
440+
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
441+
failureType: 'testCodeFailure'
442+
error: 'before'
443+
code: 'ERR_TEST_FAILURE'
444+
stack: |-
445+
*
446+
*
447+
*
448+
*
449+
*
450+
*
451+
*
452+
*
453+
*
454+
*
455+
...
456+
# Subtest: t.after throws
457+
# Subtest: 1
458+
ok 1 - 1
459+
---
460+
duration_ms: *
461+
...
462+
# Subtest: 2
463+
ok 2 - 2
464+
---
465+
duration_ms: *
466+
...
467+
1..2
468+
not ok 15 - t.after throws
469+
---
470+
duration_ms: *
471+
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
472+
failureType: 'hookFailed'
473+
error: 'after'
474+
code: 'ERR_TEST_FAILURE'
475+
stack: |-
476+
*
477+
*
478+
*
479+
*
480+
*
481+
*
482+
*
483+
*
484+
*
485+
*
486+
...
487+
# Subtest: t.after throws - no subtests
488+
not ok 16 - t.after throws - no subtests
489+
---
490+
duration_ms: *
491+
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
492+
failureType: 'hookFailed'
493+
error: 'after'
494+
code: 'ERR_TEST_FAILURE'
495+
stack: |-
496+
*
497+
*
498+
*
499+
*
500+
*
501+
*
502+
*
503+
*
504+
*
505+
*
373506
...
374507
# Subtest: t.beforeEach throws
375508
# Subtest: 1
@@ -413,7 +546,7 @@ not ok 9 - t.before throws
413546
*
414547
...
415548
1..2
416-
not ok 10 - t.beforeEach throws
549+
not ok 17 - t.beforeEach throws
417550
---
418551
duration_ms: *
419552
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -463,7 +596,7 @@ not ok 10 - t.beforeEach throws
463596
*
464597
...
465598
1..2
466-
not ok 11 - t.afterEach throws
599+
not ok 18 - t.afterEach throws
467600
---
468601
duration_ms: *
469602
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -497,7 +630,7 @@ not ok 11 - t.afterEach throws
497630
duration_ms: *
498631
...
499632
1..2
500-
not ok 12 - afterEach when test fails
633+
not ok 19 - afterEach when test fails
501634
---
502635
duration_ms: *
503636
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -512,7 +645,7 @@ not ok 12 - afterEach when test fails
512645
duration_ms: *
513646
...
514647
1..1
515-
ok 13 - afterEach context when test passes
648+
ok 20 - afterEach context when test passes
516649
---
517650
duration_ms: *
518651
...
@@ -532,7 +665,7 @@ ok 13 - afterEach context when test passes
532665
*
533666
...
534667
1..1
535-
not ok 14 - afterEach context when test fails
668+
not ok 21 - afterEach context when test fails
536669
---
537670
duration_ms: *
538671
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -581,7 +714,7 @@ not ok 14 - afterEach context when test fails
581714
*
582715
...
583716
1..2
584-
not ok 15 - afterEach throws and test fails
717+
not ok 22 - afterEach throws and test fails
585718
---
586719
duration_ms: *
587720
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -590,7 +723,7 @@ not ok 15 - afterEach throws and test fails
590723
code: 'ERR_TEST_FAILURE'
591724
...
592725
# Subtest: t.after() is called if test body throws
593-
not ok 16 - t.after() is called if test body throws
726+
not ok 23 - t.after() is called if test body throws
594727
---
595728
duration_ms: *
596729
location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -615,7 +748,7 @@ not ok 16 - t.after() is called if test body throws
615748
code: 'ERR_TEST_FAILURE'
616749
...
617750
1..1
618-
not ok 17 - run after when before throws
751+
not ok 24 - run after when before throws
619752
---
620753
duration_ms: *
621754
type: 'suite'
@@ -634,15 +767,15 @@ not ok 17 - run after when before throws
634767
*
635768
*
636769
...
637-
1..17
770+
1..24
638771
# before 1 called
639772
# before 2 called
640773
# after 1 called
641774
# after 2 called
642-
# tests 43
643-
# suites 9
644-
# pass 16
645-
# fail 24
775+
# tests 49
776+
# suites 12
777+
# pass 19
778+
# fail 27
646779
# cancelled 3
647780
# skipped 0
648781
# todo 0

‎test/fixtures/test-runner/output/hooks_spec_reporter.snapshot

+167-4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
describe hooks (*ms)
1111

12+
describe hooks - no subtests (*ms)
1213
before throws
1314
1
1415
'test did not finish before its parent and was cancelled'
@@ -29,6 +30,18 @@
2930
*
3031
*
3132

33+
before throws - no subtests (*ms)
34+
Error: before
35+
*
36+
*
37+
*
38+
*
39+
*
40+
*
41+
*
42+
*
43+
*
44+
3245
after throws
3346
1 (*ms)
3447
2 (*ms)
@@ -45,6 +58,18 @@
4558
*
4659
*
4760

61+
after throws - no subtests (*ms)
62+
Error: after
63+
*
64+
*
65+
*
66+
*
67+
*
68+
*
69+
*
70+
*
71+
*
72+
4873
beforeEach throws
4974
1 (*ms)
5075
Error: beforeEach
@@ -155,6 +180,7 @@
155180

156181
test hooks (*ms)
157182

183+
test hooks - no subtests (*ms)
158184
t.before throws
159185
1 (*ms)
160186
Error: before
@@ -184,6 +210,61 @@
184210

185211
t.before throws (*ms)
186212

213+
Error: before
214+
*
215+
*
216+
*
217+
*
218+
*
219+
*
220+
*
221+
*
222+
*
223+
*
224+
225+
t.before throws - no subtests (*ms)
226+
Error: before
227+
*
228+
*
229+
*
230+
*
231+
*
232+
*
233+
*
234+
*
235+
*
236+
*
237+
238+
t.after throws
239+
1 (*ms)
240+
2 (*ms)
241+
t.after throws (*ms)
242+
243+
Error: after
244+
*
245+
*
246+
*
247+
*
248+
*
249+
*
250+
*
251+
*
252+
*
253+
*
254+
255+
t.after throws - no subtests (*ms)
256+
Error: after
257+
*
258+
*
259+
*
260+
*
261+
*
262+
*
263+
*
264+
*
265+
*
266+
*
267+
187268
t.beforeEach throws
188269
1 (*ms)
189270
Error: beforeEach
@@ -329,10 +410,10 @@
329410
before 2 called
330411
after 1 called
331412
after 2 called
332-
tests 43
333-
suites 9
334-
pass 16
335-
fail 24
413+
tests 49
414+
suites 12
415+
pass 19
416+
fail 27
336417
cancelled 3
337418
skipped 0
338419
todo 0
@@ -361,6 +442,19 @@
361442
*
362443
*
363444

445+
*
446+
before throws - no subtests (*ms)
447+
Error: before
448+
*
449+
*
450+
*
451+
*
452+
*
453+
*
454+
*
455+
*
456+
*
457+
364458
*
365459
after throws (*ms)
366460
Error: after
@@ -374,6 +468,19 @@
374468
*
375469
*
376470

471+
*
472+
after throws - no subtests (*ms)
473+
Error: after
474+
*
475+
*
476+
*
477+
*
478+
*
479+
*
480+
*
481+
*
482+
*
483+
377484
*
378485
1 (*ms)
379486
Error: beforeEach
@@ -496,6 +603,62 @@
496603
*
497604
*
498605

606+
*
607+
t.before throws (*ms)
608+
Error: before
609+
*
610+
*
611+
*
612+
*
613+
*
614+
*
615+
*
616+
*
617+
*
618+
*
619+
620+
*
621+
t.before throws - no subtests (*ms)
622+
Error: before
623+
*
624+
*
625+
*
626+
*
627+
*
628+
*
629+
*
630+
*
631+
*
632+
*
633+
634+
*
635+
t.after throws (*ms)
636+
Error: after
637+
*
638+
*
639+
*
640+
*
641+
*
642+
*
643+
*
644+
*
645+
*
646+
*
647+
648+
*
649+
t.after throws - no subtests (*ms)
650+
Error: after
651+
*
652+
*
653+
*
654+
*
655+
*
656+
*
657+
*
658+
*
659+
*
660+
*
661+
499662
*
500663
1 (*ms)
501664
Error: beforeEach

‎test/parallel/test-runner-output.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ const tests = [
9595
{ name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },
9696
{ name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js' },
9797
{ name: 'test-runner/output/hooks-with-no-global-test.js' },
98+
{ name: 'test-runner/output/global-hooks-with-no-tests.js' },
9899
{ name: 'test-runner/output/before-and-after-each-too-many-listeners.js' },
99100
{ name: 'test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js' },
100101
{ name: 'test-runner/output/global_after_should_fail_the_test.js' },

0 commit comments

Comments
 (0)
Please sign in to comment.