Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
|
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 No outstanding decisions or follow-ups. Recommended next action: (Written by GPT-6) |
kixelated
left a comment
There was a problem hiding this comment.
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)
Review (head
|
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_hostandipv6_literal_fixed_addresseson the real clock, matching the other TLS authority tests. Keep their TLS name, HTTP Host, and fixed-address fallback assertions unchanged.Impact
Alternatives
An in-memory transport would bypass the real address dialing and TLS authority behavior these tests verify.
Validation
just rs test -p moq-tokio -E 'test(websocket::tests::)': all 8 WebSocket tests passed after the change.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)