Skip to content

Commit a1529d5

Browse files
cjihrigRafaelGSS
cjihrig
authored and
RafaelGSS
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: #58800 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Pietro Marchini <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
1 parent c9dc0a8 commit a1529d5

File tree

11 files changed

+56
-84
lines changed

11 files changed

+56
-84
lines changed

‎lib/internal/test_runner/harness.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ const {
2828
setupGlobalSetupTeardownFunctions,
2929
} = require('internal/test_runner/utils');
3030
const { queueMicrotask } = require('internal/process/task_queues');
31+
const { TIMEOUT_MAX } = require('internal/timers');
32+
const { clearInterval, setInterval } = require('timers');
3133
const { bigint: hrtime } = process.hrtime;
3234
const testResources = new SafeMap();
3335
let globalRoot;
@@ -228,11 +230,20 @@ function setupProcessState(root, globalOptions) {
228230
const rejectionHandler =
229231
createProcessEventHandler('unhandledRejection', root);
230232
const coverage = configureCoverage(root, globalOptions);
231-
const exitHandler = async () => {
233+
const exitHandler = async (kill) => {
232234
if (root.subtests.length === 0 && (root.hooks.before.length > 0 || root.hooks.after.length > 0)) {
233235
// Run global before/after hooks in case there are no tests
234236
await root.run();
235237
}
238+
239+
if (kill !== true && root.subtestsPromise !== null) {
240+
// Wait for all subtests to finish, but keep the process alive in case
241+
// there are no ref'ed handles left.
242+
const keepAlive = setInterval(() => {}, TIMEOUT_MAX);
243+
await root.subtestsPromise.promise;
244+
clearInterval(keepAlive);
245+
}
246+
236247
root.postRun(new ERR_TEST_FAILURE(
237248
'Promise resolution is still pending but the event loop has already resolved',
238249
kCancelledByParent));
@@ -252,8 +263,8 @@ function setupProcessState(root, globalOptions) {
252263
}
253264
};
254265

255-
const terminationHandler = () => {
256-
exitHandler();
266+
const terminationHandler = async () => {
267+
await exitHandler(true);
257268
process.exit();
258269
};
259270

‎lib/internal/test_runner/test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,8 @@ class Test extends AsyncResource {
651651
this.activeSubtests = 0;
652652
this.pendingSubtests = [];
653653
this.readySubtests = new SafeMap();
654+
this.unfinishedSubtests = new SafeSet();
655+
this.subtestsPromise = null;
654656
this.subtests = [];
655657
this.waitingOn = 0;
656658
this.finished = false;
@@ -754,6 +756,11 @@ class Test extends AsyncResource {
754756

755757
addReadySubtest(subtest) {
756758
this.readySubtests.set(subtest.childNumber, subtest);
759+
760+
if (this.unfinishedSubtests.delete(subtest) &&
761+
this.unfinishedSubtests.size === 0) {
762+
this.subtestsPromise.resolve();
763+
}
757764
}
758765

759766
processReadySubtestRange(canSend) {
@@ -815,6 +822,7 @@ class Test extends AsyncResource {
815822

816823
if (parent.waitingOn === 0) {
817824
parent.waitingOn = test.childNumber;
825+
parent.subtestsPromise = PromiseWithResolvers();
818826
}
819827

820828
if (preventAddingSubtests) {
@@ -937,6 +945,7 @@ class Test extends AsyncResource {
937945
// If there is enough available concurrency to run the test now, then do
938946
// it. Otherwise, return a Promise to the caller and mark the test as
939947
// pending for later execution.
948+
this.parent.unfinishedSubtests.add(this);
940949
this.reporter.enqueue(this.nesting, this.loc, this.name, this.reportedType);
941950
if (this.root.harness.buildPromise || !this.parent.hasConcurrency()) {
942951
const deferred = PromiseWithResolvers();
@@ -1061,6 +1070,10 @@ class Test extends AsyncResource {
10611070

10621071
this[kShouldAbort]();
10631072

1073+
if (this.subtestsPromise !== null) {
1074+
await SafePromiseRace([this.subtestsPromise.promise, stopPromise]);
1075+
}
1076+
10641077
if (this.plan !== null) {
10651078
const planPromise = this.plan?.check();
10661079
// If the plan returns a promise, it means that it is waiting for more assertions to be made before

‎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
@@ -215,10 +215,6 @@ const tests = [
215215
name: 'test-runner/output/unfinished-suite-async-error.js',
216216
flags: ['--test-reporter=tap'],
217217
},
218-
{
219-
name: 'test-runner/output/unresolved_promise.js',
220-
flags: ['--test-reporter=tap'],
221-
},
222218
{ name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
223219
{
224220
name: 'test-runner/output/arbitrary-output.js',

0 commit comments

Comments
 (0)