mozilla / grcov

Rust tool to collect and aggregate code coverage data for multiple source files
Mozilla Public License 2.0
1.17k stars 148 forks source link

For Rust projects, many lines are marked as partial hits that ideally would not be. #476

Open thomcc opened 4 years ago

thomcc commented 4 years ago

I'm not really sure if y'all have anything you can do about this, but we're using grcov to produce coverage in https://github.com/mozilla/application-services, and we're having some issues getting more accurate coverage information.

Specifically:

  1. assert! and such are considered partial hits by the coverage output by grcov unless you have code that both causes it to pass and fail. (We also have a similar issue with log::foo! but I think it's easier to work around, hopefully).

    This is especially frustrating as tests often have lots of asserts in them.

    This is an artifact of it being implemented as a macro, despite it being conceptually equivalent to a function call (In general IMO marking these sorts of things as partial is a violation of the macro's abstraction barrier).

  2. All lines that contain foo? are considered partial hits by the coverage output by grcov, unless you have code that causes it to both pass and fail.

    Given that ? is roughly analogous to exception propagation in other languages, and that those languages wouldn't have all lines that could potentially throw marked as partial hits, this seems unfortunate.

These are amplified by the fact that codecov.io considers partial hits and misses equivalent, although it seems easier to fix coverage producer, especially given that e.g. these issues do not seem to exist with cargo-tarpaulin (although producing the coverage with grcov is better for us for other reasons).

Also, it's totally possible this is just an issue of us using grcov wrong, and I'd love for this to be the case. However, I'm assuming it's not, since y'all have the same problem in your codecov.io reports (for example, see https://codecov.io/gh/mozilla/grcov/src/master/src/path_rewriting.rs#L1347)

Anyway, it would be nice if there were some way so that we could force some of these "partial" matches to behave as if they're full hits. (Or some other way of fixing the situation — I'm not tied to doing this one way or another). I'm totally open to this requiring some arcane specification in a configuration file, so long as it works well enough.

Thanks.

(See also https://github.com/mozilla/application-services/issues/3393 which is the tracking issue on our side of the world for finding some solution here).

thomcc commented 4 years ago

Hm, it looks like we can solve these issues to our satisfaction by disabling branch coverage. This is a bit unfortunate, but also makes sense.

That said, I stand by my statements: assert! or log::foo! don't really feel like they should count as partially-covered branches, and while ? is a lot more debatable, given how it's used in most rust code, IMO it's very unclear those branches should be counted.

grahamdyson commented 4 years ago

i've added the following CLI option to avoid the branch coverage problem when using the assert equal macro --excl-br-line assert_eq!. I expect it would work for assert too --excl-br-line assert!

rivy commented 4 years ago

I'm now using --excl-br-line "^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()" in uutils/coreutils.

Notably, this assumes fully cargo fmt'd code, with assert's on individual lines.

marco-c commented 3 years ago

Just FYI, here's the explanation for this: https://github.com/mozilla/grcov/issues/450#issuecomment-640039459.

@jcdickinson implemented those excl options exactly for this in #416, to fix #261.

marco-c commented 3 years ago

You could try source-based coverage, I've recently added support for it in grcov (version 0.6.0). Short explanation at https://marco-c.github.io/2020/11/24/rust-source-based-code-coverage.html, docs at https://github.com/mozilla/grcov#example-how-to-generate-source-based-coverage-for-a-rust-project, full example at https://github.com/marco-c/rust-code-coverage-sample.

andrewdavidmackenzie commented 3 years ago

Did you implement a workaround to ignore the log:: macros you mentioned in initial comment, so that code like this with log statements doesn't show these kinds of coverage (which depends on the log-level set I assume):

image

thomcc commented 3 years ago

I think we just turned the logging up to max, but it was a while ago. Not exactly the most satisfying answer, though.

DanielJoyce commented 3 years ago

Well technically if you want 100% coverage, you need to test the pass/fail side of assert and '?'. Other coverage tools work like this.

Ideally there would be parameters to tune this for different uses, ignore, etc

atodorov commented 3 years ago

Well technically if you want 100% coverage, you need to test the pass/fail side of assert and '?'. Other coverage tools work like this.

If I am not mistaken I'm covering both the pass & fail scenario for File::open()? but it still gets reported as partially covered. See https://github.com/atodorov/example-rust/pull/2 and https://app.codecov.io/gh/atodorov/example-rust/compare/2/tree/src/lib.rs, line 70.

FTR I have my lcov.info file printed under a DEBUG section of the GitHub logs. The relevant lines look like this:

BRDA:70,0,0,-
BRDA:70,0,1,1
BRDA:70,0,2,1