search

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

Merging coverage for multiple feature sets #50

Closed davidhewitt closed 3 years ago

davidhewitt commented 3 years ago

PyO3 has a feature (--multiple-pymethods) which alters several code paths (such as inside a proc macro crate, the feature causes different generated code to be emitted).

Ideally I'd like to measure coverage combined from runs with and without this feature enabled. It's not immediately clear to me whether this is possible with cargo-llvm-cov at the moment. Do you think that's something which would make sense to support?

At the moment PyO3's CI just makes several different invocations to cargo test and then runs grcov once at the end.

I expect it's possible I could find an external tool to merge lcov output files for now, rather than making this a part of cargo-llvm-cov.

taiki-e commented 3 years ago

Probably we can support this by adding a flag like --no-report that does the opposite of --no-run, and combining the two.

cargo llvm-cov --no-report --features a
cargo llvm-cov --no-report --features b

cargo llvm-cov --no-run --lcov
davidhewitt commented 3 years ago

That sounds ideal. I'm happy to offer a PR to add --no-report if that's helpful? I haven't looked at the code, but skipping the report step doesn't sound too hard to implement 😅

Perhaps the hardest bit is that --no-report may want to avoid cleaning up any existing profiling output? (Assuming that cargo-llvm-cov usually cleans up some output directory before running...?)

taiki-e commented 3 years ago

I'm happy to offer a PR to add --no-report

That would be great!

Perhaps the hardest bit is that --no-report may want to avoid cleaning up any existing profiling output? (Assuming that cargo-llvm-cov usually cleans up some output directory before running...?)

Perhaps we can separate the step of cleaning the artifacts, the step of running the tests, and the step of generating the report into separate functions, and combine them.

We might be able to add some heuristics for artifact cleanup in --no-report, but for now, I think it's okay to always require explicit cargo clean.

taiki-e commented 3 years ago

I will probably do this together with some other refactoring.

davidhewitt commented 3 years ago

Ok 👍

I did indeed hope to write this PR, but my time is spread super thin right now so I was expecting I wouldn't get to it before next week. Thank you for all the work on cargo-llvm-cov, it's really improved how we measure coverage in PyO3!