Question: Does it work with dune's cram test?

[Kakadu]

If yes, could you please add some instructions to README?

[Kakadu]

People in discord gave me a demo https://github.com/mooreryan/bisect_ppx_cram_test_example

[aantron]

Great, thanks @Kakadu and @mooreryan! I'm going to reopen this as an issue about improving Bisect's docs.

[mbarbin]

Hi! Thank you for this issue, and linking the discord example, this helped me setup bisect_ppx + cram in a repo.

I struggled to make the GitHub action part work, so I'm pasting here what I currently arrived at.

This took me a few tries! (The feedback loop when trying to debug a GitHub action is not convenient).

Putting it here if you want to discuss further, or perhaps help somebody else with that part. The part I struggled with I think was that I was afraid /tmp would not be available in the context of a GitHub action, and I think something wasn't working if I supply used a relative path for BISECT_FILE. Seems like using runner.tmp does the trick.

      - name: Run tests
        run: |
             mkdir $BISECT_DIR
             opam exec -- dune runtest --instrument-with bisect_ppx
        env:
          BISECT_DIR: ${{ runner.temp }}/_bisect_ppx_data
          BISECT_FILE: ${{ runner.temp }}/_bisect_ppx_data/data

      - name: Send coverage report to Coveralls
        run: opam exec -- bisect-ppx-report send-to Coveralls --coverage-path $BISECT_DIR
        env:
          BISECT_DIR: ${{ runner.temp }}/_bisect_ppx_data
          COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          PULL_REQUEST_NUMBER: ${{ github.event.number }}

This is taken from here.

Thank you for the amazing bisect_ppx!

[mooreryan]

The /tmp dir does work on the ubuntu and macOS github runners at least (that code I put in the repo originally I extracted from projects that use it in the action). I just updated that linked github repo with a github action that uses the script in the makefile, if it helps someone.

[aantron]

The main issue here was just to add BISECT_FILE, is that right?

[mbarbin]

Hi @aantron,

The impression I had was that you'd need to choose a location for the bisect files (1), remember to delete any stale file there ahead of running the tests (2), supply the env var while running the test with an absolute path that terminates with a basename prefix (3), and supply the containing directory name during bisect-ppx-report, via --coverage-path (4).

I guess I found the mention of the env variable BISECT_FILE in the demo linked to this issue and got things to work with it.

By the way, while setting this up, I remember wondering whether things could be more opinionated in order to simplify these 4 steps. I wouldn't mind opting in for a default that you'd find reasonable.

Another (perhaps complementary, or orthogonal) proposal is to publish somewhere a reference GitHub action that can serve as common action that people can use directly (like uses: ocaml/setup-ocaml@v2). That being said, I've seen some people have a few lines in their Makefile to wrap the bisect ppx steps, so they can easily run the bisect tests locally, and keep things in sync by calling that Makefile target in their action workflow. So perhaps an action wouldn't gain popularity, since it wouldn't be as straight forward to see the local and workflow version are equivalent. I don't know!

Thanks for looking back at this issue.

[aantron]

By the way, while setting this up, I remember wondering whether things could be more opinionated in order to simplify these 4 steps. I wouldn't mind opting in for a default that you'd find reasonable.

The issue with that is I don't think we can detect in Bisect that the user is running cram tests, at least not in a maintainable way. I'll probably opt to document all of this in advanced.md and link it from the main README so that it is findable by Ctrl+F.

[aantron]

Also, Bisect_ppx itself uses cram tests, and Bisect's own Makefile doesn't seem to do any of this. I'll have to take a look at why.

[mbarbin]

Also, Bisect_ppx itself uses cram tests, and Bisect's own Makefile doesn't seem to do any of this. I'll have to take a look at why.

I'm unsure about the details, but I have a vague recollection that using the BISECT_FILE env var was only useful in order to aggregate several bisect-ppx data files into a single directory, so as to accumulate the data collected for all the files during report generation. I am not sure whether this could relate to the tests you're talking about, but perhaps this can be explained a bit in the doc. Is it the case that if you only have one file to generate, not passing any extra flags just works? (as in, the file is saved into a canonical place and is found by bisect-ppx-report)

[mooreryan]

The main issue here was just to add BISECT_FILE, is that right?

^ Regarding getting it to work with cram tests? For that you need to ensure that you turn instrumentation on for the executable under test (in addition to the lib) and it will "just work". I only used the BISECT_FILE var to stick output into a single folder. (As far as I remember...I've been using that pattern from the example repo more or less unchanged for a couple years now.)

[mbarbin]

I've noticed similar issues reported in #418 and #432, which I believe might be related.

In my experience, when running tests instrumented with bisect_ppx, if the BISECT_FILE environment variable is not set, no *.coverage files are generated, preventing the report generation step.

~/dev/catch-the-bunny$ dune clean
~/dev/catch-the-bunny$ dune runtest --instrument-with bisect_ppx
~/dev/catch-the-bunny$ bisect-ppx-report summary
Error: no *.coverage files found
Hint: is the code under test linked into the test suite?
Hint: did the tests run?

However, if I set BISECT_FILE to a location outside of _build, it works:

~/dev/catch-the-bunny$ dune clean
~/dev/catch-the-bunny$ mkdir -p /tmp/coverage ; rm -rf /tmp/coverage/*
~/dev/catch-the-bunny$ BISECT_FILE=/tmp/coverage/data dune runtest --instrument-with bisect_ppx
~/dev/catch-the-bunny$ bisect-ppx-report summary --coverage-path /tmp/coverage
Coverage: 124/131 (94.66%)

I suspect that dune might be deleting unexpected files from _build (though this is just a hypothesis).

I'm preparing to use bisect_ppx more and would be grateful for any insights into the setup.

Thank you!

[mooreryan]

Here is why you need the BISECT_FILE env variable (at least with cram tests, ppx_inline_test, ppx_expect ...not sure if it will be the same for other testing frameworks):

If you have a typical setup where the dune is running the tests in a sandbox, then the coverage files are disappearing because dune will delete the contents of the sandboxed directory when the tests are finished, and it won't promote the coverage files automatically. But it is still working (if you have instrumentation set up properly of course).

To check that cov files are actually generated in your tests, you can add something like this to the end of a file containing inline or expect tests:

let () = Sys.getcwd () |> Sys.readdir |> Array.iter (fun x -> print_endline x)

To check that it is actually working in a cram test, stick this at the end of your cram file and you will see the generated coverage file:

Show coverage files

  $ ls

If you set the BISECT_FILE env variable, instrumentation appears to work because it is not dumping the coverage files into a bunch of directories in which dune removes after tests are run. Rather, it collects all coverage files into the specified directory.

[mbarbin]

Thank you, @mooreryan. Your response was enlightening! It has motivated me to delve deeper into bisect_ppx, and I've enabled it in several more repositories.

I believe your explanation about the BISECT_FILE environment variable and its interaction with dune would be a great addition to the bisect_ppx documentation. Currently, BISECT_FILE is described as a convenience tool, but your explanation suggests it's a crucial part of the setup.

I have two more questions I'd appreciate your input on:

• [ ] 1. Instrumenting testing libraries: In #432, you mentioned not wanting to generate coverage for the Inline_tester module. I've been instrumenting my inline-test libraries to check for dead code. Is there a downside to this approach that I'm overlooking?

• [x] 2. Binary coverage not being picked up: In this repo, I have a cram test written in OCaml (example). The binary defined in a sibling directory is run during the test, and I've added an instrumentation stanza to that binary. However, the coverage report shows 0.0% coverage for both keyval_server and grpc_server. I believe the BISECT_FILE environment variable should be propagated from the parent process when I run the binary using Eio.Process.spawn. Any ideas on what might be going wrong?

Thank you for your time and insights!

[mbarbin]

I have a hunch about issue 2: It seems that my server is being signaled to quit and exits before it has a chance to write the coverage data. I'll investigate the use of the BISECT_SIGTERM environment variable next, as it might help in ensuring the coverage data is written before the process terminates.

Edit: this was fixed by https://github.com/mbarbin/eio-rpc/pull/4

[aantron]

Thanks @mbarbin and @mooreryan! BISECT_FILE was originally indeed only a convenience, but with Dune cram tests standard, we need to update the instructions.

I'm currently converting Luv to cram tests and will be applying Bisect_ppx to it afterwards, so I'll get some first-hand experience with this. I wasn't aware of any issues with ppx_inline_test or ppx_expect that would require BISECT_FILE, as I use ppx_expect in Dream with Bisect without having to use the environment variable, but perhaps something has changed in recent versions of Dune. I'll take a fresh look at that.