test: extract shared _skip_preflight helper in test_runner.py - #304
Conversation
_build_runner and _build_visualize_override_runner each defined a byte-identical local async no-op _skip_preflight(url) closure used to stub out VisualQARunner._preflight_url for tests. Hoisted it to a single module-level function that both builders now reference, matching this file's established convention of factoring out duplicated test scaffolding (_build_* fixtures, _make_response, _check_messages, etc.).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus 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. 📝 WalkthroughWalkthroughChangesRunner preflight setup
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This is a localized test-only refactor with no behavior change, and no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 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
_build_runnerand_build_visualize_override_runnerintests/test_runner.pyeach defined a byte-for-byte identical local async no-op closure:used to stub out
VisualQARunner._preflight_urlso tests don't make a real HTTP HEAD probe. Neither call site's assignment relies on closure identity or a fresh object per call — both just attach the same no-op coroutine to a fresh runner instance.Improvement
Hoisted
_skip_preflightto a single module-level function; both builders now assign that shared function instead of each redefining it. This matches the file's established convention for factoring out duplicated test scaffolding (_build_*fixture builders,_make_response,_check_messages,patched_sleep, etc. — see prior PRs #167, #200, #221, #237 in this repo).Test-only, single file, +6/-8.
Safety / Verification
uv run ruff check src/ tests/— cleanuv run ruff format --check tests/test_runner.py— already formatteduv run pytest tests/test_runner.py— 43 passed (same as before the change)uv run pytest tests/— 326 passed / 21 failed, identical to the documented pre-existing baseline (the 21 failures are all environment-only Chromium-sandbox tests intest_browser.py/test_live_runner.py— same test names, unchanged by this diff)runner._preflight_urlas before, just defined once instead of twice.Generated by Claude Code
Note
Low Risk
Test scaffolding only; no production code or test behavior change.
Overview
Test-only refactor in
tests/test_runner.py: the identical no-op async stub used to bypassVisualQARunner._preflight_url(HTTP HEAD) is defined once at module scope as_skip_preflight, with a short docstring._build_runnerand_build_visualize_override_runnerno longer each define a local_skip_preflight; both still assign that shared coroutine torunner._preflight_url(setattrvs direct attribute assignment unchanged). Runtime behavior in tests is the same—only duplication is removed, aligned with other shared_build_*helpers in this file.Reviewed by Cursor Bugbot for commit f2f7873. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit