FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: reland run global after() hook earlier · nodejs/node@d839640 · GitHub

/ node Public

Commit d839640

Browse files
cjihrig
authored and
targos
committed
test_runner: reland run global after() hook earlier
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (#49059 (comment)) and an additional common.mustCall() in the added test. Refs: #49059 Refs: #49110 PR-URL: #49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent d61a505 commit d839640

7 files changed

Lines changed: 104 additions & 9 deletions

File tree

‎lib/internal/test_runner/harness.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ function setup(root) {
140140
const rejectionHandler =
141141
createProcessEventHandler('unhandledRejection', root);
142142
const coverage = configureCoverage(root, globalOptions);
143-
const exitHandler = async () => {
144-
await root.run(new ERR_TEST_FAILURE(
143+
const exitHandler = () => {
144+
root.postRun(new ERR_TEST_FAILURE(
145145
'Promise resolution is still pending but the event loop has already resolved',
146146
kCancelledByParent));
147147

@@ -150,8 +150,8 @@ function setup(root) {
150150
process.removeListener('uncaughtException', exceptionHandler);
151151
};
152152

153-
const terminationHandler = async () => {
154-
await exitHandler();
153+
const terminationHandler = () => {
154+
exitHandler();
155155
process.exit();
156156
};
157157

‎lib/internal/test_runner/test.js‎

Lines 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 || typeof this.hookType === 'string') {
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,

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

Lines 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());

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

Lines 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 *

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

Lines 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

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

Lines 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

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

Lines 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(common.mustCall());
24+
}));
25+
26+
test();

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL