fix(tests): match live-head-moved regression to #1697's intentional reorder - #1705
Closed
seonghobae wants to merge 2 commits into
Closed
fix(tests): match live-head-moved regression to #1697's intentional reorder#1705seonghobae wants to merge 2 commits into
seonghobae wants to merge 2 commits into
Conversation
…eorder #1697 (commit 5c561a6) reordered opencode-review.yml's live-state checks so closed/draft admission runs before the head-SHA-match check, and exits 0 instead of 1 for an open, ready PR whose live head has moved. A draft PR whose live head has moved is therefore exempted by the draft check first — the head-moved branch is now unreachable while still draft. test_opencode_live_draft_state_regression.py's test_draft_exemption_fails_closed_when_live_head_moved still asserted the pre-#1697 behavior (returncode 1, "head moved while validating live" in stdout) for exactly that input shape, so it fails on current main. Update it to assert the actual current behavior (returncode 0, exempted via the draft-check message), matching the equivalent direct-production-step coverage #1697 already added in test_opencode_required_verdict_regression.py. Confirmed via a clean origin/main worktree that the regression pre-dates this change and is not introduced by it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
5 tasks
This was referenced Sep 2, 2026
seonghobae
added a commit
that referenced
this pull request
Sep 2, 2026
… (#1710) * fix(tests): match live-head-moved regression to #1697's intentional reorder #1697 (commit 5c561a6) reordered opencode-review.yml's live-state checks so closed/draft admission runs before the head-SHA-match check, and exits 0 instead of 1 for an open, ready PR whose live head has moved. A draft PR whose live head has moved is therefore exempted by the draft check first — the head-moved branch is now unreachable while still draft. test_opencode_live_draft_state_regression.py's test_draft_exemption_fails_closed_when_live_head_moved still asserted the pre-#1697 behavior (returncode 1, "head moved while validating live" in stdout) for exactly that input shape, so it fails on current main. Update it to assert the actual current behavior (returncode 0, exempted via the draft-check message), matching the equivalent direct-production-step coverage #1697 already added in test_opencode_required_verdict_regression.py. Confirmed via a clean origin/main worktree that the regression pre-dates this change and is not introduced by it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 * fix(tests): stub sleep in OpenCode poll regression tests, salvage #1706 Two existing tests extract the real "Fail closed without a current-head OpenCode verdict" step's bash and run it against a fake gh, but never stubbed `sleep` -- driving the transport-failure retry path to its 3-failure threshold performed two genuine 60s sleeps per affected test run (confirmed directly: this exact gap made a 2-test run exceed a 120s timeout). Both now stub `sleep` alongside the existing fake `gh`, matching the pattern already used in test_opencode_poll_self_retirement.py: tests/test_opencode_required_verdict_regression.py::test_fail_closed_step_still_polls_for_a_non_draft_pr tests/test_opencode_live_draft_state_regression.py::test_stale_draft_verdict_event_does_not_exempt_live_ready_pr Also fixes test_opencode_poll_self_retirement.py, which was silently broken on current main: #1707's wall-clock-deadline fix to opencode-review.yml added a `poll_deadline_epoch` reference at the top of the poll loop, but this file's `_run_poll_loop` harness never declared that variable before splicing in the now-changed real loop body, so 7 of its tests failed with an empty gh-calls.log (the script aborted under `set -u` before making any call). Adds the missing `poll_deadline_epoch` line and an injectable fake `date` (extending the existing fake-gh/fake-sleep/fake-timeout harness) to prove the wall-clock deadline logic itself: the loop fails closed with the new diagnostic once the deadline is exceeded even when every gh call keeps succeeding (the exact zombie scenario the fix targets), a fast verdict is unaffected, and the production shape keeps both bounds distinct and additive. No test sleeps for real time. Full affected suite (73 tests) verified green in ~16s; the full project suite (2582 passed, 1 skipped, 21 subtests) runs in ~116s with 100% coverage and 100% docstrings, matching #1706's own claimed 236.65s -> 112.76s improvement. This is a same-file-conflict-driven successor to #1706, which also included this exact test-file delta. #1706 additionally touched .github/workflows/opencode-review.yml with the wall-clock-deadline logic itself -- that exact fix already landed separately as #1707 (bypass-merged during the org-wide capacity incident, before #1706 finished), which is why #1706 is now DIRTY/CONFLICTING against main through no fault of its own test-file changes. This PR carries only the still-valid, non-redundant test-suite-hang fix forward; #1706 is being closed in favor of this PR. Branched from and includes #1705 (fix/live-draft-regression-test-1697, a different in-flight fix to the same tests/test_opencode_live_draft_state_regression.py file, addressing an unrelated draft-head-moved logic question) to avoid a second same-file conflict. If #1705 merges to main independently before this PR, this PR's identical carried-forward hunk should merge as a no-op; if this PR merges first, #1705 should rebase onto main afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Contributor
Author
|
Superseded by #1710, which merged this branch in and landed its full content on |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
tests/test_opencode_live_draft_state_regression.py::test_draft_exemption_fails_closed_when_live_head_movedfails on currentmaintip itself — confirmed independently by three separate background agents working on unrelated PRs (#1242, #1245, #1476), each testing a cleanorigin/maincheckout in isolation, and reconfirmed here in a dedicated cleanorigin/mainworktree.Root cause
main's tip commit5c561a65("fix(opencode): retire stale draft/head dispatches without false failure (#1697)") intentionally reordered.github/workflows/opencode-review.yml's live-state checks so closed/draft admission runs before the head-SHA-match check, and changed that check's outcome for an open, ready PR fromexit 1toexit 0. #1697 updated its sibling test file (tests/test_opencode_required_verdict_regression.py) to match, but not this older file.For the specific input this test exercises (
live_draft=True, live head moved), the new check order means the draft-admission check now runs first and exits 0 with "PR is still a draft on the live exact head..." — the head-moved branch is unreachable while still draft. The old assertions (returncode == 1,"head moved while validating live" in stdout) describe behavior that no longer exists for this input.Fix
Update the test to assert the actual current behavior instead of the pre-#1697 one:
returncode == 0, exempted via the draft-check message, matching the equivalent direct-production-step coverage #1697 already added intest_opencode_required_verdict_regression.py(test_request_review_step_exempts_a_draft_pr_whose_live_head_has_moved/test_fail_closed_step_exempts_a_draft_pr_whose_live_head_has_moved).Verification
coverage run -m pytest tests -q: 2577 passed, 1 skipped, 21 subtests passed (0 failed)coverage report --show-missing: 100% statements / 100% branches onscripts/ciinterrogate: 100.0% docstring coverageorigin/mainworktree (no PR branch involved) that this regression pre-dates and is independent of this change.Impact
This regression was blocking at least two other in-flight PRs (#1245, #1476) from completing their own merge-conflict resolution and passing full verification, since both share this same test file.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
Generated by Claude Code