diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 648bfd064b..06694fcbca 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -342,7 +342,20 @@ def _load_changed_files(api_url: str, repository: str, pull_request: int, token: raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") if len(payload) < 100: return tuple(files) - raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") + # Unreachable by construction, not a live fallback: every one of the 31 + # `range(1, 32)` iterations that reaches this point already returned a + # page whose length is >= 100 (a page under 100 items hits the `return` + # two lines up first), so 31 such pages accumulate at least 3,100 files + # -- strictly more than the 3,000 cap above, which is checked after + # every single appended item, not just at page boundaries. That in-loop + # check therefore always raises no later than partway through the 31st + # page, before the `for` loop can ever exhaust its range. Kept as a + # structural fail-closed guard (so a future change to PAGE_COUNT, + # per_page, or the 3,000 cap that breaks this invariant fails loudly + # instead of silently truncating evidence) rather than deleted; see + # test_changed_file_pagination_bound_is_provably_unreachable, which + # pins the arithmetic relationship itself. + raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") # pragma: no cover def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str, token: str, opener: OpenJson) -> bytes: diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index 26434207cb..70bb1bc970 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -4,6 +4,8 @@ import base64 import importlib.util +import inspect +import re import sys from io import BytesIO from pathlib import Path @@ -491,6 +493,35 @@ def opener(url: str, _token: str) -> object: assert calls[-1].endswith("page=31") +def test_changed_file_pagination_bound_is_provably_unreachable() -> None: + """Pin the arithmetic invariant that makes the loop's trailing raise dead code. + + ``_load_changed_files`` raises inside its item loop the moment + ``len(files) > 3_000`` (checked after every appended item, not only at + page boundaries) and returns early the moment one page has fewer than + 100 items -- so the ``# pragma: no cover``-marked ``raise`` after the + ``for page in range(...)`` loop can only execute if every one of that + many pages returns at least 100 items while the cumulative total never + exceeds 3,000. That requires ``page_count * per_page <= 3_000``, which + the real page count (31) and per_page (100) violate (3,100 > 3,000) -- + the in-loop raise always fires first. This test reads those literals + from the actual source rather than duplicating them, so it fails loudly + if a future edit to any of the three breaks the inequality -- exactly + when the trailing raise becomes reachable again and needs a real + covering test instead of the pragma. + """ + source = inspect.getsource(policy._load_changed_files) + start, stop = (int(n) for n in re.search(r"range\((\d+),\s*(\d+)\)", source).groups()) + page_count = len(range(start, stop)) + per_page = int(re.search(r"per_page=(\d+)", source).group(1)) + cap = int(re.search(r"len\(files\) > (\d[\d_]*)", source).group(1).replace("_", "")) + assert page_count * per_page > cap, ( + "the trailing pagination raise in _load_changed_files is no longer " + "provably unreachable; remove its '# pragma: no cover' and add a " + "test that actually covers it" + ) + + @pytest.mark.parametrize( ("payload", "message"), [