Skip to content

Commit c5f4939

Browse files
cjihrigcjihrig
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.
1 parent 009d53e commit c5f4939

File tree

11 files changed

+59
-87
lines changed

11 files changed

+59
-87
lines changed

‎lib/internal/test_runner/harness.js

Lines changed: 16 additions & 0 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();
@@ -217,6 +219,15 @@ function setupProcessState(root, globalOptions) {
217219
// Run global before/after hooks in case there are no tests
218220
await root.run();
219221
}
222+
223+
if (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));
@@ -233,6 +244,11 @@ function setupProcessState(root, globalOptions) {
233244

234245
const terminationHandler = () => {
235246
exitHandler();
247+
248+
if (!root.reported) {
249+
process.exitCode = kGenericUserError;
250+
}
251+
236252
process.exit();
237253
};
238254

‎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 & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,13 @@
2424
*[39m
2525
*[39m
2626

27-
[31m✖ should pass but parent fail [90m(*ms)[39m[39m
28-
[32m'test did not finish before its parent and was cancelled'[39m
29-
27+
[32m✔ should pass but parent fail [90m(*ms)[39m[39m
3028
[31m✖ parent [90m(*ms)[39m[39m
3129
[34mℹ tests 6[39m
3230
[34mℹ suites 0[39m
33-
[34mℹ pass 1[39m
31+
[34mℹ pass 2[39m
3432
[34mℹ fail 3[39m
35-
[34mℹ cancelled 1[39m
33+
[34mℹ cancelled 0[39m
3634
[34mℹ skipped 1[39m
3735
[34mℹ todo 0[39m
3836
[34mℹ duration_ms *[39m
@@ -63,7 +61,3 @@
6361
*[39m
6462
*[39m
6563
*[39m
66-
67-
*
68-
[31m✖ should pass but parent fail [90m(*ms)[39m[39m
69-
[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 & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,6 @@
118118
level 0a (*ms)
119119
top level
120120
+long running (*ms)
121-
'test did not finish before its parent and was cancelled'
122-
123121
+short running
124122
++short running (*ms)
125123
+short running (*ms)
@@ -302,9 +300,9 @@
302300
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.
303301
tests 75
304302
suites 0
305-
pass 34
306-
fail 25
307-
cancelled 3
303+
pass 36
304+
fail 24
305+
cancelled 2
308306
skipped 9
309307
todo 4
310308
duration_ms *
@@ -415,10 +413,6 @@
415413
sync throw non-error fail (*ms)
416414
Symbol(thrown symbol from sync throw non-error fail)
417415

418-
*
419-
+long running (*ms)
420-
'test did not finish before its parent and was cancelled'
421-
422416
*
423417
sync skip option is false fail (*ms)
424418
Error: this should be executed

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,6 @@
118118
level 0a (*ms)
119119
top level
120120
+long running (*ms)
121-
'test did not finish before its parent and was cancelled'
122-
123121
+short running
124122
++short running (*ms)
125123
+short running (*ms)
@@ -305,9 +303,9 @@
305303
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.
306304
tests 76
307305
suites 0
308-
pass 35
309-
fail 25
310-
cancelled 3
306+
pass 37
307+
fail 24
308+
cancelled 2
311309
skipped 9
312310
todo 4
313311
duration_ms *
@@ -418,10 +416,6 @@
418416
sync throw non-error fail (*ms)
419417
Symbol(thrown symbol from sync throw non-error fail)
420418

421-
*
422-
+long running (*ms)
423-
'test did not finish before its parent and was cancelled'
424-
425419
*
426420
sync skip option is false fail (*ms)
427421
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)