Coverlet: Coverlet assumes coverage of 3rd party files with embedded ppdb

[clairernovotny]

Hi,

Looking here: https://dev.azure.com/dotnet/ReactiveUI/_build/results?buildId=18689&view=codecoverage-tab

I see that it's trying to measure coverage of System.Reactive. Rx has embedded ppdb's in it. I'm assuming Coverlet assumes to measure coverage if it finds a pdb, but many 3rd party libraries have embedded ones.

Is there a way to better detect this so people don't have to have large amounts of config by defualt?

[MarcoRossignoli]

AFAIK we instrument also with embedded pdb https://github.com/tonerdo/coverlet/blob/f64337e74ef605b3ad3d9a7d304e02274c4383a6/src/coverlet.core/Helpers/InstrumentationHelper.cs#L82
When coverlet is not able to instrument you should see a warning in console https://github.com/tonerdo/coverlet/blob/09f91f2afa623dbd011acac8a10f6023d9513812/src/coverlet.core/Instrumentation/Instrumenter.cs#L61
Can you enable verbose logs to understand which is o isn't instrumented? https://github.com/tonerdo/coverlet/blob/master/Documentation/Troubleshooting.md

[clairernovotny]

My point is that I don't want it to instrument/include coverage for those third parties by default.

[MarcoRossignoli]

Mmm how does coverlet know that are "third parties"?
I mean at the moment coverlet analyze dlls, it doesn't use csproj metadata to chose what instrument(as you already know for that we provide filters).

[MarcoRossignoli]

Maybe there is a way to filter out 0% asm from ReportGenerator cc: @danielpalme

[clairernovotny]

Is there a way for it to detect if the source files are present for it (based on the pdb data)? That could be the most fool-proof method.

You could get the source documents from the PDB: https://github.com/NuGetPackageExplorer/NuGetPackageExplorer/blob/master/Core/AssemblyMetadata/AssemblyDebugParser.cs#L129

And determine if they exist.

[MarcoRossignoli]

We could take a look...maybe could be some interaction with source link switch cc: @tonerdo

[clairernovotny]

Where in the code base does that processing happen?

[MarcoRossignoli]

Instrument check https://github.com/tonerdo/coverlet/blob/5a139b200f7bf51c21660b93f41906f4571c971b/src/coverlet.core/Coverage.cs#L96
Source link data gathering https://github.com/tonerdo/coverlet/blob/09f91f2afa623dbd011acac8a10f6023d9513812/src/coverlet.core/Instrumentation/Instrumenter.cs#L108
Get source link https://github.com/tonerdo/coverlet/blob/5a139b200f7bf51c21660b93f41906f4571c971b/src/coverlet.core/Coverage.cs#L240

[clairernovotny]

Thanks. For this option, I don't think sourcelink is relevant. That provides a path to map the local file to some other web location. In this case, all we need to check is if the local file exists. I think all we need is to gather the document paths and skip outputing any coverage data for those. I think it's okay to leave them instrumented. Outputting/suppressing data for missing files could be an option (with the default to suppress).

[MarcoRossignoli]

That provides a path to map the local file to some other web location

I thought that with embedded pdb+sourcelink we can generate report also for third party lib, but I didn't go deep on that part of coverlet yet so my idea was likely wrong.

[clairernovotny]

I thought that with embedded pdb+sourcelink we can generate report also for third party lib, but I didn't go deep on that part of coverlet yet so my idea was likely wrong.

In theory you could, if you downloaded the source files from sourcelink. That said, I would expect that most people are interested in coverage of their own code, not of external things. That's why I'd suggest that be the default, and possibly enable coverage of external code as an option.

[MarcoRossignoli]

@tonerdo a pair of question to go on with this
1) You wrote source link integration...is your original idea allow to generate report also for 3d party libs?
I mean report generator download file from web thanks to https://github.com/tonerdo/coverlet/blob/5a139b200f7bf51c21660b93f41906f4571c971b/src/coverlet.core/Coverage.cs#L246
2) What do you think about the idea of

Outputting/suppressing data for missing files could be an option (with the default to suppress).

[danielpalme]

Maybe there is a way to filter out 0% asm from ReportGenerator cc: @danielpalme

You can specify filters in ReportGenerator, but you have to supply the name of the assembly.
There is no way to filter based on coverage.

[tonerdo]

@MarcoRossignoli @onovotny

You wrote source link integration...is your original idea allow to generate report also for 3d party libs?
I mean report generator download file from web thanks to

Not really. It was meant to allow coverage reports to always link to the specific version of the source code.

Outputting/suppressing data for missing files could be an option (with the default to suppress)

This is actually a good idea. Although I'll take it a step further and not instrument to begin with

We'll need to update this method https://github.com/tonerdo/coverlet/blob/d5d0a0b59c4d52ab759711e2e0a5fddab7ef87d7/src/coverlet.core/Helpers/InstrumentationHelper.cs#L25 to check if a document in the PDB is present on the local machine

I'm happy to work on it this weekend

[MarcoRossignoli]

Thank's Toni!

[clairernovotny]

@tonerdo any luck here?

We're seeing users get confused by this leading to unexpected errors: https://github.com/dotnet/reactive/issues/984. Rx/Ix has embedded ppdb's, which is what's tripping up that person.

[aalmada]

I can find in the coverage report when it considers unwanted 3rd party libraries but, in this case, no report was being generated. I was getting the following cryptic error message:

_System.IO.FileLoadException : Could not load file or assembly 'System.Interactive, Version=4.0.0.0, Culture=neutral, PublicKeyToken=94bc3704cddfc263' or one of its dependencies. Strong name signature could not be verified. The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045)_

It would be great if, at least, a friendlier message was shown warning that a signed assembly was found and that it cannot be handled by coverlet.

Thanks for the amazing work!

[MarcoRossignoli]

@aalmada just open a PR to solve the issue https://github.com/tonerdo/coverlet/pull/510 the idea si to skip instrumentation for modules with embedded pdb without local source available.

[tonerdo]

@onovotny can you try with the latest nightly build to see if things work as expected?