Skip to content

Commit 6ef7329

Browse files
romainmenkeRafaelGSS
romainmenke
authored and
RafaelGSS
committed
Revert "test_runner: automatically wait for subtests to finish"
This reverts commit aa3523e. PR-URL: #58282 Fixes: #58227 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>
1 parent d0e42ff commit 6ef7329

File tree

11 files changed

+84
-56
lines changed

11 files changed

+84
-56
lines changed

‎lib/internal/test_runner/harness.js‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ 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');
3331
const { bigint: hrtime } = process.hrtime;
3432
const testResources = new SafeMap();
3533
let globalRoot;
@@ -230,20 +228,11 @@ function setupProcessState(root, globalOptions) {
230228
const rejectionHandler =
231229
createProcessEventHandler('unhandledRejection', root);
232230
const coverage = configureCoverage(root, globalOptions);
233-
const exitHandler = async (kill) => {
231+
const exitHandler = async () => {
234232
if (root.subtests.length === 0 && (root.hooks.before.length > 0 || root.hooks.after.length > 0)) {
235233
// Run global before/after hooks in case there are no tests
236234
await root.run();
237235
}
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-
247236
root.postRun(new ERR_TEST_FAILURE(
248237
'Promise resolution is still pending but the event loop has already resolved',
249238
kCancelledByParent));
@@ -263,8 +252,8 @@ function setupProcessState(root, globalOptions) {
263252
}
264253
};
265254

266-
const terminationHandler = async () => {
267-
await exitHandler(true);
255+
const terminationHandler = () => {
256+
exitHandler();
268257
process.exit();
269258
};
270259

‎lib/internal/test_runner/test.js‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -651,8 +651,6 @@ 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;
656654
this.subtests = [];
657655
this.waitingOn = 0;
658656
this.finished = false;
@@ -756,11 +754,6 @@ class Test extends AsyncResource {
756754

757755
addReadySubtest(subtest) {
758756
this.readySubtests.set(subtest.childNumber, subtest);
759-
760-
if (this.unfinishedSubtests.delete(subtest) &&
761-
this.unfinishedSubtests.size === 0) {
762-
this.subtestsPromise.resolve();
763-
}
764757
}
765758

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

823816
if (parent.waitingOn === 0) {
824817
parent.waitingOn = test.childNumber;
825-
parent.subtestsPromise = PromiseWithResolvers();
826818
}
827819

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

10711062
this[kShouldAbort]();
10721063

1073-
if (this.subtestsPromise !== null) {
1074-
await SafePromiseRace([this.subtestsPromise.promise, stopPromise]);
1075-
}
1076-
10771064
if (this.plan !== null) {
10781065
const planPromise = this.plan?.check();
10791066
// 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: 7 additions & 3 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-
[32m✔ should pass but parent fail [90m(*ms)[39m[39m
6+
[31m✖ 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 2[39m
10+
[34mℹ pass 1[39m
1111
[34mℹ fail 3[39m
12-
[34mℹ cancelled 0[39m
12+
[34mℹ cancelled 1[39m
1313
[34mℹ skipped 1[39m
1414
[34mℹ todo 0[39m
1515
[34mℹ duration_ms *[39m
@@ -40,3 +40,7 @@
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: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
..XX...X..XXX.X.....
2-
XXX............X....
2+
XXX.....X..X...X....
33
.....X...XXX.XX.....
44
XXXXXXX...XXXXX
55

@@ -93,6 +93,10 @@ 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'
96100
✖ sync skip option is false fail (*ms)
97101
Error: this should be executed
98102
*

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,12 @@ 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="0" skipped="0" hostname="HOSTNAME">
190-
<testcase name="+long running" time="*" classname="test"/>
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>
191195
<testsuite name="+short running" time="*" disabled="0" errors="0" tests="1" failures="0" skipped="0" hostname="HOSTNAME">
192196
<testcase name="++short running" time="*" classname="test"/>
193197
</testsuite>
@@ -512,9 +516,9 @@ Error [ERR_TEST_FAILURE]: test could not be started because its parent finished
512516
<!-- 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. -->
513517
<!-- tests 75 -->
514518
<!-- suites 0 -->
515-
<!-- pass 36 -->
516-
<!-- fail 24 -->
517-
<!-- cancelled 2 -->
519+
<!-- pass 34 -->
520+
<!-- fail 25 -->
521+
<!-- cancelled 3 -->
518522
<!-- skipped 9 -->
519523
<!-- todo 4 -->
520524
<!-- duration_ms * -->

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,35 @@
11
TAP version 13
22
# Subtest: does not keep event loop alive
33
# Subtest: +does not keep event loop alive
4-
ok 1 - +does not keep event loop alive
4+
not 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+
*
814
...
915
1..1
10-
ok 1 - does not keep event loop alive
16+
not ok 1 - does not keep event loop alive
1117
---
1218
duration_ms: *
1319
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+
*
1426
...
1527
1..1
1628
# tests 2
1729
# suites 0
18-
# pass 2
30+
# pass 0
1931
# fail 0
20-
# cancelled 0
32+
# cancelled 2
2133
# skipped 0
2234
# todo 0
2335
# duration_ms *

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,14 @@ ok 23 - level 0a
288288
...
289289
# Subtest: top level
290290
# Subtest: +long running
291-
ok 1 - +long running
291+
not 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'
295299
...
296300
# Subtest: +short running
297301
# Subtest: ++short running
@@ -307,10 +311,14 @@ ok 23 - level 0a
307311
type: 'test'
308312
...
309313
1..2
310-
ok 24 - top level
314+
not ok 24 - top level
311315
---
312316
duration_ms: *
313317
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'
314322
...
315323
# Subtest: invalid subtest - pass but subtest fails
316324
ok 25 - invalid subtest - pass but subtest fails
@@ -777,9 +785,9 @@ not ok 62 - invalid subtest fail
777785
# 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.
778786
# tests 75
779787
# suites 0
780-
# pass 36
781-
# fail 24
782-
# cancelled 2
788+
# pass 34
789+
# fail 25
790+
# cancelled 3
783791
# skipped 9
784792
# todo 4
785793
# duration_ms *

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,14 @@ ok 23 - level 0a
288288
...
289289
# Subtest: top level
290290
# Subtest: +long running
291-
ok 1 - +long running
291+
not 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'
295299
...
296300
# Subtest: +short running
297301
# Subtest: ++short running
@@ -307,10 +311,14 @@ ok 23 - level 0a
307311
type: 'test'
308312
...
309313
1..2
310-
ok 24 - top level
314+
not ok 24 - top level
311315
---
312316
duration_ms: *
313317
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'
314322
...
315323
# Subtest: invalid subtest - pass but subtest fails
316324
ok 25 - invalid subtest - pass but subtest fails
@@ -791,9 +799,9 @@ ok 63 - last test
791799
1..63
792800
# tests 77
793801
# suites 0
794-
# pass 38
795-
# fail 24
796-
# cancelled 2
802+
# pass 36
803+
# fail 25
804+
# cancelled 3
797805
# skipped 9
798806
# todo 4
799807
# duration_ms *

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

Lines changed: 7 additions & 3 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 36
94-
fail 24
95-
cancelled 2
93+
pass 34
94+
fail 25
95+
cancelled 3
9696
skipped 9
9797
todo 4
9898
duration_ms *
@@ -203,6 +203,10 @@
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+
206210
*
207211
sync skip option is false fail (*ms)
208212
Error: this should be executed

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

Lines changed: 7 additions & 3 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 37
97-
fail 24
98-
cancelled 2
96+
pass 35
97+
fail 25
98+
cancelled 3
9999
skipped 9
100100
todo 4
101101
duration_ms *
@@ -206,6 +206,10 @@
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+
209213
*
210214
sync skip option is false fail (*ms)
211215
Error: this should be executed

0 commit comments

Comments
 (0)