Coverage in package_id, per-configuration test jobs, and a report-only Coverage gate - #128
Conversation
5725256 to
d180151
Compare
Review: xmsconan PR #128 — coverage in
|
|
Addressed in a700775. What changed, what did not, and one question back. AppliedM2 — two pybind build types render one GitLab job key. Confirmed, and worse than stated: the collision is not between the two instrumented jobs, it is between the Release configuration's
M1 — The two unguarded YAML keys (
Docs. Broader than the finding stated:
Not appliedBoth CRITICALs — dropped on evidence during validation, before this commit:
~19 test-organization and template-duplication entries from the unvalidated minor pool. Declined as scope: they are pre-existing structure, not something this branch introduced, and folding them in would make the diff harder to review than the restructure it is reviewing. Question back
|
Post-mortem items 1-3 from the xmsvtk coverage investigation. Item 1 is the
root cause; items 2 and 3 are what it was hiding, and the pipeline
restructure is what item 3 turned out to require.
Instrumentation reached the build through XMS_COVERAGE=1 in a profile's
[buildenv], which conan does not see. An instrumented binary and a
production binary therefore shared a package_id, so a coverage run could
satisfy --build=missing for a production build and vice versa. The
generated recipe grows a `coverage` option instead, so instrumentation
travels with the identity; package_id() dels it when False, which keeps
every uninstrumented id byte-identical to the ids this recipe produced
before the option existed -- no republish, no cache invalidation for normal
builds. The generated CMakeLists.txt no longer reads $ENV{XMS_COVERAGE} as
a fallback, and plan_cmake_presets writes the variable into every configure
preset's cacheVariables, because a raw `cmake --preset` build never runs the
recipe's build(). Only the two coverage legs are instrumented, not every
configuration the matrix builds.
The split-out GitLab test job found its artifact directory by falling back
to the first of Debug-testing, Release-testing that existed. A matrix
building both therefore compiled the Release runner on every pipeline and
never executed it, with nothing red to show for it -- the warning naming the
choice went to a job log nobody reads. The template now emits one
"Run C++ Tests - <label>" job per surviving testing configuration, each
passing --label and uploading only its own test_artifacts/<label>/. The
labels come from ci_filter_effects rather than from ci_build_types, which
counts every survivor: a `pybind = true` pin keeps Release configurations
while staging no runner at all. _config_label became the module-level
config_label, so the generator names those directories with the same
function the build writes them with; a template concatenating
"<build_type>-testing" would be a second implementation free to drift.
split_tests with no surviving testing configuration is now rejected at
generation, because splitting exports XMS_SKIP_CXX_TESTS=1 and the suite
would run nowhere while the pipeline passed having tested nothing.
The Coverage job did everything -- two instrumented conan builds, gcovr, the
HTML render, and the threshold gate -- in one job at the end of the
pipeline, so the slowest work started last and the gate could only be
exercised by driving two real conan builds. run_coverage now takes --phase.
coverage-status.json carries what no report file records: whether either
leg's tests failed, and whether each layer was measured. Thresholds stay out
of it, so tightening one re-gates existing artifacts without rebuilding.
allow_failure: exit_codes: [3] stays on the report job only -- collect
cannot produce a 3, so anything it fails on is a real breakage -- and the
coverage: regex reads the report phase's own "Coverage total: <pct>%" line,
so an unmeasured C++ layer prints n/a and cannot publish a percentage that
looks like a measurement.
Splitting collect from report exposed the real cost. "Coverage Build"
recompiled the Debug testing and pybind configurations, because
instrumentation is part of the package_id and no production binary can
satisfy it: 503 of the 1334 objects a pipeline compiled were that second
copy, and the single "Conan Build" compiled its whole matrix sequentially
inside one container. So a wheel_only repository now gets one build job per
surviving Linux configuration, each declaring `needs: []` so it starts at
t=0 rather than at its stage's turn, and a third phase -- `--phase measure
--leg <cpp|python>` -- builds one instrumented leg and reads its .gcda where
it was compiled. What leaves each job is a tracefile of a few hundred KB: at
245 MiB of .gcno against 5.5 MiB of .gcda, moving a build folder would cost
more than the compile it saves. Tag pipelines build only what ships, so
testing configurations are branch-only and the instrumented pybind job has a
clean `only: tags` twin, since --coverage changes the binary a release
publishes. Every taggable job exports its own cache tarball, because with
one container per configuration `conan cache save` in the pybind job sees
the pybind package and nothing else -- naming a single exporter would
publish one package id and silently drop the rest. The shape is gated on
[matrix].wheel_only: it was designed and measured against a matrix with no
library configurations, and a repository that publishes library packages
keeps what it had until that shape is proven too.
Two build types in [matrix].pybind_build_types would have collided in the
new fan-out. A pybind job's name carried the ABI but not the build type, so
the Release configuration's `only: tags` twin and the Debug configuration's
job both rendered as "Python Build" -- and a GitLab job name is a YAML
mapping key, so the later block silently replaced the earlier and that
configuration was never built. The build type now joins the name when the
fan-out spans two, a second pybind build type is no longer instrumented
(the Python leg's filter pins one, via a shared COVERAGE_PYBIND_BUILD_TYPE
the CI planner and the coverage run both read), and naming ends with a
uniqueness check that raises rather than shipping a pipeline that quietly
builds less than the matrix asked for. Alongside it, `dependencies:` on the
deploy job and `needs:` on the Coverage job are guarded on a non-empty
list: both render as null under a narrowing filter and GitLab rejects the
file, including in the case coverage_conflicts() warns about and then
generates anyway.
USAGE.md documents the measure phase and --leg, corrects the claim that the
report phase is pure JSON in (it shells out to gcovr to merge per-leg
tracefiles), and describes the wheel_only Coverage stage.
a700775 to
756f949
Compare
1 participant
Post-mortem items 1-3 from the xmsvtk coverage investigation, plus the pipeline restructure that item 3 turned out to require. Thirteen commits; each stands alone, and they are ordered because everything downstream depends on item 1's option being what carries instrumentation.
1. Coverage instrumentation is part of the package identity (
653dadb)Instrumentation reached the build through
XMS_COVERAGE=1in a profile's[buildenv], which conan does not see. An instrumented binary and a production binary therefore shared apackage_id, so a coverage run could satisfy--build=missingfor a production build — and vice versa.coverageoption,[True, False], defaultFalse, so instrumentation travels with thepackage_id.package_id()dels the option whenFalse, keeping every uninstrumented id byte-identical to the ids this recipe produced before the option existed. No republish, no cache invalidation for normal builds.plan_cmake_presetswritesXMS_COVERAGEinto every configure preset'scacheVariables, because a rawcmake --presetbuild never runs the recipe'sbuild().CMakeLists.txtno longer reads$ENV{XMS_COVERAGE}as a fallback. The option is the sole driver.bd32377narrows it further: only the two coverage legs are instrumented, not every configuration the matrix builds.2. One test job per staged testing configuration (
298fb5c)The split-out GitLab test job found its artifact directory by falling back to the first of
Debug-testing,Release-testingthat existed. A matrix building both therefore compiled the Release runner on every pipeline and never executed it, with nothing red to show for it — the warning naming the choice went to a job log nobody reads.ci_filter_effectsreportstest_labels, the Linux testing configurations that survive the filter. Deliberately not derived fromci_build_types, which counts every survivor: apybind = truepin keeps Release configurations while staging no runner at all.Run C++ Tests - <label>job per label, each passing--labeland uploading only its owntest_artifacts/<label>/._config_labelbecame the module-levelconfig_label, so the generator names the directories with the same function the build writes them with.split_testswith no surviving testing configuration is rejected at generation: the suite would run nowhere and the pipeline would pass having tested nothing.6aeaf28raises a shard's timeout to 20 minutes;11153adlets ctest parallelize the instrumented suite again.3. Coverage splits into phases (
754e5ba,d180151)The Coverage job did everything — two instrumented conan builds, gcovr, the HTML render, and the threshold gate — in one job at the end of the pipeline. The slowest work started last, and the gate could only be exercised by driving two real conan builds.
run_coveragetakes--phase.collectdoes both builds, gcovr, and every rendered artifact;reportapplies the gate. The default,all, still does both in one process.coverage-status.jsoncarries what no report file records: whether either leg's tests failed, and whether each layer was measured. Thresholds stay out of it —reportre-readsbuild.toml, so tightening one re-gates existing artifacts without rebuilding.allow_failure: exit_codes: [3]stays on the report job only. Collect cannot produce a3, so anything it fails on is a real breakage.coverage:regex reads theCoverage total: <pct>%line the report phase prints; an unmeasured C++ layer printsn/aand matches nothing, so it cannot publish a percentage that looks like a measurement.04607f7documents why the legs combine as gcovr tracefiles rather than merged.gcda;9a0d226logs how much gcov data each build folder holds, which is the measurement the next section rests on.4. The concurrent build stage (
73f4916,087575d,51eb94d)Splitting collect from report exposed the real cost:
Coverage Buildrecompiled the Debug testing and pybind configurations because instrumentation is part of thepackage_id, so no production binary could satisfy it. 503 of the 1334 objects a pipeline compiled were that second copy, and the singleConan Buildcompiled its whole matrix sequentially inside one container.needs: []so it starts at t=0 rather than at its stage's turn.--phase measure --leg <cpp|python>, builds one instrumented leg and reads its.gcdawhere it was compiled. The instrumented jobs are ordinary Build-stage jobs;Coverage Buildis gone. Each leg writes a gcovr tracefile of a few hundred KB, so no build folder travels — at 245 MiB of.gcnoagainst 5.5 MiB of.gcda, moving one would cost more than the compile it saves.only: tagstwin, since--coveragechanges the binary a release publishes.conan cache savein the pybind job sees the pybind package and nothing else, so naming a single exporter would publish one package id and silently drop the rest.087575dgates the whole shape on[matrix].wheel_only. It was designed and measured against a wheel_only matrix, which has no library configurations; a repository that publishes library packages keeps what it had.51eb94druns wheel repair only in the pipeline that builds a wheel.5. Job names, and the keys a narrowed
[filter]can empty (a700775)From the review on this PR.
[matrix].pybind_build_typesaccepts two build types, and the pybind job name carried the ABI but not the build type — so the Release configuration'sonly: tagstwin and the Debug configuration's job both rendered as"Python Build". A GitLab job name is a YAML mapping key, so the later block silently replaced the earlier and that configuration was never built. The build type now joins the name when the fan-out spans two, a second pybind build type is no longer instrumented (the Python leg's filter pins one, via the sharedCOVERAGE_PYBIND_BUILD_TYPE), and naming ends with a uniqueness check that raises.dependencies:on the deploy job andneeds:on the Coverage job are guarded on a non-empty list. Both render asnullunder a narrowing filter, and GitLab rejects the file — including in the casecoverage_conflicts()warns about and then generates anyway.wheel_onlyfan-out rather than theConan Buildjob that shape does not emit..get.Verification
2325 passed, 5 skipped; flake8 clean across the repository.Run C++ Tests - Release-testingjob, which is the behavior the split was meant to preserve.