Skip to content

Add unified C++/Python coverage via xmsconan 2.15.2#126

Merged
gagelarsen merged 5 commits into
masterfrom
feature/coverage
May 17, 2026
Merged

Add unified C++/Python coverage via xmsconan 2.15.2#126
gagelarsen merged 5 commits into
masterfrom
feature/coverage

Conversation

@gagelarsen

@gagelarsen gagelarsen commented May 14, 2026

Copy link
Copy Markdown
Member

Summary

Adds a two-build instrumented Debug pass that produces both C++ (gcovr) and Python (pytest-cov) coverage reports for every push and pull request to master. Pure additive — no source code changes, only build-config and CI plumbing.

What this PR does

  • build.toml: sets [ci].coverage = true and adds a [coverage] table with both thresholds at 0 (report-only mode while we establish the baseline). The existing python_namespaced_dir = "core" is already in place, which is the only other thing the coverage flow needs.
  • .github/workflows/Coverage.yaml (new, generated by xmsconan ci): Linux-only job running directly on ubuntu-latest (containerless under xmsconan 2.15.2, since the old container baked xmsconan in and caused silent floor-pin no-ops). Installs gcovr>=7,<9 and xmsconan>=2.15.2, runs xmsconan_coverage, uploads cov-{cpp,py}.xml and coverage-html-{cpp,py}/ as run artifacts.
  • .github/workflows/XmsCore-CI.yaml: regenerated, xmsconan pin bumped 2.12.12.15.2 and all install steps gained --upgrade so the floor is real (not silently overridden by a pre-installed copy).
  • .flake8, pytest.ini: new files emitted by xmsconan gen from the same template the rest of the family uses.
  • .gitignore: ignores cov-*.xml, cov-*-summary.json, coverage-html-*/ so a developer running coverage locally doesn't accidentally commit a snapshot.
  • README.md: short "Coverage" section explaining the workflow + artifacts.

Baseline numbers from the first green Coverage CI run on the two-build flow

From XmsCore-Coverage run 25978408642 against xmsconan==2.15.2:

Layer Line rate Lines covered
C++ 88.21% 3503 / 3971
Python 72.73% 104 / 143

The C++ jump versus earlier runs of this PR (9.36% on 124/1325 lines) is structural, not a sudden test-writing surge: xmsconan 2.15.2 splits the instrumented build into a CxxTest-driven testing pass and a pytest-driven pybind pass. cov-cpp.xml now reflects coverage from running the CxxTest binaries directly instead of whatever fraction of C++ symbols pytest happened to reach through the pybind ABI. The total line count grew because the testing build compiles the full set of library + testing_sources, whereas the prior pytest-only path only compiled the pybind-linked subset.

Per-file C++ breakdown — current gaps

From the same gcovr report. Files marked "production" are real coverage gaps; testing/TestTools.\* is the test-scaffold library itself, which won't ever be 100% (it has helpers no test currently invokes).

File Lines Coverage Notes
xmscore/misc/DynBitset.cpp 11 0% production — no test currently exercises it
xmscore/misc/XmLog.cpp 104 60% production — error-path branches missing
xmscore/misc/Singleton.h 10 70% template helpers
xmscore/points/functors.h 55 72% inline math helpers
xmscore/misc/Observer.cpp 129 81% progress / observer corner cases
xmscore/misc/XmError.cpp 145 81% ENSURE failure paths
xmscore/misc/StringUtil.cpp 807 89% format / encoding edge cases
xmscore/points/pt.h 590 91% inline point ops
xmscore/time/TimeConversion.cpp 155 92% invalid-input branches
xmscore/dataio/daStreamIo.cpp 1147 96% stream-failure paths
xmscore/testing/TestTools.cpp 156 14% test-scaffold — unused helpers
xmscore/testing/TestTools.h 43 39% test-scaffold — unused helpers

Other files (math.cpp, math.h, Progress.cpp, functors.cpp, pt.cpp, XmError.h) are at 100%.

The thresholds are deliberately 0 so the gate never fails while the baseline is being established. Once test-writing fills the remaining gaps, raise them in a follow-up.

Upstream prerequisites

This works because xmsconan 2.15.2 finally has the coverage flow wired up correctly. The rollout went 2.14.0 → 2.15.2 over several releases, each one uncovering the next downstream gap once the previous one was fixed:

  • 2.14.0 — initial unified coverage release.
  • 2.14.1 (#61 / #62 / #64 / #65 via PR #63): generate_configurations Debug+pybind gate, filter shape, testing/pybind matrix exclusivity, multi-Python pinning.
  • 2.14.2 (#66 via PR #67): conan cache path --folder=source reference-shape fix.
  • 2.14.3 (#69 and #71 via PR #70 + PR #72): XMS_COVERAGE into CMake buildenv; rglob for pytest-cov artifacts; loud-fail guard for empty gcovr summaries.
  • 2.14.4 (direct on master, commit 2e978fe): pass XMS_COVERAGE to CMake as a -D cache variable since conan's [buildenv] activation isn't reaching the CMake subprocess.
  • 2.14.5 (direct on master, commit 0131f6d): anchor gcovr --root and the doubled filter against the build folder, not the conan source folder — sources are copied into the build folder before compilation, so .gcno paths live there.
  • 2.15.2 (PR #76): the combined pybind=True + testing=True + Debug config was structurally fragile — the pybind .so could pick up undefined CxxTest symbols (CxxTest::charToString) at dlopen. Replaced with two independent builds (CxxTest-only testing build for cov-cpp.xml; pybind-only build for cov-py.xml). Also drops the Coverage container so the canary always exercises whatever xmsconan is on devpi, and adds --upgrade to all CI install steps so the in-repo floor is real.

Test plan

  • XmsCore-CI workflow on feature/coverage passes on all three platforms (Mac, Linux, Windows × py3.10/3.13).
  • XmsCore-Coverage workflow produces real artifacts and exits 0.
  • cov-cpp.xml is non-empty Cobertura XML with line-rate="0.8821" and lines-valid="3971".
  • cov-py.xml is non-empty Cobertura XML with line-rate="0.7273" and lines-valid="143".
  • CxxTest binary actually runs during the coverage flow (Total Test time (real) = 1.48 sec in the Coverage job log), and pytest actually runs (25 passed in 0.47s) — both contribute to their respective cov-{cpp,py}.xml outputs.

Follow-ups (not in this PR)

  • Backfill tests for the real C++ gaps: misc/DynBitset.cpp (11 lines, 0%) first, then the error-path branches in XmLog.cpp (60%), Observer.cpp (81%), and XmError.cpp (81%). testing/TestTools.{cpp,h} is the test-scaffold and is expected to stay partially covered.
  • Raise [coverage].cpp_threshold and [coverage].python_threshold once the remaining gaps land, so regressions break the build. cpp_threshold could reasonably go to 80 already.

🤖 Generated with Claude Code

gagelarsen and others added 5 commits May 13, 2026 21:20
- Enable [ci].coverage and add [coverage] table (report-only thresholds)
- Bump xmsconan pin to >=2.14.0 in regenerated CI workflows
- Regenerate CI workflows including new Coverage.yaml
- Document coverage workflow in README

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Running `xmsconan coverage --version <ver> build.toml` locally (e.g. via
WSL) writes the same artifacts the CI workflow uploads into the
workspace root: cov-{cpp,py}.xml, cov-{cpp,py}-summary.json, and the
coverage-html-{cpp,py}/ directories. Keep them out of git so a
developer running coverage doesn't accidentally commit a snapshot.

The recipe-side .gcno/.gcda were already ignored; this rounds out the
list with the workspace-level outputs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xmsconan 2.14.0 had two issues now fixed upstream:
  * The pybind extension was linked against CxxTest, so the coverage
    job's pytest collection failed with an undefined symbol for
    CxxTest::charToString.
  * The C++ coverage report only counted lines exercised by the
    pytest tests; the CxxTest binary's coverage was not included.

Pinning the floor to 2.15.1 forces fresh CI environments to pull the
fix instead of any cached 2.14.x. The README pointer is bumped to
match. Note: these YAMLs carry an "auto-generated by xmsconan_ci"
banner — if the workflows are regenerated, the floor will need to be
re-applied (or set in the generator).
The previous commit raised the version floor to 2.15.1, but the
Coverage workflow still failed with the same CxxTest undefined symbol
because the GitHub runner container
(ghcr.io/aquaveo/conan-gcc13-py3.13:latest) ships with xmsconan
pre-installed, and `pip install xmsconan>=X` is a no-op when the
existing install already satisfies the constraint. Whether a run
passes therefore depends on which xmsconan happens to be baked into
that mutable :latest image at run time — not on the floor we declare.

Add --upgrade so pip actively pulls the newest version that satisfies
the floor from the dev index, regardless of what is pre-installed.
Also `pip show xmsconan` after install so future log inspections show
exactly which xmsconan version actually ran.

Applied to all four pip-install steps: Coverage.yaml plus the three
XmsCore-CI.yaml legs (Linux build, Linux test, Windows).
xmsconan 2.15.2 splits the coverage build into two independent
passes — a testing-only build drives CxxTest under gcov for
cov-cpp.xml, and a separate pybind-only build drives pytest-cov for
cov-py.xml. The single combined pybind+testing config that produced
the `undefined symbol: CxxTest::charToString` leak is no longer in
the matrix.

Regenerated with `xmsconan_ci --version 0.0.0 build.toml`. Notable
changes from the previous generation:

  * Coverage.yaml drops the `container:` block — the workflow now
    runs directly on ubuntu-latest with actions/setup-python@v5.
    This breaks the silent-no-op coupling where the container's
    pre-installed xmsconan satisfied `>=X.Y.Z` and locked the canary
    to whatever xmsconan the image happened to carry.
  * All four pip-install steps stay on `pip install --upgrade
    "xmsconan>=2.15.2"` so they continue to pull the latest matching
    release from devpi on every run.

The previous manual `pip show xmsconan` diagnostic lines were
dropped — they aren't part of the canonical template, and with the
container removed there's no longer a hidden-version footgun for
that diagnostic to surface. README pointer bumped to match.
@gagelarsen gagelarsen changed the title Add unified C++/Python coverage via xmsconan 2.14.5 Add unified C++/Python coverage via xmsconan 2.15.2 May 17, 2026

@gagelarsen gagelarsen left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review of PR #126

Scope: changes in this PR only — pre-existing code is out of scope per the user's review policy.

Pure additive build-config/CI plumbing — no production C++ or Python source touched, so coverage/testing policy has nothing to enforce on this diff. The PR body is unusually well-grounded (numbers, per-file table, and upstream issue chain all verified against the latest XmsCore-Coverage run 25978408642). All three CI legs are green at 634155b9. No blockers. One scope-correct observation worth flagging: the new .flake8 is effectively inert under the (pre-existing) CI lint invocation.

Blockers

None.

Recommendations

  • .flake8 (new file) is not consulted by the existing CI lint step.github/workflows/XmsCore-CI.yaml already invokes flake8 --isolated … with a hand-rolled argument list (that line is pre-existing, not changed by this PR — git diff master..feature/coverage -- .github/workflows/XmsCore-CI.yaml only touches the three xmsconan>=2.15.2 --upgrade install lines). Because --isolated ignores every config file, the rules the new .flake8 introduces beyond the CLI args — the B028, W503 additions to ignore, the banned-modules = osgeo.*=… rule, and the tests/files/* / tests/fixtures/* excludes — are not actually enforced on PRs. Scope-respecting recommendation: either drop .flake8 from this PR (since nothing exercises it), or accept that it's local-dev-only and document that in a comment at the top of the file. A CI-side change to honour .flake8 (removing --isolated and the inline args) is the cleaner long-term fix but is out of scope for this PR — file it as a follow-up. Not a release blocker.

Verified

  • All three install steps in XmsCore-CI.yaml that this PR touches pin xmsconan>=2.15.2 with --upgrade and consistent quoting (mac line 108, linux line 242, windows line 387). These are the only lines this PR changes in that file.
  • Coverage.yaml (new) is containerless on ubuntu-latest, uses actions/setup-python@v5, installs gcovr>=7,<9 and xmsconan>=2.15.2 --upgrade, and uploads exactly the two artifact bundles the README documents.
  • build.toml diff adds [ci].coverage = true and a [coverage] table with thresholds of 0, matching the PR body's "report-only while baseline is being established" framing.
  • .gitignore diff covers all six local artifact patterns: cov-{cpp,py}.xml, cov-{cpp,py}-summary.json, coverage-html-{cpp,py}/.
  • README.md diff adds only a new Coverage section that accurately describes the new flow (Linux-only, two HTML reports + two XML files, thresholds at 0). No other README sections are touched by this PR.
  • pytest.ini (new) defines testpaths = _package/tests and python_files = test_*.py *_test.py *_pyt.py, matching what Coverage CI consumes.
  • PR body numbers cross-check exactly against the Coverage run log: gcovr TOTAL 3971 3503 88%, pytest 25 passed in 0.47s, CxxTest Total Test time (real) = 1.48 sec. Per-file table matches the gcovr report line-for-line (DynBitset 11/0/0%, XmLog 104/63/60%, Singleton 10/7/70%, functors.h 55/40/72%, Observer 129/105/81%, XmError 145/118/81%, StringUtil 807/720/89%, pt.h 590/540/91%, TimeConversion 155/144/92%, daStreamIo 1147/1108/96%, TestTools.cpp 156/22/14%, TestTools.h 43/17/39%). "Other files at 100%" claim also verified: math.cpp 58, math.h 26, Progress.cpp 86, functors.cpp 105, pt.cpp 342, XmError.h 2 — all 100% in the log.
  • Two-build flow visible in the run log: first build with pybind=False testing=True (testing pass → cov-cpp.xml from CxxTest runner), second build with pybind enabled (pytest pass → cov-py.xml).
  • Branch history is clean — only commits relevant to the coverage feature: 22d4df7 (gitignore), 90199f7 (initial 2.14.0 wire-up), e013bce (2.15.1 bump), e3c8003 (--upgrade fix), 634155b (regenerate at 2.15.2).
  • git diff master..feature/coverage --stat is 147 lines added across 7 files. No accidentally committed binaries, secrets, or local config.
  • Code conventions: nothing to check — no C++/Python production code in the diff.

Notes

  • .flake8 line 20 sets isolated = true as a config-file key. That key has no effect inside a config file (it's a CLI flag); harmless but slightly misleading if someone later edits .flake8 expecting that knob to mean something. In-scope because .flake8 is new in this PR.
  • Coverage.yaml (new in this PR) uses actions/checkout@v2 while it uses @v5 for setup-python and @v4 for upload-artifact. The newly-added file is internally inconsistent on action versions; worth aligning to @v4 at minimum within this PR's file.
  • Per the PR body's own follow-up list, raising cpp_threshold to 80 once the gaps land is the obvious next step — and DynBitset.cpp at 0% / 11 lines is the cheapest gap to close first. Worth filing as an issue tied to this PR's merge so the follow-up doesn't get lost.

Scope correction note: A prior version of this review flagged (a) the --isolated flag in XmsCore-CI.yaml's lint step and (b) the README Testing section recommending python -m unittest discover. Both of those targets are pre-existing code that this PR does not modify, so per the user's "changes-only" review policy they have been dropped from the recommendations. The .flake8-is-inert observation is retained but reframed to avoid prescribing changes to pre-existing CI lines.

@gagelarsen gagelarsen merged commit b0715bc into master May 17, 2026
24 checks passed
@gagelarsen gagelarsen deleted the feature/coverage branch May 17, 2026 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant