Skip to content

Comments

test: avoid flaky debugger restart waits#61773

Open
inoway46 wants to merge 1 commit intonodejs:mainfrom
inoway46:debugger-flaky-61762
Open

test: avoid flaky debugger restart waits#61773
inoway46 wants to merge 1 commit intonodejs:mainfrom
inoway46:debugger-flaky-61762

Conversation

@inoway46
Copy link
Contributor

@inoway46 inoway46 commented Feb 11, 2026

Summary

  • replace stepCommand('restart') with command('restart') in:
    • test/parallel/test-debugger-restart-message.js
    • test/parallel/test-debugger-run-after-quit-restart.js
  • in test-debugger-restart-message, wait for Debugger attached. and prompt before asserting the listening message count
  • in test-debugger-run-after-quit-restart, continue to assert behavior via waitForInitialBreak() and breakInfo

Context

test-macOS has shown intermittent timeouts while waiting for BREAK_MESSAGE in restart-related debugger tests.
In failing logs, reconnect succeeds (Debugger attached. is printed), but the expected break ... in line is not observed before timeout.

Hypothesis

The flake is likely caused by a timing race in CLI output ordering around restart/reconnect:

  • stepCommand() requires BREAK_MESSAGE to appear
  • under some timing conditions (seen on macOS unusual-path runs), attach/prompt output arrives, but break ... in output is delayed or missed by this wait pattern

This change does not claim to prove a single root cause. It may be related to timing behavior in test/common/debugger.js, but this PR only narrows these two tests to a less fragile synchronization path aligned with what each test is actually verifying.

Why this approach

  • keep test intent intact
  • reduce dependence on a brittle BREAK_MESSAGE wait specifically for restart
  • minimize blast radius by changing only the affected tests

Testing

  • out/Release/node test/parallel/test-debugger-restart-message.js (passes locally)
  • out/Release/node test/parallel/test-debugger-run-after-quit-restart.js (passes locally)
  • Additional local stress runs were executed from an unusual-character path on both this PR branch and main. (test: avoid flaky debugger restart waits #61773 (comment))
  • CI passes on macOS

Refs: #61762

@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 Feb 11, 2026
@inoway46 inoway46 force-pushed the debugger-flaky-61762 branch 3 times, most recently from 6dd9cf9 to c3b82fc Compare February 11, 2026 09:58
@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.77%. Comparing base (5babc8d) to head (e6adfbe).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61773      +/-   ##
==========================================
- Coverage   89.78%   89.77%   -0.01%     
==========================================
  Files         674      674              
  Lines      204957   204957              
  Branches    39391    39398       +7     
==========================================
- Hits       184011   184006       -5     
- Misses      13214    13221       +7     
+ Partials     7732     7730       -2     

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

@inoway46 inoway46 force-pushed the debugger-flaky-61762 branch 2 times, most recently from 87a24df to 58aeec7 Compare February 12, 2026 01:48
@inoway46
Copy link
Contributor Author

Quick update: this flake reproduced again in Test macOS (run https://github.com/nodejs/node/actions/runs/22255995247), with the same timeout path in test-debugger-restart-message.js (stepCommand waiting for BREAK_MESSAGE after restart).
This PR is intended as a targeted mitigation for that exact failure.
Happy to split/follow up if you’d prefer a broader fix in test/common/debugger.js.

@inoway46 inoway46 force-pushed the debugger-flaky-61762 branch from 58aeec7 to e6adfbe Compare February 22, 2026 04:40
@inoway46
Copy link
Contributor Author

Quick update on local verification (macOS 15.7.4, Apple M2 Pro).

I ran stress checks from an unusual-character path to match the CI scenario:

UNUSUAL="$HOME/dir with \$unusual\"chars?'åß∂ƒ©∆¬…\`"
mkdir -p "$UNUSUAL"
ln -sfn "$PWD" "$UNUSUAL/node"
cd "$UNUSUAL/node"

./tools/test.py -p actions -j1 --repeat 50 \
  parallel/test-debugger-restart-message \
  parallel/test-debugger-run-after-quit-restart

./tools/test.py -p actions -j4 --repeat 100 \
  parallel/test-debugger-restart-message \
  parallel/test-debugger-run-after-quit-restart

Results:

  • This PR branch: no local failures in the runs above.
  • main: no local failures in the same runs.

Since I could not reproduce this locally, I cannot claim high confidence that this change fixes the flake; however, if possible, running CI multiple times on this diff should help validate whether it reduces recurrence.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2026
@nodejs-github-bot
Copy link
Collaborator

@inoway46
Copy link
Contributor Author

node-test-linux-alpine-latest-x64 failed during checkout (before running tests):
fatal: update_ref failed for ref 'HEAD': cannot lock ref 'HEAD': Unable to create '/home/iojs/build/workspace/node/.git/HEAD.lock': File exists.

Looks like a CI workspace issue unrelated to this PR.
Could someone re-run the failed job (or the workflow)? Thanks!

https://ci.nodejs.org/job/node-test-commit-linux/nodes=alpine-latest-x64/68594/console

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.

3 participants