Skip to content

test(redir): wait for relay handlers before tests reuse shared config - #1332

Merged
rsdmike merged 2 commits into
mainfrom
test/relay-handler-race
Oct 16, 2026
Merged

rsdmike merged 2 commits into
mainfrom
test/relay-handler-race

Conversation

@rsdmike

@rsdmike rsdmike commented Oct 16, 2026

Copy link
Copy Markdown
Member

TestWebSocketHandlerRealUpgrader (added in #1236) returns while hijacked relay handlers are still reading config.ConsoleConfig at redirect.go:75. The next test, TestWebSocketHandlerTokenValidation, calls config.NewConfig() and the race detector fails the run. This has failed main CI intermittently since 2026-09-25.

httptest.Server.Close does not wait for hijacked connections, so the test now wraps the router to track in-flight handlers and waits for them in cleanup. Test-only change.

@rsdmike
rsdmike requested a review from a team as a code owner October 16, 2026 22:02
Copilot AI balanced review requested due to automatic review settings October 16, 2026 22:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused test-only change correctly waits for relay handlers before shared configuration is reused.

No open issues 🎉

What changed in this PR

Fixes an intermittent test race by waiting for relay handlers to finish before subsequent tests reuse shared configuration.

Changes:

  • Tracks active HTTP handlers with a wait group.
  • Waits for handler completion during test cleanup; production code is unchanged.
File Description
internal/​controller/​ws/​v1/​redirect_test.go Adds handler tracking and cleanup synchronization to the real-upgrader test.

Review effort: Balanced

TestWebSocketHandlerRealUpgrader returned while hijacked relay
handlers were still reading config.ConsoleConfig, racing
TestWebSocketHandlerTokenValidation's config.NewConfig() under -race.

httptest.Server.Close does not wait for hijacked connections, so the
test now tracks in-flight handlers and waits for them in cleanup.
@rsdmike
rsdmike force-pushed the test/relay-handler-race branch from c750634 to 480d3d9 Compare October 16, 2026 22:03
@rsdmike
rsdmike enabled auto-merge (squash) October 16, 2026 22:04
@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1332   +/-   ##
=======================================
  Coverage   62.24%   62.24%           
=======================================
  Files         159      159           
  Lines       13417    13417           
=======================================
  Hits         8351     8351           
  Misses       5066     5066           

☔ View full report in Codecov by Harness.
📢 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.

@rsdmike
rsdmike merged commit 62f4fba into main Oct 16, 2026
22 checks passed
@rsdmike
rsdmike deleted the test/relay-handler-race branch October 16, 2026 22:16
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.

3 participants