Make bisect_ppx actually instrument the library — coverage was measuring nothing - #713
Conversation
The coverage job runs, passes, and measures nothing.
$ dune runtest --force --instrument-with bisect_ppx
537 tests OK, exit 0
$ bisect-ppx-report summary
Error: no *.coverage files found
Zero .coverage files are written anywhere in the tree.
== Root cause
lib/dune declares `(preprocess (pps ...))` but no instrumentation backend.
`--instrument-with bisect_ppx` only instruments libraries that opt in with an
`(instrumentation (backend bisect_ppx))` stanza; without one, dune instruments
nothing, the tests run uninstrumented, and no coverage data exists to report.
== Why nobody noticed
.github/workflows/ci.yml:213 sets `continue-on-error: true` on the step that
calls bisect-ppx-report. So the report failed on every run, the failure was
swallowed, and the coverage job went green while producing no data. The same
hollow-gate shape as a test that passes without running.
The `continue-on-error` is left in place — it is defensible for a
visibility-only artefact step, and with this fix the step now has something to
report. Worth revisiting separately if coverage ever becomes gating.
== After the fix
.coverage files: 0 -> 2
Coverage: 8103/16243 (49.89%)
That number has never been measurable before, so it is a baseline rather than a
regression or an improvement.
== What it says needs attention
Core paths are reasonably covered:
parser.ml 75.64% 969/1281
quantity.ml 79.28% 241/304
typecheck.ml 68.37% 1042/1524
borrow.ml 68.03% 800/1176
resolve.ml 59.21% 405/684
lexer.ml 58.33% 49/84
The gaps are concentrated in backends and tooling, not the front end:
wasm_gc.ml 0.00% 0/178 <- entirely unexercised
wasm.ml 0.24% 1/423
types.ml 9.95% 19/191
lean_codegen 10.48% 13/124
value.ml 13.13% 26/198
c_codegen.ml 14.60% 67/459
lsp_server.ml 15.47% 43/278
wasm_gc.ml at 0/178 is the standout: 178 points, none reached by any of the 537
tests.
One change only: lib/dune.
|
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review ✅ ApprovedAdds the missing bisect_ppx instrumentation stanza to lib/dune, enabling code coverage measurement and establishing a 49.89% baseline. No issues found.
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Important Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
1 participant
The coverage job runs, passes, and measures nothing.
Zero
.coveragefiles are written anywhere in the tree.Root cause
lib/dunedeclares(preprocess (pps …))but no instrumentation backend.--instrument-with bisect_ppxonly instruments libraries that opt in with an(instrumentation (backend bisect_ppx))stanza. Without one, dune instruments nothing, the tests run uninstrumented, and there is no coverage data to report.Why nobody noticed
.github/workflows/ci.yml:213setscontinue-on-error: trueon the step that callsbisect-ppx-report. So the report failed on every run, the failure was swallowed, and the coverage job went green while producing no data — the same hollow-gate shape as a test that passes without running.The
continue-on-erroris left in place: it's defensible for a visibility-only artefact step, and with this fix the step now has something to report. Worth revisiting separately if coverage ever becomes gating.After the fix
.coveragefilesThat number has never been measurable before, so treat it as a baseline — not a regression, and not an improvement.
What it says needs attention
Core paths are reasonably covered:
parser.mlquantity.mltypecheck.mlborrow.mlresolve.mllexer.mlThe gaps are concentrated in backends and tooling, not the front end:
wasm_gc.mlwasm.mltypes.mllean_codegen.mlvalue.mlc_codegen.mllsp_server.mlwasm_gc.mlat 0/178 is the standout — 178 points, none reached by any of the 537 tests.Two things I checked and did not change
The 32
[SKIP]s inres-to-affine-walkerare correct behaviour, not a fake gate.test_walker.ml:65skips with an actionable message when the tree-sitter CLI isn't on PATH, and CI does install it (ci.yml:59–68) — so those tests run in CI and skip only locally. Good design; left alone.The suite is otherwise healthy: 537 tests run, 0 fail. Better than the estate norm.
Scope
One file changed:
lib/dune.🤖 Generated with Claude Code