Skip to content

Commit aa3523e

Browse files
cjihrigjasnell
cjihrig
authored and
jasnell
committed
test_runner: automatically wait for subtests to finish
This commit updates the test runner to automatically wait for subtests to finish. This makes the experience more consistent with suites and removes the need to await anything. PR-URL: #56664 Fixes: #51292 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Pietro Marchini <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
1 parent ba4587c commit aa3523e

11 files changed

+57
-84
lines changed

‎lib/internal/test_runner/harness.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ const {
2727
shouldColorizeTestFiles,
2828
} = require('internal/test_runner/utils');
2929
const { queueMicrotask } = require('internal/process/task_queues');
30+
const { TIMEOUT_MAX } = require('internal/timers');
31+
const { clearInterval, setInterval } = require('timers');
3032
const { bigint: hrtime } = process.hrtime;
3133
const resolvedPromise = PromiseResolve();
3234
const testResources = new SafeMap();
@@ -212,11 +214,20 @@ function setupProcessState(root, globalOptions) {
212214
const rejectionHandler =
213215
createProcessEventHandler('unhandledRejection', root);
214216
const coverage = configureCoverage(root, globalOptions);
215-
const exitHandler = async () => {
217+
const exitHandler = async (kill) => {
216218
if (root.subtests.length === 0 && (root.hooks.before.length > 0 || root.hooks.after.length > 0)) {
217219
// Run global before/after hooks in case there are no tests
218220
await root.run();
219221
}
222+
223+
if (kill !== true && root.subtestsPromise !== null) {
224+
// Wait for all subtests to finish, but keep the process alive in case
225+
// there are no ref'ed handles left.
226+
const keepAlive = setInterval(() => {}, TIMEOUT_MAX);
227+
await root.subtestsPromise.promise;
228+
clearInterval(keepAlive);
229+
}
230+
220231
root.postRun(new ERR_TEST_FAILURE(
221232
'Promise resolution is still pending but the event loop has already resolved',
222233
kCancelledByParent));
@@ -231,8 +242,8 @@ function setupProcessState(root, globalOptions) {
231242
}
232243
};
233244

234-
const terminationHandler = () => {
235-
exitHandler();
245+
const terminationHandler = async () => {
246+
await exitHandler(true);
236247
process.exit();
237248
};
238249

‎lib/internal/test_runner/test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,8 @@ class Test extends AsyncResource {
579579
this.activeSubtests = 0;
580580
this.pendingSubtests = [];
581581
this.readySubtests = new SafeMap();
582+
this.unfinishedSubtests = new SafeSet();
583+
this.subtestsPromise = null;
582584
this.subtests = [];
583585
this.waitingOn = 0;
584586
this.finished = false;
@@ -682,6 +684,11 @@ class Test extends AsyncResource {
682684

683685
addReadySubtest(subtest) {
684686
this.readySubtests.set(subtest.childNumber, subtest);
687+
688+
if (this.unfinishedSubtests.delete(subtest) &&
689+
this.unfinishedSubtests.size === 0) {
690+
this.subtestsPromise.resolve();
691+
}
685692
}
686693

687694
processReadySubtestRange(canSend) {
@@ -743,6 +750,7 @@ class Test extends AsyncResource {
743750

744751
if (parent.waitingOn === 0) {
745752
parent.waitingOn = test.childNumber;
753+
parent.subtestsPromise = PromiseWithResolvers();
746754
}
747755

748756
if (preventAddingSubtests) {
@@ -865,6 +873,7 @@ class Test extends AsyncResource {
865873
// If there is enough available concurrency to run the test now, then do
866874
// it. Otherwise, return a Promise to the caller and mark the test as
867875
// pending for later execution.
876+
this.parent.unfinishedSubtests.add(this);
868877
this.reporter.enqueue(this.nesting, this.loc, this.name, this.reportedType);
869878
if (this.root.harness.buildPromise || !this.parent.hasConcurrency()) {
870879
const deferred = PromiseWithResolvers();
@@ -982,6 +991,11 @@ class Test extends AsyncResource {
982991
}
983992

984993
this[kShouldAbort]();
994+
995+
if (this.subtestsPromise !== null) {
996+
await SafePromiseRace([this.subtestsPromise.promise, stopPromise]);
997+
}
998+
985999
this.plan?.check();
9861000
this.pass();
9871001
await afterEach();

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
[90m﹣ should skip [90m(*ms)[39m # SKIP[39m
44
▶ parent
55
[31m✖ should fail [90m(*ms)[39m[39m
6-
[31m✖ should pass but parent fail [90m(*ms)[39m[39m
6+
[32m✔ should pass but parent fail [90m(*ms)[39m[39m
77
[31m✖ parent [90m(*ms)[39m[39m
88
[34mℹ tests 6[39m
99
[34mℹ suites 0[39m
10-
[34mℹ pass 1[39m
10+
[34mℹ pass 2[39m
1111
[34mℹ fail 3[39m
12-
[34mℹ cancelled 1[39m
12+
[34mℹ cancelled 0[39m
1313
[34mℹ skipped 1[39m
1414
[34mℹ todo 0[39m
1515
[34mℹ duration_ms *[39m
@@ -40,7 +40,3 @@
4040
*[39m
4141
*[39m
4242
*[39m
43-
44-
*
45-
[31m✖ should pass but parent fail [90m(*ms)[39m[39m
46-
[32m'test did not finish before its parent and was cancelled'[39m

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
..XX...X..XXX.X.....
2-
XXX.....X..X...X....
2+
XXX............X....
33
.....X...XXX.XX.....
44
XXXXXXX...XXXXX
55

@@ -93,10 +93,6 @@ Failed tests:
9393
'1 subtest failed'
9494
✖ sync throw non-error fail (*ms)
9595
Symbol(thrown symbol from sync throw non-error fail)
96-
✖ +long running (*ms)
97-
'test did not finish before its parent and was cancelled'
98-
✖ top level (*ms)
99-
'1 subtest failed'
10096
✖ sync skip option is false fail (*ms)
10197
Error: this should be executed
10298
*

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,8 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail
186186
<testcase name="level 1c" time="*" classname="test"/>
187187
<testcase name="level 1d" time="*" classname="test"/>
188188
</testsuite>
189-
<testsuite name="top level" time="*" disabled="0" errors="0" tests="2" failures="1" skipped="0" hostname="HOSTNAME">
190-
<testcase name="+long running" time="*" classname="test" failure="test did not finish before its parent and was cancelled">
191-
<failure type="cancelledByParent" message="test did not finish before its parent and was cancelled">
192-
[Error [ERR_TEST_FAILURE]: test did not finish before its parent and was cancelled] { code: 'ERR_TEST_FAILURE', failureType: 'cancelledByParent', cause: 'test did not finish before its parent and was cancelled' }
193-
</failure>
194-
</testcase>
189+
<testsuite name="top level" time="*" disabled="0" errors="0" tests="2" failures="0" skipped="0" hostname="HOSTNAME">
190+
<testcase name="+long running" time="*" classname="test"/>
195191
<testsuite name="+short running" time="*" disabled="0" errors="0" tests="1" failures="0" skipped="0" hostname="HOSTNAME">
196192
<testcase name="++short running" time="*" classname="test"/>
197193
</testsuite>
@@ -519,9 +515,9 @@ Error [ERR_TEST_FAILURE]: test could not be started because its parent finished
519515
<!-- Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:269:1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -->
520516
<!-- tests 75 -->
521517
<!-- suites 0 -->
522-
<!-- pass 34 -->
523-
<!-- fail 25 -->
524-
<!-- cancelled 3 -->
518+
<!-- pass 36 -->
519+
<!-- fail 24 -->
520+
<!-- cancelled 2 -->
525521
<!-- skipped 9 -->
526522
<!-- todo 4 -->
527523
<!-- duration_ms * -->

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

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,23 @@
11
TAP version 13
22
# Subtest: does not keep event loop alive
33
# Subtest: +does not keep event loop alive
4-
not ok 1 - +does not keep event loop alive
4+
ok 1 - +does not keep event loop alive
55
---
66
duration_ms: *
77
type: 'test'
8-
location: '/test/fixtures/test-runner/output/no_refs.js:(LINE):11'
9-
failureType: 'cancelledByParent'
10-
error: 'Promise resolution is still pending but the event loop has already resolved'
11-
code: 'ERR_TEST_FAILURE'
12-
stack: |-
13-
*
148
...
159
1..1
16-
not ok 1 - does not keep event loop alive
10+
ok 1 - does not keep event loop alive
1711
---
1812
duration_ms: *
1913
type: 'test'
20-
location: '/test/fixtures/test-runner/output/no_refs.js:(LINE):1'
21-
failureType: 'cancelledByParent'
22-
error: 'Promise resolution is still pending but the event loop has already resolved'
23-
code: 'ERR_TEST_FAILURE'
24-
stack: |-
25-
*
2614
...
2715
1..1
2816
# tests 2
2917
# suites 0
30-
# pass 0
18+
# pass 2
3119
# fail 0
32-
# cancelled 2
20+
# cancelled 0
3321
# skipped 0
3422
# todo 0
3523
# duration_ms *

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -288,14 +288,10 @@ ok 23 - level 0a
288288
...
289289
# Subtest: top level
290290
# Subtest: +long running
291-
not ok 1 - +long running
291+
ok 1 - +long running
292292
---
293293
duration_ms: *
294294
type: 'test'
295-
location: '/test/fixtures/test-runner/output/output.js:(LINE):5'
296-
failureType: 'cancelledByParent'
297-
error: 'test did not finish before its parent and was cancelled'
298-
code: 'ERR_TEST_FAILURE'
299295
...
300296
# Subtest: +short running
301297
# Subtest: ++short running
@@ -311,14 +307,10 @@ ok 23 - level 0a
311307
type: 'test'
312308
...
313309
1..2
314-
not ok 24 - top level
310+
ok 24 - top level
315311
---
316312
duration_ms: *
317313
type: 'test'
318-
location: '/test/fixtures/test-runner/output/output.js:(LINE):1'
319-
failureType: 'subtestsFailed'
320-
error: '1 subtest failed'
321-
code: 'ERR_TEST_FAILURE'
322314
...
323315
# Subtest: invalid subtest - pass but subtest fails
324316
ok 25 - invalid subtest - pass but subtest fails
@@ -787,9 +779,9 @@ not ok 62 - invalid subtest fail
787779
# Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:(LINE):1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event.
788780
# tests 75
789781
# suites 0
790-
# pass 34
791-
# fail 25
792-
# cancelled 3
782+
# pass 36
783+
# fail 24
784+
# cancelled 2
793785
# skipped 9
794786
# todo 4
795787
# duration_ms *

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -288,14 +288,10 @@ ok 23 - level 0a
288288
...
289289
# Subtest: top level
290290
# Subtest: +long running
291-
not ok 1 - +long running
291+
ok 1 - +long running
292292
---
293293
duration_ms: *
294294
type: 'test'
295-
location: '/test/fixtures/test-runner/output/output.js:(LINE):5'
296-
failureType: 'cancelledByParent'
297-
error: 'test did not finish before its parent and was cancelled'
298-
code: 'ERR_TEST_FAILURE'
299295
...
300296
# Subtest: +short running
301297
# Subtest: ++short running
@@ -311,14 +307,10 @@ ok 23 - level 0a
311307
type: 'test'
312308
...
313309
1..2
314-
not ok 24 - top level
310+
ok 24 - top level
315311
---
316312
duration_ms: *
317313
type: 'test'
318-
location: '/test/fixtures/test-runner/output/output.js:(LINE):1'
319-
failureType: 'subtestsFailed'
320-
error: '1 subtest failed'
321-
code: 'ERR_TEST_FAILURE'
322314
...
323315
# Subtest: invalid subtest - pass but subtest fails
324316
ok 25 - invalid subtest - pass but subtest fails
@@ -801,9 +793,9 @@ ok 63 - last test
801793
1..63
802794
# tests 77
803795
# suites 0
804-
# pass 36
805-
# fail 25
806-
# cancelled 3
796+
# pass 38
797+
# fail 24
798+
# cancelled 2
807799
# skipped 9
808800
# todo 4
809801
# duration_ms *

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@
9090
Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:269:1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event.
9191
tests 75
9292
suites 0
93-
pass 34
94-
fail 25
95-
cancelled 3
93+
pass 36
94+
fail 24
95+
cancelled 2
9696
skipped 9
9797
todo 4
9898
duration_ms *
@@ -203,10 +203,6 @@
203203
sync throw non-error fail (*ms)
204204
Symbol(thrown symbol from sync throw non-error fail)
205205

206-
*
207-
+long running (*ms)
208-
'test did not finish before its parent and was cancelled'
209-
210206
*
211207
sync skip option is false fail (*ms)
212208
Error: this should be executed

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@
9393
Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:269:1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event.
9494
tests 76
9595
suites 0
96-
pass 35
97-
fail 25
98-
cancelled 3
96+
pass 37
97+
fail 24
98+
cancelled 2
9999
skipped 9
100100
todo 4
101101
duration_ms *
@@ -206,10 +206,6 @@
206206
sync throw non-error fail (*ms)
207207
Symbol(thrown symbol from sync throw non-error fail)
208208

209-
*
210-
+long running (*ms)
211-
'test did not finish before its parent and was cancelled'
212-
213209
*
214210
sync skip option is false fail (*ms)
215211
Error: this should be executed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,6 @@ const tests = [
199199
name: 'test-runner/output/unfinished-suite-async-error.js',
200200
flags: ['--test-reporter=tap'],
201201
},
202-
{
203-
name: 'test-runner/output/unresolved_promise.js',
204-
flags: ['--test-reporter=tap'],
205-
},
206202
{ name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
207203
{
208204
name: 'test-runner/output/arbitrary-output.js',

0 commit comments

Comments
 (0)