Skip to content

test: make failed-opening-handshake websocket test deterministic#5233

Closed
mcollina wants to merge 1 commit intomainfrom
fix-5223-websocket-flaky
Closed

test: make failed-opening-handshake websocket test deterministic#5233
mcollina wants to merge 1 commit intomainfrom
fix-5223-websocket-flaky

Conversation

@mcollina
Copy link
Copy Markdown
Member

@mcollina mcollina commented May 6, 2026

Summary

Fixes a flaky websocket test by replacing an environment-dependent failed connection with a deterministic failed opening handshake against a local HTTP server.

Changes

  • rename test/websocket/issue-3697-2399493917.js to test/websocket/failed-opening-handshake-ready-state.js
  • replace wss://localhost with a local 127.0.0.1 HTTP server that responds with 404
  • assert that a failed opening handshake still transitions the WebSocket to CLOSED

Fixes #5223

Testing

  • node --test test/websocket/failed-opening-handshake-ready-state.js
  • npx borp --timeout 180000 -p "test/websocket/failed-opening-handshake-ready-state.js"
  • npm run test:websocket

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.27%. Comparing base (e58fff7) to head (026f03f).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5233   +/-   ##
=======================================
  Coverage   93.27%   93.27%           
=======================================
  Files         110      110           
  Lines       36353    36353           
=======================================
  Hits        33909    33909           
  Misses       2444     2444           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

Copy link
Copy Markdown
Member

@KhafraDev KhafraDev left a comment

Choose a reason for hiding this comment

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

The tests seem to be fundamentally different. In the new test, a connection was opened, where the previous test's purpose was to test before we have a connection.

@mcollina mcollina closed this May 6, 2026
@mcollina
Copy link
Copy Markdown
Member Author

mcollina commented May 6, 2026

The tests seem to be fundamentally different. In the new test, a connection was opened, where the previous test's purpose was to test before we have a connection.

I'm not sure how to make that test not flaky.

@KhafraDev
Copy link
Copy Markdown
Member

I have no idea either :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[flaky] test\websocket\issue-3697-2399493917.js

3 participants