From 66ff22c77722cedf031c717f47dcec4fcd848ade Mon Sep 17 00:00:00 2001 From: Rafael Palomar Date: Fri, 11 Sep 2026 08:47:33 +0200 Subject: [PATCH 1/2] COMP: Stop the Codecov flag filters from dropping three modules The reported whole-tree percentage was measured over a subset of the extension. Three faults, all silent -- no warning, no failed job: `flags..paths` in .codecov.yml removes files from the report rather than merely un-flagging them, and the list had fallen behind the module layout: it named LiverMarkups/ (module deleted) and Modeling/ (never existed), placed LiverResectionsLib/ one level too high, pointed the py flag at **/Testing/Python/ (which the ignore list discards anyway), and omitted Liver/, LiverSegmentation/ and SlicerLiverInteractionLib/. Those three modules were therefore absent from the percentage even though the coverage job measured them. Flag membership now comes from the upload alone. A single codecov-action step with `files: a,b` + `flags: cxx,py` is one session carrying both flags, so neither flag isolated its language -- the tell was `sessions: 1` on a repo with two flags. Upload once per flag instead. SlicerLiverInteractionLib was missing from the pytest --cov targets, so the base four modules share was never measured at all, and the three newer staged Lib packages had no .coveragerc [paths] alias, leaving their launched-leg records on build-tree paths instead of merging onto the source files. Because the first two faults change the denominator, they also made the percentage non-comparable across commits: the 56.29 -> 51.61 step in August falls on an ADR-plus-workflow commit that adds no source. Coverage stays informational per ADR-0021; this fixes what the number is computed over, not whether it gates. --- .codecov.yml | 30 +++++++++++++++++------------- .coveragerc | 9 +++++++++ .github/workflows/ci.yml | 22 +++++++++++++++++++--- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 0df420b1..52bcfc56 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -25,7 +25,9 @@ codecov: # some notification settings. notify: wait_for_ci: true - after_n_builds: 1 + # Both flags upload from the same job as separate steps; wait for + # both before posting so the comment never reflects half the tree. + after_n_builds: 2 coverage: precision: 2 @@ -46,21 +48,23 @@ comment: require_changes: true layout: "diff,flags,files" +# Flag membership comes from the upload itself: the coverage job emits +# coverage-cxx.xml (gcovr) and coverage-py.xml (coverage.py) and uploads +# each under its own flag. Do NOT re-filter by `paths` here. +# +# A stale `paths` list silently DROPS files from the report rather than +# merely un-flagging them: the previous list named LiverMarkups/ (module +# deleted) and Modeling/ (never existed), placed LiverResectionsLib/ at +# the wrong level, pointed the py flag at **/Testing/Python/ (which the +# ignore list below discards anyway), and omitted Liver/, +# LiverSegmentation/ and SlicerLiverInteractionLib/ entirely -- so those +# three modules were absent from the whole-tree percentage even though +# the coverage job measured them. flags: cxx: - paths: - - LiverResections/ - - LiverResectionsLib/ - - LiverMarkups/ - - VascularTerritories/ - - LiverVolumetry/ - - Modeling/ + carryforward: false py: - paths: - - LiverResections/Testing/Python/ - - LiverResectionsLib/Testing/Python/ - - VascularTerritories/ - - LiverVolumetry/ + carryforward: false # Exclude generated, test-only, and third-party code from both # coverage % and PR comment annotations. These paths are uninteresting diff --git a/.coveragerc b/.coveragerc index 498b7869..949339b3 100644 --- a/.coveragerc +++ b/.coveragerc @@ -29,3 +29,12 @@ liverresectionslib = liversegmentationlib = LiverSegmentation/LiverSegmentationLib/ */qt-scripted-modules/LiverSegmentationLib/ +livervolumetrylib = + LiverVolumetry/LiverVolumetryLib/ + */qt-scripted-modules/LiverVolumetryLib/ +slicerliverinteractionlib = + SlicerLiverInteractionLib/ + */qt-scripted-modules/SlicerLiverInteractionLib/ +vascularterritorieslib = + VascularTerritories/VascularTerritoriesLib/ + */qt-scripted-modules/VascularTerritoriesLib/ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e72bd0c0..4bd859ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -662,6 +662,7 @@ jobs: --cov=LiverSegmentation \ --cov=LiverResections/LiverResectionsLib \ --cov=LiverVolumetry \ + --cov=SlicerLiverInteractionLib \ --cov=VascularTerritories \ --cov-report= \ || true @@ -721,12 +722,18 @@ jobs: XML } - - name: Upload coverage to Codecov + # Two uploads, one per flag. A single step with + # ``files: a,b`` + ``flags: cxx,py`` produces ONE session carrying + # BOTH flags, so neither flag isolates its own language and the + # per-flag figures on the Codecov UI are meaningless (the symptom + # is ``sessions: 1`` on every commit while two flags exist). + # Splitting the step is what makes the cxx/py split real. + - name: Upload C++ coverage to Codecov if: github.event_name != 'pull_request' || steps.changes.outputs.any_changed == 'true' uses: codecov/codecov-action@v5 with: - files: ./coverage-cxx.xml,./coverage-py.xml - flags: cxx,py + files: ./coverage-cxx.xml + flags: cxx # Non-blocking: per ADR-0021 the job's pass/fail is not a # branch-protection signal. An upload failure (Codecov # outage, OAuth not yet configured by the maintainer, etc.) @@ -745,3 +752,12 @@ jobs: # ``CODECOV_TOKEN`` secret. Rotating the token is a # maintainer-only step on the Codecov side. token: ${{ secrets.CODECOV_TOKEN }} + + - name: Upload Python coverage to Codecov + if: github.event_name != 'pull_request' || steps.changes.outputs.any_changed == 'true' + uses: codecov/codecov-action@v5 + with: + files: ./coverage-py.xml + flags: py + fail_ci_if_error: false + token: ${{ secrets.CODECOV_TOKEN }} From 0d1a247e0589811f45405f9c9a833b79359b67d5 Mon Sep 17 00:00:00 2001 From: Rafael Palomar Date: Fri, 11 Sep 2026 08:47:43 +0200 Subject: [PATCH 2/2] DOC: Record the Codecov denominator traps in ADR-0021 Both faults fixed in the preceding commit corrupt the reported percentage without surfacing an error, and both are easy to reintroduce: a flag path list that silently drops files, and a single upload step that collapses two flags into one session. Also note the standing obligation: a new Python sub-package staged into qt-scripted-modules/ needs a .coveragerc [paths] alias, or its launched-leg records never merge onto the source file. Adds the reading rule that would have caught this sooner -- when a coverage swing has no plausible cause in the diff, check the file and line totals before treating it as a test-quality regression. --- Docs/adr/0021-coverage-measurement.md | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Docs/adr/0021-coverage-measurement.md b/Docs/adr/0021-coverage-measurement.md index 2cf1b7b5..408be2e8 100644 --- a/Docs/adr/0021-coverage-measurement.md +++ b/Docs/adr/0021-coverage-measurement.md @@ -109,6 +109,34 @@ absolute-% threshold; ratchet pressure is per-PR. PR comment; tests still run. - Coverage % drift on rebases can produce noisy PR comments. +## Configuration traps (learned 2026-09-11) + +The whole-tree percentage is only as honest as the upload wiring, and +two settings can corrupt it silently -- no warning, no failed job, just +a wrong number: + +- **`flags..paths` in `.codecov.yml` DROPS files, it does not + merely un-flag them.** A path list that has fallen behind the module + layout removes those modules from the reported percentage entirely. + This is the trap that hid `Liver/`, `LiverSegmentation/` and + `SlicerLiverInteractionLib/` while the coverage job was measuring + them. Flag membership should come from the upload, not from a second + filter that has to be kept in sync with the tree. +- **One `codecov-action` step with `files: a,b` + `flags: x,y` is ONE + session carrying BOTH flags**, so neither flag isolates its language. + The tell is `sessions: 1` on a repo with two flags. Upload once per + flag. + +Because both faults change the *denominator*, they also make the +percentage non-comparable across commits: a docs-only commit can appear +to move coverage by several points. When a coverage swing has no +plausible cause in the diff, check the file and line totals before +reading it as a test-quality regression. + +Any new Python sub-package staged into `qt-scripted-modules/` needs a +`[paths]` alias in `.coveragerc`, or its launched-leg records stay on +build-tree paths and never merge onto the source file. + ## References - [ADR-0003][adr-0003] — Testability invariant.