Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6346bdc

Browse filesBrowse files
cjihrigcjihrig
committed
test_runner: run global after() hook earlier
This commit moves the global after() hook execution from the 'beforeExit' event to the point where all tests have finished running. This gives the global after() a chance to clean up handles that would otherwise prevent the 'beforeExit' event from being emitted. PR-URL: #49059 Fixes: #49056 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 4e61c22 commit 6346bdc
Copy full SHA for 6346bdc

File tree

Expand file treeCollapse file tree

7 files changed

+104
-9
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+104
-9
lines changed
Open diff view settings
Collapse file

‎lib/internal/test_runner/harness.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/harness.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ function setup(root) {
142142
const rejectionHandler =
143143
createProcessEventHandler('unhandledRejection', root);
144144
const coverage = configureCoverage(root, globalOptions);
145-
const exitHandler = async () => {
146-
await root.run(new ERR_TEST_FAILURE(
145+
const exitHandler = () => {
146+
root.postRun(new ERR_TEST_FAILURE(
147147
'Promise resolution is still pending but the event loop has already resolved',
148148
kCancelledByParent));
149149

@@ -152,8 +152,8 @@ function setup(root) {
152152
process.removeListener('uncaughtException', exceptionHandler);
153153
};
154154

155-
const terminationHandler = async () => {
156-
await exitHandler();
155+
const terminationHandler = () => {
156+
exitHandler();
157157
process.exit();
158158
};
159159

Collapse file

‎lib/internal/test_runner/test.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/test.js
+23-4Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ class Test extends AsyncResource {
574574
}
575575
}
576576

577-
async run(pendingSubtestsError) {
577+
async run() {
578578
if (this.parent !== null) {
579579
this.parent.activeSubtests++;
580580
}
@@ -662,9 +662,16 @@ class Test extends AsyncResource {
662662
}
663663
}
664664

665-
// Clean up the test. Then, try to report the results and execute any
666-
// tests that were pending due to available concurrency.
667-
this.postRun(pendingSubtestsError);
665+
if (this.parent !== null) {
666+
// Clean up the test. Then, try to report the results and execute any
667+
// tests that were pending due to available concurrency.
668+
//
669+
// The root test is skipped here because it is a special case. Its
670+
// postRun() method is called when the process is getting ready to exit.
671+
// This helps catch any asynchronous activity that occurs after the tests
672+
// have finished executing.
673+
this.postRun();
674+
}
668675
}
669676

670677
postRun(pendingSubtestsError) {
@@ -706,6 +713,18 @@ class Test extends AsyncResource {
706713
this.parent.addReadySubtest(this);
707714
this.parent.processReadySubtestRange(false);
708715
this.parent.processPendingSubtests();
716+
717+
if (this.parent === this.root &&
718+
this.root.activeSubtests === 0 &&
719+
this.root.pendingSubtests.length === 0 &&
720+
this.root.readySubtests.size === 0 &&
721+
this.root.hooks.after.length > 0) {
722+
// This is done so that any global after() hooks are run. At this point
723+
// all of the tests have finished running. However, there might be
724+
// ref'ed handles keeping the event loop alive. This gives the global
725+
// after() hook a chance to clean them up.
726+
this.root.run();
727+
}
709728
} else if (!this.reported) {
710729
const {
711730
diagnostics,
Collapse file

‎test/fixtures/test-runner/output/async-test-scheduling.mjs‎

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as common from '../../../common/index.mjs';
2+
import { describe, test } from 'node:test';
3+
import { setTimeout } from 'node:timers/promises';
4+
5+
test('test', common.mustCall());
6+
describe('suite', common.mustCall(async () => {
7+
test('test', common.mustCall());
8+
await setTimeout(10);
9+
test('scheduled async', common.mustCall());
10+
}));
11+
12+
await setTimeout(10);
13+
test('scheduled async', common.mustCall());
Collapse file

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

Copy file name to clipboard
+37Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
TAP version 13
2+
# Subtest: test
3+
ok 1 - test
4+
---
5+
duration_ms: *
6+
...
7+
# Subtest: suite
8+
# Subtest: test
9+
ok 1 - test
10+
---
11+
duration_ms: *
12+
...
13+
# Subtest: scheduled async
14+
ok 2 - scheduled async
15+
---
16+
duration_ms: *
17+
...
18+
1..2
19+
ok 2 - suite
20+
---
21+
duration_ms: *
22+
type: 'suite'
23+
...
24+
# Subtest: scheduled async
25+
ok 3 - scheduled async
26+
---
27+
duration_ms: *
28+
...
29+
1..3
30+
# tests 4
31+
# suites 1
32+
# pass 4
33+
# fail 0
34+
# cancelled 0
35+
# skipped 0
36+
# todo 0
37+
# duration_ms *
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.j
2222
*
2323
*
2424
*
25-
*
2625
...
2726
1..1
2827
# tests 1
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-runner-output.mjs
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const tests = [
7474
{ name: 'test-runner/output/unresolved_promise.js' },
7575
{ name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
7676
{ name: 'test-runner/output/arbitrary-output.js' },
77+
{ name: 'test-runner/output/async-test-scheduling.mjs' },
7778
!skipForceColors ? {
7879
name: 'test-runner/output/arbitrary-output-colored.js',
7980
transform: snapshot.transform(specTransform, replaceTestDuration), tty: true
Collapse file

‎test/parallel/test-runner-root-after-with-refed-handles.js‎

Copy file name to clipboard
+26Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { before, after, test } = require('node:test');
4+
const { createServer } = require('node:http');
5+
6+
let server;
7+
8+
before(common.mustCall(() => {
9+
server = createServer();
10+
11+
return new Promise(common.mustCall((resolve, reject) => {
12+
server.listen(0, common.mustCall((err) => {
13+
if (err) {
14+
reject(err);
15+
} else {
16+
resolve();
17+
}
18+
}));
19+
}));
20+
}));
21+
22+
after(common.mustCall(() => {
23+
server.close();
24+
}));
25+
26+
test();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.