Would be nice to be able to ignore lines from coverage

[dart-lang]

Would be nice to be able to ignore lines from coverage

[wouterhardeman]

Hey @cbracken,

I'm currently working on a PR. Thanks for the pointers. My current solution searches the source file for certain comments. It extracts the ignored line numbers and excludes them from the hitmap. Currently I have implemented:

• // coverage:ignore-line: ignores the current line of code
• // coverage:ignore-start: starts ignoring all next lines
• // coverage:ignore-end: stops ignoring lines

I still have to implement the opt-in argument.

I'll create a pull request tomorrow. I'm looking forward to your review!

[simc]

Is there any news on this feature? It would be really convenient 👍

[vysotsky]

Ok. If someone's interested, I've solved the problem by editing lcov.info itself.

flutter test --coverage
lcov --remove coverage/lcov.info "lib/generated/*" -o coverage/lcov_cleaned.info
genhtml -o coverage coverage/lcov_cleaned.info --no-function-coverage -s -p `pwd`

It helps to remove unneeded files

[jtmcdole]

It is not a thing, I was just suggesting a thing!

[zoechi]

or

// ignore: coverage

like used to suppress linter warnings

[Justin-Randall]

Whatever happened with the PR?

[fatherOfLegends]

According to the readme it appears like this was completed.

Ignore lines from coverage
// coverage:ignore-line to ignore one line.
// coverage:ignore-start and // coverage:ignore-end to ignore range of lines inclusive.
// coverage:ignore-file to ignore the whole file.

Looking in the change log I see this:

0.14.0 - 2020-06-04
Add flag --check-ignore that is used to ignore lines from coverage depending on the comments.
Use // coverage:ignore-line to ignore one line. Use // coverage:ignore-start and // coverage:ignore-end to ignore range of lines inclusive. Use // coverage:ignore-file to ignore the whole file.

#302

I think this issue can be closed.

That said I want to use this in a Flutter context.

I see this in the flutter codebase: flutter/flutter#61408
Looks like it should be in the latest stable release of Flutter but it does not appear to work.

[jtmcdole]

What about @coverage(false) or @NotCovered

[eseidelGoogle]

I didn't know that was a thing. Thanks!

[eseidelGoogle]

Oh, sorry.

[eseidelGoogle]

The only downside of that, it that it likely means you have to incldue package:coverage as a dependency in your package, which seems suboptimal.

[jtmcdole]

Solid point & agreed. I was trying to use:

  void _();

And convert the class to abstract (its a static warehouse), but I'm getting warnings about it being unused in strong mode... which seems odd at first and then expected since it is private and nothing in the library is using it.

[cbracken]

It'd be a relatively substantial change to support driving this from the source level, since we don't look at it at all at the moment. All the data we need is available (in most cases) though.

Right now coverage collection doesn't look at the source at all, though it's available via the VM service so long as you're not running from a snapshot. When running from a snapshot, the VM will provide source reconstituted from the tokenised source (and line numbers will match up) but comments are stripped. I suspect that in most cases, it's reasonable to require that developers run from source rather than a snapshot for coverage, but that'll exclude dart: libraries for example. For such cases, we could support a path (or paths) to the code on disk and go from there.

I like the idea of doing it inline with the source since it reduces the likelihood of data getting out of date, compared to something like a list of ignored ranges, etc.

[cbracken]

On the specific issue here, it seems like a reasonable idea to identify a set of conditions under which code should be marked as non-coverable by default like the case @eseidelGoogle mentions above: private constructors with no invocations within the library.

[dnfield]

Would it make sense to have this be driven upstream by the observatory report? Perhaps having some kind of flag/attribute/comment the observatory could use to indicate that a file/class/method/set of lines are to be excluded.

[fatherOfLegends]

Please add this.

[JZ-at-TP]

Almost a year later. There is currently no way to get 100% coverage without this, or the ability to mock static class methods.

[cbracken]

Just to set expectations, this is not something that's likely to be worked on by us in the near future. I'd be more than happy to review community contributions that cleanly resolve this issue though.

A proper solution likely involves integrating with package:analyzer. A couple things that make this a bit trickier:

1. We probably want to avoid relying on defining any annotations in this package. Using something like @coverage(false) requires package:coverage to be a real dependency not just a dev_dependency in pub. A simple option here is to do what the linter does and rely on a comment immediately above the line/block of code in question. e.g. // coverage: false.
2. For cases such as the example at the top, ideally we wouldn't require any markup to ignore it, we'd have a set of rules that identifies code as either unhittable, or maybe-unhittable. e.g. in the case above we might mark as un-hittable only if we find it's not hit. Specifically, there could be cases where such a method is hit -- e.g. if construction were driven entirely by a static factory method that calls the private ctor.
3. Coverage collection doesn't currently rely on the source code (formatting in pretty print mode does, but doesn't compute or resolve an AST). We'd want any dependency on the source code to be strictly opt-in, since there are cases where the code being executed and the code on disk may differ -- e.g. when codegen is in use. As a particularly egregious case, imagine something akin to pub transformers, which are fortunately no longer supported. Script sources can be collected from the VM, but I can't remember if comments are retained.
4. Another reason to make this opt-in and disabled by default is the likely performance hit involved in reading in the source code, computing an AST, then applying a set of rules to that AST to selectively ignore coverage. I'd expect that adding the additional step of parsing the source code of some of the more massive apps internally at Google could very significantly impact collection times.

As noted above, I'm more than happy to review any community contributions that implement this.

[vysotsky]

Is there any chance to get this feature working? We're using code generation for Protobuf and seems we're getting wrong coverage 😞

[rknell]

@vysotsky does that just remove references to files? Is there a way to filter individual lines?

[egarson]

@rknell: No, that's not supported by lcov.
While we're at it, there is also a pub package that will post-process an lcov file to remove files: remove_from_coverage.

[cbracken]

Yep this can definitely be closed. This should be on the current beta branch for Flutter, and will roll out in the next stable release.

There are a few things that have changed since this was first opened, a key one being that in-memory source transformers are no longer part of the Dart ecosystem, so libraries loaded in the VM are almost always 1:1 with code on disk, even generated code. If anyone runs into any edge cases around the mapping between the source as known to the VM and the source on disk that we now resolve to for the purposes of ignoring lines, I'd love to know. Please file issues.

[fatherOfLegends]

@cbracken Thanks for your help! I see the commit linked below in the Flutter codebase and it is tagged with 1.22.0 the current stable release. I thought that would mean it is available. I am excited to see it coming in the next stable release but I must be misunderstanding how to determine what release a feature will be in. I am curious if you can tell me how to determine what flutter version this in included in? Thanks again!

flutter/flutter@5c9d09f#commitcomment-42931558