Skip to content

fix: stabilize idle timeout sender tests#172

Merged
BunsDev merged 2 commits into
mainfrom
fix/castcodes-idle-timeout-test
Jun 28, 2026
Merged

fix: stabilize idle timeout sender tests#172
BunsDev merged 2 commits into
mainfrom
fix/castcodes-idle-timeout-test

Conversation

@BunsDev

@BunsDev BunsDev commented Jun 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Stabilize IdleTimeoutSender tests by polling for delayed oneshot delivery instead of relying on a fixed post-timeout sleep.
  • Keep the existing immediate None assertion so the tests still verify that delayed sends do not complete before the timeout.
  • Leave production sender behavior unchanged.

Root Cause

The macOS 26 scheduled CI runner failed idle_timeout_sender_later_send_after_supersedes_earlier because the test slept for 100ms after scheduling a 50ms timer and assumed the spawned timer thread had also been scheduled and delivered by then. Under CI load, the receiver was still empty, producing left: None, right: Some(2).

Verification

  • cargo nextest run -p warp-app idle_timeout_sender --features gui --profile default
  • cargo fmt --check
  • ./script/check_ai_attribution
  • ./script/check_rebrand
  • cargo check -p warp-app --bin cast-codes --features gui

@BunsDev BunsDev marked this pull request as ready for review June 28, 2026 00:02
Copilot AI review requested due to automatic review settings June 28, 2026 00:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Stabilizes the IdleTimeoutSender unit tests by replacing a fixed post-timeout sleep with a polling helper that waits up to a bounded duration for oneshot delivery, reducing CI scheduling flakiness without changing production sender behavior.

Changes:

  • Add a recv_within helper that repeatedly try_recvs until a timeout.
  • Replace sleep + try_recv assertions with recv_within(..., 1s) in the delayed-delivery and “later send supersedes earlier” tests.
  • Keep the existing immediate None assertion to ensure delayed sends don’t complete before the configured timeout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CI's `clippy::disallowed_types` lint forbids `std::time::Instant`
(it isn't implemented for wasm targets) and requires `instant::Instant`,
which the rest of the crate already uses. Swap the import; `Duration`
stays from std (it is allowed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@BunsDev

BunsDev commented Jun 28, 2026

Copy link
Copy Markdown
Member Author

Pushed 3925c9c to fix the CI blocker: the new tests imported std::time::Instant, which this repo's clippy::disallowed_types lint forbids (it's not implemented for wasm targets). Swapped to instant::Instant — the same type the rest of the crate already uses — keeping Duration from std.

Heads up on the prior Run Windows tests failure: the only failing test was terminal::local_tty::windows::child::tests::test_event_is_emitted_when_child_exits, a Windows-specific terminal child-exit test unrelated to this PR (which only touches app/src/ai/agent_sdk/driver_tests.rs). It looks like an environmental/flaky failure and should clear on re-run.

@BunsDev BunsDev merged commit b0695a3 into main Jun 28, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants