Skip to content

Commit d00a7b1

Browse files
committed
test_runner: fix run() none-isolation teardown hang
1 parent f38a739 commit d00a7b1

File tree

3 files changed

+64
-0
lines changed

3 files changed

+64
-0
lines changed

lib/internal/test_runner/runner.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,12 @@ function run(options = kEmptyObject) {
944944
finishBootstrap();
945945
return root.processPendingSubtests();
946946
};
947+
948+
if (!isTestRunner) {
949+
// run() API consumers can keep handles alive (e.g., IPC). Finalize
950+
// without waiting for beforeExit so the stream can close promptly.
951+
teardown = () => root.harness.teardown();
952+
}
947953
}
948954
}
949955

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
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+
cluster.fork().on('exit', (code, signal) => {
9+
if (signal !== null) {
10+
process.stderr.write(`worker exited with signal ${signal}\n`);
11+
process.exit(1);
12+
}
13+
14+
process.exit(code ?? 0);
15+
});
16+
} else {
17+
// Repro based on: https://github.com/nodejs/node/issues/60020
18+
const stream = run({
19+
isolation: 'none',
20+
files: [
21+
join(__dirname, 'default-behavior', 'test', 'random.cjs'),
22+
],
23+
});
24+
25+
async function consumeStream() {
26+
for await (const data of stream) {
27+
if (data === undefined) {
28+
continue;
29+
}
30+
}
31+
process.stdout.write('on end\n');
32+
process.exit(0);
33+
}
34+
35+
consumeStream().catch((err) => {
36+
process.stderr.write(`worker error: ${err}\n`);
37+
process.exit(1);
38+
});
39+
40+
setTimeout(() => {
41+
process.stderr.write('worker timed out waiting for end\n');
42+
process.exit(1);
43+
}, 3000).unref();
44+
}

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(5000),
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)