Skip to content

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

  1. Adds directives coverage-setup, coverage-report, coverage-clean to the Makefile
  2. fixes lint.sh who doesn't like the pp.ml* files generated by ppx_bisect
  3. updates install_build_deps.sh to install bisect_ppx along with the other developer tools.
  4. updates .gitignore to ignore tempory files
  5. 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 to bisect-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 fix glob expressions in dune files to avoid passing pp.ml{i} files to lint.sh. I wasn't able to do so)

  • .gitignore, make coverage-report and make clean-report expect coverage data to be in _coverage_output. This target directory is specified with environment variable BISECT_FILE. Unfortunately, this can only be set outside of the Makefile. The solution proposed in this MR is to have a target in the Makefile coverage-setup to check that BISECT_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
Edited by Philippe B.

Merge request reports