OpenCover / opencover

A code coverage tool for .NET 2 and above (WINDOWS OS only), support for 32 and 64 processes with both branch and sequence points
https://blog.many-monkeys.com
Other
1.31k stars 248 forks source link

Excludebyattribute not excluding the method #779

Open dwkane opened 6 years ago

dwkane commented 6 years ago

Please provide the following information when submitting an issue.

Where appropriate replace the [ ] with a [X]

My Framework

My Environment

I have already...

My issue is related to (check only those which apply):

Expected Behavior

I am expecting that when using an attribute on a method and then using the -excludebyattribute filter, that the entire method would be excluded from coverage reporting.

Actual Behavior

The line within the method is still reported as uncovered. See repro steps for details.

opencover

Steps to reproduce the problem:

OpenCover command: "%userprofile%\.nuget\packages\OpenCover\4.6.801\tools\OpenCover.Console.exe" -oldstyle -output:"opencover.xml" -register:user -target:"powershell.exe" -targetargs:"C:\Sentinel\buildtest.ps1" -filter:"+[*]Sentinel.* -[Fake*]* -[Fluent*]*" -mergebyhash -skipautoprops -hideskipped:All -coverbytest:* -excludebyattribute:*.Exclude*

The buildtest.ps1 contains: dotnet build dotnet vstest (Get-ChildItem -recurse -File *.Tests.*dll | ? { $_.FullName -notmatch "\\obj\\?" })

Output XML attached as well: opencover.zip

Method is: Sentinel.Registration.API\Sentinel.Registration\Sentinel.Registration.Api\Controllers\RegistrantController.cs:Get (as the example shown here)

If I don't use the exclude attribute, then line 44 in the image attachment is shown uncovered as well. This is considered a pass through method for us and we are saying it doesn't need unit test coverage (because the getRegistrantById class has it's own tests).

Here's an additional screenshot showing the Exclude attribute working for the constructor. You can see three methods here. Two with the Exclude attribute and one without. It correctly excludes the constructor. Is this an issue with delegates in a method?

excludebyattribute

How can we truly skip this method (and others like it)? In the output XML, I don't see any of the skippeddueto=Attribute things that would indicate that the method should be skipped.

I have reproduced this on the latest release (4.6.519) and the latest build (4.6.801).

Any ideas? Am I missing something?

dwkane commented 6 years ago

@sawilde Just checking in on this issue. Could you help with why the method isn't being skipped properly?

sawilde commented 6 years ago

Hi @dwkane - from your description, it is possibly due to it being an async method.

dwkane commented 6 years ago

@sawilde Yes, after a bit of searching existing issues, I would guess it is related to the async. This would be great to be unblocked on as I am looking to reduce the amount of noise in the reports for methods that don't need coverage. Thanks. :)

sawilde commented 6 years ago

@dwkane I'll have a look but having issue with my build server at the moment :(

dwkane commented 6 years ago

@sawilde Just one more screenshot showing awaits being indicated as uncovered branches. It might be related to my issue.

opencover2

sawilde commented 6 years ago

@dwkane OpenCover looks at IL branches and does its best to align them with source code - async/await may look simple to write but the IL produced it quite complex. If you want to diagnose it then you should start decompiling into IL and see if you can see the cause yourself.

SteveGilham commented 6 years ago

The await section in this code will be in a generated inner class called <Get>d__# where # is a numeral.

In general C# helper types generated from code in namespace.class.method will be types namespace.class/<method>xxx for some decorator xxx; or are in class namespace.class/<>xxx with the logic in a method called <method>xxx (lambdas take the latter shape). (F# is a different matter with generated types being inner types of the surrounding type but with a class name containing @<line number> rather than a direct indication of the enclosing function).

In any case, neither inherit the attributes on the lexically containing method, so it becomes a matter of IL navigation to go from the name to the enclosing method.

The uncovered branches in are implementation detail and reflect parts of a state machine in synthetic methods <ExecuteAsync>d__#.MoveNext() and <Action>d__#.MoveNext() -- there's a different path taken if the wait completes promptly (it does a synchronous return) than in the case where it does not (and it has to defer completion to a continuation); this is the sort of thing that will be difficult to force tests to exercise.

SteveGilham commented 6 years ago

Doing some more testing in this area, I have the following cases where the attribute on the function isn't propagated to all the syntactic scope of the function body C# 779a F# 779b

SteveGilham commented 6 years ago

It should also come as no surprise that C#7.0 local functions are similarly not excluded even if the enclosing method is.

arphox commented 5 years ago

I just stumbled upon this issue as well:

coverage_bug

For me it seems to be that lambdas that cause the problem. I hope it helps.

sawilde commented 5 years ago

Yup - it is slated for a future release