test: deflake two timing-dependent tests by erskingardner · Pull Request #235 · marmot-protocol/transponder

erskingardner · 2026-07-05T14:17:37Z

Fixes the two pre-existing intermittent flakes surfaced during the root-cause refactor program (both pass in isolation, fail occasionally under full-suite concurrency / coverage instrumentation).

shutdown::wait_for_signal_or_trigger_returns_on_internal_trigger relied on two real wall-clock timeouts — a 10ms window to prove the wait was still pending, and a 1s window for it to resolve after the trigger (exceedable when a starved task isn't polled within 1s under load). Now: a single deterministic poll with a no-op waker proves Pending (the first poll subscribes the receiver and registers the signal handlers, so with neither fired the future is provably pending), and the triggered future is awaited directly. No wall-clock remains.

events::test_process_rejects_future_notification_rumor dated the event only +1s beyond the future-skew tolerance. validate_notification_freshness re-reads Timestamp::now() (1-second granularity) at process time, so a whole-second tick between building the event and that read collapsed the +1 margin onto the exact threshold (> became ==) and spuriously accepted the event. Widened to +1h; there's no injectable clock at the check site, so a drift-proof margin is the deterministic fix without a production API change. No assertion weakened — it still exercises the created_at > now + skew rejection path.

Verified: 20/20 each under cargo llvm-cov (the instrumentation that surfaced the flakes), full suite 647 pass, clippy -D warnings clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests
- Improved shutdown and notification tests to be more deterministic and less flaky.
- Strengthened future-dated notification coverage by using a larger time offset, avoiding boundary timing issues.
- Reworked a shutdown assertion to check for pending state directly before confirming the internal trigger completes the operation.

Both tests intermittently failed under full-suite concurrency / coverage instrumentation and passed in isolation. shutdown::wait_for_signal_or_trigger_returns_on_internal_trigger relied on two real wall-clock timeouts: a 10ms window to prove the wait was still pending, and a 1s window for it to resolve after the trigger. Under load the 1s bound could be exceeded by a starved task. Replace the 10ms check with a single deterministic poll (no-op waker) — the first poll subscribes the receiver and registers the signal handlers, so with neither fired the future is provably Pending — and await the triggered future directly instead of racing a timeout. events::test_process_rejects_future_notification_rumor dated the event only +1s beyond the future-skew tolerance. process() re-reads Timestamp::now() (1-second granularity) when validating freshness, so a whole-second tick between building the event and that read collapsed the margin onto the exact threshold (> became ==) and spuriously accepted the event. Widen the margin to +1h; there is no injectable clock at the check site, so a drift-proof margin is the deterministic fix without a production API change. Verified 20/20 each under cargo llvm-cov; full suite 647 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cursor · 2026-07-05T14:17:41Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

stage-review · 2026-07-05T14:17:49Z

Ready to review this PR? Stage has broken it down into 3 individual chapters for you:

	Title
1	Deflake shutdown trigger test with deterministic polling
2	Widen future-skew margin in notification tests
3	Verify operator-facing shutdown signal labels

_{Chapters generated by Stage for commit d252e27 on Jul 5, 2026 2:31pm UTC.}

coderabbitai · 2026-07-05T14:17:59Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 46 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 072c35b2-279c-4327-97bc-90f6162c36fd

📥 Commits

Reviewing files that changed from the base of the PR and between 8264a3e and d252e27.

📒 Files selected for processing (1)

src/shutdown.rs

Walkthrough

Two unit tests are modified to reduce timing-based flakiness: one widens the clock-skew offset used in a future-notification-rumor test, and the other replaces a wall-clock timeout assertion with deterministic polling using a noop waker.

Changes

Deterministic Test Timing Fixes

Layer / File(s)	Summary
Widen clock-skew margin in notification test `src/nostr/events.rs`	The future `created_at` offset is increased from skew+1 second to skew+3600 seconds, with comments explaining avoidance of second-granularity boundary flakiness.
Deterministic polling for shutdown trigger test `src/shutdown.rs`	The pending-state check is replaced with a single-step `poll` using `Context::from_waker(Waker::noop())` instead of a `timeout`-based wall-clock check, while the rest of the test flow is unchanged.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Related Issues: None specified

Related PRs: None specified

Suggested labels: tests, flaky-test-fix

Suggested reviewers: None specified

Poem:
A rabbit hopped through clock and thread,
No more flaky timeouts to dread,
With noop wakers polling true,
And skew margins wide and new,
Tests now sleep soundly in their bed. 🐰⏰

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: deflaking two timing-dependent tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/deflake-timing-tests

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-05T14:19:23Z

✅ Code Coverage Report

Metric	Value
Line Coverage	96.90%
Lines Covered	17054 / 17599
Change from master	+0.03% ✅

View detailed coverage

Download the coverage-html artifact from this workflow run to view the detailed coverage report.

Or run locally:

cargo llvm-cov --html
open target/llvm-cov/html/index.html

The matches!(poll, Poll::Pending) form introduced an implicit non-matching wildcard arm that the always-Pending poll never exercises, a new uncovered region that tripped the fail-on-any-decrease coverage gate by ~0.0002pp. Use Poll::is_pending() (same assert-on-bool shape as the wall-clock assertion it replaced), which has no uncovered arm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cursor · 2026-07-05T14:24:07Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

name() renders the operator-facing signal label in shutdown logs but was entirely uncovered — it is only invoked inside warn!/info! field expressions, which tracing skips when tests run without a subscriber. A direct unit test covers both match arms, and offsets the coverage the deterministic-poll rewrite removed with the wall-clock assertions (the fail-on-any-decrease gate tripped on the net removal of covered lines, not on the logic change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

cursor · 2026-07-05T14:31:07Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

erskingardner merged commit 6428fef into master Jul 5, 2026
10 checks passed

erskingardner deleted the fix/deflake-timing-tests branch July 5, 2026 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: deflake two timing-dependent tests#235

test: deflake two timing-dependent tests#235
erskingardner merged 3 commits into
masterfrom
fix/deflake-timing-tests

erskingardner commented Jul 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

cursor Bot commented Jul 5, 2026

Uh oh!

stage-review Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 5, 2026 •

edited

Loading

Review limit reached

Uh oh!

github-actions Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

cursor Bot commented Jul 5, 2026

Uh oh!

cursor Bot commented Jul 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

erskingardner commented Jul 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

cursor Bot commented Jul 5, 2026

Uh oh!

stage-review Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Code Coverage Report

Uh oh!

cursor Bot commented Jul 5, 2026

Uh oh!

cursor Bot commented Jul 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

erskingardner commented Jul 5, 2026 •

edited by coderabbitai Bot

Loading

stage-review Bot commented Jul 5, 2026 •

edited

Loading

coderabbitai Bot commented Jul 5, 2026 •

edited

Loading

github-actions Bot commented Jul 5, 2026 •

edited

Loading