From 2009bcb9ffcdb1ab4f91c772d698778576ee6f3d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 15:08:29 +0900 Subject: [PATCH 1/2] test(scheduler): close current-main RCA coverage gaps --- ...est_scheduler_1541_coverage_regressions.py | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/test_scheduler_1541_coverage_regressions.py diff --git a/tests/test_scheduler_1541_coverage_regressions.py b/tests/test_scheduler_1541_coverage_regressions.py new file mode 100644 index 0000000000..3d03ce56db --- /dev/null +++ b/tests/test_scheduler_1541_coverage_regressions.py @@ -0,0 +1,102 @@ +"""Regression coverage for the scheduler branches introduced by PR #1541.""" + +from __future__ import annotations + +import pytest + +from scripts.ci import pr_review_fix_scheduler as fix +from scripts.ci import pr_review_merge_scheduler as merge + + +def _pr(**overrides: object) -> dict[str, object]: + """Return a minimal same-repository pull-request fixture.""" + value: dict[str, object] = { + "number": 7, + "isDraft": False, + "baseRefName": "main", + "baseRefOid": "b" * 40, + "headRefName": "feature", + "headRefOid": "a" * 40, + "headRepository": {"nameWithOwner": "owner/repo"}, + "mergeStateStatus": "CLEAN", + "reviews": {"nodes": []}, + "reviewThreads": {"nodes": []}, + } + value.update(overrides) + return value + + +def test_conflicted_draft_skips_before_repair_authority() -> None: + """A conflicted draft remains a draft skip rather than an RCA dispatch.""" + args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) + + assert fix.inspect_pr( + "owner/repo", + _pr(isDraft=True, mergeStateStatus="DIRTY"), + args, + ) == ("skip", ("draft PR",)) + + +def test_conflicted_unapproved_pr_fails_closed_without_repair_authority() -> None: + """A conflict without explicit unreviewed-repair authority stays closed.""" + args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) + + assert fix.inspect_pr( + "owner/repo", + _pr(mergeStateStatus="DIRTY"), + args, + ) == ("skip", ("merge conflict is not authorized for repair",)) + + +def test_workflow_name_rest_fallback_paginates_and_filters_rows( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Workflow identity pagination keeps only rows with usable names.""" + page_one = [{"check_suite_id": index, "name": f"workflow-{index}"} for index in range(99)] + page_one.append({"check_suite_id": 99, "name": ""}) + calls: list[str] = [] + + def fake_api(path: str) -> dict[str, object]: + calls.append(path) + if path.endswith("page=1"): + return {"workflow_runs": page_one} + return {"workflow_runs": [{"check_suite_id": 100, "name": "opencode-review"}]} + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40) + + assert names[0] == "workflow-0" + assert 99 not in names + assert names[100] == "opencode-review" + assert calls == [ + f"repos/owner/repo/actions/runs?head_sha={'a' * 40}&per_page=100&page=1", + f"repos/owner/repo/actions/runs?head_sha={'a' * 40}&per_page=100&page=2", + ] + + +def test_workflow_name_rest_fallback_treats_permission_denial_as_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An inaccessible Actions inventory returns an empty fail-closed map.""" + + def fake_api(_path: str) -> dict[str, object]: + raise RuntimeError("Resource not accessible by integration") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + assert merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40) == {} + + +def test_workflow_name_rest_fallback_propagates_unrelated_failures( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Transport failures other than permission denial remain visible.""" + + def fake_api(_path: str) -> dict[str, object]: + raise RuntimeError("gh: HTTP 502 (exhausted retries)") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + with pytest.raises(RuntimeError, match="HTTP 502"): + merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40) From 563a766c5be935a8cdba7afc0cc9684af741e72e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 15:46:35 +0900 Subject: [PATCH 2/2] test(scheduler): close remaining current-main coverage gaps --- tests/test_pr_review_fix_scheduler.py | 86 +++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 9860eeaec7..daeefae573 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -177,6 +177,92 @@ def test_prepare_autofix_slot_preserves_new_head_workers_after_head_advance(monk workflow_repository=fix.DEFAULT_AUTOFIX_REPOSITORY, dry_run=False, ) is None + + +def test_prepare_autofix_slot_returns_same_head_with_no_stale_workers(monkeypatch): + """No stale workers means the cancellation branch is never entered.""" + head = "a" * 40 + monkeypatch.setattr( + fix, + "run_json", + lambda _args: { + "workflow_runs": [ + { + "id": 1, + "status": "in_progress", + "display_title": f"PR Review Autofix owner/repo#7@{head}", + } + ] + }, + ) + monkeypatch.setattr( + fix, + "force_cancel_workflow_runs", + lambda *_args: pytest.fail("there is no stale worker to cancel"), + ) + monkeypatch.setattr( + fix, + "live_head_matches", + lambda *_args: pytest.fail( + "staleness is only ever checked when a stale worker exists" + ), + ) + + assert fix.prepare_autofix_slot( + "owner/repo", + make_pr(headRefOid=head), + workflow=fix.DEFAULT_AUTOFIX_WORKFLOW, + workflow_repository=fix.DEFAULT_AUTOFIX_REPOSITORY, + dry_run=False, + ) + + +def test_live_head_matches_compares_the_live_head_to_the_cached_snapshot(monkeypatch): + """The real head-matching implementation reads GitHub's current PR head. + + Every other test in this module monkeypatches ``live_head_matches`` away, + so its own body (the ``gh api`` read, the payload-shape guard, and the + case-insensitive comparison) was never exercised by the suite at all. + """ + pr = make_pr(headRefOid="a" * 40) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "A" * 40}}) + assert fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "b" * 40}}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "short"}}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": None}}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": "not-a-dict"}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: "not-a-dict") + assert not fix.live_head_matches("owner/repo", pr) + + +def test_inspect_pr_reports_active_autofix_worker_without_dispatch(monkeypatch): + """An already-running current-head worker waits instead of re-dispatching.""" + args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) + monkeypatch.setattr(fix, "needs_autofix", lambda _pr: (True, ("review",))) + monkeypatch.setattr(fix, "issue_comments", lambda _repo, _number: []) + monkeypatch.setattr(fix, "prepare_autofix_slot", lambda *_args, **_kwargs: True) + monkeypatch.setattr( + fix, + "dispatch_autofix", + lambda *_args, **_kwargs: pytest.fail("an active worker must not be redispatched"), + ) + + assert fix.inspect_pr("owner/repo", make_pr(), args) == ( + "wait", + ("current-head autofix run is already queued or running",), + ) + + def test_terminal_failed_check_triggers_rca_without_prior_opencode_review(): """Exact-head check evidence can start RCA without a circular review prerequisite.""" pr = make_pr(