Repository navigation
Reject invalid Python regression-lane timeouts - #18476
Conversation
Co-authored-by: OpenAI Codex <noreply@openai.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Python test lane now rejects timeout values that are non-finite or less than or equal to zero. A regression test checks rejection before the lane lists tests. ChangesTimeout validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Invalid timeout values are rejected before tests are selected or started, while existing valid-timeout behavior is preserved. No outstanding merge risk was identified in the reviewed change. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches🧪 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 |
6639835
left a comment
There was a problem hiding this comment.
Reviewed lane-runner parsing and execution order at 68ba1ed. Positive-finite timeout validation precedes registry/test/environment/process work, rejecting zero, negative, NaN and infinite values while preserving defaults. The regression checks diagnostic and absence of listed tests. No actionable issue found within this diff. Static review only; CI lane/unittest execution was not performed.
|
Thank you @BlueRaddish! |
|
Merge receipt for |
29661b9 gh-merge-green: allow explicit Vercel status override (manaflow-ai#18614) dd6e295 fix: preserve SSH ProxyCommand child environment (manaflow-ai#18285) f0a2bad Reject invalid Python regression-lane timeouts (manaflow-ai#18476) 3430354 Preserve PR media referenced through GitHub blob URLs (manaflow-ai#18562) 3b71b41 Reset a browser pane's selected frame and element refs when the page navigates (manaflow-ai#18577) 04e1d68 Clear force-close bypass when a confirmed close is rejected (manaflow-ai#18414) 8d86447 Treat Copilot value flags as value options when restoring (manaflow-ai#18470) f6c678a Keep __proto__ keys in whole-area browser storage reads (manaflow-ai#18527) 7e97128 Keep minimized windows in the Dock when the global hotkey reveals cmux (manaflow-ai#18533)
Summary
The Python regression-lane runner now rejects zero, negative, NaN, and infinite
--timeoutvalues before selecting or starting tests. NaN and infinity could disable the promised timeout bound; nonpositive values could kill valid tests immediately. Existing positive finite timeouts and the 900-second default keep their behavior.Testing
python3 tests/test_ci_test_execution_registry.py LaneRunnerTests.test_invalid_timeouts_are_rejected_before_listing_testsfailed for all five new CLI cases on regression commit805a906b445d2e96fc78781a0ac2383c07d06ff1. With the fix,python3 tests/test_ci_test_execution_registry.pypasses all 33 tests, including real process concurrency, failure reporting, and hung-test termination.git diff --checkpasses.The suite ran under WSL on the existing reviewed Windows checkout; no app build or runtime test was needed.
python3 scripts/verify-local.pycould not establish its base because native Linux Git cannot resolve this Windows worktree's absolute.gitpointer. A native verification checkout was abandoned during asset fetching when the host ran low on memory; no full-static pass is claimed. The first complete-suite attempt also lacked sparse.githubfiles; after materializing those existing files, all 33 tests passed. No hook was bypassed. The diagnostic is internal CI tooling and does not add app/catalog strings.AI assistance from OpenAI Codex was used for diagnosis, implementation, and tests. Open-PR checks for
run_python_test_laneand lane/timeout symptoms found no matching fix; matching candidate file patches were inspected.Changelog
none
Proof
Internal test-runner behavior; the committed CLI regression and red/green suite are the proof.
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Low Risk
Internal CI test-runner CLI validation only; no app, auth, or production runtime impact.
Overview
The Python regression-lane runner (
run_python_test_lane.py) now validates--timeoutat CLI parse time: it must be a positive finite number. Zero, negative, NaN, and ±infinity exit with--timeout must be a positive finite numberinstead of being passed through tosubprocess.wait(timeout=...), where non-finite values could break the per-test kill bound and nonpositive values could time out tests immediately.A new
LaneRunnerTests.test_invalid_timeouts_are_rejected_before_listing_testscovers0,-1,nan,inf, and-infwith--listso invalid timeouts fail before any registry listing or test execution. Existing positive finite timeouts (including the 900s default) are unchanged.Reviewed by Cursor Bugbot for commit 68ba1ed. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit