test(tui): fix decision-toast notification race on py3.14#14
Conversation
`test_decision_banner_shows_and_clears` asserted the decision toast was present synchronously, right after waiting for `decision_pending`. But the toast is emitted via `self.notify()`, which posts a message onto textual's async pump — it only lands in `app._notifications` a tick after `_decision` is set in the same `_apply` call. The bare assert could therefore run before the pump processed the Notify message; under CI's parallel IO contention this flaked on py3.14 (3.11–3.13 happened to win the race). Wait for the toast via `until()` instead, matching every other notification test in this file (e.g. `test_attach_without_tmux_notifies`). Pure test-timing fix — no product change. Verified green on py3.13 and py3.14. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe TUI test now waits for the decision-question notification to appear before asserting banner behavior, using an async loop around the notification check. ChangesTUI decision banner test timing
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
The post-merge CI run for #13 failed a single job —
test (py3.14)— ontests/test_tui_app.py::test_decision_banner_shows_and_clears. 3.11–3.13 passed. The failure is a test-timing race, not a product bug.Root cause
The test waits for
screen.decision_pending, then synchronously asserts the decision toast is inapp._notifications:But the toast is emitted with
self.notify(), which posts a message onto textual's async pump.decision_pending(_decision) is set synchronously in_apply, while the notification only lands inapp._notificationsa tick later when the pump processes theNotifymessage. So the bareassertcan run before the notification is observable. Under CI's parallel IO contention the py3.14 runner lost that race; the faster interpreters happened to win it.Fix
Wait for the toast via
until()instead of asserting synchronously — the exact pattern every other notification test in this file already uses (e.g.test_attach_without_tmux_notifies). One line, test-only, no product change.Verification
Ran the full
tests/test_tui_app.pymodule on both py3.13 and py3.14 (CI-matchinguv sync --locked --all-extras): 39 passed on each.🤖 Generated with Claude Code
Summary by CodeRabbit