Skip to content

test_runner: fix run() none-isolation teardown hang#62056

Closed
inoway46 wants to merge 1 commit intonodejs:mainfrom
inoway46:fix-60020-run-none-teardown
Closed

test_runner: fix run() none-isolation teardown hang#62056
inoway46 wants to merge 1 commit intonodejs:mainfrom
inoway46:fix-60020-run-none-teardown

Conversation

@inoway46
Copy link
Contributor

@inoway46 inoway46 commented Mar 1, 2026

When run({ isolation: 'none' }) is used from a cluster worker, live handles such as IPC can keep the process open after the tests finish. In that case teardown may be delayed and the run() stream can hang waiting for end.

Set an explicit teardown callback for the non-isTestRunner run() path so the stream can finalize even if the process is still being kept alive by open handles.

Add a regression test based on the reproduction from #60020.

Fixes: #60020

@nodejs-github-bot
Copy link
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 Mar 1, 2026
@inoway46 inoway46 force-pushed the fix-60020-run-none-teardown branch 2 times, most recently from 2e503c7 to d00a7b1 Compare March 8, 2026 14:36
@inoway46 inoway46 force-pushed the fix-60020-run-none-teardown branch from d00a7b1 to c8bf2a8 Compare March 8, 2026 14:41
@inoway46 inoway46 changed the title test_runner: fix run() isolation=none hang in cluster workers test_runner: fix run() none-isolation teardown hang Mar 8, 2026
@inoway46 inoway46 marked this pull request as ready for review March 8, 2026 14:54
@codecov
Copy link

codecov bot commented Mar 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.64%. Comparing base (3725bd2) to head (c8bf2a8).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62056      +/-   ##
==========================================
- Coverage   89.65%   89.64%   -0.02%     
==========================================
  Files         676      676              
  Lines      206543   206549       +6     
  Branches    39547    39548       +1     
==========================================
- Hits       185184   185155      -29     
- Misses      13480    13523      +43     
+ Partials     7879     7871       -8     
Files with missing lines Coverage Δ
lib/internal/test_runner/runner.js 93.23% <100.00%> (+0.04%) ⬆️

... and 41 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.

@JakobJingleheimer
Copy link
Member

I think this might be a duplicate of #57394

@inoway46
Copy link
Contributor Author

inoway46 commented Mar 8, 2026

@JakobJingleheimer
You're right, I missed that this was already addressed in #57394. Thanks for pointing it out. I'll close this.

@inoway46 inoway46 closed this Mar 8, 2026
@inoway46 inoway46 deleted the fix-60020-run-none-teardown branch March 8, 2026 16:29
@JakobJingleheimer JakobJingleheimer added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Mar 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate Issues and PRs that are duplicates of other issues or PRs. 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 with isolation=none does not work in child process

3 participants