test: avoid truncating watched worker deps by inoway46 · Pull Request #63083 · nodejs/node · GitHub
Skip to content

test: avoid truncating watched worker deps#63083

Closed
inoway46 wants to merge 1 commit into
nodejs:mainfrom
inoway46:fix-test-watch-mode-worker
Closed

test: avoid truncating watched worker deps#63083
inoway46 wants to merge 1 commit into
nodejs:mainfrom
inoway46:fix-test-watch-mode-worker

Conversation

@inoway46

@inoway46 inoway46 commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This fixes a flaky sequential/test-watch-mode-worker failure where the nested CJS worker dependency case can log {} instead of sub-dep v1.

The test rewrites watched worker dependency files to trigger --watch restarts, but default writeFileSync() temporarily truncates the file. A restarted worker can read the dependency during that empty-file window.

This change keeps the write-based restart trigger, but uses r+ to overwrite without truncation.

Reported in: https://github.com/nodejs/reliability/blob/main/reports/2026-05-02.md

not ok 5323 sequential/test-watch-mode-worker
  ---
  Test failure: 'should watch changes to nested worker dependencies - cjs'
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
    [
      'sub-dep v1',
      "Completed running '/home/iojs/node-tmp/.tmp.5322/2.js'. Waiting for file changes before restarting...",
      "Restarting '/home/iojs/node-tmp/.tmp.5322/2.js'",
  +   '{}',
  -   'sub-dep v1',
      "Completed running '/home/iojs/node-tmp/.tmp.5322/2.js'. Waiting for file changes before restarting..."
    ]

Testing:

$ ./tools/test.py sequential/test-watch-mode-worker

$ ./tools/test.py -j1 --repeat 20 sequential/test-watch-mode-worker

Refs: #62368

@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 2, 2026
@inoway46 inoway46 force-pushed the fix-test-watch-mode-worker branch from b27b615 to da136db Compare May 2, 2026 14:44
@inoway46 inoway46 changed the title test: fix flaky test-watch-mode-worker test: avoid truncating watched worker deps May 2, 2026
@inoway46
test: avoid truncating watched worker deps
cfcd3e7 
Signed-off-by: inoway46 <inoueyuya416@gmail.com>
@inoway46 inoway46 force-pushed the fix-test-watch-mode-worker branch from da136db to cfcd3e7 Compare May 2, 2026 14:54
@inoway46 inoway46 marked this pull request as ready for review May 2, 2026 14:57
@codecov

codecov Bot commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown

@GrinZero

GrinZero commented May 9, 2026

Copy link
Copy Markdown
Contributor

LGTM

@inoway46

inoway46 commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

A newer reliability failure shows a different failure log for the same test:
https://github.com/nodejs/reliability/blob/main/reports/2026-05-11.md

not ok 5567 sequential/test-watch-mode-worker
  ---
  Test failure: 'should watch changes to worker - cjs'
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
    [
      'worker running',
      "Completed running '/home/iojs/node-tmp/.tmp.5566/0.js'. Waiting for file changes before restarting...",
      "Restarting '/home/iojs/node-tmp/.tmp.5566/0.js'",
  -   'worker running',
      "Completed running '/home/iojs/node-tmp/.tmp.5566/0.js'. Waiting for file changes before restarting..."
    ]

This is consistent with the same truncate-window race: the watched worker entry file can be observed while temporarily truncated, so the restarted worker exits without printing worker running.

@inoway46

inoway46 commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Could someone please request CI?

@inoway46

inoway46 commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

@MoLow
Sorry for the mention. This flake still appears several times, so could you please request a full CI?
Ref: nodejs/reliability#1542

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2026
@nodejs-github-bot

nodejs-github-bot commented May 16, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73498/

@trivikr

trivikr commented May 18, 2026

Copy link
Copy Markdown
Member

I'd independently worked on an alternative which uses appendFileSync in #63384.
Commenting here, if you want to take a look. I discovered this PR while browsing.

@inoway46

inoway46 commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@inoway46 inoway46 closed this May 21, 2026
@inoway46 inoway46 deleted the fix-test-watch-mode-worker branch May 21, 2026 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MoLow MoLow MoLow approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@inoway46 @GrinZero @nodejs-github-bot @trivikr @MoLow @lpinca