Skip to content

test_runner: avoid waiting on inherited stdio with force exit - #66390

Open
dayun6530 wants to merge 2 commits into
nodejs:mainfrom
dayun6530:fix/test-force-exit-inherited-stdio
Open

dayun6530 wants to merge 2 commits into
nodejs:mainfrom
dayun6530:fix/test-force-exit-inherited-stdio

Conversation

@dayun6530

Copy link
Copy Markdown
Contributor

Fixes: #66336

When --test-force-exit is used, the parent test runner waits for both
the test file process to exit and its stdout stream to finish.

If the test file leaks a child process with inherited stdio, that child
keeps the stdout pipe open even after the test file process exits. This
causes the test runner to remain alive until the leaked child exits.

Use the test:summary event as an additional completion signal when
--test-force-exit is enabled. Once the summary has been received, the
test report has completed and the parent no longer needs to wait for EOF
from stdout.

The existing stdout completion path is kept as a fallback.

A regression test is included for a leaked child process with inherited
stdio.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Sep 29, 2026
@addaleax

Copy link
Copy Markdown
Member

If the test file leaks a child process with inherited stdio, that child
keeps the stdout pipe open even after the test file process exits. This
causes the test runner to remain alive until the leaked child exits.

That can happen regardless of the forced exit flag, right?

@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.43%. Comparing base (4486dce) to head (7d48358).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/test_runner/runner.js 86.95% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66390      +/-   ##
==========================================
+ Coverage   90.42%   90.43%   +0.01%     
==========================================
  Files         791      791              
  Lines      275611   275591      -20     
  Branches    52845    52843       -2     
==========================================
+ Hits       249209   249229      +20     
+ Misses      16810    16763      -47     
- Partials     9592     9599       +7     
Files with missing lines Coverage Δ
lib/internal/test_runner/runner.js 94.97% <86.95%> (-0.16%) ⬇️

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Dayun <dlekdbs6530@gmail.com>
Signed-off-by: Dayun <dlekdbs6530@gmail.com>
@dayun6530
dayun6530 force-pushed the fix/test-force-exit-inherited-stdio branch from c929287 to 7d48358 Compare October 2, 2026 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_runner: --test-force-exit does not end a run held open by a leaked child with inherited stdio (Linux)

3 participants