Skip to content

Commit c2f1830

Browse files
rluvatonUlisesGascon
rluvaton
authored and
UlisesGascon
committed
test_runner: cleanup test timeout abort listener
fix#48475 PR-URL: #48915 Backport-PR-URL: #49225 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 75333f3 commit c2f1830

6 files changed

Lines changed: 194 additions & 10 deletions

‎lib/internal/test_runner/test.js‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ const {
2020
SafeSet,
2121
SafePromiseAll,
2222
SafePromiseRace,
23+
SymbolDispose,
24+
ObjectDefineProperty,
2325
Symbol,
2426
}=primordials;
27+
const{ addAbortListener }=require('events');
2528
const{ AsyncResource }=require('async_hooks');
26-
const{ once }=require('events');
2729
const{ AbortController }=require('internal/abort_controller');
2830
const{
2931
codes: {
@@ -52,7 +54,7 @@ const {
5254
validateOneOf,
5355
validateUint32,
5456
}=require('internal/validators');
55-
const{ setTimeout }=require('timers/promises');
57+
const{ setTimeout }=require('timers');
5658
const{TIMEOUT_MAX}=require('internal/timers');
5759
const{ availableParallelism }=require('os');
5860
const{bigint: hrtime}=process.hrtime;
@@ -76,15 +78,42 @@ const { testNamePatterns, testOnlyFlag } = parseCommandLine();
7678
letkResistStopPropagation;
7779

7880
functionstopTest(timeout,signal){
81+
constdeferred=createDeferredPromise();
82+
constabortListener=addAbortListener(signal,deferred.resolve);
83+
lettimer;
84+
letdisposeFunction;
85+
7986
if(timeout===kDefaultTimeout){
80-
returnonce(signal,'abort');
87+
disposeFunction=abortListener[SymbolDispose];
88+
}if(timeout!==kDefaultTimeout){
89+
timer=setTimeout(()=>deferred.resolve(),timeout);
90+
timer.unref();
91+
92+
ObjectDefineProperty(deferred,'promise',{
93+
__proto__: null,
94+
configurable: true,
95+
writable: true,
96+
value: PromisePrototypeThen(deferred.promise,()=>{
97+
thrownewERR_TEST_FAILURE(
98+
`test timed out after ${timeout}ms`,
99+
kTestTimeoutFailure,
100+
);
101+
}),
102+
});
103+
104+
disposeFunction=()=>{
105+
abortListener[SymbolDispose]();
106+
timer[SymbolDispose]();
107+
};
81108
}
82-
returnPromisePrototypeThen(setTimeout(timeout,null,{__proto__: null,ref: false, signal }),()=>{
83-
thrownewERR_TEST_FAILURE(
84-
`test timed out after ${timeout}ms`,
85-
kTestTimeoutFailure,
86-
);
109+
110+
ObjectDefineProperty(deferred.promise,SymbolDispose,{
111+
__proto__: null,
112+
configurable: true,
113+
writable: true,
114+
value: disposeFunction,
87115
});
116+
returndeferred.promise;
88117
}
89118

90119
classTestContext{
@@ -549,14 +578,16 @@ class Test extends AsyncResource {
549578
}
550579
});
551580

581+
letstopPromise;
582+
552583
try{
553584
if(this.parent?.hooks.before.length>0){
554585
awaitthis.parent.runHook('before',this.parent.getRunArgs());
555586
}
556587
if(this.parent?.hooks.beforeEach.length>0){
557588
awaitthis.parent.runHook('beforeEach',{__proto__: null, args, ctx });
558589
}
559-
conststopPromise=stopTest(this.timeout,this.signal);
590+
stopPromise=stopTest(this.timeout,this.signal);
560591
construnArgs=ArrayPrototypeSlice(args);
561592
ArrayPrototypeUnshift(runArgs,this.fn,ctx);
562593

@@ -603,6 +634,8 @@ class Test extends AsyncResource {
603634
this.fail(newERR_TEST_FAILURE(err,kTestCodeFailure));
604635
}
605636
}finally{
637+
stopPromise?.[SymbolDispose]();
638+
606639
// Do not abort hooks and the root test as hooks instance are shared between tests suite so aborting them will
607640
// cause them to not run for further tests.
608641
if(this.parent!==null){
@@ -817,6 +850,7 @@ class Suite extends Test {
817850
asyncrun(){
818851
consthookArgs=this.getRunArgs();
819852

853+
letstopPromise;
820854
try{
821855
this.parent.activeSubtests++;
822856
awaitthis.buildSuite;
@@ -834,7 +868,7 @@ class Suite extends Test {
834868

835869
awaitthis.runHook('before',hookArgs);
836870

837-
conststopPromise=stopTest(this.timeout,this.signal);
871+
stopPromise=stopTest(this.timeout,this.signal);
838872
constsubtests=this.skipped||this.error ? [] : this.subtests;
839873
constpromise=SafePromiseAll(subtests,(subtests)=>subtests.start());
840874

@@ -848,6 +882,8 @@ class Suite extends Test {
848882
}else{
849883
this.fail(newERR_TEST_FAILURE(err,kTestCodeFailure));
850884
}
885+
}finally{
886+
stopPromise?.[SymbolDispose]();
851887
}
852888

853889
this.postRun();

‎test/fixtures/test-runner/output/before-and-after-each-too-many-listeners.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
'use strict';
2+
const{ beforeEach, afterEach, test}=require("node:test");
3+
beforeEach(()=>{});
4+
afterEach(()=>{});
5+
6+
for(leti=1;i<=11;++i){
7+
test(`${i}`,()=>{});
8+
}

‎test/fixtures/test-runner/output/before-and-after-each-too-many-listeners.snapshot‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
TAP version 13
2+
# Subtest: 1
3+
ok 1 - 1
4+
---
5+
duration_ms: *
6+
...
7+
# Subtest: 2
8+
ok 2 - 2
9+
---
10+
duration_ms: *
11+
...
12+
# Subtest: 3
13+
ok 3 - 3
14+
---
15+
duration_ms: *
16+
...
17+
# Subtest: 4
18+
ok 4 - 4
19+
---
20+
duration_ms: *
21+
...
22+
# Subtest: 5
23+
ok 5 - 5
24+
---
25+
duration_ms: *
26+
...
27+
# Subtest: 6
28+
ok 6 - 6
29+
---
30+
duration_ms: *
31+
...
32+
# Subtest: 7
33+
ok 7 - 7
34+
---
35+
duration_ms: *
36+
...
37+
# Subtest: 8
38+
ok 8 - 8
39+
---
40+
duration_ms: *
41+
...
42+
# Subtest: 9
43+
ok 9 - 9
44+
---
45+
duration_ms: *
46+
...
47+
# Subtest: 10
48+
ok 10 - 10
49+
---
50+
duration_ms: *
51+
...
52+
# Subtest: 11
53+
ok 11 - 11
54+
---
55+
duration_ms: *
56+
...
57+
1..11
58+
# tests 11
59+
# suites 0
60+
# pass 11
61+
# fail 0
62+
# cancelled 0
63+
# skipped 0
64+
# todo 0
65+
# duration_ms *

‎test/fixtures/test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
'use strict';
2+
const{ beforeEach, afterEach, test}=require("node:test");
3+
beforeEach(()=>{},{timeout: 10000});
4+
afterEach(()=>{},{timeout: 10000});
5+
6+
for(leti=1;i<=11;++i){
7+
test(`${i}`,()=>{});
8+
}

‎test/fixtures/test-runner/output/before-and-after-each-with-timeout-too-many-listeners.snapshot‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
TAP version 13
2+
# Subtest: 1
3+
ok 1 - 1
4+
---
5+
duration_ms: *
6+
...
7+
# Subtest: 2
8+
ok 2 - 2
9+
---
10+
duration_ms: *
11+
...
12+
# Subtest: 3
13+
ok 3 - 3
14+
---
15+
duration_ms: *
16+
...
17+
# Subtest: 4
18+
ok 4 - 4
19+
---
20+
duration_ms: *
21+
...
22+
# Subtest: 5
23+
ok 5 - 5
24+
---
25+
duration_ms: *
26+
...
27+
# Subtest: 6
28+
ok 6 - 6
29+
---
30+
duration_ms: *
31+
...
32+
# Subtest: 7
33+
ok 7 - 7
34+
---
35+
duration_ms: *
36+
...
37+
# Subtest: 8
38+
ok 8 - 8
39+
---
40+
duration_ms: *
41+
...
42+
# Subtest: 9
43+
ok 9 - 9
44+
---
45+
duration_ms: *
46+
...
47+
# Subtest: 10
48+
ok 10 - 10
49+
---
50+
duration_ms: *
51+
...
52+
# Subtest: 11
53+
ok 11 - 11
54+
---
55+
duration_ms: *
56+
...
57+
1..11
58+
# tests 11
59+
# suites 0
60+
# pass 11
61+
# fail 0
62+
# cancelled 0
63+
# skipped 0
64+
# todo 0
65+
# duration_ms *

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ const tests = [
3737
{name: 'test-runner/output/describe_nested.js'},
3838
{name: 'test-runner/output/hooks.js'},
3939
{name: 'test-runner/output/hooks-with-no-global-test.js'},
40+
{name: 'test-runner/output/before-and-after-each-too-many-listeners.js'},
41+
{name: 'test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js'},
4042
{name: 'test-runner/output/no_refs.js'},
4143
{name: 'test-runner/output/no_tests.js'},
4244
{name: 'test-runner/output/only_tests.js'},

0 commit comments

Comments
 (0)