Skip to content

test(tokio): run fixed-address WebSocket TLS dials on real time - #4722

Open
kixelated wants to merge 2 commits into
quest/m1/test-flakes-2/READMEfrom
quest/m1/test-flakes-2/websocket-paused-tls
Open

kixelated wants to merge 2 commits into
quest/m1/test-flakes-2/READMEfrom
quest/m1/test-flakes-2/websocket-paused-tls

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

The fixed-address WebSocket tests pause Tokio's clock while dialing TLS over real loopback sockets. Virtual time can advance before the OS delivers socket events, making any timeout on that path race the handshake.

Approach

Run fixed_addresses_keep_tls_name_and_request_host and ipv6_literal_fixed_addresses on the real clock, matching the other TLS authority tests. Keep their TLS name, HTTP Host, and fixed-address fallback assertions unchanged.

Impact

  • Public API: none.
  • Wire: none.

Alternatives

An in-memory transport would bypass the real address dialing and TLS authority behavior these tests verify.

Validation

  • Focused baseline: both named tests passed before the change.
  • Nix just rs test -p moq-tokio -E 'test(websocket::tests::)': all 8 WebSocket tests passed after the change.
  • Nix just check: passed, including 3,153 default, 387 capture, and 132 playback tests, feature checks, and lints.
  • quest check: 431 documents valid.

Follow-ups

None.

(Written by GPT-6)

kixelated and others added 2 commits October 2, 2026 07:57
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Completed the quest: the two fixed-address WebSocket TLS tests run on the real clock, matching the neighboring TLS tests. All eight WebSocket tests and the required Nix just check passed. Public API and wire impact: none.

No outstanding decisions or follow-ups. Recommended next action: /quest-merge after review and CI. Leaving this PR a draft as requested for this batch.

(Written by GPT-6)

@kixelated
kixelated marked this pull request as ready for review October 2, 2026 15:35

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit 9631e3247fd80740ceb20d27da012557bafa295b.

No actionable bugs found. Removing the two clock pauses in rs/moq-tokio/src/websocket.rs:720-733 is the right, minimal change for tests that perform real loopback TCP/TLS I/O. The shared helper still checks the certificate/SNI and HTTP Host, and its silent first fixed address still exercises fallback to the completing TLS handshake (websocket.rs:808-822). The tests contain no virtual-time assertions; stagger timing remains covered by the separate paused-clock tests in failover.rs:257-273. No public API or wire change.

Verification: source review of the tests, shared helper, dial/failover paths and test configuration only. Rust/Nix tooling is unavailable here, so I did not run the tests or independently reproduce the author's local results. GitHub currently reports the Check and Platform workflows successful for this commit.

(Written by OpenAI Codex)

Copy link
Copy Markdown
Collaborator Author

Review (head 9631e3247fd80740ceb20d27da012557bafa295b)

MERGE

Removes tokio::time::pause() from fixed_addresses_keep_tls_name_and_request_host and ipv6_literal_fixed_addresses, so those fixed-address WebSocket TLS dials run on real time like the neighboring check_tls_authority tests. Quest doc + README entry for the flake are completed/removed.

Findings

No concrete bugs or regressions found.

  • check_tls_authority does a real loopback TCP + TLS + WebSocket handshake (plus a silent pinned peer for the fixed-address race). Pausing Tokio’s clock over that path is the same class of flake test(auth): run the outage tests on a paused clock without sockets #4527 removed elsewhere; dropping pause is the right fix and matches tls_host_name_override_dials_url_address / ipv6_literal / ipv6_tls_host_name_override, which never paused.
  • Assertions (SNI / HTTP Host / fixed-address fallback) are unchanged. No production code changes.
  • After the change there is no remaining tokio::time::pause() in this file. Tests will pay the real DEFAULT_DELAY (200ms) sleep in connect; the sibling TLS tests already do that.
  • CI green on this head (Check, Test, Quest, Windows, macOS).

This is an automated review, not the maintainer's decision
(Written by Grok)

This branch has not been deployed

No deployments
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.

1 participant