Add code coverage tooling (bisect_ppx)#8416
Open
JonoPrest wants to merge 5 commits intorescript-lang:masterfrom
Open
Add code coverage tooling (bisect_ppx)#8416JonoPrest wants to merge 5 commits intorescript-lang:masterfrom
JonoPrest wants to merge 5 commits intorescript-lang:masterfrom
Conversation
Wires bisect_ppx instrumentation through every OCaml library and executable in the workspace (compiler, analysis, tools, ounit tests) and adds make targets for producing reports. - `make coverage` builds an instrumented toolchain, runs the full test suite via `node scripts/test.js -all`, and emits HTML + summary reports under `_coverage/`. - `make coverage-super-errors` is a focused flow: it runs only the super_errors fixtures and prints per-file coverage for the type checker / error reporting modules so it's easy to spot which error paths the snapshot fixtures don't exercise. - Frozen (`parsetree0.ml`) and cppo-generated `compiler/ext` variants are filtered from reports via `--ignore-files` patterns. - bisect_ppx is added as a `with-dev-setup` dep in `rescript.opam.template`, so default test builds are unaffected.
The four libraries that use cppo (compiler/{common,ml,core,ext}) had a
library-wide `(preprocess (action (run cppo ...)))` stanza, which dune
disallows in combination with `(instrumentation ...)`. That blocked
adding bisect_ppx coverage on the modules where most of the type
checker and error reporting lives.
Of the ~250 .ml files across these four libraries, only 10 actually
use cppo directives — all toggling the existing BROWSER and RELEASE
flags for the JSOO playground build and stripped debug logging. The
fix is mechanical:
- Rename those 10 source files to `*.cppo.ml` (or `*.cppo.mli`)
- Add a per-file `(rule (target X.ml) (deps X.cppo.ml) (action cppo))`
for each, matching the convention compiler/ext already uses for its
template files (hash_set.cppo.ml, vec.cppo.ml, etc.)
- Drop the library-wide `(preprocess ...)` stanzas
The same `%{env:CPPO_FLAGS=}` substitution is preserved in each rule,
so the dev / release / static / browser profile behavior in
`compiler/dune` is unchanged. js_reserved_map needs cppo's `-V OCAML:`
flag for its `#if OCAML_VERSION >= (5, 0, 0)` directive, which is
preserved on its rule specifically.
Side benefit: cppo only runs on the 10 files that need it instead of
every file in those libraries.
The flags I'd written for the report subcommands didn't match what bisect_ppx 2.8 actually accepts: - `--ignore-files <regex>` (used for excluding generated files) does not exist. The 2.8 API takes path globs via `--expect`/ `--do-not-expect`. Dropped the regex filter for now — generated files don't add meaningful noise to the report. We can revisit excluding `compiler/ext/*_string.ml` etc. with the path-based API later if it becomes useful. - `--ignore-missing-files` is only accepted by `html` and `cobertura`, not by `summary`. Removed it from summary invocations. - `cobertura` takes the output file as a positional argument, not via `-o`. Fixed. Also pulls in the regenerated rescript.opam (`bisect_ppx` was added as a `with-dev-setup` dep in rescript.opam.template earlier; dune regenerates the .opam file on build). `make coverage-super-errors` now runs end-to-end and produces a per-file table for the type-checker / error-reporting modules.
ocamlformat was failing on the renamed .cppo.ml/.mli files because the ignore list still referenced their old paths. Replaced the explicit file list with `**/*.cppo.ml` and `**/*.cppo.mli` globs, which covers the renamed files plus the existing `compiler/ext/*.cppo.ml` templates that were already listed individually. Future cppo files added under the new convention are covered automatically. Also includes the auto-promoted dune format applied to the js_reserved_map rule in compiler/ext/dune (`dune build @fmt` reformatted the multi-arg cppo invocation onto one arg per line).
Trim the coverage surface to what's actually needed for local inspection + agent queries: - Remove `coverage-report-cobertura` — Cobertura XML is only useful for CI dashboards (Codecov/Coveralls upload). Not wiring CI for now. - Remove `coverage-super-errors` and `coverage-run-super-errors` — super_errors fixtures are already executed under `make coverage` via scripts/test.js -all (which iterates tests/build_tests/*). Querying for super_errors-relevant modules is easy with jq on coverage.json or a grep on `summary --per-file`. - Add `coveralls` JSON output to `coverage-report`. Goes to _coverage/coverage.json with the structure documented in the Makefile header so an agent / jq one-liner can consume it directly. - Inline the COVERAGE_IGNORE variable since `--ignore-missing-files` is the only flag now. User-facing surface: `make coverage`, `make clean-coverage`, plus internal phases (coverage-build, coverage-prepare, coverage-lib, coverage-run, coverage-report) for re-running individual stages.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
JonoPrest
commented
May 7, 2026
Comment on lines
-4
to
-6
| (preprocess | ||
| (action | ||
| (run %{bin:cppo} %{env:CPPO_FLAGS=} %{input-file}))) |
Contributor
Author
There was a problem hiding this comment.
Dune was unabled to have both this preprocess stanza and the ppx stanza. There were such few files actually using cppo flags that I just converted them into per file rules.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add code coverage tooling (bisect_ppx)
Why
We want a coverage workflow we can lean on as we move toward larger
agent-driven PRs. The goal isn't coverage itself — it's confidence:
an agent (or reviewer) can run
make coverage, see which lines achange leaves dark, and close those gaps with new fixtures before
merging. CI integration is intentionally out of scope; one config file
away when we want it.
What
make coveragerebuilds the toolchain with bisect_ppx instrumentation,runs
scripts/test.js -all, and emits:_coverage/html/index.html— human-browsable line-level report_coverage/coverage.json— Coveralls-format JSON for agents / jqbisect_ppx is added to
rescript.opam.templateaswith-dev-setup,so default builds and CI runs are unaffected. Opt-in:
opam install bisect_ppx && make coverage.Why the dune surgery
(instrumentation (backend bisect_ppx))is incompatible withaction-style
(preprocess (action (run cppo ...))). Four libraries —compiler/{ml,core,common,ext}— used that pattern, and they'reexactly where the type checker and error reporting live.
Only 10 of the ~250 files in those libraries actually use cppo
directives (BROWSER / RELEASE / OCAML_VERSION). I renamed them to
*.cppo.mland added per-file(rule ...)blocks invoking cppo —same convention
compiler/ext/dunealready uses for itshash_set.cppo.ml/vec.cppo.mltemplates. Library-wide(preprocess ...)stanzas dropped. Validated all four build profiles(dev / release / static / browser) produce identical cppo output to
the old setup.
Verification
make(default build),make test-syntax, browser profile build:clean
make coverage: runs end-to-end, 49.79% baseline line coveragewith no diffs after the cppo refactor
representative files