Use MS coverage instead of coverlet #6211

Evangelink · 2022-10-19T09:33:28Z

This solves the "Unable to read beyond the end of the stream." we keep facing on CI builds

Full error:

D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error : Unable to read beyond the end of the stream. [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at System.IO.BinaryReader.FillBuffer(Int32 numBytes) [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at System.IO.BinaryReader.ReadInt32() [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at Coverlet.Core.Coverage.CalculateCoverage() in /_/src/coverlet.core/Coverage.cs:line 414 [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at Coverlet.Core.Coverage.GetCoverageResult() in /_/src/coverlet.core/Coverage.cs:line 161 [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at Coverlet.MSbuild.Tasks.CoverageResultTask.Execute() in /_/src/coverlet.msbuild.tasks/CoverageResultTask.cs:line 83 [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]

mavasani · 2022-10-19T11:16:28Z

@sharwell

Evangelink · 2022-10-19T12:09:38Z

Looks like this will require a change in arcade (or customization). I will sync with them to see how best to do so.

In the meantime, moving this back to draft.

This solves the "Unable to read beyond the end of the stream." we keep facing on CI builds

eng/CodeCoverage.proj

codecov · 2022-10-20T13:03:31Z

Codecov Report

Merging #6211 (1ad571e) into main (df911e6) will decrease coverage by 6.98%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #6211       +/-   ##
===========================================
- Coverage   96.14%   89.17%    -6.98%     
===========================================
  Files        1365     1165      -200     
  Lines      317523    70799   -246724     
  Branches    10269     6791     -3478     
===========================================
- Hits       305295    63135   -242160     
+ Misses       9790     6128     -3662     
+ Partials     2438     1536      -902

Evangelink · 2022-10-20T13:07:13Z

@sharwell @mavasani PR is ready. There is a change in coverage between the tools, I will discuss with colleagues to try to understand if something is wrong on our coverage side or if that's just more accurate.

Evangelink · 2022-10-20T13:15:04Z

To really complete this PR, it would be nice to add some .runsettings file to configure Microsoft.CodeCoverage so that we don't try to analyzer native and so that we can skip external dlls (xunit, humanizer...).

eng/CodeCoverage.proj

mavasani · 2023-01-03T09:51:33Z

@sharwell @jmarolf Please let us know if you have any concerns here.

mavasani · 2023-01-03T09:54:30Z

/azp run

azure-pipelines · 2023-01-03T09:54:45Z

Azure Pipelines successfully started running 3 pipeline(s).

Directory.Build.targets

stephentoub · 2023-01-10T21:31:03Z

The cited issue looks like coverlet-coverage/coverlet#865.

Switching away from coverlet to address it seems like a really big hammer.

What functionality is gained / lost by switching from coverlet to mstest?

Evangelink · 2023-01-11T08:00:53Z

The cited issue looks like coverlet-coverage/coverlet#865.

Switching away from coverlet to address it seems like a really big hammer.

What functionality is gained / lost by switching from coverlet to mstest?

@MarcoRossignoli is owner of coverlet and also working on MS coverage, I think he will give more accurate answers than what I can do.

sharwell · 2023-01-19T20:13:00Z

@Evangelink in the latest report, it appears the line numbers are off by 2 in a bunch of the new data

sharwell · 2023-01-19T20:18:46Z

Here's a file with confusing data:
https://app.codecov.io/gh/dotnet/roslyn-analyzers/commit/1c2be93b081e7a96422516c874ddd6a0c2696b93/blob/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Security/Helpers/InsecureDeserializationTypeDecider.cs

Looks like none of the local functions are tracked.

sharwell · 2023-01-19T20:19:22Z

Here's one where the line numbers seem incorrect:
https://app.codecov.io/gh/dotnet/roslyn-analyzers/commit/1c2be93b081e7a96422516c874ddd6a0c2696b93/blob/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Security/DoNotAlwaysSkipTokenValidationInDelegates.cs

Directory.Build.targets

jakubch1 · 2023-01-20T15:19:32Z

Here's one where the line numbers seem incorrect: https://app.codecov.io/gh/dotnet/roslyn-analyzers/commit/1c2be93b081e7a96422516c874ddd6a0c2696b93/blob/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Security/DoNotAlwaysSkipTokenValidationInDelegates.cs

I am not able to reproduce issue locally. Can we rerun this codecov logic? Maybe source changed in the meantime and we use old coverage report.

sharwell · 2023-01-20T16:19:09Z

@jakubch1 I'm looking at the .txt file uploaded to codecov.io (which contains the embedded Coburtura XML content) and it looks like the line numbers in the file above don't match the content of that file.

sharwell · 2023-01-20T16:23:35Z

@jakubch1 It looks like codecov.io is rendering content of this file from prior to d32f4d9, which added 3 lines to the start of the file. This would occur because the CI build doesn't execute against the head of this branch, but rather against the merge commit.

sharwell · 2023-01-20T16:26:02Z

@Evangelink normally I would avoid merging main into the PR branch unless there's a merge conflict, but in this case it seems like it will help us get a better understanding of the coverage difference.

Evangelink · 2023-01-20T18:26:15Z

@sharwell I will try to have a look at this either during weekend or on Monday.

jakubch1 · 2023-01-23T07:01:03Z

Thanks for the detailed explanation. How should we be thinking about coverlet in other repos then? Should we be switching dotnet/runtime's code coverage as in this PR?

If

you don't need to know how many times each line was executed
it's not mandatory for you to use only open-source tools in your repository
you don't need to get coverage in a constrained environment like the case of System.Private.Corelib.dll where there is no support of BCL

we recommend switching to VS code coverage for better performance and reliability.

stephentoub · 2023-01-23T12:52:31Z

Thanks for the detailed explanation. How should we be thinking about coverlet in other repos then? Should we be switching dotnet/runtime's code coverage as in this PR?

If

you don't need to know how many times each line was executed

it's not mandatory for you to use only open-source tools in your repository

you don't need to get coverage in a constrained environment like the case of System.Private.Corelib.dll where there is no support of BCL

we recommend switching to VS code coverage for better performance and reliability.

Oh, that rules out using it in dotnet/runtime.

jakubch1 · 2023-01-23T13:13:17Z

Here's a file with confusing data: https://app.codecov.io/gh/dotnet/roslyn-analyzers/commit/1c2be93b081e7a96422516c874ddd6a0c2696b93/blob/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Security/Helpers/InsecureDeserializationTypeDecider.cs

Looks like none of the local functions are tracked.

We add local functions in separate classes in the report. It looks like reportgenerator doesn't like it. I will investigate it.

sharwell · 2023-01-23T21:28:22Z

you don't need to know how many times each line was executed

The performance of this feature was so poor in Coverlet that we always disabled it anyway. I wouldn't consider removal of this feature a downside mostly because I never saw it work in practice.

it's not mandatory for you to use only open-source tools in your repository

I find this pretty concerning. Why would this tool not be open source?

Evangelink · 2023-01-24T09:29:28Z

Oh, that rules out using it in dotnet/runtime.

I don't want to say something wrong but I think it's technically possible for you to use coverlet for CoreLib and Code Coverage for the rest. @MarcoRossignoli @jakubch1 Am I wrong?

I find this pretty concerning. Why would this tool not be open source?

This was a collegial decision made with management based on proprietary code, tools and some business directions. I don't think there is any plan to revisit this decision in the short term. @jakubch1 and @pavelhorak might be able to give you more info.

This PR was a proposition to fix recurring issue on this repo and at the same time move to a tool that seems more suited (i.e. whose benefits are bigger for the need of this repo). If you prefer to stick with coverlet for any reason, that's perfectly fine and so please feel free to close this PR. FYI I can file a different PR to do a small change in the way you are using coverlet that would fix the issue and leave the rest of the layout the same.

stephentoub · 2023-01-24T11:30:21Z

I think it's technically possible for you to use coverlet for CoreLib and Code Coverage for the rest.

Why would we want to complicate things by using two different tools? Is the perf difference so substantial as to warrant such complication? What's the technical limitation on supporting corelib? Note we frequently want to get the coverage for multiple libs, including corelib, in the same run.

sharwell · 2023-01-24T16:40:05Z

@stephentoub My understanding is the performance comparison was not run with --single-hit, which means it didn't allow coverlet to benefit from coverlet-coverage/coverlet#291 and coverlet-coverage/coverlet#309. Once configured to have the same behavior (threshold at 1 hit), I would not expect there to be a significant performance difference between the two tools.

jakubch1 · 2023-01-25T14:07:54Z

@stephentoub My understanding is the performance comparison was not run with --single-hit, which means it didn't allow coverlet to benefit from coverlet-coverage/coverlet#291 and coverlet-coverage/coverlet#309. Once configured to have the same behavior (threshold at 1 hit), I would not expect there to be a significant performance difference between the two tools.

I've run comparison for 1 test project from roslyn repository (Windows, Debug, .NET 7, Microsoft.CodeAnalysis.UnitTests project). I will try to use some tooling to run comparison for more projects also. I will need to investigate also difference between dynamic and static instrumentation because in past we were getting very similar performance.

Scenario	Time	Line coverage	Covered lines	Valid lines
No coverage	95s
MS Coverage (static instrumentation)	130s	25.7%	182619	710269
MS Coverage (dynamic instrumentation)	237s	25.6%	182610	712305
Coverlet (single hit)	396s	25.4%	179522	705684
Coverlet	440s	25.4%	179560	705684

logs.txt

jakubch1 · 2023-01-31T07:31:48Z

@sharwell @stephentoub I've run comparison for all projects in roslyn repository. It shows with MS coverage ~3x faster tests execution. Attaching file with logs from execution where you can find results for specific projects.

Tool	Tests execution time	Ratio
Coverlet (single hit)	8h 17m 59s	1.00
MS Coverage (dynamic instrumentation)	3h 36m 23s	0.43
MS Coverage (static instrumentation)	2h 51m 21s	0.34

fullrun1.txt

mavasani · 2023-02-01T04:21:05Z

FYI: We are frequently running into failures on PRs and CI due to code coverage, almost every other run. I think it is likely that we turn off code coverage on the repo until we have a stable and reliable story.

Calculating coverage result...
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error : Unable to read beyond the end of the stream. [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at System.IO.BinaryReader.FillBuffer(Int32 numBytes) [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at System.IO.BinaryReader.ReadInt32() [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at Coverlet.Core.Coverage.CalculateCoverage() in /_/src/coverlet.core/Coverage.cs:line 422 [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at Coverlet.Core.Coverage.GetCoverageResult() in /_/src/coverlet.core/Coverage.cs:line 161 [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
D:\a\_work\1\s\.packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error :    at Coverlet.MSbuild.Tasks.CoverageResultTask.Execute() in /_/src/coverlet.msbuild.tasks/CoverageResultTask.cs:line 83 [D:\a\_work\1\s\src\Roslyn.Diagnostics.Analyzers\UnitTests\Roslyn.Diagnostics.Analyzers.UnitTests.csproj::TargetFramework=netcoreapp3.1]
##[error].packages\coverlet.msbuild\3.1.2\build\coverlet.msbuild.targets(71,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) Unable to read beyond the end of the stream.
   at System.IO.BinaryReader.FillBuffer(Int32 numBytes)
   at System.IO.BinaryReader.ReadInt32()
   at Coverlet.Core.Coverage.CalculateCoverage() in /_/src/coverlet.core/Coverage.cs:line 422
   at Coverlet.Core.Coverage.GetCoverageResult() in /_/src/coverlet.core/Coverage.cs:line 161

MarcoRossignoli · 2023-02-01T07:39:50Z

@mavasani to fix that issue you should move to the datacollector, here we're using msbuild task integration that suffers the "test platform process kill". Here the doc https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/VSTestIntegration.md

sharwell · 2023-02-01T14:42:45Z

@mavasani The coverlet story is quite reliable (we fixed the reliability issues 3-4 years ago). Unfortunately, this repository uses a large amount of build customization (Arcade) that makes it difficult to integrate with when tools have been designed for standard toolchains.

Considering that the reliability issues are 100% self-inflicted (the company that owns this repository is responsible for all of Arcade, microsoft/vstest#1900, and the fact that coverlet's data collector integration is required to be so complex), I see no value in penalizing coverlet (the one working component in this) by disabling it even temporarily.

jakubch1 · 2023-02-02T08:10:10Z

What is the reason coverlet.collector integration is so complex? In this PR we proposed changing to MS data collector and all worked in terms of collector integration so I think we can do it in same way.

Evangelink requested a review from a team as a code owner October 19, 2022 09:33

Evangelink marked this pull request as draft October 19, 2022 12:08

Evangelink added 4 commits October 19, 2022 16:33

Use MS coverage instead of coverlet

0e85287

This solves the "Unable to read beyond the end of the stream." we keep facing on CI builds

Fix conditions

e55e2b4

Try update switch escape

0c16bd1

new try

0533bad

Evangelink force-pushed the coverage branch from 0140dab to 0533bad Compare October 19, 2022 14:34

Evangelink added 3 commits October 19, 2022 16:46

Fix typo

18fe947

update

ffb049a

Revert unwanted change to azure-pipelines.yml

e62e677

Evangelink commented Oct 20, 2022

View reviewed changes

eng/CodeCoverage.proj Outdated Show resolved Hide resolved

Evangelink added 2 commits October 20, 2022 11:14

Update coverage report location

4762eab

Specify source dir

a47c391

Evangelink marked this pull request as ready for review October 20, 2022 13:06

Evangelink mentioned this pull request Oct 20, 2022

Add Code Coverage targets dotnet/arcade#1946

Open

premun reviewed Oct 20, 2022

View reviewed changes

eng/CodeCoverage.proj Show resolved Hide resolved

mavasani approved these changes Jan 3, 2023

View reviewed changes

jmarolf self-assigned this Jan 5, 2023

jmarolf reviewed Jan 10, 2023

View reviewed changes

Directory.Build.targets Outdated Show resolved Hide resolved

jmarolf approved these changes Jan 10, 2023

View reviewed changes

Add runsettings file and pass it to CI test command

856660f

jakubch1 reviewed Jan 20, 2023

View reviewed changes

Directory.Build.targets Outdated Show resolved Hide resolved

Evangelink added 4 commits January 23, 2023 09:21

Merge branch 'main' into coverage

e172c97

Fix path to runsettings

693e4c5

Use $(RepoRoot) instead

53b7d7a

Fix variable name

1ad571e

Evangelink closed this Jan 25, 2023

Evangelink deleted the coverage branch January 25, 2023 08:19

jakubch1 mentioned this pull request Feb 13, 2023

Huge performance drop when using --collect:"XPlat Code Coverage" in dotnet test (.NET 6) coverlet-coverage/coverlet#1192

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use MS coverage instead of coverlet #6211

Use MS coverage instead of coverlet #6211

Evangelink commented Oct 19, 2022

mavasani commented Oct 19, 2022

Evangelink commented Oct 19, 2022

codecov bot commented Oct 20, 2022 •

edited

Loading

Evangelink commented Oct 20, 2022

Evangelink commented Oct 20, 2022

mavasani commented Jan 3, 2023

mavasani commented Jan 3, 2023

azure-pipelines bot commented Jan 3, 2023

stephentoub commented Jan 10, 2023

Evangelink commented Jan 11, 2023

sharwell commented Jan 19, 2023

sharwell commented Jan 19, 2023

sharwell commented Jan 19, 2023

jakubch1 commented Jan 20, 2023

sharwell commented Jan 20, 2023

sharwell commented Jan 20, 2023

sharwell commented Jan 20, 2023

Evangelink commented Jan 20, 2023

jakubch1 commented Jan 23, 2023

stephentoub commented Jan 23, 2023

jakubch1 commented Jan 23, 2023

sharwell commented Jan 23, 2023 •

edited

Loading

Evangelink commented Jan 24, 2023

stephentoub commented Jan 24, 2023

sharwell commented Jan 24, 2023

jakubch1 commented Jan 25, 2023 •

edited

Loading

jakubch1 commented Jan 31, 2023

mavasani commented Feb 1, 2023

MarcoRossignoli commented Feb 1, 2023

sharwell commented Feb 1, 2023 •

edited

Loading

jakubch1 commented Feb 2, 2023

Use MS coverage instead of coverlet #6211

Use MS coverage instead of coverlet #6211

Conversation

Evangelink commented Oct 19, 2022

mavasani commented Oct 19, 2022

Evangelink commented Oct 19, 2022

codecov bot commented Oct 20, 2022 • edited Loading

Codecov Report

Evangelink commented Oct 20, 2022

Evangelink commented Oct 20, 2022

mavasani commented Jan 3, 2023

mavasani commented Jan 3, 2023

azure-pipelines bot commented Jan 3, 2023

stephentoub commented Jan 10, 2023

Evangelink commented Jan 11, 2023

sharwell commented Jan 19, 2023

sharwell commented Jan 19, 2023

sharwell commented Jan 19, 2023

jakubch1 commented Jan 20, 2023

sharwell commented Jan 20, 2023

sharwell commented Jan 20, 2023

sharwell commented Jan 20, 2023

Evangelink commented Jan 20, 2023

jakubch1 commented Jan 23, 2023

stephentoub commented Jan 23, 2023

jakubch1 commented Jan 23, 2023

sharwell commented Jan 23, 2023 • edited Loading

Evangelink commented Jan 24, 2023

stephentoub commented Jan 24, 2023

sharwell commented Jan 24, 2023

jakubch1 commented Jan 25, 2023 • edited Loading

jakubch1 commented Jan 31, 2023

mavasani commented Feb 1, 2023

MarcoRossignoli commented Feb 1, 2023

sharwell commented Feb 1, 2023 • edited Loading

jakubch1 commented Feb 2, 2023

codecov bot commented Oct 20, 2022 •

edited

Loading

sharwell commented Jan 23, 2023 •

edited

Loading

jakubch1 commented Jan 25, 2023 •

edited

Loading

sharwell commented Feb 1, 2023 •

edited

Loading