test: extract shared _build_overlay_call_order_fixtures helper in test_actions.py - #311
Conversation
…t_actions.py Six overlay-preview tests each hand-rolled the same four-statement arrange block: a call-order list, an overlay-enabled FakePage recording into it, a bare MagicMock overlay double, and the overlay-wired executor/viewport pair from _build_overlay_action_fixtures. They differ only in which overlay coroutines and page mouse methods they wire up afterwards. Extract the common part into _build_overlay_call_order_fixtures and route all six sites through it, including _build_move_down_up_overlay_fixtures (which previously duplicated it inline) and the _preview_types inner helper (which additionally re-implemented _build_overlay_action_fixtures by hand). Test-only, no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W5i16kvsHTAB61hk5Ehqy1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesThe test suite centralizes overlay call-order fixture construction in Overlay test fixture consolidation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This test-only refactor centralizes duplicated fixture setup while preserving the existing test behavior and assertions. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Issue
Six overlay-preview tests in
tests/test_actions.pyeach hand-rolled the same arrange block before wiring their own overlay behaviour:The sites are
_build_move_down_up_overlay_fixturesand theleft_click,goto_url,key_press/F5, andhovertests. A sixth — the_preview_typesinner helper intest_execute_action_key_press_shows_copy_paste_glyph_only_for_bare_chords— additionally re-implemented_build_overlay_action_fixturesby hand, repeating theinstantiate_with_supported_kwargs(module.ActionExecutor, navigation_timeout_ms=1_000, settle_delay_seconds=0)+executor.overlay = overlaypair that_build_action_executoralready owns.This is the same duplication class the file has already factored out three times (
_build_action_executor,_build_overlay_action_fixtures,_make_overlay_enabled_page); these last sites just weren't covered.Improvement
Add
_build_overlay_call_order_fixtures(module), returning the(call_order, page, overlay, executor, viewport)tuple that_build_move_down_up_overlay_fixturesalready returns, and route all six sites through it. Each test keeps its ownpreview_action/set_status/ mouse wiring — the helper stops at the part that was genuinely identical.Net
-33 / +27lines in one test file.Safety
_build_overlay_action_fixturesnow runs before the test attaches its overlay coroutines rather than after. That is equivalent:_build_overlay_action_fixturesonly doesexecutor = _build_action_executor(module); executor.overlay = overlay; viewport = ViewportConfig()— it never reads the overlay, and the executor holds a reference to the sameMagicMockobject, so side effects attached afterwards still take effect. The docstring records this._preview_typesnow uses the helper'sviewportinstead of constructingViewportConfig()inline at the call — identical, both are the default-constructed config.uv run pytest tests/gives 328 passed / 21 failed, and the failing setdiffs byte-identical against the same command onorigin/main(all pre-existing Chromium-sandbox environment failures intest_browser.py/test_live_runner.py).tests/test_actions.pyalone: 40 passed.uv run ruff check src/ tests/clean;uv run ruff format --check tests/test_actions.pyreports already formatted.Generated by Claude Code
Note
Low Risk
Changes are limited to test helpers and assertions are unchanged; no production code is modified.
Overview
Test-only refactor in
tests/test_actions.py: introduces_build_overlay_call_order_fixtures(module)to centralize the repeated overlay call-order arrange block (call_order, overlay-enabledFakePage,MagicMockoverlay, and executor/viewport from_build_overlay_action_fixtures).Six sites now call the helper instead of duplicating that setup:
_build_move_down_up_overlay_fixtures, the left-click preview ordering test, navigationset_status, semantic F5 key shortcut, hover preview ordering, and the_preview_typesinner helper in the copy/paste glyph test. Each test still wires its ownpreview_action,set_status, or mouse mocks afterward; the helper’s docstring notes that ordering relative to_build_overlay_action_fixturesdoes not change behavior because the executor keeps a reference to the same overlay mock.The copy/paste test also drops hand-rolled
ActionExecutorconstruction in favor of the shared fixtures (same timeout/settle tuning and defaultViewportConfig).Reviewed by Cursor Bugbot for commit cacd6f6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit