Add code coverage tooling (bisect_ppx)#8416
Open
JonoPrest wants to merge 7 commits into
Open
Conversation
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.
JonoPrest
force-pushed
the
jono/add-coverage
branch
from
a36eab0 to
ab311b7
Compare
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.
Runs make coverage on a single Linux/ARM job and uploads the coveralls-format report to Codecov. The codecov.yml keeps checks informational and suppresses inline annotations so PR coverage feedback stays low-noise for contributors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JonoPrest
force-pushed
the
jono/add-coverage
branch
from
ab311b7 to
8c2b726
Compare
Member
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4f755aa31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+255
to
+257
| @$(foreach bin,$(COMPILER_BIN_NAMES), \ | ||
| cp $(DUNE_BIN_DIR)/$(bin)$(PLATFORM_EXE_EXT) $(BIN_DIR)/$(bin).exe && \ | ||
| chmod 755 $(BIN_DIR)/$(bin).exe;) |
There was a problem hiding this comment.
coverage-build copies instrumented compiler executables into packages/@rescript/<platform>/bin, but nothing switches them back afterward. Because compiler/lib are stamp-based (_build/log, runtime build stamp), subsequent make test/make lib runs can reuse these instrumented binaries without rebuilding, so normal workflows keep running with coverage instrumentation (extra overhead and stray bisect*.coverage output) until a manual clean/rebuild. Keep the instrumented artifacts isolated or explicitly rebuild/copy uninstrumented binaries at the end of coverage.
Useful? React with 👍 / 👎.
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
2 participants
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.
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 / jq / Codecovbisect_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.
Codecov integration
A dedicated
.github/workflows/coverage.ymlrunsmake coverageon asingle Linux/ARM job and uploads
_coverage/coverage.jsonto Codecov.Design points:
ci.yml. Coverage needswith-dev-setup(bisect_ppx) and instrumented builds — differentartifacts, different cache slot (
opam-env-coverage-v1-...) so itdoesn't contaminate
ci.yml'sopam-env-v8-...cache.it on every OS/arch combo would multiply CI cost for no extra
information. Linux/ARM picked to align with the existing fast lane.
cancel-in-progresson PRs sorapid pushes don't queue up stale coverage runs.
codecov.ymlkeeps PR feedback low-noise:informational: trueon both project and patch checks — coveragedeltas surface as context, never as a blocking red ✗.
threshold: 0.5%absorbs jitter from non-deterministic tests.github_checks.annotations: false— no inline "this line isn'tcovered" comments cluttering the diff view.
comment.require_changes: true— Codecov only comments whencoverage actually moves.
fail_ci_if_error: falseon the upload step — a Codecov outageshouldn't redden the PR.
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