Skip to content

Commit 9d90188

Browse files
sbryngelsonclaude
andcommitted
Add case.fpp, cmake/, coverage.py to ALWAYS_RUN_ALL; fix retry loop and missing binary warning
- case.fpp and toolchain/cmake/ are invisible to gcov (Fypp-inlined or non-Fortran) but affect all tests. coverage.py itself must trigger a full run to validate pruning logic changes. - Fix commit-cache retry: pushed=true was emitted even when all 3 push attempts failed. - Warn when a target binary is missing during cache build so cache quality issues are visible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 240ef0a commit 9d90188

File tree

3 files changed

+40
-6
lines changed

3 files changed

+40
-6
lines changed

.github/workflows/test.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,12 +404,13 @@ jobs:
404404
git commit -m "Regenerate gcov coverage cache
405405
406406
Automatically rebuilt because cases.py changed."
407+
pushed=false
407408
for i in 1 2 3; do
408-
git pull --rebase && git push && break
409+
git pull --rebase && git push && { pushed=true; break; }
409410
echo "Push attempt $i failed, retrying in 5s..."
410411
sleep 5
411412
done
412-
echo "pushed=true" >> "$GITHUB_OUTPUT"
413+
echo "pushed=$pushed" >> "$GITHUB_OUTPUT"
413414
fi
414415
415416
- name: Post PR Comment

toolchain/mfc/test/coverage.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,21 @@
4545
"src/common/include/omp_macros.fpp",
4646
"src/common/include/shared_parallel_macros.fpp",
4747
"src/common/include/macros.fpp",
48+
"src/common/include/case.fpp",
4849
"toolchain/mfc/test/cases.py",
4950
"toolchain/mfc/test/case.py",
5051
"toolchain/mfc/params/definitions.py",
5152
"toolchain/mfc/run/input.py",
5253
"toolchain/mfc/case_validator.py",
54+
"toolchain/mfc/test/coverage.py",
5355
"CMakeLists.txt",
5456
])
5557

58+
# Directory prefixes: any changed file under these paths triggers full suite.
59+
ALWAYS_RUN_ALL_PREFIXES = (
60+
"toolchain/cmake/",
61+
)
62+
5663

5764
def _get_gcov_version(gcov_binary: str) -> str:
5865
"""Return the version string from gcov --version."""
@@ -304,6 +311,8 @@ def _run_single_test_direct(test_info: dict, gcda_dir: str, strip: str) -> tuple
304311
failures = []
305312
for target_name, bin_path in binaries:
306313
if not os.path.isfile(bin_path):
314+
cons.print(f"[yellow]Warning: binary {target_name} not found "
315+
f"at {bin_path} for test {uuid}[/yellow]")
307316
continue
308317
cmd = mpi_cmd + [bin_path]
309318
try:
@@ -574,12 +583,16 @@ def get_changed_files(root_dir: str, compare_branch: str = "master") -> Optional
574583

575584
def should_run_all_tests(changed_files: set) -> bool:
576585
"""
577-
Return True if any changed file is in ALWAYS_RUN_ALL.
586+
Return True if any changed file is in ALWAYS_RUN_ALL or under
587+
ALWAYS_RUN_ALL_PREFIXES.
578588
579-
GPU macro files and toolchain files cannot be correctly analyzed by CPU
580-
coverage — changes to them must always trigger the full test suite.
589+
GPU macro files, Fypp includes, and build system files cannot be
590+
correctly analyzed by CPU coverage — changes to them must always
591+
trigger the full test suite.
581592
"""
582-
return bool(changed_files & ALWAYS_RUN_ALL)
593+
if changed_files & ALWAYS_RUN_ALL:
594+
return True
595+
return any(f.startswith(ALWAYS_RUN_ALL_PREFIXES) for f in changed_files)
583596

584597

585598
def filter_tests_by_coverage(

toolchain/mfc/test/test_coverage_unit.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,26 @@ def test_cmakelists_triggers_all(self):
254254
{"CMakeLists.txt"}
255255
) is True
256256

257+
def test_case_fpp_triggers_all(self):
258+
assert should_run_all_tests(
259+
{"src/common/include/case.fpp"}
260+
) is True
261+
262+
def test_coverage_py_triggers_all(self):
263+
assert should_run_all_tests(
264+
{"toolchain/mfc/test/coverage.py"}
265+
) is True
266+
267+
def test_cmake_dir_triggers_all(self):
268+
assert should_run_all_tests(
269+
{"toolchain/cmake/FindFFTW.cmake"}
270+
) is True
271+
272+
def test_cmake_subdir_triggers_all(self):
273+
assert should_run_all_tests(
274+
{"toolchain/cmake/some/nested/file.cmake"}
275+
) is True
276+
257277
def test_simulation_module_does_not_trigger_all(self):
258278
assert should_run_all_tests(
259279
{"src/simulation/m_rhs.fpp"}

0 commit comments

Comments
 (0)