From e1caf43fc01c3536699ca649cb8829126aba9a02 Mon Sep 17 00:00:00 2001 From: Jeff Gardner <202880+erskingardner@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:56:22 +0100 Subject: [PATCH 1/2] test: deflake retry jitter test under coverage instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_with_retry_repeated_retry_after_zero_sleeps_before_each_retry advanced the paused clock and then relied on a single yield_now for the spawned retry task to cross the fired-timer boundary before asserting the attempt count. Under llvm-cov's slower, perturbed scheduling one yield is not always enough, so the count lagged (left: 1, right: 2) and the Coverage CI job flaked. Replace the single yield before each positive count assertion with a bounded wait_for_attempt_count() poll loop that yields until the expected count is observed (capped to avoid hangs). The negative 'must not retry before the floor' assertions keep the single yield — the 100ms floor makes early firing impossible, so they are deterministic. Timing behavior verified unchanged: 25/25 passes under cargo llvm-cov. Co-Authored-By: Claude Fable 5 --- src/push/retry.rs | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/push/retry.rs b/src/push/retry.rs index 72ee13e..95c95d9 100644 --- a/src/push/retry.rs +++ b/src/push/retry.rs @@ -750,6 +750,31 @@ mod tests { assert!(elapsed < Duration::from_secs(1)); } + /// Wait until `count` reaches at least `expected`, yielding to the scheduler + /// in a bounded loop. + /// + /// A paused-clock test can require more than one scheduler pass for a + /// spawned task to cross a fired timer boundary and run its next attempt — + /// especially under coverage instrumentation, where scheduling is slower and + /// perturbed — so a single `yield_now` is not a reliable synchronization + /// point. The retry task runs from "sleep fires" through incrementing the + /// counter to re-parking on the next sleep in a single poll (no intermediate + /// await), so observing the incremented count does not race ahead of the + /// task re-parking. The iteration cap prevents a hang if progress never + /// happens. + async fn wait_for_attempt_count(count: &AtomicU32, expected: u32) { + for _ in 0..10_000 { + if count.load(Ordering::SeqCst) >= expected { + return; + } + tokio::task::yield_now().await; + } + panic!( + "timed out waiting for attempt_count to reach {expected}; last observed {}", + count.load(Ordering::SeqCst) + ); + } + #[tokio::test] async fn test_with_retry_repeated_retry_after_zero_sleeps_before_each_retry() { tokio::time::pause(); @@ -785,7 +810,7 @@ mod tests { .await }); - tokio::task::yield_now().await; + wait_for_attempt_count(&attempt_count, 1).await; assert_eq!(attempt_count.load(Ordering::SeqCst), 1); assert!(!task.is_finished()); @@ -796,7 +821,7 @@ mod tests { assert_eq!(attempt_count.load(Ordering::SeqCst), 1); tokio::time::advance(Duration::from_millis(51)).await; - tokio::task::yield_now().await; + wait_for_attempt_count(&attempt_count, 2).await; assert_eq!(attempt_count.load(Ordering::SeqCst), 2); // The fallback backoff advances even while Retry-After is supplied, so @@ -806,7 +831,7 @@ mod tests { assert_eq!(attempt_count.load(Ordering::SeqCst), 2); tokio::time::advance(Duration::from_millis(101)).await; - tokio::task::yield_now().await; + wait_for_attempt_count(&attempt_count, 3).await; assert_eq!(attempt_count.load(Ordering::SeqCst), 3); // The third zero-header retry uses the next 400ms fallback slot. From 9928c79c14b569a3d51d42fd6b8ec525c0cf8143 Mon Sep 17 00:00:00 2001 From: Jeff Gardner <202880+erskingardner@users.noreply.github.com> Date: Sun, 5 Jul 2026 14:02:53 +0100 Subject: [PATCH 2/2] test: cover the bounded-wait helper timeout path The wait_for_attempt_count helper's iteration-cap panic branch was never exercised by passing tests, leaving one uncovered line that tripped the fail-on-any-decrease coverage gate. Add a should_panic test that drives the helper with a count that never arrives, covering the timeout diagnostic. Co-Authored-By: Claude Fable 5 --- src/push/retry.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/push/retry.rs b/src/push/retry.rs index 95c95d9..b2bcd49 100644 --- a/src/push/retry.rs +++ b/src/push/retry.rs @@ -846,6 +846,16 @@ mod tests { assert_eq!(attempt_count.load(Ordering::SeqCst), 4); } + #[tokio::test] + #[should_panic(expected = "timed out waiting for attempt_count")] + async fn wait_for_attempt_count_panics_when_progress_never_happens() { + // Guard the bounded-wait helper's timeout path: if the awaited count is + // never reached (nothing increments it here), the helper must not spin + // forever — it exhausts its iteration cap and panics with a diagnostic. + let count = AtomicU32::new(0); + wait_for_attempt_count(&count, 1).await; + } + #[tokio::test] async fn test_with_retry_success_false() { let config = RetryConfig::default();