From 8457134eef153e333f775eed780ef5047d9abd5b Mon Sep 17 00:00:00 2001 From: yuneng Date: Sun, 4 Oct 2026 00:38:58 +0000 Subject: [PATCH] ci: stop crediting --ignore paths as invoked in assert_ci_coverage --- .github/scripts/assert_ci_coverage.py | 50 +++++++++++++++++++++++---- tests/unit/test_assert_ci_coverage.py | 22 ++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/.github/scripts/assert_ci_coverage.py b/.github/scripts/assert_ci_coverage.py index 99decc64328..878f7ab8fc5 100644 --- a/.github/scripts/assert_ci_coverage.py +++ b/.github/scripts/assert_ci_coverage.py @@ -26,6 +26,7 @@ DOCKERFILE_INPUT_KEYS = frozenset({"file", "dockerfile"}) TEST_RUNNER_RE = re.compile(r"\bpytest\b|\bcircleci tests\b|\bhelm unittest\b|\bplaywright test\b|\bpython[0-9.]*\s") IMAGE_BUILD_RE = re.compile(r"\bdocker\s+(?:buildx\s+)?build\b") TEST_TOKEN_RE = re.compile(r"tests/[A-Za-z0-9_./*?-]+") +IGNORE_ARG_RE: Final = re.compile(r"--ignore(?:-glob)?[= ](\S+)") DOCKERFILE_TOKEN_RE = re.compile(r"[A-Za-z0-9_./-]*Dockerfile[A-Za-z0-9_.-]*") COMMENT_RE = re.compile(r"^\s*#.*$", re.MULTILINE) GLOB_CHARS = frozenset("*?") @@ -71,6 +72,17 @@ class Scalar: value: str +@dataclass(frozen=True, slots=True) +class Selection: + included: frozenset[str] + ignored: frozenset[str] + + def covers(self, relative_path: str) -> bool: + return any(_token_covers(token, relative_path) for token in self.included) and not any( + _token_covers(token, relative_path) for token in self.ignored + ) + + @dataclass(frozen=True, slots=True) class Finding: subject: str @@ -110,13 +122,32 @@ def _uncommented(value: str) -> str: return COMMENT_RE.sub("", value) -def _invoked_test_tokens(scalars: Iterable[Scalar]) -> frozenset[str]: - return frozenset( - match.group(0).rstrip("/") +def _selection_for_scalar(scalar: Scalar) -> Selection: + text: Final = _uncommented(scalar.value) + ignored: Final = frozenset().union( + *( + frozenset(token.rstrip("/") for token in TEST_TOKEN_RE.findall(ignored_argument)) + for ignored_argument in IGNORE_ARG_RE.findall(text) + ) + ) + included: Final = frozenset( + match.group(0).rstrip("/") for match in TEST_TOKEN_RE.finditer(IGNORE_ARG_RE.sub("", text)) + ) + return Selection(included=included, ignored=ignored) + + +def _invoked_selections(scalars: Iterable[Scalar]) -> tuple[Selection, ...]: + selected_scalars: Final = tuple( + scalar for scalar in scalars if scalar.key in TEST_PATH_KEYS or TEST_RUNNER_RE.search(scalar.value) - for match in TEST_TOKEN_RE.finditer(_uncommented(scalar.value)) ) + selections: Final = tuple(_selection_for_scalar(scalar) for scalar in selected_scalars) + return tuple(selection for selection in selections if selection.included) + + +def _invoked_test_tokens(scalars: Iterable[Scalar]) -> frozenset[str]: + return frozenset().union(*(selection.included for selection in _invoked_selections(scalars))) def _built_dockerfile_tokens(scalars: Iterable[Scalar]) -> frozenset[str]: @@ -179,11 +210,12 @@ def _dockerfiles() -> tuple[str, ...]: ) -def _uncovered_tests(allowlist: Allowlist, tokens: frozenset[str]) -> tuple[Finding, ...]: +def _uncovered_tests(allowlist: Allowlist, selections: tuple[Selection, ...]) -> tuple[Finding, ...]: uncovered = tuple( relative_path for relative_path in _test_files() - if not any(_token_covers(token, relative_path) for token in tokens) and not allowlist.covers_test(relative_path) + if not any(selection.covers(relative_path) for selection in selections) + and not allowlist.covers_test(relative_path) ) directories = tuple(dict.fromkeys(path.rsplit("/", 1)[0] for path in uncovered)) return tuple( @@ -643,7 +675,11 @@ def main() -> int: integration_paths, ownership_findings = _integration_ownership() test_findings = ( - _uncovered_tests(allowlist, _invoked_test_tokens(scalars) | integration_paths) + _uncovered_tests( + allowlist, + _invoked_selections(scalars) + + (Selection(included=integration_paths, ignored=frozenset()),), + ) + ownership_findings ) dockerfile_findings = _uncovered_dockerfiles(allowlist, _built_dockerfile_tokens(scalars)) diff --git a/tests/unit/test_assert_ci_coverage.py b/tests/unit/test_assert_ci_coverage.py index aa3b1eb7909..be577b66112 100644 --- a/tests/unit/test_assert_ci_coverage.py +++ b/tests/unit/test_assert_ci_coverage.py @@ -196,6 +196,28 @@ def test_shards_are_credited_only_from_explicit_test_paths() -> None: assert coverage._invoked_test_tokens(scalars) == frozenset({"tests/unit/wired", "tests/unit/also_wired"}) +def test_an_ignored_path_is_not_credited_as_invoked() -> None: + scalars: Final = ( + coverage.Scalar(key="test-path", value="tests/unit/a\n--ignore=tests/unit/b/test_x.py"), + ) + assert coverage._invoked_test_tokens(scalars) == frozenset({"tests/unit/a"}) + + +def test_a_file_its_only_shard_ignores_is_not_covered_by_that_shards_glob() -> None: + selections: Final = coverage._invoked_selections( + ( + coverage.Scalar( + key="test-path", + value="tests/unit/proxy/test_*.py --ignore=tests/unit/proxy/test_update_spend.py", + ), + ) + ) + assert len(selections) == 1 + selection: Final = selections[0] + assert selection.covers("tests/unit/proxy/test_other.py") is True + assert selection.covers("tests/unit/proxy/test_update_spend.py") is False + + def test_check_shards_passes_on_the_repo_as_it_stands(capsys): assert coverage._check_shards() == 0