Double-digit losses of coverage using coverlet.msbuild 3.0.0 #1037

martincostello · 2021-01-11T08:47:33Z

In multiple projects' dependabot updates to coverlet.msbuild from 2.9.0 to 3.0.0, there is a double-digit loss of apparent code coverage with zero other changes.

This seems to me like a bug of some kind, rather than a bug fix of aberrant data in a previous release, but if it genuinely is a reflection of true metric misreporting in v2 then fair enough.

Here are links to the generated pull requests and their respective drop in coverage for investigation:

PR	Coverage Change
justeattakeaway/JustSaying#846 (comment)	-21%
martincostello/alexa-london-travel#444 (comment)	-26%
martincostello/alexa-london-travel-site#856 (comment)	-13.44%
martincostello/api#539 (comment)	-22.73%
martincostello/azure-functions#221 (comment)	-13.38%
martincostello/website#686 (comment)	-17.93%

Dave-EMIS · 2021-01-11T09:38:42Z

Just tested V3 on my repo and seeing the line coverage drop from 4027 to 2100 (of 4085) using coverlet.collector.

Branch coverage remains the same.

This is a private repo, so I cannot share details.

I'm looking at the HTML in dotnet-reportgenerator-globaltool 4.8.1 using the OpenCover format (set via a runsettings file).

Some of the methods have "random" missing blocks - I cannot determine a pattern for the lines missed, but this is one generic example:

This is fully covered in 1.3.0

martincostello · 2021-01-11T10:20:02Z

Looks like line coverage has been affected where the "logical code line" spans over more than one line in the source file, such as a multiple-line constructor initializer:

var foo = new Bar()
{
    Baz = "qux"
};

MarcoRossignoli · 2021-01-11T10:26:39Z

Thanks for reporting this. We'll take a look asap.

Dave-EMIS · 2021-01-11T10:27:42Z

Looks like line coverage has been affected where the "logical code line" spans over more than one line in the source file, such as a multiple-line constructor initializer:
var foo = new Bar()
{
    Baz = "qux"
};

Yeah, that seems to be the case now you mention it, it is obvious :D

MarcoRossignoli · 2021-01-11T10:29:54Z

Weird, we had some guys using nightly and this issue wasn't reported and also strange for UTs.

MarcoRossignoli · 2021-01-11T10:35:01Z

cc: @daveMueller is you have some fast idea.

JSkimming · 2021-01-11T11:06:39Z

I've found the same issue in several of my projects, for example, JSkimming/Castle.Core.AsyncInterceptor#119

Here's the coverage report for reference:

https://2024-60778941-gh.circle-artifacts.com/0/Report/index.html

MarcoRossignoli · 2021-01-11T11:40:42Z

If you can confirm the issue with a small repro could be great(like the above done by Martin)

JSkimming · 2021-01-11T11:52:14Z

@MarcoRossignoli I may have time to create something small later today, though JSkimming/Castle.Core.AsyncInterceptor#119 could be exactly what you're looking for.

It's only 5 classes, and it had 100% coverage until this update, so every instance where it's missing coverage is an instance of this issue. There's also a simple bash script (coverage.sh) to execute the tests and produce the coverage report in the root.

daveMueller · 2021-01-11T12:43:39Z

I couldn't reproduce this with the multiple-line constructor initializer. I also tested the multi-line initializer within generated IL methods but this also worked.

I will try to look into the repo @JSkimming mentioned later that day. Sorry for not find a fast solution.

MarcoRossignoli · 2021-01-11T13:48:30Z

I will try to look into the repo @JSkimming mentioned later that day. Sorry for not find a fast solution.

np at all, thx for your invaluable help!

Malivil · 2021-01-11T14:06:03Z

I created a repo with four reproduction cases using coverlet.msbuild 3.0.0

https://github.com/Malivil/Coverlet300Repro

Run the GenerateCoverageReport.bat in Coverlet300ReproTests

Malivil · 2021-01-11T14:22:24Z

I did also verify it is unrelated to the SkipAutoProps flag and also unrelated to my recent upgrade to .Net 5 (The only two things that I changed other than updating Coverlet)

MarcoRossignoli · 2021-01-11T14:24:04Z

@daveMueller pretty sure the regression was introduced with this one #1006 we should try to revert for fast check.

Dave-EMIS · 2021-01-11T14:24:46Z

If it matters, I'm using .NET 5.0.101 SDK and building netcoreapp3.1...

Malivil · 2021-01-11T14:26:15Z

If it matters, I'm using .NET 5.0.101 SDK and building netcoreapp3.1...

I tried .Net 5.0.101 SDK and building netcoreapp3.1 as well as net5.0

I did not try with the .Net 3.1.x SDK though

bkaid · 2021-01-11T15:40:46Z

Weird, we had some guys using nightly and this issue wasn't reported and also strange for UTs.

I actually did run into this with the nightly and meant to report it but was going to try to put a repro sample together first. Sorry about that.

MarcoRossignoli · 2021-01-11T16:04:38Z

I actually did run into this with the nightly and meant to report it but was going to try to put a repro sample together first. Sorry about that.

Oh no problem at all...I'm curious to know, we have some UT's with multiline and lambda etc(like David sample above) so it's weird we didn't catch before. Anyway np we'll fix it.

daveMueller · 2021-01-12T03:09:26Z

@MarcoRossignoli I guess i found the issue and as you assumed it is related to #1006. Here

coverlet/src/coverlet.core/Coverage.cs

Lines 397 to 404 in d43f83d

    
           foreach (HitCandidate hitCandidate in result.HitCandidates) 
        
           { 
        
               if (hitCandidate.isBranch || hitCandidate.end == hitCandidate.start) 
        
               { 
        
                   continue; 
        
               } 
        
               foreach (HitCandidate hitCandidateToCompare in result.HitCandidates)

we don't consider the docIndex of the HitCandidates which means we are comparing lines of different classes. For a quick test I just added

HitCandidate hitCandidateToCompare in result.HitCandidates.Where(x => x.docIndex.Equals(hitCandidate.docIndex))

and it seemed to work. I will double check this tomorrow and create a PR. This would also explain why we didn't see this in our UT because there is only one class loaded per test. I hope this is the only issue and I don't find other problems tomorrow.

Thanks @Malivil for the great repro. I compiled my 'quick' fix and tested it on the project.

Malivil · 2021-01-12T03:20:34Z

Glad to be of assistance, more glad that you found the cause =)

MarcoRossignoli · 2021-01-12T06:53:35Z

Good catch @daveMueller!
Can you try to fix and build package from your branch using

dotnet pack -c release /p:TF_BUILD=true /p:PublicRelease=true

And attach the three packages to this issue?
So our kind/patiences friend here can try on their projects and confirm the fix?

I expect a package with "preview" tag on version.

This would also explain why we didn't see this in our UT because there is only one class loaded per test.

😢

skovalyova · 2021-01-12T14:05:52Z

The same issue for .NET 5 after updating coverlet to 3.0.0 - coverage is 20% less than for 2.9.0 (json, opencover, coberture report types). FineCodeCoverage (https://github.com/FortuneN/FineCodeCoverage) shows correct coverage.

Malivil · 2021-01-12T14:14:25Z

FineCodeCoverage installs an old version by default but will use a newer version if you update the coverlet tool yourself. If you were to update the tool it would show the same coverage problem.

daveMueller · 2021-01-13T06:17:39Z

Sorry for the late response but now here I have attached the packages with the potential fix. Would be cool if someone could test this and give some feedback. 😏
coverlet.3.0.1-preview.4.zip

JSkimming · 2021-01-13T07:15:39Z

@daveMueller I've just tried it on Castle.Core.AsyncInterceptor and it looks like you've fixed the issue: 100% (328 of 328), whereas 3.0.0 counted 89% (196 of 220).

Here's the output.

+------------------------------+------+--------+--------+
| Module                       | Line | Branch | Method |
+------------------------------+------+--------+--------+
| Castle.Core.AsyncInterceptor | 100% | 100%   | 100%   |
+------------------------------+------+--------+--------+

+---------+------+--------+--------+
|         | Line | Branch | Method |
+---------+------+--------+--------+
| Total   | 100% | 100%   | 100%   |
+---------+------+--------+--------+
| Average | 100% | 100%   | 100%   |
+---------+------+--------+--------+

Interestingly, the numbers are now much closer to OpenCover (I used this before coverlet) which counts 100% (333 of 333).

MarcoRossignoli · 2021-01-13T08:10:23Z

Thanks @JSkimming for the fast test
@Dave-EMIS @martincostello can you try to double check 🙇 ?

@daveMueller open the PR that we'll discuss the fix.

martincostello · 2021-01-13T10:44:48Z

Using the 3.0.1-preview.4 package with martincostello/api#539 gives the following results:

+-----------+--------+--------+--------+
| Module    | Line   | Branch | Method |
+-----------+--------+--------+--------+
| API       | 84.17% | 57.08% | 98.46% |
+-----------+--------+--------+--------+
| API.Views | 88.99% | 66.41% | 83.33% |
+-----------+--------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total   | 84.76% | 60.25% | 96.62% |
+---------+--------+--------+--------+
| Average | 86.58% | 61.74% | 90.89% |
+---------+--------+--------+--------+

So that actually improves on the previous coverage in the main branch of 84.20%. 🥳

Malivil · 2021-01-13T12:44:42Z

I'm seeing good results with the 3.0.1 preview as well

2.9.0 - 92.2% (1726 of 1871)
3.0.0 - 85.9% (1559 of 1813)
3.0.1 - 96% (1742 of 1813)

EDIT: Disregard my previous edit - I'm dumb =)

MarcoRossignoli · 2021-01-14T08:05:53Z

Thanks to super @daveMueller this fix tomorrow will be on nightly https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md
Asap I'll push on nuget(bit busy atm).

skovalyova · 2021-01-16T08:26:24Z

Bug is closed, but update is still not available from nuget package manager in VS - when will it be added?

MarcoRossignoli · 2021-01-16T09:38:18Z

Just released new version with fix to nuget.org

ulrichb · 2021-01-16T10:21:46Z

Okay, it got way better with 3.0.1, but there are still false negatives.

Example 1:

Example 2:

MarcoRossignoli · 2021-01-16T10:25:03Z

@ulrichb can you open a new issue and provide a small repro?
I think this is a different issue unrelated to this one, as we tested for that specific issue where the bug drops the numbers a lot.

AraHaan · 2022-04-09T00:51:12Z

Using the Following Packages on my Source Generator Tests produce the following Results:

coverlet.collector/3.1.2
coverlet.msbuild/3.1.2
Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.XUnit/1.1.2-beta1.22205.2
Microsoft.CodeAnalysis/4.0.1
Microsoft.NET.Test.Sdk/17.3.0-preview-20220408-15
xunit.runner.visualstudio/2.4.3
xunit/2.4.2-pre.22

+--------+------+--------+--------+
| Module | Line | Branch | Method |
+--------+------+--------+--------+

+---------+------+--------+--------+
|         | Line | Branch | Method |
+---------+------+--------+--------+
| Total   | 0%   | 0%     | 0%     |
+---------+------+--------+--------+
| Average | 0%   | 0%     | 0%     |
+---------+------+--------+--------+

MarcoRossignoli · 2022-04-09T07:12:09Z

@AraHaan are you using the collector integration? If not that can be the issue as it's a known one

https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/KnownIssues.md

AraHaan · 2022-04-09T16:14:21Z

I discovered the issue, the source generator known as: Microsoft.CodeAnalysis.ResxSourceGenerator generates it's code without the .g.cs ending (which my source generator uses to generate code to it's resx file).

My issue will now be able to be fixed with: dotnet/roslyn-analyzers#5959

MarcoRossignoli added the untriaged To be investigated label Jan 11, 2021

MarcoRossignoli added tenet-coverage Issue related to possible incorrect coverage bug Something isn't working labels Jan 11, 2021

MarcoRossignoli pinned this issue Jan 11, 2021

This was referenced Jan 11, 2021

Bump coverlet.msbuild from 2.9.0 to 3.0.0 JSkimming/Castle.Core.AsyncInterceptor#119

Closed

Bump coverlet.msbuild from 2.9.0 to 3.0.0 JSkimming/abioc#151

Closed

Bump coverlet.msbuild from 2.9.0 to 3.0.0 JSkimming/tesla-net#121

Closed

sys27 mentioned this issue Jan 11, 2021

Bump coverlet.msbuild from 2.9.0 to 3.0.0 in /xFunc.Tests sys27/xFunc#394

Closed

eoehen mentioned this issue Jan 12, 2021

Bump coverlet.msbuild from 2.9.0 to 3.0.0 in /src eoehen/arguard#88

Closed

daveMueller mentioned this issue Jan 13, 2021

Fix loss in coverage #1043

Merged

MarcoRossignoli closed this as completed in #1043 Jan 14, 2021

This was referenced Jan 14, 2021

Coverage went down after Upgrade to version 3.0.0 #1048

Closed

Lower covered lines after using coverlet 3.0.0 #1049

Closed

petli mentioned this issue Jan 14, 2021

Wrong coverage report #1050

Closed

MarcoRossignoli mentioned this issue Jan 14, 2021

Lower coverage reported than Visual Studio #1029

Closed

MarcoRossignoli unpinned this issue Jan 16, 2021

MarcoRossignoli mentioned this issue Jan 16, 2021

Deprecate and unlist package version 3.0.0 and change release habits #1055

Closed

ulrichb mentioned this issue Jan 16, 2021

Multi-line lambda false negatives in 3.0.1 #1056

Closed

JimBobSquarePants mentioned this issue Feb 17, 2021

Fix Code Coverage SixLabors/ImageSharp#1554

Merged

4 tasks

Double-digit losses of coverage using coverlet.msbuild 3.0.0 #1037

Double-digit losses of coverage using coverlet.msbuild 3.0.0 #1037

Comments

martincostello commented Jan 11, 2021

Dave-EMIS commented Jan 11, 2021

martincostello commented Jan 11, 2021 • edited Loading

MarcoRossignoli commented Jan 11, 2021

Dave-EMIS commented Jan 11, 2021

MarcoRossignoli commented Jan 11, 2021 • edited Loading

MarcoRossignoli commented Jan 11, 2021

JSkimming commented Jan 11, 2021

MarcoRossignoli commented Jan 11, 2021

JSkimming commented Jan 11, 2021

daveMueller commented Jan 11, 2021

MarcoRossignoli commented Jan 11, 2021

Malivil commented Jan 11, 2021 • edited Loading

Malivil commented Jan 11, 2021 • edited Loading

MarcoRossignoli commented Jan 11, 2021

Dave-EMIS commented Jan 11, 2021

Malivil commented Jan 11, 2021

bkaid commented Jan 11, 2021

MarcoRossignoli commented Jan 11, 2021

daveMueller commented Jan 12, 2021

Malivil commented Jan 12, 2021

MarcoRossignoli commented Jan 12, 2021 • edited Loading

skovalyova commented Jan 12, 2021 • edited Loading

Malivil commented Jan 12, 2021 • edited Loading

daveMueller commented Jan 13, 2021

JSkimming commented Jan 13, 2021

MarcoRossignoli commented Jan 13, 2021 • edited Loading

martincostello commented Jan 13, 2021

Malivil commented Jan 13, 2021 • edited Loading

MarcoRossignoli commented Jan 14, 2021 • edited Loading

skovalyova commented Jan 16, 2021

MarcoRossignoli commented Jan 16, 2021

ulrichb commented Jan 16, 2021

MarcoRossignoli commented Jan 16, 2021 • edited Loading

AraHaan commented Apr 9, 2022

MarcoRossignoli commented Apr 9, 2022 • edited Loading

AraHaan commented Apr 9, 2022 • edited Loading

martincostello commented Jan 11, 2021 •

edited

Loading

MarcoRossignoli commented Jan 11, 2021 •

edited

Loading

Malivil commented Jan 11, 2021 •

edited

Loading

Malivil commented Jan 11, 2021 •

edited

Loading

MarcoRossignoli commented Jan 12, 2021 •

edited

Loading

skovalyova commented Jan 12, 2021 •

edited

Loading

Malivil commented Jan 12, 2021 •

edited

Loading

MarcoRossignoli commented Jan 13, 2021 •

edited

Loading

Malivil commented Jan 13, 2021 •

edited

Loading

MarcoRossignoli commented Jan 14, 2021 •

edited

Loading

MarcoRossignoli commented Jan 16, 2021 •

edited

Loading

MarcoRossignoli commented Apr 9, 2022 •

edited

Loading

AraHaan commented Apr 9, 2022 •

edited

Loading