Re: [MirageOS-devel] tests, coverage, and the modern duniverse

[Anil Madhavapeddy <anil@xxxxxxxxxx>]

On 9 Apr 2019, at 17:30, Rudi Grinberg <rudi.grinberg@xxxxxxxxx> wrote:

Indeed this feature has been on my to-do list forever. This problem is complicated by the fact that we want to be able to instrument preprocessed code. Consider running bisect on code sources that use cppo for example.

Thanks for the details, Rudi!

It strikes me that we have relatively little use of ppx in core Mirage libraries — indeed, we’ve been steadily removing the hard dependency on many core libraries such as ipaddr, uri, nocrypto and so on.  So I don’t think this is a big limitation initially.  And the coverage information from most ppx invocations don’t seem hugely important to track _vs_ human written code, but others may correct me on this.  I don’t think we have a lot of use of cppo, which would be the exception to this rule since it guards human-written code.

Another design decision that needs review is the interaction with profiles. Originally, my idea was to have a special bisect profile that would setup instrumentation. However, people have been raising concerns that this is too inflexible. So we are likely to have this as a context option instead.

A context option does sound more flexible — this means that we can build it alongside any existing build configurations.  Bisect coverage testing seems like something we want to enable as widely as possible, so this also sounds good to me.

Since this is becoming an issue for more people, I can certainly bump its priority. The first version of this is unlikely to address the limitation I've mentioned initially, would it still be useful for mirage?

I think so!  It would also get rid of another source of ocaml jbuild files.

regards,

Anil

On Tue, Apr 9, 2019 at 11:11 PM Anil Madhavapeddy <anil@xxxxxxxxxx> wrote:

On 9 Apr 2019, at 00:02, Mindy Preston <mindy@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Hi all,
>
> I recently got inspired to revisit test coverage information generation. It seems that the current state of this ecosystem uses `dune` for building and some instructions in `preprocess` stanzas to invoke `bisect_ppx`, which itself has logic to only produce coverage information if an environment variable is set when called in a certain mode.
>
> Unfortunately, while the *invocation* of `bisect_ppx` can be set conditionally (see bisect_ppx's instructions[1]) for details), the dependency on `bisect_ppx` is unconditional - even if the environment variable isn't set, `dune build` will fail if `bisect_ppx` is not installed.
>
> It seems that most projects using `bisect_ppx` use a solution that involves some pre-release massaging of `dune` files to remove `bisect_ppx` from `preprocess (pps` stanzas, and then release an `opam` file that doesn't mention `bisect_ppx`, but keep `bisect_ppx` in their repository-local `opam` files.
>
> I think this kind of workflow is OK for repositories that have one or two very involved maintainers, but it seems error-prone for MirageOS repositories, where there's a team of maintainers that have varying amounts of involvement.  I can very easily imagine myself going to make a release of a repository with this strategy and accidentally releasing the bisected version.
>
> I'm interested if anyone has a solution in mind for this that's a bit more automatic.
>

This is indeed pending a fix in Dune; see https://github.com/ocaml/dune/issues/57

I’ve copied Rudi in case he can comment on any blockers from the dune end and whether this is a candidate to target for dune 1.10 (the 1.9 release is just about ready to go out of the door in the next few days).

regards,
Anil

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/mirageos-devel

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/mirageos-devel