From 75270c7f12c5438d6cc1b9337e0d0fe8df3a89eb Mon Sep 17 00:00:00 2001 From: Navaneeth Prabha Date: Fri, 31 Jul 2026 14:21:57 +0000 Subject: [PATCH 1/2] test_runner: flush report stream before force exit Signed-off-by: Navaneeth Prabha --- lib/internal/test_runner/test.js | 20 +++++- .../test-runner-force-exit-no-verdict-loss.js | 71 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100755 test/parallel/test-runner-force-exit-no-verdict-loss.js diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 38ce54e4ea9b..45ec921b14cf 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -85,6 +85,7 @@ const { } = require('internal/validators'); const { clearTimeout, + setImmediate, setTimeout, } = require('timers'); const { TIMEOUT_MAX } = require('internal/timers'); @@ -1476,7 +1477,19 @@ class Test extends AsyncResource { if (!destination.closed && typeof destination.close === 'function') { destination.close(resolve); } else { - resolve(); + // The destination has no close() to await - for example a child + // process's stdout, which is a pipe to the test runner's parent + // process. Writes to a pipe are asynchronous, so the process.exit() + // below can truncate buffered output and silently drop test + // results (https://github.com/nodejs/node/issues/64833). Switch the + // underlying handle to blocking so any queued bytes are flushed to + // the OS, then end the stream and wait for it to finish before + // exiting. + if (destination._handle && + typeof destination._handle.setBlocking === 'function') { + destination._handle.setBlocking(true); + } + destination.end(resolve); } }); })); @@ -1484,6 +1497,11 @@ class Test extends AsyncResource { this.harness.teardown(); await SafePromiseAllReturnVoid(promises); + // Yield once more so the event loop can drain any report events that are + // still in flight from child processes (their stdout is piped into this + // process) before we force the exit. The blocking flush above guarantees + // the writable side; this covers the readable side on the parent. + await new Promise((resolve) => setImmediate(resolve)); process.exit(); } } diff --git a/test/parallel/test-runner-force-exit-no-verdict-loss.js b/test/parallel/test-runner-force-exit-no-verdict-loss.js new file mode 100755 index 000000000000..782f24af0eed --- /dev/null +++ b/test/parallel/test-runner-force-exit-no-verdict-loss.js @@ -0,0 +1,71 @@ +'use strict'; +require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const { writeFileSync, mkdirSync, readFileSync } = require('node:fs'); +const { join } = require('node:path'); + +// Regression test for https://github.com/nodejs/node/issues/64833. +// +// When --test-force-exit is combined with process isolation and concurrency, +// each test file runs in a child process that streams its results back to the +// parent over a pipe. Writes to a pipe are asynchronous, so a forced +// process.exit() could truncate the still-flushing report stream, silently +// dropping test verdicts while the process still exited with code 0. +// +// Generate several test files, each of which keeps itself alive with a ref'd +// handle (so --test-force-exit is required to finish), then assert that every +// single verdict survives the forced exit. + +const FILES = 12; +const TESTS_PER_FILE = 1000; +const EXPECTED = FILES * TESTS_PER_FILE; + +tmpdir.refresh(); +const testsDir = tmpdir.resolve('tests'); +mkdirSync(testsDir); + +for (let f = 0; f < FILES; f++) { + let body = "'use strict';\n"; + body += "const { test } = require('node:test');\n"; + // A ref'd handle keeps the child alive so it never exits on its own; only + // --test-force-exit ends it, forcing a process.exit() while the child's + // stdout may still be flushing verdicts to the parent. + body += 'setInterval(() => {}, 1_000_000);\n'; + for (let t = 0; t < TESTS_PER_FILE; t++) { + body += `test('f${f}_t${t}', () => {});\n`; + } + writeFileSync(join(testsDir, `f${f}.test.js`), body); +} + +// Write the aggregated report to a file rather than piping it through +// spawnSync (12k TAP lines would exceed the default maxBuffer). Running from +// within testsDir lets the runner auto-discover the *.test.js files. +const destination = tmpdir.resolve('out.tap'); +const args = [ + '--test', + '--test-force-exit', + '--test-concurrency=16', + '--test-reporter=tap', + `--test-reporter-destination=${destination}`, +]; +const child = spawnSync(process.execPath, args, { encoding: 'utf8', cwd: testsDir }); + +assert.strictEqual( + child.status, + 0, + `unexpected exit code ${child.status} (signal ${child.signal})\n${child.stderr}`, +); + +const output = readFileSync(destination, 'utf8'); +const pass = output.match(/# pass (\d+)/); +const fail = output.match(/# fail (\d+)/); +assert.notStrictEqual(pass, null, `missing pass summary in output:\n${output}`); +assert.strictEqual( + Number(pass[1]), + EXPECTED, + `expected ${EXPECTED} passing verdicts but only ${pass[1]} reached the reporter ` + + `(force-exit truncated the child -> parent report stream)`, +); +assert.strictEqual(Number(fail?.[1]), 0, `unexpected failures:\n${output}`); From 6a7d3c42fe032ebd0d8450eb0eb228cccaacfa87 Mon Sep 17 00:00:00 2001 From: Navaneeth Prabha Date: Sun, 16 Aug 2026 10:07:47 +0000 Subject: [PATCH 2/2] test_runner: bound the force-exit flush wait Ensure --test-force-exit cannot hang if a reporter destination never signals completion, by capping the per-destination flush wait. The normal path still clears the timer as soon as the stream finishes, so a destination that is actively flushing is never cut short. Signed-off-by: Navaneeth Prabha --- lib/internal/test_runner/test.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 45ec921b14cf..c5cab5ab9d14 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -94,6 +94,10 @@ const { relative } = require('path'); const { availableParallelism } = require('os'); const { innerOk } = require('internal/assert/utils'); const { bigint: hrtime } = process.hrtime; +// Upper bound (in milliseconds) on how long --test-force-exit waits for a +// single reporter destination to flush before giving up, so a forced exit +// can never hang on a destination that fails to signal completion. +const kForceExitFlushTimeout = 5000; const kCallbackAndPromisePresent = 'callbackAndPromisePresent'; const kCancelledByParent = 'cancelledByParent'; const kAborted = 'testAborted'; @@ -1473,9 +1477,22 @@ class Test extends AsyncResource { const { destination } = reporterScope.reporters[i]; ArrayPrototypePush(promises, new Promise((resolve) => { + // Force exit must never hang. If a destination never signals + // completion (for example one that already errored before this + // ran), fall back to a bounded timeout so the process.exit() below + // cannot be blocked indefinitely. In the normal path the completion + // callback fires first and clears this timer, so it never cuts off + // a destination that is still flushing. The timer is unref'd so it + // does not by itself keep the event loop alive. + const timer = setTimeout(resolve, kForceExitFlushTimeout); + timer.unref(); + const done = () => { + clearTimeout(timer); + resolve(); + }; destination.on('unpipe', () => { if (!destination.closed && typeof destination.close === 'function') { - destination.close(resolve); + destination.close(done); } else { // The destination has no close() to await - for example a child // process's stdout, which is a pipe to the test runner's parent @@ -1489,7 +1506,7 @@ class Test extends AsyncResource { typeof destination._handle.setBlocking === 'function') { destination._handle.setBlocking(true); } - destination.end(resolve); + destination.end(done); } }); }));