Skip to content

test: add comprehensive e2e tests for interrupt/cancellation#107

Merged
emal-avala merged 1 commit intomainfrom
test/interrupt-cancellation-e2e
Apr 15, 2026
Merged

test: add comprehensive e2e tests for interrupt/cancellation#107
emal-avala merged 1 commit intomainfrom
test/interrupt-cancellation-e2e

Conversation

@emal-avala
Copy link
Copy Markdown
Member

Summary

Follow-up to #106. The initial e2e test only verified cancellation during a single turn with a hanging provider. This PR adds coverage for the specific regression scenarios the original fix was designed to prevent — most importantly the back-to-back-turns test that directly exercises the stale-token-clone bug.

New tests

  • cancel_works_across_multiple_turns — the actual cannot interrupt it #103 regression. Cancels turn 1, then runs turn 2 and verifies it's still cancellable via the same shared handle. Pre-fix this would hang forever on turn 2 because the signal handler held a stale clone.
  • cancel_does_not_poison_next_turn — pre-cancels the shared token, then runs a completing turn to verify run_turn_with_sink resets to a fresh un-cancelled token at turn start.
  • cancel_before_first_event_interrupts_cleanly — race-condition edge case where cancel fires during or before the first rx.recv().
  • cancelled_turn_emits_warning_to_sink — verifies the sink is notified via on_warning(\"Cancelled\"), confirming the UI feedback path works.

Refactoring

  • Hoisted the HangingProvider mock out of the test function so it can be shared
  • Added a CompletingProvider that emits text + Done normally
  • Added build_engine and schedule_cancel helpers to keep new scenarios concise

What's still not covered (out of scope here)

  • The actual spawn_escape_watcher thread reading real crossterm events (requires a pty-based test)
  • The install_signal_handler SIGINT path (would require sending real OS signals)
  • Terminal raw-mode cleanup on EscapeWatcherGuard::drop

These remain manual-test items in the README checklist for #106.

Test plan

  • All new tests pass locally / in CI
  • Refactored existing test still passes

Follow-up to #106. The initial e2e test only verified cancellation
during a single turn. This adds coverage for the specific regression
scenarios the original fix was designed to prevent.

New tests:
- cancel_works_across_multiple_turns: the actual #103 regression.
  Cancels turn 1, then runs turn 2 and verifies it's still cancellable
  via the same shared handle. Pre-fix this would hang forever because
  the signal handler held a stale token clone.
- cancel_does_not_poison_next_turn: pre-cancels the shared token, then
  runs a normal turn to verify run_turn_with_sink resets to a fresh
  un-cancelled token.
- cancel_before_first_event_interrupts_cleanly: race condition edge
  case where cancel fires during or before the first rx.recv().
- cancelled_turn_emits_warning_to_sink: verifies the sink is notified
  via on_warning("Cancelled"), confirming the UI feedback path works.

Refactors the existing test to share a build_engine/schedule_cancel
helper so new scenarios stay concise.
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@emal-avala emal-avala added the run-e2e Trigger E2E test suite on this PR label Apr 15, 2026
@emal-avala emal-avala merged commit 6fd51e2 into main Apr 15, 2026
13 of 16 checks passed
@emal-avala emal-avala deleted the test/interrupt-cancellation-e2e branch April 15, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-e2e Trigger E2E test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant