Skip to content

Commit 67ad62d

Browse files
committed
test_runner: fix run() none-isolation teardown hang
1 parent 6964b53 commit 67ad62d

File tree

3 files changed

+71
-1
lines changed

3 files changed

+71
-1
lines changed

lib/internal/test_runner/runner.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,8 +942,19 @@ function run(options = kEmptyObject) {
942942
debug('beginning test execution');
943943
root.entryFile = null;
944944
finishBootstrap();
945-
return root.processPendingSubtests();
945+
const pendingSubtestsPromise = root.processPendingSubtests();
946+
if (!isTestRunner) {
947+
PromisePrototypeThen(pendingSubtestsPromise, undefined, triggerUncaughtException);
948+
return;
949+
}
950+
return pendingSubtestsPromise;
946951
};
952+
953+
if (!isTestRunner) {
954+
// run() API consumers can keep handles alive (e.g., IPC). Finalize
955+
// without waiting for beforeExit so the stream can close promptly.
956+
teardown = () => root.harness.teardown();
957+
}
947958
}
948959
}
949960

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
3+
const cluster = require('node:cluster');
4+
const { join } = require('node:path');
5+
const { run } = require('node:test');
6+
7+
if (cluster.isPrimary) {
8+
const worker = cluster.fork();
9+
worker.on('exit', (code, signal) => {
10+
if (signal !== null) {
11+
process.stderr.write(`worker exited with signal ${signal}\n`);
12+
process.exit(1);
13+
}
14+
15+
process.exit(code ?? 0);
16+
});
17+
} else {
18+
// Repro based on: https://github.com/nodejs/node/issues/60020
19+
// We pin `files` for deterministic test discovery in CI.
20+
const stream = run({
21+
isolation: 'none',
22+
files: [
23+
join(__dirname, 'default-behavior', 'test', 'random.cjs'),
24+
],
25+
});
26+
27+
stream.on('error', (err) => {
28+
process.stderr.write(`worker error: ${err}\n`);
29+
process.exit(1);
30+
});
31+
32+
stream.on('data', (data) => {
33+
process.stdout.write(`on data ${data.type}\n`);
34+
});
35+
36+
stream.on('end', () => {
37+
process.stdout.write('on end\n');
38+
process.exit(0);
39+
});
40+
41+
setTimeout(() => {
42+
process.stderr.write('worker timed out waiting for end\n');
43+
process.exit(1);
44+
}, 3000).unref();
45+
}

test/parallel/test-runner-run.mjs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as common from '../common/index.mjs';
22
import * as fixtures from '../common/fixtures.mjs';
33
import { join } from 'node:path';
4+
import { spawnSync } from 'node:child_process';
45
import { describe, it, run } from 'node:test';
56
import { dot, spec, tap } from 'node:test/reporters';
67
import consumers from 'node:stream/consumers';
@@ -646,6 +647,19 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
646647
assert.strictEqual(diagnostics.includes(entry), true);
647648
}
648649
});
650+
651+
// Regression test for https://github.com/nodejs/node/issues/60020
652+
it('should not hang in cluster workers when isolation is none', () => {
653+
const fixture = fixtures.path('test-runner', 'run-isolation-none-in-cluster.js');
654+
const { status, signal, stdout, stderr } = spawnSync(process.execPath, [fixture], {
655+
encoding: 'utf8',
656+
timeout: common.platformTimeout(15_000),
657+
});
658+
659+
assert.strictEqual(signal, null);
660+
assert.strictEqual(status, 0, stderr);
661+
assert.match(stdout, /on end/);
662+
});
649663
});
650664

651665
describe('env', () => {

0 commit comments

Comments
 (0)