ci: skip pre-commit hook on automated cache commit#1319
ci: skip pre-commit hook on automated cache commit#1319sbryngelson wants to merge 11 commits intoMFlowCode:masterfrom
Conversation
…rify empty gcno_copies return
…uld_run_all_tests
Claude Code ReviewHead SHA: 04a9f93 Files changed:
Summary:
Findings: 1. github.event_name == 'push' &&
(needs.file-changes.outputs.cases_py == 'true' || ...)There is no 2. Rebase-then-push race in git rebase origin/master
git push origin HEAD:refs/heads/masterThere is no retry loop around the push. If another commit lands on master between the 3. New-file coverage gap: no test covers a brand-new 4. 5. Fortran source lint / convention — no violations found 6. gcda_count = sum(1 for _, _, fns in os.walk(sample_build) for f in fns if f.endswith(".gcda"))The loop variable 7. Thread-safety note: cons output interleaving (coverage.py build_coverage_cache) |
There was a problem hiding this comment.
Pull request overview
This PR introduces file-level gcov coverage caching and CI-driven test pruning to speed up PR test runs, and adjusts CI to rebuild/commit the cache automatically (including skipping pre-commit hooks for the automated cache commit).
Changes:
- Add a file-level gcov coverage cache builder (
--build-coverage-cache) and a pruning mode (--only-changes) that selects tests based on changed.fppfiles. - Extend CI to (a) detect cache-staleness triggers, (b) rebuild/upload/commit the cache, and (c) run pruned test suites on PRs using the downloaded cache.
- Update build system to improve gcov compatibility (Fypp line markers, disable LTO for gcov, adjust Release optimization for gcov).
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
toolchain/mfc/test/coverage.py |
New coverage cache builder + changed-file detection + test filtering logic |
toolchain/mfc/test/test.py |
Wire --only-changes pruning and --build-coverage-cache execution into test runner |
toolchain/mfc/cli/commands.py |
Add CLI flags for building cache and pruning based on changes |
.github/workflows/test.yml |
Add cache rebuild job, artifact flow, gating, and --no-verify commit; enable pruning on PRs |
.github/workflows/common/rebuild-cache.sh |
New script to build gcov-instrumented binaries and generate cache on SLURM runner |
.github/workflows/common/test.sh |
Prune tests on PRs only (keeps full suite on master pushes) |
CMakeLists.txt |
Adjust gcov flags/behavior (Release -O1, disable LTO for gcov, Fypp line markers) |
toolchain/mfc/test/case.py |
Refactor post_process param mods into shared constants/helpers used by cache builder |
toolchain/mfc/test/test_coverage_unit.py |
Add offline unit tests for coverage cache logic |
toolchain/mfc/test/test_coverage_cache.json.gz |
Add committed gzipped cache artifact |
.github/file-filter.yml |
Add common workflow paths + cases_py filter |
.github/scripts/submit-slurm-job.sh |
Pass GITHUB_EVENT_NAME into SLURM job script |
.gitignore |
Ignore legacy raw JSON cache file |
toolchain/mfc/test/cases.py |
Skip 1D_qbmm example (gfortran 12 overflow note) |
toolchain/mfc/viz/interactive.py |
Remove stray whitespace line |
| if n_jobs is None: | ||
| # Caller should pass n_jobs explicitly on SLURM systems; | ||
| # os.cpu_count() may exceed the SLURM allocation. | ||
| n_jobs = max(os.cpu_count() or 1, 1) | ||
| # Cap Phase 2 test parallelism: each test spawns gcov-instrumented MPI | ||
| # processes (~2-5 GB each under gcov). Too many concurrent tests cause OOM. | ||
| # Phase 3 gcov workers run at full n_jobs (gcov is lightweight by comparison). | ||
| phase2_jobs = min(n_jobs, 16) | ||
| cons.print(f"[bold]Building coverage cache for {len(cases)} tests ({phase2_jobs} test workers, {n_jobs} gcov workers)...[/bold]") |
| # Create minimal stubs for toolchain modules so coverage.py can be imported | ||
| # without the full MFC toolchain being on sys.path. | ||
| def _make_stub(name): | ||
| mod = types.ModuleType(name) | ||
| sys.modules[name] = mod | ||
| return mod | ||
|
|
||
|
|
||
| for _mod_name in [ | ||
| "toolchain", | ||
| "toolchain.mfc", | ||
| "toolchain.mfc.printer", | ||
| "toolchain.mfc.common", | ||
| "toolchain.mfc.build", | ||
| "toolchain.mfc.test", | ||
| "toolchain.mfc.test.case", | ||
| ]: | ||
| if _mod_name not in sys.modules: | ||
| _make_stub(_mod_name) | ||
|
|
||
| # Provide the attributes coverage.py needs from its relative imports | ||
| _printer_stub = sys.modules.get("toolchain.mfc.printer", _make_stub("toolchain.mfc.printer")) | ||
|
|
||
|
|
||
| class _FakeCons: | ||
| def print(self, *args, **kwargs): | ||
| pass # suppress output during tests | ||
|
|
||
|
|
||
| _printer_stub.cons = _FakeCons() | ||
|
|
||
| _common_stub = sys.modules.get("toolchain.mfc.common", _make_stub("toolchain.mfc.common")) | ||
| _common_stub.MFC_ROOT_DIR = "/fake/repo" | ||
|
|
||
|
|
||
| class _FakeMFCException(Exception): | ||
| pass | ||
|
|
||
|
|
||
| _common_stub.MFCException = _FakeMFCException | ||
|
|
||
| _build_stub = sys.modules.get("toolchain.mfc.build", _make_stub("toolchain.mfc.build")) | ||
| _build_stub.PRE_PROCESS = "pre_process" | ||
| _build_stub.SIMULATION = "simulation" | ||
| _build_stub.POST_PROCESS = "post_process" | ||
|
|
||
| _case_stub = sys.modules.get("toolchain.mfc.test.case", _make_stub("toolchain.mfc.test.case")) | ||
| _case_stub.input_bubbles_lagrange = lambda case: None | ||
| _case_stub.get_post_process_mods = lambda params: {} | ||
| _case_stub.POST_PROCESS_3D_PARAMS = { | ||
| "fd_order": 1, | ||
| "omega_wrt(1)": "T", | ||
| "omega_wrt(2)": "T", | ||
| "omega_wrt(3)": "T", | ||
| } | ||
|
|
||
| # Load coverage.py by injecting stubs into sys.modules so relative imports resolve. | ||
| _COVERAGE_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "coverage.py") | ||
|
|
||
| sys.modules.pop("toolchain.mfc.test.coverage", None) # reset if already loaded | ||
|
|
||
| _spec = importlib.util.spec_from_file_location("toolchain.mfc.test.coverage", _COVERAGE_PATH, submodule_search_locations=[]) | ||
| _coverage_mod = importlib.util.module_from_spec(_spec) | ||
| _coverage_mod.__package__ = "toolchain.mfc.test" | ||
|
|
||
| sys.modules["toolchain.mfc.test"] = types.ModuleType("toolchain.mfc.test") | ||
| sys.modules["toolchain.mfc.test"].__package__ = "toolchain.mfc.test" | ||
|
|
| def test_parse_single_file(self): | ||
| result = _parse_diff_files("src/simulation/m_rhs.fpp\n") | ||
| assert result == {"src/simulation/m_rhs.fpp"} | ||
|
|
||
| def test_parse_multiple_files(self): | ||
| text = "src/simulation/m_rhs.fpp\nsrc/simulation/m_weno.fpp\nREADME.md\n" | ||
| result = _parse_diff_files(text) | ||
| assert result == { | ||
| "src/simulation/m_rhs.fpp", | ||
| "src/simulation/m_weno.fpp", | ||
| "README.md", | ||
| } | ||
|
|
||
| def test_parse_empty(self): | ||
| assert _parse_diff_files("") == set() | ||
| assert _parse_diff_files("\n") == set() | ||
|
|
||
| def test_parse_ignores_blank_lines(self): | ||
| text = "src/simulation/m_rhs.fpp\n\n\nsrc/simulation/m_weno.fpp\n" | ||
| result = _parse_diff_files(text) | ||
| assert result == {"src/simulation/m_rhs.fpp", "src/simulation/m_weno.fpp"} | ||
|
|
||
| def test_parse_mixed_extensions(self): | ||
| text = "src/simulation/m_rhs.fpp\ntoolchain/mfc/test/cases.py\nCMakeLists.txt\n" | ||
| result = _parse_diff_files(text) | ||
| assert len(result) == 3 | ||
| assert "toolchain/mfc/test/cases.py" in result | ||
| assert "CMakeLists.txt" in result | ||
|
|
| rebuild-cache: | ||
| name: Rebuild Coverage Cache | ||
| needs: [lint-gate, file-changes] | ||
| if: >- | ||
| github.repository == 'MFlowCode/MFC' && | ||
| ( | ||
| (github.event_name == 'pull_request' && | ||
| (needs.file-changes.outputs.cases_py == 'true' || | ||
| needs.file-changes.outputs.dep_changed == 'true')) || | ||
| (github.event_name == 'push' && | ||
| (needs.file-changes.outputs.cases_py == 'true' || | ||
| needs.file-changes.outputs.dep_changed == 'true')) || | ||
| github.event_name == 'workflow_dispatch' | ||
| ) | ||
| timeout-minutes: 240 | ||
| runs-on: | ||
| group: phoenix | ||
| labels: gt | ||
| permissions: | ||
| contents: write # Required for Commit Cache to Master on push events | ||
| steps: | ||
| - name: Clone | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} | ||
| clean: false | ||
|
|
||
| - name: Rebuild Cache via SLURM | ||
| run: bash .github/scripts/submit-slurm-job.sh .github/workflows/common/rebuild-cache.sh cpu none phoenix | ||
|
|
||
| - name: Print Logs | ||
| if: always() | ||
| run: cat rebuild-cache-cpu-none.out | ||
|
|
||
| - name: Upload Cache Artifact | ||
| if: github.event_name == 'pull_request' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coverage-cache | ||
| path: toolchain/mfc/test/test_coverage_cache.json.gz | ||
| retention-days: 1 | ||
|
|
||
| - name: Commit Cache to Master | ||
| if: (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && github.ref == 'refs/heads/master' | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| git add toolchain/mfc/test/test_coverage_cache.json.gz | ||
| if git diff --cached --quiet; then | ||
| echo "Coverage cache unchanged." | ||
| else | ||
| git commit --no-verify -m "Regenerate gcov coverage cache [skip ci]" | ||
| # Rebase onto latest master in case it advanced during the build | ||
| # (which can take 20-240 minutes). Only test_coverage_cache.json.gz | ||
| # is changed, so rebase conflicts are essentially impossible. | ||
| git fetch origin master | ||
| git rebase origin/master | ||
| git push origin HEAD:refs/heads/master | ||
| fi |
| # Override Release -O3 with -O1 for gcov: coverage instrumentation is | ||
| # inaccurate at -O3, and aggressive codegen (e.g. AVX-512 FP16 on | ||
| # Granite Rapids) can emit instructions that older assemblers reject. | ||
| set(CMAKE_Fortran_FLAGS_RELEASE "-O1 -DNDEBUG" CACHE STRING "" FORCE) |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request introduces a comprehensive coverage cache and test filtering infrastructure for the MFC toolchain. It adds GitHub Actions workflows to build and maintain a gcov-based coverage cache, new CLI options (--build-coverage-cache and --only-changes) to support coverage-driven test selection, CMake support for gcov instrumentation without LTO/IPO conflicts, and a new coverage module that computes file-level coverage data and filters tests based on changed files. The implementation includes workflow orchestration via SLURM, cache artifact management, test pruning logic with fallback mechanisms, and extensive unit test coverage for the new functionality. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Important Merge conflicts detected (Beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can get early access to new features in CodeRabbit.Enable the |
Summary
rebuild-cacheCI job'sgit committriggers the pre-commit hook, which runsprecheckon the entire repo. If any other merged PR introduced a spelling/lint issue, the cache commit fails — even though onlytest_coverage_cache.json.gzis being committed.--no-verifyto the automatedgit commitin the "Commit Cache to Master" step.Test plan
rebuild-cachejob succeeds on the next push to master🤖 Generated with Claude Code