From f82690b5296ceb76c01f31180f1ba9920571e8b4 Mon Sep 17 00:00:00 2001 From: Dayun Date: Tue, 29 Sep 2026 15:39:46 +0900 Subject: [PATCH 1/2] test_runner: avoid waiting on inherited stdio with force exit Signed-off-by: Dayun --- lib/internal/test_runner/runner.js | 24 ++++++++++- .../test-runner/force-exit-inherited-stdio.js | 15 +++++++ .../test-runner-force-exit-inherited-stdio.js | 41 +++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/test-runner/force-exit-inherited-stdio.js create mode 100644 test/parallel/test-runner-force-exit-inherited-stdio.js diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 98947c742b21..f1e2407dc136 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -22,6 +22,7 @@ const { SafePromiseAll, SafePromiseAllReturnVoid, SafePromiseAllSettledReturnVoid, + SafePromiseRace, SafeSet, String, StringFromCharCode, @@ -291,10 +292,15 @@ class FileTest extends Test { #rawBufferSize = 0; #reportedChildren = 0; #pendingPartialV8Header = false; + #reportFinished; + #resolveReportFinished; failedSubtests = false; constructor(options) { super(options); + const { promise, resolve } = PromiseWithResolvers(); + this.#reportFinished = promise; + this.#resolveReportFinished = resolve; this.loc ??= { __proto__: null, line: 1, @@ -304,6 +310,10 @@ class FileTest extends Test { this.timeout = null; } + get reportFinished() { + return this.#reportFinished; + } + willBeFilteredByTags() { // File wrappers have no tags of their own. Tag filtering applies to the // tests inside the file, which run in a child process (or in-process @@ -544,6 +554,10 @@ class FileTest extends Test { this.#rawBufferSize = TypedArrayPrototypeGetLength(remaining); this.#rawBuffer = this.#rawBufferSize !== 0 ? [remaining] : []; + if (item.type === 'test:summary') { + this.#resolveReportFinished(); + } + this.addToReport(item); } } @@ -618,9 +632,17 @@ function runTestFile(path, filesWatcher, opts) { }); }); + const stdoutFinished = finished( + child.stdout, + { __proto__: null, signal: t.signal }, + ); + const reportFinished = opts.forceExit ? + SafePromiseRace([stdoutFinished, subtest.reportFinished]) : + stdoutFinished; + const { 0: { 0: code, 1: signal } } = await SafePromiseAll([ once(child, 'exit', { __proto__: null, signal: t.signal }), - finished(child.stdout, { __proto__: null, signal: t.signal }), + reportFinished, ]); // Close readline interface to prevent memory leak diff --git a/test/fixtures/test-runner/force-exit-inherited-stdio.js b/test/fixtures/test-runner/force-exit-inherited-stdio.js new file mode 100644 index 000000000000..098cb30ed625 --- /dev/null +++ b/test/fixtures/test-runner/force-exit-inherited-stdio.js @@ -0,0 +1,15 @@ +'use strict'; + +const { spawn } = require('node:child_process'); +const { test } = require('node:test'); + +test('leaks a child with inherited stdio', () => { + spawn( + process.execPath, + [ + '-e', + `setTimeout(() => {}, ${process.env.TEST_RUNNER_STALL_MS})`, + ], + { stdio: 'inherit' }, + ); +}); diff --git a/test/parallel/test-runner-force-exit-inherited-stdio.js b/test/parallel/test-runner-force-exit-inherited-stdio.js new file mode 100644 index 000000000000..cb85424c6ac0 --- /dev/null +++ b/test/parallel/test-runner-force-exit-inherited-stdio.js @@ -0,0 +1,41 @@ +'use strict'; + +const common = require('../common'); + +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const fixtures = require('../common/fixtures'); + +const fixture = + fixtures.path('test-runner/force-exit-inherited-stdio.js'); + +const maxDuration = common.platformTimeout(2000); +const stallDuration = common.platformTimeout(4000); +const start = Date.now(); + +const result = spawnSync( + process.execPath, + [ + '--test', + '--test-force-exit', + fixture, + ], + { + encoding: 'utf8', + env: { + ...process.env, + TEST_RUNNER_STALL_MS: String(stallDuration), + }, + }, +); + +const duration = Date.now() - start; + +assert.strictEqual(result.status, 0); +assert.strictEqual(result.signal, null); +assert.strictEqual(result.stderr, ''); + +assert.ok( + duration < maxDuration, + `test runner took ${duration}ms to exit`, +); From 7d483580b11603641a0e61052002fc5aed9752b5 Mon Sep 17 00:00:00 2001 From: Dayun Date: Sat, 3 Oct 2026 00:05:29 +0900 Subject: [PATCH 2/2] test_runner: avoid shadowing path resolve Signed-off-by: Dayun --- lib/internal/test_runner/runner.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index f1e2407dc136..d65f6f743162 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -298,9 +298,9 @@ class FileTest extends Test { constructor(options) { super(options); - const { promise, resolve } = PromiseWithResolvers(); + const { promise, resolve: resolveReportFinished } = PromiseWithResolvers(); this.#reportFinished = promise; - this.#resolveReportFinished = resolve; + this.#resolveReportFinished = resolveReportFinished; this.loc ??= { __proto__: null, line: 1,