test: make waiter deadline regressions deterministic - #269
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes two cloud waiter deadline regression tests deterministic by removing real sleep calls and instead controlling the router’s registration instant in test builds. It adds a small cfg(test) seam to ResponseRouter while keeping the production registration path (Instant::now()) unchanged.
Changes:
- Refactor
ResponseRouter::registerto delegate to a sharedregister_at(...)implementation. - Add
ResponseRouter::register_at_for_test(...)(test-only) to allow deterministic waiter age modeling in unit tests. - Update two cloud integration unit tests to remove wall-clock sleeps and use explicit
registered_atinstants (expired vs live waiter).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/runtime/router.rs |
Introduces an internal register_at helper and a cfg(test) test seam for injecting registration instants without changing production behavior. |
src/runtime/event_loop/cloud_integration/tests.rs |
Replaces timing sleeps with deterministic registration instants to model expired/live waiters reliably. |
Suppressed comments (1)
src/runtime/event_loop/cloud_integration/tests.rs:6678
runtime_response_timeoutis increased from 200ms to 5s here as well. Even though the test no longer sleeps, a 5s timeout can significantly slow CI feedback if a regression causes the waiter routing to hang/timeout. Consider keeping a shorter timeout (consistent with the rest of this module) and adjusting the syntheticregistered_atoffsets accordingly.
el.runtime_response_timeout = Duration::from_secs(5);
append_cloud_async_put(&mut el)?;
let (first_segment, first_max_sequence) = seal_segment_without_remote_proof_for_test(&mut el)?;
append_cloud_async_put(&mut el)?;
let (second_segment, second_max_sequence) =
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace real sleeps in two cloud waiter deadline regressions with explicit registration instants. A test-only response-router seam preserves the production registration path while allowing the tests to represent one expired waiter and one live waiter deterministically.
Production routing, deadline, durability, and cloud behavior are unchanged.
Why are you making this contribution?
Final-main Platform CI run https://github.com/cntryl/midge/actions/runs/33530091023 passed Windows but failed macOS after scheduler overshoot caused both nominally live waiters to expire in two unit tests. The tests used a 200 ms runtime timeout plus 150 ms and 75 ms sleeps, leaving no scheduling margin on a loaded macOS runner.
Linked issues
No issue: this is a test-only follow-up to the red final-main macOS Platform attempt after PR #268.
Acceptance audit
Evidence: should_use_latest_surviving_waiter_deadline_given_older_waiter_already_expired and should_preserve_later_segment_waiter_when_earlier_gap_waiter_expired each passed 50 of 50 focused repetitions, and the full all-features workspace suite passed.
Production entry point: ResponseRouter::register still records Instant::now; the injected registration time is compiled only for tests and exercises the same pending-request state.
Resolution: Register the old waiters six seconds in the past and the live waiters 250 ms in the past against a five-second timeout.
Risk and compatibility
Low risk. The shipping registration method delegates to an unchanged private implementation using Instant::now. The only exposed seam is cfg(test), and no persistence format, public API, runtime protocol, or production deadline behavior changes.
Verification
Tool assistance disclosure
Codex inspected the hosted macOS failure, reproduced and stress-ran the affected tests, implemented the test-only timing seam, and ran the reported verification.
Contributor responsibility