taiki-e / cargo-llvm-cov

Cargo subcommand to easily use LLVM source-based code coverage (-C instrument-coverage).
Apache License 2.0
933 stars 57 forks source link

Exit code based on coverage percentage #137

Closed vmiklos closed 2 years ago

vmiklos commented 2 years ago

Coverage tools for some other languages allow exiting with a non-0 exit code in case a certain type of coverage is below a specified minimum. For example:

The motivation for these flags is that they typically have some good way to exclude non-interesting code and then they can require good coverage for the rest of the code.

It seems to me that cargo-llvm-cov has no such feature and probably it would not be too complex to add one. Would a patch implementing this be accepted, after the usual review?

I think the closest in cargo-llvm-cov terms would be line coverage, perhaps something like:

cargo llvm-cov --fail-under-lines 100

could be a syntax to request non-0 exit code when line coverage is not 100%?

Thanks.

taiki-e commented 2 years ago

Sounds good to me! I would accept a PR to support this.

vmiklos commented 2 years ago

I looked into this briefly, just to realize that the current summary is not produced by cargo-llvm-cov itself, but by the underlying llvm-cov report, which doesn't seem to have an option to fail under a certain coverage. I naively assumed that llvm-cov already deals with percentages and this issue is a matter of reflecting that percentage in the exit status code. :-)

One way to do this could be to add a parser for the JSON output of llvm-cov, but https://clang.llvm.org/docs/SourceBasedCodeCoverage.html doesn't really document the format, so this might involve a bit of llvm-cov source code reading (at least the format is not trivial to my eyes).

An other way could be to export to lcov, then lcov --summary t.lcov --fail-under-lines 100 can provide this feature. And then perhaps document this in cargo-llvm-cov or just claim this is not in scope for the project after all.

Third option is to first add something like this to llvm-cov and once that's accepted, then cargo-llvm-cov could just pass this option. This may be the most painful, given how much time it may take till the patch is accepted + it reaches the rust-provided llvm-cov.

Do you have a preference between 1), 2) and 3)?

taiki-e commented 2 years ago

We have a tool for testing that parses JSON output (i.e., the implementation of 1's parser), so using that is probably the easiest way to go.

https://github.com/taiki-e/cargo-llvm-cov/blob/0798e99594fba80abb2f27d9ef2ae2910679b77d/tests/auxiliary/json.rs#L109-L122

vmiklos commented 2 years ago

Once the above PR is in, the actual percentage check can be added. I have a first cut of that at https://github.com/vmiklos/cargo-llvm-cov/commits/fail-under-lines2. I imagine it's easier to review these as two separate PRs, but in case that's not the case, then let me know.