Skip to content

Commit 4a5e462

Browse files
committed
fix(coverage): invert ALWAYS_RUN_ALL to unattributable-catch-all and validate map shape
Fix 1: Replace the allowlist of run-all files with an inverted design: define a small, conservative allowlist of provably test-irrelevant files (docs/*.md, LICENSE, etc.) and treat any changed file that is not .fpp, not cases.py, and not in that allowlist as unattributable -> run all. This closes the gap where mfc.sh, .github/**, tests/**, toolchain/pyproject.toml, and similar files would silently fall through to rung-7 (skip-all) under --select-enforce. Fix 2: In load_map, validate that every entry is str -> list[str] after popping _meta. A malformed entry returns (None, None) so the caller runs the full suite rather than silently misrouting tests. Tests: expand test_docs_only_still_skips_all to cover docs/, LICENSE, .claude/; add test_unattributable_nonsource_change_runs_all for mfc.sh / pyproject.toml / tests/** / .github/workflows; add test_load_map_rejects_malformed_entry.
1 parent a1152d1 commit 4a5e462

2 files changed

Lines changed: 48 additions & 6 deletions

File tree

toolchain/mfc/test/coverage.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ def save_map(path: Path, entries: dict, *, n_tests: int, git_sha: str, gfortran_
5555
"toolchain/bootstrap/", # build/run scripts
5656
)
5757

58+
# Conservative allowlist of files that are provably irrelevant to test outcomes.
59+
# Only add files here if you are 100% certain a change cannot affect any test result.
60+
# Err toward run-all: a false "irrelevant" classification causes under-inclusion (skipping
61+
# a test that should run), which violates the soundness invariant.
62+
_IRRELEVANT_SUFFIXES = (".md", ".rst")
63+
_IRRELEVANT_EXACT = frozenset(["LICENSE", "LICENSE.md", ".gitignore", ".gitattributes", ".mailmap", ".editorconfig", "CITATION.cff"])
64+
_IRRELEVANT_PREFIXES = ("docs/", ".claude/", ".github/ISSUE_TEMPLATE/")
65+
66+
67+
def _is_test_irrelevant(f: str) -> bool:
68+
"""Return True iff the file is provably irrelevant to test outcomes (docs, config)."""
69+
return f.endswith(_IRRELEVANT_SUFFIXES) or f in _IRRELEVANT_EXACT or f.startswith(_IRRELEVANT_PREFIXES)
70+
5871

5972
def is_always_run_all(changed_files: set) -> bool:
6073
"""True if any changed file forces the full suite (un-attributable by the CPU map)."""
@@ -89,6 +102,9 @@ def load_map(path: Path) -> Tuple[Optional[dict], Optional[dict]]:
89102
if not isinstance(data, dict) or "_meta" not in data:
90103
return None, None
91104
meta = data.pop("_meta")
105+
for k, v in data.items():
106+
if not isinstance(k, str) or not isinstance(v, list) or not all(isinstance(x, str) for x in v):
107+
return None, None
92108
return data, meta
93109

94110

@@ -120,11 +136,17 @@ def select_tests(cases, coverage_map, changed_files):
120136
return list(cases), [], "rung3: hand-written .f90/.f changed"
121137

122138
changed_fpp = {f for f in changed_files if f.endswith(".fpp")}
123-
# Skip-all only when nothing test-relevant changed. If cases.py changed (no .fpp), fall
124-
# through to per-test: new/modified tests have a fresh param_hash absent from the map and
125-
# run via rung 5; unchanged tests have no .fpp overlap and are skipped.
139+
# Unattributable-change guard: if any changed file is not a .fpp (handled by the
140+
# coverage map), not cases.py (handled by rung 5), and not provably test-irrelevant
141+
# (docs), we cannot attribute the change to individual tests -> run all. This is the
142+
# catch-all that covers mfc.sh, .github/**, tests/**, toolchain/pyproject.toml, etc.
143+
# The old ALWAYS_RUN_ALL_* sets are still checked first (rungs 2-3) for common cases.
144+
unattributable = [f for f in changed_files if not f.endswith(".fpp") and f != CASES_PY and not _is_test_irrelevant(f)]
145+
if unattributable:
146+
return list(cases), [], "rung2: unattributable change (not .fpp/cases.py/docs) -> run all"
147+
# Only .fpp + cases.py + irrelevant docs remain.
126148
if not changed_fpp and CASES_PY not in changed_files:
127-
return [], list(cases), "rung7: no Fortran or test-definition change"
149+
return [], list(cases), "rung7: only docs/irrelevant files changed"
128150

129151
# Rung 4: a changed .fpp that no test covers -> run all (GPU-only blind spot).
130152
covered = _covered_fpp(coverage_map)

toolchain/mfc/test/test_coverage_unit.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,30 @@ def test_cases_py_change_runs_new_tests_not_skipall():
299299
assert [c.coverage_key() for c in skip] == ["mapped"]
300300

301301

302+
def test_unattributable_nonsource_change_runs_all():
303+
cases = _cases("a")
304+
for f in ("mfc.sh", "toolchain/pyproject.toml", "tests/ABC12345/golden.txt", ".github/workflows/test.yml"):
305+
run, skip, reason = select_tests(cases, {"a": ["src/x.fpp"]}, {f})
306+
assert len(run) == 1 and "run all" in reason, f
307+
308+
302309
def test_docs_only_still_skips_all():
303310
cases = _cases("a")
304-
run, skip, reason = select_tests(cases, {"a": ["src/x.fpp"]}, {"README.md"})
305-
assert run == [] and len(skip) == 1 and "rung7" in reason
311+
for f in ("README.md", "docs/foo.rst", "LICENSE", ".claude/x.md"):
312+
run, skip, reason = select_tests(cases, {"a": ["src/x.fpp"]}, {f})
313+
assert run == [] and "rung7" in reason, f
314+
315+
316+
def test_load_map_rejects_malformed_entry(tmp_path):
317+
import gzip
318+
import json
319+
320+
from mfc.test.coverage import load_map
321+
322+
p = tmp_path / "m.json.gz"
323+
with gzip.open(p, "wt") as fh:
324+
json.dump({"_meta": {"built_at": "x"}, "good": ["a.fpp"], "bad": "not-a-list"}, fh)
325+
assert load_map(Path(p)) == (None, None)
306326

307327

308328
def test_uppercase_fortran_extension_forces_all():

0 commit comments

Comments
 (0)