Skip to content

test: keep finalization ref alive until exit#63394

Draft
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:flaky-test-process-finalization
Draft

test: keep finalization ref alive until exit#63394
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:flaky-test-process-finalization

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 17, 2026

Fixes an intermittent failure in parallel/test-process-finalization.

The different-registry-per-thread.mjs fixture registered an object with
process.finalization.register() but did not keep that object strongly
reachable until process exit. Since finalization stores weak references, GC
could collect the main-thread object before shutdown, causing the expected
shutdown on main thread output to be missing.

This keeps the registered reference reachable through the exit phase so the
test verifies per-thread finalization behavior without depending on GC timing.

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-05-15.md#jstest-failure


Assisted-by: openai:gpt-5.5

Keep the registered finalization object strongly reachable until
process exit so the test does not depend on GC timing.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 17, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (bc90667) to head (fc775f0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #63394   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files         714      714           
  Lines      225656   225704   +48     
  Branches    42696    42717   +21     
=======================================
+ Hits       203214   203267   +53     
- Misses      14220    14231   +11     
+ Partials     8222     8206   -16     

see 49 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.

@trivikr trivikr marked this pull request as draft May 18, 2026 01:44
@trivikr
Copy link
Copy Markdown
Member Author

trivikr commented May 18, 2026

Converted to draft, as the flaky test is likely fixed by #63209
It can be closed in future, if the test failures are not repeated.

The PRs for which the CI failed on https://github.com/nodejs/reliability/blob/main/reports/2026-05-15.md#jstest-failure did not contain the fix from #63209.

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 Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants