From 404e263b068f652a9b5d15d074cdf58f366286eb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 04:18:38 +0000 Subject: [PATCH] test(ci): close main's coverage gap in two REST-fallback scheduler paths main is currently red on its own 100%-coverage gate (99%, 4 statements + 1 partial branch missing), which blocks the coverage-evidence check for every PR that rebases onto it, independent of that PR's own diff. Root cause: two functions have execution paths no existing test reaches. - pr_review_merge_scheduler.py: fetch_workflow_names_by_check_suite_rest is exercised only via test_pr_review_fix_scheduler_rest_workflow_identity.py's two rest_pr_node()-level fixtures (single-page happy path; resource- inaccessible-returns-{}), which never build a page of exactly 100 workflow_runs (pagination's page += 1), never include a run missing check_suite_id/name (the admission branch's False arm), and never raise a non-access RuntimeError (the fail-closed re-raise). Added three direct unit tests for the function itself, mirroring the sibling fetch_all_pr_reviews_rest's existing direct-call test style. - pr_review_fix_scheduler.py: inspect_pr's conflicted-branch draft check (line 417) and unauthorized-conflict skip (line 425) were never hit -- every existing draft-PR fixture used mergeStateStatus="CLEAN", so it fell through to the *other*, separately-tested draft check after the conflict branch (line 437) instead. Added two assertions building an actually-conflicted (mergeStateStatus="DIRTY") draft and non-draft PR, reaching both previously-dead return statements directly. No production code changed -- this is test-only, closing coverage gaps in already-correct existing logic. Full suite: 2228 passed, 1 skipped, 21 subtests. coverage report: 100%. interrogate: 100%. --- tests/test_pr_review_fix_scheduler.py | 7 ++ ...ew_fix_scheduler_rest_workflow_identity.py | 67 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 3b4416bdc3..1507f6d5fb 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -1152,6 +1152,13 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch): assert fix.inspect_pr("owner/repo", make_pr(headRepository={"nameWithOwner": "fork/repo"}), args)[1] == ( "external PR head is not writable by repository workflow credentials", ) + assert fix.inspect_pr( + "owner/repo", make_pr(mergeStateStatus="DIRTY", isDraft=True), args + ) == ("skip", ("draft PR",)) + assert fix.inspect_pr("owner/repo", make_pr(mergeStateStatus="DIRTY"), args) == ( + "skip", + ("merge conflict is not authorized for repair",), + ) monkeypatch.setattr(fix, "needs_autofix", lambda pr: (False, ())) assert fix.inspect_pr("owner/repo", make_pr(), args) == ( diff --git a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py index c24cfb05f9..f261ce5beb 100644 --- a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py +++ b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py @@ -154,3 +154,70 @@ def fake_api(path: str) -> Any: assert merge.is_strix_context(context) assert merge.strix_evidence_state(pr) == expected_state assert fix.current_head_failed_checks(pr) == () + + +def test_fetch_workflow_names_by_check_suite_rest_paginates_past_100( + monkeypatch: Any, +) -> None: + """A first page of exactly 100 runs must fetch a second page and merge both.""" + head_sha = "e" * 40 + page1 = [ + {"check_suite_id": i, "name": f"workflow-{i}"} for i in range(100) + ] + page2 = [{"check_suite_id": 100, "name": "workflow-100"}] + calls: list[str] = [] + + def fake_api(path: str) -> Any: + calls.append(path) + if path.endswith("page=1"): + return {"workflow_runs": page1} + if path.endswith("page=2"): + return {"workflow_runs": page2} + raise AssertionError(f"unexpected path {path}") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) + + assert names == {i: f"workflow-{i}" for i in range(101)} + assert calls == [ + f"repos/owner/repo/actions/runs?head_sha={head_sha}&per_page=100&page=1", + f"repos/owner/repo/actions/runs?head_sha={head_sha}&per_page=100&page=2", + ] + + +def test_fetch_workflow_names_by_check_suite_rest_skips_entries_missing_suite_id_or_name( + monkeypatch: Any, +) -> None: + """A run with no check-suite id or a blank name must not populate the map.""" + head_sha = "f" * 40 + + def fake_api(path: str) -> Any: + return { + "workflow_runs": [ + {"check_suite_id": None, "name": "orphaned run"}, + {"check_suite_id": 900, "name": ""}, + {"check_suite_id": 901, "name": "kept run"}, + ] + } + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) + + assert names == {901: "kept run"} + + +def test_fetch_workflow_names_by_check_suite_rest_propagates_non_access_errors( + monkeypatch: Any, +) -> None: + """A page-fetch failure unrelated to integration access must fail closed.""" + head_sha = "0" * 40 + + def fake_api(path: str) -> Any: + 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", head_sha)