Skip to content

Commit f248697

Browse files
rluvatonCeres6
rluvaton
authored and
Ceres6
committed
test_runner: fix timeout in *Each hook failing further tests
PR-URL: nodejs#48925 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent b8a4981 commit f248697

7 files changed

Lines changed: 385 additions & 15 deletions

File tree

‎lib/internal/test_runner/test.js‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,8 @@ class SuiteContext {
193193
}
194194

195195
class Test extends AsyncResource {
196-
#abortController;
197-
#outerSignal;
196+
abortController;
197+
outerSignal;
198198
#reportedSubtest;
199199

200200
constructor(options) {
@@ -292,16 +292,16 @@ class Test extends AsyncResource {
292292
fn = noop;
293293
}
294294

295-
this.#abortController = new AbortController();
296-
this.#outerSignal = signal;
297-
this.signal = this.#abortController.signal;
295+
this.abortController = new AbortController();
296+
this.outerSignal = signal;
297+
this.signal = this.abortController.signal;
298298

299299
validateAbortSignal(signal, 'options.signal');
300300
if (signal) {
301301
kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
302302
}
303303

304-
this.#outerSignal?.addEventListener(
304+
this.outerSignal?.addEventListener(
305305
'abort',
306306
this.#abortHandler,
307307
{ __proto__: null, [kResistStopPropagation]: true },
@@ -441,7 +441,7 @@ class Test extends AsyncResource {
441441
}
442442

443443
#abortHandler = () => {
444-
const error = this.#outerSignal?.reason || new AbortError('The test was aborted');
444+
const error = this.outerSignal?.reason || new AbortError('The test was aborted');
445445
error.failureType = kAborted;
446446
this.#cancel(error);
447447
};
@@ -459,7 +459,7 @@ class Test extends AsyncResource {
459459
);
460460
this.startTime = this.startTime || this.endTime; // If a test was canceled before it was started, e.g inside a hook
461461
this.cancelled = true;
462-
this.#abortController.abort();
462+
this.abortController.abort();
463463
}
464464

465465
createHook(name, fn, options) {
@@ -527,7 +527,7 @@ class Test extends AsyncResource {
527527
if (this.signal.aborted) {
528528
return true;
529529
}
530-
if (this.#outerSignal?.aborted) {
530+
if (this.outerSignal?.aborted) {
531531
this.#abortHandler();
532532
return true;
533533
}
@@ -639,7 +639,7 @@ class Test extends AsyncResource {
639639
// Do not abort hooks and the root test as hooks instance are shared between tests suite so aborting them will
640640
// cause them to not run for further tests.
641641
if (this.parent !== null) {
642-
this.#abortController.abort();
642+
this.abortController.abort();
643643
}
644644
}
645645

@@ -679,7 +679,7 @@ class Test extends AsyncResource {
679679
this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed));
680680
}
681681

682-
this.#outerSignal?.removeEventListener('abort', this.#abortHandler);
682+
this.outerSignal?.removeEventListener('abort', this.#abortHandler);
683683
this.mock?.reset();
684684

685685
if (this.parent !== null) {
@@ -795,6 +795,14 @@ class TestHook extends Test {
795795
super({ __proto__: null, fn, timeout, signal });
796796
}
797797
run(args) {
798+
if (this.error && !this.outerSignal?.aborted) {
799+
this.passed = false;
800+
this.error = null;
801+
this.abortController.abort();
802+
this.abortController = new AbortController();
803+
this.signal = this.abortController.signal;
804+
}
805+
798806
this.#args = args;
799807
return super.run();
800808
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Flags: --no-warnings
2+
'use strict';
3+
const { before, beforeEach, describe, it, after, afterEach } = require('node:test');
4+
5+
describe('1 before describe', () => {
6+
const ac = new AbortController();
7+
before(() => {
8+
console.log('before');
9+
ac.abort()
10+
}, {signal: ac.signal});
11+
12+
it('test 1', () => {
13+
console.log('1.1');
14+
});
15+
it('test 2', () => {
16+
console.log('1.2');
17+
});
18+
});
19+
20+
describe('2 after describe', () => {
21+
const ac = new AbortController();
22+
after(() => {
23+
console.log('after');
24+
ac.abort()
25+
}, {signal: ac.signal});
26+
27+
it('test 1', () => {
28+
console.log('2.1');
29+
});
30+
it('test 2', () => {
31+
console.log('2.2');
32+
});
33+
});
34+
35+
describe('3 beforeEach describe', () => {
36+
const ac = new AbortController();
37+
beforeEach(() => {
38+
console.log('beforeEach');
39+
ac.abort()
40+
}, {signal: ac.signal});
41+
42+
it('test 1', () => {
43+
console.log('3.1');
44+
});
45+
it('test 2', () => {
46+
console.log('3.2');
47+
});
48+
});
49+
50+
describe('4 afterEach describe', () => {
51+
const ac = new AbortController();
52+
afterEach(() => {
53+
console.log('afterEach');
54+
ac.abort()
55+
}, {signal: ac.signal});
56+
57+
it('test 1', () => {
58+
console.log('4.1');
59+
});
60+
it('test 2', () => {
61+
console.log('4.2');
62+
});
63+
});

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

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
before
2+
2.1
3+
2.2
4+
after
5+
beforeEach
6+
4.1
7+
afterEach
8+
4.2
9+
TAP version 13
10+
# Subtest: 1 before describe
11+
# Subtest: test 1
12+
not ok 1 - test 1
13+
---
14+
duration_ms: ZERO
15+
failureType: 'cancelledByParent'
16+
error: 'test did not finish before its parent and was cancelled'
17+
code: 'ERR_TEST_FAILURE'
18+
...
19+
# Subtest: test 2
20+
not ok 2 - test 2
21+
---
22+
duration_ms: ZERO
23+
failureType: 'cancelledByParent'
24+
error: 'test did not finish before its parent and was cancelled'
25+
code: 'ERR_TEST_FAILURE'
26+
...
27+
1..2
28+
not ok 1 - 1 before describe
29+
---
30+
duration_ms: *
31+
type: 'suite'
32+
failureType: 'hookFailed'
33+
error: 'This operation was aborted'
34+
code: 20
35+
name: 'AbortError'
36+
stack: |-
37+
*
38+
*
39+
*
40+
*
41+
*
42+
*
43+
*
44+
*
45+
*
46+
*
47+
...
48+
# Subtest: 2 after describe
49+
# Subtest: test 1
50+
ok 1 - test 1
51+
---
52+
duration_ms: *
53+
...
54+
# Subtest: test 2
55+
ok 2 - test 2
56+
---
57+
duration_ms: *
58+
...
59+
1..2
60+
not ok 2 - 2 after describe
61+
---
62+
duration_ms: *
63+
type: 'suite'
64+
failureType: 'hookFailed'
65+
error: 'This operation was aborted'
66+
code: 20
67+
name: 'AbortError'
68+
stack: |-
69+
*
70+
*
71+
*
72+
*
73+
*
74+
*
75+
*
76+
*
77+
*
78+
*
79+
...
80+
# Subtest: 3 beforeEach describe
81+
# Subtest: test 1
82+
not ok 1 - test 1
83+
---
84+
duration_ms: *
85+
failureType: 'hookFailed'
86+
error: 'This operation was aborted'
87+
code: 20
88+
name: 'AbortError'
89+
stack: |-
90+
*
91+
*
92+
*
93+
*
94+
*
95+
*
96+
*
97+
*
98+
*
99+
async Promise.all (index 0)
100+
...
101+
# Subtest: test 2
102+
not ok 2 - test 2
103+
---
104+
duration_ms: *
105+
failureType: 'hookFailed'
106+
error: 'This operation was aborted'
107+
code: 20
108+
name: 'AbortError'
109+
stack: |-
110+
*
111+
*
112+
*
113+
*
114+
*
115+
*
116+
*
117+
*
118+
*
119+
async Promise.all (index 0)
120+
...
121+
1..2
122+
not ok 3 - 3 beforeEach describe
123+
---
124+
duration_ms: *
125+
type: 'suite'
126+
failureType: 'subtestsFailed'
127+
error: '2 subtests failed'
128+
code: 'ERR_TEST_FAILURE'
129+
...
130+
# Subtest: 4 afterEach describe
131+
# Subtest: test 1
132+
not ok 1 - test 1
133+
---
134+
duration_ms: *
135+
failureType: 'hookFailed'
136+
error: 'This operation was aborted'
137+
code: 20
138+
name: 'AbortError'
139+
stack: |-
140+
*
141+
*
142+
*
143+
*
144+
*
145+
*
146+
*
147+
*
148+
*
149+
*
150+
...
151+
# Subtest: test 2
152+
not ok 2 - test 2
153+
---
154+
duration_ms: *
155+
failureType: 'hookFailed'
156+
error: 'This operation was aborted'
157+
code: 20
158+
name: 'AbortError'
159+
stack: |-
160+
*
161+
*
162+
*
163+
*
164+
*
165+
*
166+
*
167+
*
168+
*
169+
*
170+
...
171+
1..2
172+
not ok 4 - 4 afterEach describe
173+
---
174+
duration_ms: *
175+
type: 'suite'
176+
failureType: 'subtestsFailed'
177+
error: '2 subtests failed'
178+
code: 'ERR_TEST_FAILURE'
179+
...
180+
1..4
181+
# tests 8
182+
# suites 4
183+
# pass 2
184+
# fail 4
185+
# cancelled 2
186+
# skipped 0
187+
# todo 0
188+
# duration_ms *

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ not ok 3 - after throws
134134
*
135135
*
136136
*
137-
async Promise.all (index 0)
138-
*
139137
*
140138
...
141139
1..2
@@ -183,7 +181,6 @@ not ok 4 - beforeEach throws
183181
*
184182
*
185183
*
186-
async Promise.all (index 0)
187184
*
188185
...
189186
1..2
@@ -265,7 +262,6 @@ not ok 6 - afterEach when test fails
265262
*
266263
*
267264
*
268-
async Promise.all (index 0)
269265
*
270266
...
271267
1..2

0 commit comments

Comments
 (0)