Code coverage
This MR makes a few adjustements to the Makefile and to some scripts to facilitate the computation of code coverage using bisect_ppx
. It
- Adds directives
coverage-setup
,coverage-report
,coverage-clean
to theMakefile
- fixes
lint.sh
who doesn't like thepp.ml*
files generated by ppx_bisect - updates
install_build_deps.sh
to installbisect_ppx
along with the other developer tools. - updates
.gitignore
to ignore tempory files - adds a script to instrument dune files
You can try this MR with
make build-dev-deps # install bisect_ppx.1.4.2 (may need to update opam repo)
# add directive in some dune file to tell it to instrument the code
# (see discussion). This can be done manually, for instance to
# instrument lib_p2p:
# sed -i '2i(preprocess (pps bisect_ppx))' src/lib_p2p/dune
#
# We use a convenience script to instrument lib_p2p and proto_alpha
./scripts/instrument_dune_bisect.sh src/lib_p2p/dune src/proto_alpha/lib_protocol/dune.inc
# compile
make
# run individual tests. Need to set up the coverage dir and BISECT_FILE
# coverage will contain the coverage data (!= coverage report)
# BISECT_FILE tells bisect where to put the generated output
# and how to name it
make coverage-setup # creates the directory to store the coverage output
# warn that $BISECT_FILE isn't set up
export BISECT_FILE=${PWD}/_coverage_output/bisect
dune build @runtest_p2p_pool --force # run some unit test
pytest tests_python/tests/test_p2p.py # run some python test
# generate report (it should be in _coverage_report/index.html)
make coverage-report
git status # make sure .gitignore ignores new file
git checkout -- src/lib_p2p/dune src/proto_alpha/lib_protocol/dune.inc # reset dune file
make coverage-clean # make sure coverage information is cleaned
Discussion:
-
The doc for this is being written in !1538 (merged) (currently it describes the procedure before this MR, it will need to be updated to reflect the workflow introduced by this MR)
-
This MR is less ambitious than MR !1404 (merged) which tried to integrate this in the CI. After some work on !1404 (merged), I think it's easier to start with this local version (CI integration adds a few orthogonal difficulties)
-
It's annoying to add the processing directive. We could put it permanently in the dune files (it won't instrument the compiled code by default if we use
(preprocess (pps bisect_ppx --conditional))
but we would need to add an extra-dependency tobisect-ppx
in all Opam files. I don't know if it's an issue or not. In the meantime, we can use the script./scripts/instrument_dune_bisect.sh
-
Maybe a bash guru can come up with a better fix for
lint.sh
(or fixglob
expressions indune
files to avoid passingpp.ml{i}
files tolint.sh
. I wasn't able to do so) -
.gitignore
,make coverage-report
andmake clean-report
expect coverage data to be in_coverage_output
. This target directory is specified with environment variableBISECT_FILE
. Unfortunately, this can only be set outside of the Makefile. The solution proposed in this MR is to have a target in the Makefilecoverage-setup
to check thatBISECT_FILE
is properly set (and instruct the user if it is not). -
instrument_dune_bisect.sh
could be more defensive/robust (check parameters are only dune files, could be idempotent...). If needed I'll rewrite it in python.
Issues
- When covering the protocol, the following error comes up
▶ bisect-ppx-report html -o ${COVERAGE_REPORT} --coverage-path ${COVERAGE_OUTPUT}
*** system error: src/proto_alpha/lib_protocol/environment.ml: No such file or directory
Which is solved by adding the option --ignore-missing-files
. This should be investigating further.
-
Bisect_2.1.0
has just been released. Unfortunately, building the instrumented code gives this error
File "src/proto_alpha/lib_protocol/delegate_services.ml", line 297, characters 25-39:
Error: This expression has type ?offset:int32 -> Raw_level.t -> Level.t
but an expression was expected of type 'a -> 'b
I wasn't able to find what caused this error. A bug in bisect_ppx
? I suggest sticking to 1.4.2
. Otherwise, there are two changes to apply.
.PHONY: coverage-report
coverage-report:
- @bisect-ppx-report -ignore-missing-files -html ${COVERAGE_REPORT} ${COVERAGE_OUTPUT}/*.out
+ @bisect-ppx-report html --ignore-missing-files -o ${COVERAGE_REPORT} --coverage-path ${COVERAGE_OUTPUT}
@echo "Report should be available in ${COVERAGE_REPORT}/index.html"
.PHONY: build-sandbox
diff --git a/scripts/install_build_deps.sh b/scripts/install_build_deps.sh
index 4098ee4085..4de11fef24 100755
--- a/scripts/install_build_deps.sh
+++ b/scripts/install_build_deps.sh
@@ -43,5 +43,5 @@ opam install --yes opam-depext
if [ -n "$dev" ]; then
opam repository add default --rank=-1 > /dev/null 2>&1 || true
- opam install merlin odoc bisect_ppx --criteria="-changed,-removed"
+ opam install merlin odoc bisect_ppx.2.1.0 --criteria="-changed,-removed"
fi