Coverlet: Async methods excluded from coverage after updating to 2.8.1

Created on 6 Apr 2020  路  32Comments  路  Source: coverlet-coverage/coverlet

Using coverage format "OpenCover" and the coverlet.msbuild package
In version 2.8.0 an async method in my Web API class shows no coverage (because I haven't written tests yet):
BeforeCoverage

In version 2.8.1, the same method is just excluded from coverage entirely:
AfterCoverage

The coverage.opencover.xml shows a complete missing section for the method:
Diff

I do not have a minimal reproduction example but if needed I can probably make one.
May be related to 528956bc1189eb541a6f7183819261f4b20e5dcc

as-designed tenet-coverage

All 32 comments

Do you have ExcludeFromCodeCoverage attribute somewhere?
Do you see missing coverage only for this method in source or for all method of same class?

I do not have a minimal reproduction example but if needed I can probably make one.

Would be great!It's hard without repro.

cc: @matteoerigozzi

This class does not have ExcludeFromCodeCoverage
Only the two methods marked async are not showing coverage.

I will try to make a simple reproduction project. I'll comment here with the repo when it's complete.

Ok thanks a lot!

Here's a repro with the minimum example: https://github.com/Malivil/CoverletAsyncRepro
If you run the .bat file within the .Tests project it will generate a report in the TestResults folder

Changing the version of the coverlet.msbuild in the .Tests csproj to 2.8.0 and then re-running the .bat will show the function as uncovered as it should.

Great, stay on 2.8.0 for now I'll come back asap.

Hi there,

I've also noticed this issue recently, it appears to be something to do with the /p:ExcludeByAttribute=CompilerGeneratedAttribute argument, as removing this will cause the coverage to be successfully reported. Thanks for the tip of going back to 2.8.0 as that also solves the issue for now.

@Malivil can you confirm that removing /p:ExcludeByAttribute=CompilerGeneratedAttribute solve(disappear, but it's clearly a bug) issue?

@MarcoRossignoli I feel that I need to clarify; athough removing the argument appears to solve this issue, it will cause all our auto-implemented properties in our DTOs to record no coverage without being explictly tested, which we did not have to do prior to 2.8.1.

@bengoymer yes I mean this is a bug but if we know that is related to that parameter we can fix more fast.

@MarcoRossignoli perfect, thank you.

@bengoymer can you try to remove that param and filter out DTO using source file filtering(if DTO are in separate files)?
https://github.com/tonerdo/coverlet/blob/master/Documentation/MSBuildIntegration.md#source-files

@MarcoRossignoli yes, just done that. If I filter out the DTOs using the source file filtering, and remove ExcludeByAttribute the coverage of the async functions is as I'd expect.

@MarcoRossignoli Removing that exclude does fix this issue. The reason I had that was mostly to exclude generated property Getters and Setters.

@Malivil sorry I wasn't clear, this is only to confirm that the issue disappear if you remove that attribute for those async method, it won't fix the issue it's to have a double confirmation on where issue could be.

@MarcoRossignoli I understand. I was just adding clarity as to why I had it there in the first place =)

I guess I should have used a different word than "fix"

So if you remove that attribute you see async methods uncovered yet?

Yes, removing the attribute exclude shows the async methods as being uncovered, that's what I meant when I said it "[fixed] the issue". Sorry for not being clear =)

Seeing this too, workaround:

<!-- <ExcludeByAttribute>Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute</ExcludeByAttribute> -->
<ExcludeByAttribute>Obsolete</ExcludeByAttribute>

@Malivil can you try the workaround(temporary) and tell me if it works?

Where does that go? In the test .csproj?

@Malivil It depends on your usage. We use the collector, so we add this to the runsettings file.

We use the msbuild integration as shown in the test repo: https://github.com/Malivil/CoverletAsyncRepro

That's what I thought.

@MarcoRossignoli: This workaround behaves the same as the command-line change which I think is expected.

This workaround behaves the same as the command-line change which I think is expected.

Not clear what you mean, /p:ExcludeByAttribute=Obsolete for msbuild

This workaround behaves the same as the command-line change which I think is expected.

Not clear what you mean, /p:ExcludeByAttribute=Obsolete for msbuild

Using that switch behaves the same as if I don't use ExcludeByAttribute at all. I don't have anything annotated with Obsolete so it has no affect.

Using that switch behaves the same as if I don't use ExcludeByAttribute at all. I don't have anything annotated with Obsolete so it has no affect.

Yes the test was only to understand if we're excluding something generated by compiler and also with Obsolete and maybe the result could be a bit different. But ok thanks for help on investigation.

If I use Obsolete, GeneratedCodeAttribute, or a combination of both it does not exclude the async method. If I add CompilerGeneratedAttribute back in to either the switch or the test .csproj then it does exclude the async method again. So it seems to be narrowed to CompilerGeneratedAttribute specifically.

So it seems to be narrowed to CompilerGeneratedAttribute specifically.

Yep, my hope was find a temporary workaround with no version downgrade, but seem not possible at the moment.

@Malivil we did some check, actually the behaviour is correct, we fixed a bug where CompilerGeneratedAttribute wasn't honored, so now correct behaviour is to exclude class with that attribute and in your case every async state machine(autogenerated class by roslyn compiler for async methods) has got that attribute.
You cannot use that filter to remove autoprops from instrumentation anymore and it's correct.
A solution for now is to use ExcludeByFile filter and remove al DTOs source files.

We should add support for skip autoprops https://github.com/tonerdo/coverlet/issues/328

I cannot exclude the every file that has properties in it. Looks like I'll need to remove that exclude and have my coverage drop until #328 is complete

Understood, for now feel free to close this issue, I'll move that issue up in queue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

IGx89 picture IGx89  路  4Comments

MarcoRossignoli picture MarcoRossignoli  路  6Comments

hlubovac picture hlubovac  路  5Comments

chaoticsoftware picture chaoticsoftware  路  7Comments

ghost picture ghost  路  5Comments