diff --git a/python/lean_pool/merge_queue_cleanup.py b/python/lean_pool/merge_queue_cleanup.py index aba26a319c..d8bcd03483 100644 --- a/python/lean_pool/merge_queue_cleanup.py +++ b/python/lean_pool/merge_queue_cleanup.py @@ -36,12 +36,16 @@ def discarded_runs( def branch_exists(repository: str, branch: str) -> bool: """Report a branch as missing only on a definite 404, never on other errors.""" - result = subprocess.run( - ["gh", "api", f"repos/{repository}/git/ref/heads/{branch}"], - capture_output=True, - text=True, - timeout=60, - ) + try: + result = subprocess.run( + ["gh", "api", f"repos/{repository}/git/ref/heads/{branch}"], + capture_output=True, + text=True, + timeout=60, + ) + except subprocess.TimeoutExpired: + LOGGER.warning("Timed out looking up %s; keeping its run", branch) + return True if result.returncode == 0: return True if "HTTP 404" in result.stderr: @@ -75,20 +79,37 @@ def queue_runs(repository: str) -> list[dict[str, Any]]: def cancel_discarded(repository: str, current_run_id: int) -> list[int]: - """Cancel every discarded queue run and return the cancelled run IDs.""" - cancelled = discarded_runs( + """Request cancellation of discarded runs and return accepted request IDs.""" + discarded = discarded_runs( queue_runs(repository), current_run_id, lambda branch: branch_exists(repository, branch), ) - for run_id in cancelled: - LOGGER.info("Cancelling run %s: its merge group was discarded", run_id) - subprocess.run( - ["gh", "run", "cancel", str(run_id), "--repo", repository], - check=False, - timeout=60, + accepted: list[int] = [] + for run_id in discarded: + try: + result = subprocess.run( + ["gh", "run", "cancel", str(run_id), "--repo", repository], + check=False, + capture_output=True, + text=True, + timeout=60, + ) + except subprocess.TimeoutExpired: + LOGGER.warning("Timed out requesting cancellation of run %s", run_id) + continue + if result.returncode != 0: + LOGGER.warning( + "Could not request cancellation of run %s: %s", + run_id, + result.stderr.strip(), + ) + continue + LOGGER.info( + "Requested cancellation of run %s: its merge group was discarded", run_id ) - return cancelled + accepted.append(run_id) + return accepted def main() -> None: diff --git a/python/tests/test_merge_queue_cleanup.py b/python/tests/test_merge_queue_cleanup.py index 67e9709e64..b4c91d4a30 100644 --- a/python/tests/test_merge_queue_cleanup.py +++ b/python/tests/test_merge_queue_cleanup.py @@ -82,3 +82,63 @@ def fake_run(command: list[str], **kwargs: Any) -> subprocess.CompletedProcess[s monkeypatch.setattr(subprocess, "run", fake_run) assert cancel_discarded("owner/repository", 99) == [1, 3] assert [command[3] for command in cancelled] == ["1", "3"] + + +def test_ref_timeout_keeps_run_and_scans_later_discarded( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unknown first ref must not hide a later definite missing ref.""" + looked_up: list[str] = [] + cancellation_requests: list[str] = [] + monkeypatch.setattr( + merge_queue_cleanup, + "queue_runs", + lambda repository: [_run(1, GONE), _run(3, ALSO_GONE)], + ) + + def fake_run(command: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + if command[:2] == ["gh", "api"]: + looked_up.append(command[2]) + if command[2] == f"repos/owner/repository/git/ref/heads/{GONE}": + raise subprocess.TimeoutExpired(command, 60) + assert command[2] == f"repos/owner/repository/git/ref/heads/{ALSO_GONE}" + return subprocess.CompletedProcess(command, 1, "", "HTTP 404") + assert command[:3] == ["gh", "run", "cancel"] + cancellation_requests.append(command[3]) + return subprocess.CompletedProcess(command, 0, "", "") + + monkeypatch.setattr(subprocess, "run", fake_run) + assert cancel_discarded("owner/repository", 99) == [3] + assert looked_up == [ + f"repos/owner/repository/git/ref/heads/{GONE}", + f"repos/owner/repository/git/ref/heads/{ALSO_GONE}", + ] + assert cancellation_requests == ["3"] + + +@pytest.mark.parametrize("failure", ["timeout", "nonzero"]) +def test_failed_cancellation_continues_and_returns_only_accepted( + monkeypatch: pytest.MonkeyPatch, failure: str +) -> None: + """One failed request must not stop later requests or count as accepted.""" + cancellation_requests: list[str] = [] + monkeypatch.setattr( + merge_queue_cleanup, + "queue_runs", + lambda repository: [_run(1, GONE), _run(3, ALSO_GONE)], + ) + + def fake_run(command: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + if command[:2] == ["gh", "api"]: + return subprocess.CompletedProcess(command, 1, "", "HTTP 404") + assert command[:3] == ["gh", "run", "cancel"] + cancellation_requests.append(command[3]) + if command[3] == "1": + if failure == "timeout": + raise subprocess.TimeoutExpired(command, 60) + return subprocess.CompletedProcess(command, 1, "", "HTTP 403") + return subprocess.CompletedProcess(command, 0, "", "") + + monkeypatch.setattr(subprocess, "run", fake_run) + assert cancel_discarded("owner/repository", 99) == [3] + assert cancellation_requests == ["1", "3"]