mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(git): dedupe check-runs per name so cancelled duplicates can't mask green
_classify_check_runs counted any completed cancelled check-run as failing. The push + pull_request double-trigger leaves cancelled same-name check-runs on the same head SHA next to the surviving run's green, so the pr_pass gate saw permanent red on a genuinely green PR. Keep only the newest (highest-id) run per check name before classifying.
This commit is contained in:
+25
-4
@@ -346,6 +346,20 @@ _FAILING_CHECK_CONCLUSIONS = frozenset(
|
||||
_HTTP_NOT_FOUND = 404
|
||||
|
||||
|
||||
def _latest_check_runs_by_name(
|
||||
check_runs: list[dict[str, Any]],
|
||||
) -> dict[str, dict[str, Any]]:
|
||||
"""Newest (highest-id) check-run per name — GitHub check-run ids are
|
||||
globally monotonic, so the highest id is the most recent attempt."""
|
||||
latest: dict[str, dict[str, Any]] = {}
|
||||
for cr in check_runs:
|
||||
name = str(cr.get("name") or "check")
|
||||
prev = latest.get(name)
|
||||
if prev is None or int(cr.get("id") or 0) > int(prev.get("id") or 0):
|
||||
latest[name] = cr
|
||||
return latest
|
||||
|
||||
|
||||
def _select_ci_head_run(runs: list[dict[str, Any]]) -> dict[str, Any]:
|
||||
"""Pick the run reflecting the branch's current-HEAD CI conclusion.
|
||||
|
||||
@@ -3246,16 +3260,23 @@ class GitService(BaseService):
|
||||
def _classify_check_runs(
|
||||
check_runs: list[dict[str, Any]], head_sha: str
|
||||
) -> dict[str, Any]:
|
||||
"""Map a non-empty check-runs list to a failure/pending/success state."""
|
||||
"""Map a non-empty check-runs list to a failure/pending/success state.
|
||||
|
||||
Deduped per check name first (see ``_latest_check_runs_by_name``): a
|
||||
superseded duplicate workflow run (the push + pull_request
|
||||
double-trigger) leaves cancelled same-name check-runs on the same
|
||||
SHA that would otherwise mask the surviving run's green forever.
|
||||
"""
|
||||
latest = _latest_check_runs_by_name(check_runs)
|
||||
failing = [
|
||||
str(cr.get("name") or "check")
|
||||
for cr in check_runs
|
||||
name
|
||||
for name, cr in latest.items()
|
||||
if cr.get("status") == "completed"
|
||||
and cr.get("conclusion") in _FAILING_CHECK_CONCLUSIONS
|
||||
]
|
||||
if failing:
|
||||
return {"state": "failure", "failing_checks": failing, "head_sha": head_sha}
|
||||
if any(cr.get("status") != "completed" for cr in check_runs):
|
||||
if any(cr.get("status") != "completed" for cr in latest.values()):
|
||||
return {"state": "pending", "head_sha": head_sha}
|
||||
return {"state": "success", "head_sha": head_sha}
|
||||
|
||||
|
||||
@@ -116,6 +116,73 @@ async def test_failing_check_names_it() -> None:
|
||||
assert out["head_sha"] == _SHA
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancelled_duplicate_run_superseded_by_newer_green() -> None:
|
||||
"""A superseded duplicate workflow run's cancelled check-run must not mask
|
||||
the newer same-name run's green (the push + pull_request double-trigger
|
||||
leaves both on the same head SHA)."""
|
||||
checks = _resp(
|
||||
200,
|
||||
json_payload={
|
||||
"check_runs": [
|
||||
{
|
||||
"id": 1,
|
||||
"name": "tests",
|
||||
"status": "completed",
|
||||
"conclusion": "cancelled",
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"name": "tests",
|
||||
"status": "completed",
|
||||
"conclusion": "success",
|
||||
},
|
||||
]
|
||||
},
|
||||
)
|
||||
client = _client(_pr_head_resp(), checks)
|
||||
with (
|
||||
_patch_project(),
|
||||
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||
):
|
||||
out = await _service().get_pr_ci_status("roboco", _PR)
|
||||
assert out == {"state": "success", "head_sha": _SHA}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_newest_same_name_run_failing_still_fails() -> None:
|
||||
"""Dedup keeps the newest run per name — a red re-run supersedes an older
|
||||
green, never the reverse."""
|
||||
checks = _resp(
|
||||
200,
|
||||
json_payload={
|
||||
"check_runs": [
|
||||
{
|
||||
"id": 1,
|
||||
"name": "tests",
|
||||
"status": "completed",
|
||||
"conclusion": "success",
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"name": "tests",
|
||||
"status": "completed",
|
||||
"conclusion": "failure",
|
||||
},
|
||||
]
|
||||
},
|
||||
)
|
||||
client = _client(_pr_head_resp(), checks)
|
||||
with (
|
||||
_patch_project(),
|
||||
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||
):
|
||||
out = await _service().get_pr_ci_status("roboco", _PR)
|
||||
assert out is not None
|
||||
assert out["state"] == "failure"
|
||||
assert out["failing_checks"] == ["tests"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_still_running_check_is_pending() -> None:
|
||||
checks = _resp(
|
||||
|
||||
Reference in New Issue
Block a user