Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revive coverage testing, first steps #6142

Merged
merged 6 commits into from
Sep 26, 2020
Merged

Conversation

psteckler
Copy link
Member

@psteckler psteckler commented Sep 25, 2020

First step in reviving unit test coverage.

Update bisect_ppx in opam.export (and some other libraries).

Add script to generate coverage files. Update Makefile to invoke that script, which generates coverage files for all libraries. You can call the script with just the libraries you want coverage for, if desired.

Change -conditional flag in dune preprocessing clauses to --conditional.

This PR addresses some of the tasks listed in #6090.

@psteckler psteckler requested review from a team as code owners September 25, 2020 00:36
@psteckler psteckler added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Sep 25, 2020
@netlify
Copy link

netlify bot commented Sep 25, 2020

Deploy preview for mina-staging ready!

Built with commit e7833c7

https://deploy-preview-6142--mina-staging.netlify.app

@mobileink
Copy link
Contributor

By coincidence I was just researching bisect_ppx. If I understand the source (register.ml), the --conditional flag unconditionally enables the lib. Similarly the env var BISECT_ENABLE=YES unconditionally enables.

Plus it looks like the plan is to deprecate --conditional: aantron/bisect_ppx#323

@mergify mergify bot merged commit 5950212 into develop Sep 26, 2020
@mergify mergify bot deleted the fix/revive-coverage-testing branch September 26, 2020 00:45
@psteckler
Copy link
Member Author

psteckler commented Sep 26, 2020

If I understand the source (register.ml), the --conditional flag unconditionally enables the lib. Similarly the env var BISECT_ENABLE=YES unconditionally enables.

I don't believe so. The code reads:

("--conditional",
  Arg.Set conditional,
  " Instrument only when BISECT_ENABLE is YES");

If --conditional is given, then instrumentation only happens when the env var is set.
That's the way it's worked up to now, except for the flag name.

Now that this PR is merged, I rebuilt from develop. I saw a small number (5ish) of .coverage files, in cases where the preprocessing clause did not include bisect_ppx and the flag. I'll create a fix PR for those.

Plus it looks like the plan is to deprecate --conditional: aantron/bisect_ppx#323

It will be easy enough to update the dune files and scripts when things change. The example in the dune docs that uses --instrument-with (or some such), which I believe is the plan, doesn't currently work.

@mobileink
Copy link
Contributor

mobileink commented Sep 26, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch ready-to-merge-into-develop
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants