-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Would be nice to be able to ignore lines from coverage #580
Comments
What about @coverage(false) or @NotCovered |
I didn't know that was a thing. Thanks! |
It is not a thing, I was just suggesting a thing! |
Oh, sorry. |
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. |
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. |
or // ignore: coverage like used to suppress linter warnings |
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 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. |
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. |
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. |
Is there any news on this feature? It would be really convenient 👍 |
Please add this. |
Almost a year later. There is currently no way to get 100% coverage without this, or the ability to mock static class methods. |
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:
As noted above, I'm more than happy to review any community contributions that implement this. |
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:
I still have to implement the opt-in argument. I'll create a pull request tomorrow. I'm looking forward to your review! |
Whatever happened with the PR? |
Is there any chance to get this feature working? We're using code generation for Protobuf and seems we're getting wrong coverage 😞 |
Ok. If someone's interested, I've solved the problem by editing lcov.info itself.
It helps to remove unneeded files |
@vysotsky does that just remove references to files? Is there a way to filter individual lines? |
@rknell: No, that's not supported by lcov. |
According to the readme it appears like this was completed.
Looking in the change log I see this:
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 |
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. |
@cbracken Thanks for your help! I see the commit linked below in the Flutter codebase and it is tagged with |
I'm not sure how one best would implement this (or if it's even possible), but there are certain lines of code in dart which are intentionally unreachable, an example of such is a private constructor.
Which is a pattern used to make a class non-constructable. It's silly that coverage tells me that this line is uncovered:
https://coveralls.io/builds/9847810/source?filename=lib%2Fsrc%2Fmaterial%2Fcolors.dart
But I'm aware of no way with package:coverage to avoid that?
I could imagine a comment syntax, or maybe just teaching package:coverage about this specific idiom? Not sure.
This is not a high priority, but would be nice to have for projects trying for high line or file coverage numbers.
The text was updated successfully, but these errors were encountered: