fix(uniflight): eliminate race condition in panic propagation tests#342
fix(uniflight): eliminate race condition in panic propagation tests#342kate-shine merged 2 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 210 210
Lines 15540 15540
=======================================
Hits 15540 15540 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR makes the uniflight panic-propagation integration tests deterministic by removing sleep-based synchronization and relying on Merger::execute()’s eager cell registration to ensure the follower joins the leader consistently.
Changes:
- Replaces
tokio::spawn+sleep(10ms)coordination with sequential creation ofexecute()futures. - Drives leader and follower concurrently via
tokio::join!to eliminate timing dependencies and flakiness on single-threaded runtimes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kate-shine
force-pushed
the
kchuranov/fix-uniflight-flaky-panic-test
branch
from
e3e2d54 to
bbc0db4
Compare
kate-shine
force-pushed
the
kchuranov/fix-uniflight-flaky-panic-test
branch
from
bbc0db4 to
e735b6e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kate-shine
force-pushed
the
kchuranov/fix-uniflight-flaky-panic-test
branch
from
e735b6e to
cdbb710
Compare
kate-shine
force-pushed
the
kchuranov/fix-uniflight-flaky-panic-test
branch
from
cdbb710 to
9e51e20
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace tokio::spawn + sleep-based synchronization with a Notify handshake protocol that deterministically coordinates leader and follower tasks. The original tests relied on sleep(10ms) to ensure the follower task was polled before the leader completed. In tokio's single-threaded runtime, the leader could finish and clean up the DashMap entry before the follower task was ever polled, causing the follower to become a new leader and return Ok instead of Err(LeaderPanicked). The fix uses two Notify signals to enforce strict ordering: 1. Leader signals after calling execute() (cell registered) 2. Follower signals after calling execute() (cell joined) 3. Leader waits for follower's signal before panicking This guarantees the follower is actively waiting on the leader's cell when the panic occurs, regardless of whether execute() registers cells eagerly or lazily. Also simplifies panic expressions by using panic!() as the tail expression (returns !) instead of dead code with #[expect(unreachable_code)]. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kate-shine
force-pushed
the
kchuranov/fix-uniflight-flaky-panic-test
branch
from
9e51e20 to
e8f297f
Compare
Replace tokio::spawn + sleep-based synchronization with eager cell registration via sequential execute() calls and tokio::join!.
The original tests relied on sleep(10ms) to ensure the follower task was polled before the leader completed. In tokio's single-threaded runtime, the leader could finish and clean up the DashMap entry before the follower task was ever polled, causing the follower to become a new leader and return Ok instead of Err(LeaderPanicked).
The fix exploits the fact that execute() eagerly calls get_or_create_cell() before returning the future. Creating both futures sequentially guarantees the follower joins the leader's cell deterministically, with no timing dependency.