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

Combining coverage from unit & integration tests produces incorrect summary #276

Closed michaelvlach closed 1 year ago

michaelvlach commented 1 year ago

When running coverage against a package with both unit tests and integration tests the resulting report contains two distinct instantiations on functions. This is due to different compilation flags (unit tests being with cfg(test) and integration tests without). This confuses up summary line coverage calculation. Detailed report shows all lines as covered with each instantiation showing respective partial coverage. The summary however claims some lines are uncovered, specifically those covered only by unit tests as seemingly only the integration tests instantiation is taken into account for summary.

Minimal reproduction:

llvm-cov-merged-report-incorrect.zip

Run cargo llvm-cov --html --open.

Some findings:

This is not an issue of cargo-llvm-cov per se but rather an issue of LLVM, Rust compiler or some llvm-cov config (missing flag or somesuch). But I wonder if there is some way to work around this in cargo-llvm-cov.

taiki-e commented 1 year ago

Maybe the same issue as https://github.com/taiki-e/cargo-llvm-cov/issues/43

We are aware of the issue that the llvm-cov summary report is somewhat different from the full report, and I think it may be possible to output a more accurate summary by exporting the coverage in JSON format and then converting it (--show-missing-lines and --codecov are actually implemented in this way).

michaelvlach commented 1 year ago

It definitely seems related. I think it's LLVM treating them same as templates in C++. They are technically not templates/generics but Rust's compilation model recompiles everything but with different settings so from LLVM's perspective they are technically different instantiations (like they were templates / generics).

And thanks for the suggestions!

I think the workaround for now is to substitute --fail-uncovered-lines for parsing --show-missing-lines verifying there are indeed none. Would it perhaps make sense to change the logic behind --fail-uncovered-lines from parsing the (inaccurate) report to what --show-missing-lines does?

michaelvlach commented 1 year ago

Feel free to decline the PR if you disagree as I know it's not as clean cut decision here since the current logic follows (and is consistent with) the report (however inaccurate). On the other hand when it fails and the report shows uncovered lines then running with --show-missing-lines shows nothing. So I think it would make sense making it a bit more consistent that way.

Dushistov commented 1 year ago

Oh, that's why not generic functions some times have several instantiation according to html generated by cargo-llvm-cov:

generic_not_generic

Ekleog-NEAR commented 9 months ago

Is this actually solved? I’m still seeing the issue described just above with the latest llvm-cov version.

It looks to me like the solution would probably be the same as #43, but the way it manifests is different, because it shows as multiple instantiations of a single function due to different #[cfg] flags.

Does that make sense?

taiki-e commented 9 months ago

@Ekleog-NEAR Could you open a new issue?

What was initially reported in this issue has been fixed and what you linked to is a half off-topic comment on a different API than that.

If you think there is another issue, please open an individual issue. Addressing several different issues in a single GH issue can be confusing, and it can slow down and obstruct solving the problem. Just as I struggled in https://github.com/taiki-e/cargo-llvm-cov/issues/300#issuecomment-1688304667.

Ekleog-NEAR commented 9 months ago

Of course! I just opened https://github.com/taiki-e/cargo-llvm-cov/issues/344 to track it :)