File path issue with ProjectReference + SourceLink

[TimHess]

Hello,

We're using coverlet in our project and we've been experiencing one of the known issues with the msbuild integration, so I'm working on switching to collectors in this PR. Almost everything is working correctly now, except that when tests run with a .NET Framework target and project references are involved, the path for files in the referenced project shows up as the local path instead of the sourcelink path like this.

Steeltoe is a pretty large set of packages and interconnected pieces, I can provide some more specific details if that will help but first I wanted to check if this is more likely to be an issue here or with SourceLink

[TimHess]

I was able to get past this issue by setting <DebugType>full</DebugType>, which resulted in all file paths in the report showing a local path on the build agent's file system.

I don't know that I care if the code coverage report shows a local path on the build server or the SourceLink path, so long as files don't show up twice. I am interested in hearing if there's a way that it should work or if there's something I could/should be doing differently in this space, thanks

[MarcoRossignoli]

Thank's for reporting this.

Do you get this issue only on .NET Framework?No issues on .NET Core?

[TimHess]

That's correct - on that branch of that repo, most tests run for net461, netcoreapp2.1 and netcoreapp3.1. I isolated the issue by just running the tests in Common.sln to get to that conclusion.

I made a repro that was failing the same way but I must have made some inadvertent change and now it all seems to be working and showing the sourcelink paths no matter what I do

This is how I've been running the tests and generating a report from the results:

dotnet test --blame -c Release --settings coverlet.runsettings --logger trx --results-directory C:\temp\testresults /p:TF_BUILD=true
reportgenerator -reports:c:\temp\testresults\**\coverage.opencover.xml -targetdir:c:\temp\testresults\CodeCoverage -reporttypes:"HtmlInline_AzurePipelines;Cobertura" -plugins: -assemblyfilters:+* -classfilters:+* -filefilters:+* -verbosity:Info

[TimHess]

It turns out my workaround isn't a real solution as now nuget.org won't validate the symbols package, so I'm back to trying to figure this out again.

Here are the steps to see the issue in the Steeltoe codebase:

git clone https://github.com/SteeltoeOSS/Steeltoe.git
cd Steeltoe
git checkout coverage
dotnet build .\src\Common\Common.sln -c Release /p:TF_BUILD=true
dotnet test .\src\Common\Common.sln --no-build -c Release --settings coverlet.runsettings --logger trx --results-directory C:\temp\testresults /p:TF_BUILD=true

# and then optionally
reportgenerator -reports:c:\temp\testresults\**\coverage.opencover.xml -targetdir:c:\temp\testresults\CodeCoverage -reporttypes:"HtmlInline_AzurePipelines;Cobertura" -plugins: -assemblyfilters:+* -classfilters:+* -filefilters:+* -verbosity:Info

I was pretty sure the issue only happened with net461, but today it looks like it's happening for netcoreapp3.1 also. netcoreapp2.1 doesn't seem to have the issue, but I'm also missing results for the base library so it might not be that it is correct but rather something else is wrong. Nothing jumps out at me as a relevant difference between my simple sample and Steeltoe, but I must be missing something. Also it looks like I forgot to mention that deterministic is enabled (in both the simple sample and Steeltoe)

[TimHess]

Here are some logs in case they can help
logs.zip

[clairernovotny]

Couple thoughts:

1. Can you use either embedded or portable PDB types? I prefer embedded as no separate snupkg is needed. NuGet.org doesn't support Full symbols and full symbols only work on Windows.
2. Use the coverlet.collectors package reference and the 16.7+ MSTest SDK. You'll also need this in your targets until Coverlet updates to use the built-in target: https://github.com/dotnet/reactive/blob/main/Rx.NET/Source/Directory.build.targets#L45-L53
3. You can see how Rx runs the tests. Note that Rx multi-targets and has tests on .NET Core and .NET Framework, so it should be similar to what you're doing. It uses embedded symbols: https://github.com/dotnet/reactive/blob/main/azure-pipelines.rx.yml#L77
4. The full pipeline is in that file if it's helpful to you. It generates multiple code coverage files per project (due to the multi targeting) that is merged into a single one published to DevOps.

[TimHess]

@clairernovotny thanks for the response

1. I don't have a preference, we weren't previously specifying what type was produced at all, it just needs to work. Unfortuntely, switching to portable or embedded doesn't seem to have had any effect on the problem that brought me here
2. Already did all of that
3. I was wrong when I said it was related to the target framework
4. Thanks, we're already doing that too.

To restate the problem, the code coverage reports for tests on projects that include a projectreference are reporting a local path for the ProjectReference'd files , where the library being directly tested will use sourcelink paths... Please review the attached file and note the difference between the paths for files under Steeltoe.Common vs Steeltoe.Common.Net

coverage.opencover.txt

[clairernovotny]

Coverlet should be getting the map for each project reference here to generate a multi map of project reference to local location:

coverlet/src/coverlet.collector/build/netstandard1.0/coverlet.collector.targets

Lines 25 to 32 in 8cf2b3c

	<Target Name="ReferencedPathMaps" BeforeTargets="CoreCompile" DependsOnTargets="ResolveProjectReferences" >
	<MSBuild Projects="@(AnnotatedProjects->'%(FullPath)')"
	Targets="CoverletGetPathMap"
	Properties="TargetFramework=%(AnnotatedProjects.NearestTargetFramework)"
	SkipNonexistentTargets="true">
	<Output TaskParameter="TargetOutputs"
	ItemName="_LocalTopLevelSourceRoot" />
	</MSBuild>

Is there a CoverletSourceRootsMapping file in your output directory that you can attach here?

[TimHess]

I don't see one

[TimHess]

I was looking in the wrong output and search failed me... found one
CoverletSourceRootsMapping.zip

[TimHess]

OK, so I found something else weird.. This looks like it's happening in test runs for Steeltoe.Common.Net, but not Steeltoe.Common.Http. These both target netstandard2.0, have tests running the same target frameworks and both depend on the same base library... I have no idea what could be different between the two that would cause this behavior to be different

Here are my latest CoverletSourceRootsMapping files and opencover reports
latestresults.zip

[TimHess]

@clairernovotny do you have any time to look into this, or a direction to point me in to help get this figured out? I have another release I need to ship soon and I need this resolved somehow first

[clairernovotny]

The coverlet team is going to need to look into it. In the interim, what I did when coverlet had issues with deterministic builds was to do the build/pack first, then run the build/dotnet test command with /p:ContinuousIntegrationBuild=false to make it build with local paths for the tests.

[MarcoRossignoli]

Sorry @TimHess for the delay and thanks a lot @clairernovotny for the help 🙇

@TimHess can you confirm that this is a way to repro #951 (comment) ?

I'll take a look this week

[TimHess]

No problem @MarcoRossignoli - yes, the steps above are consistently reproducing the problem for me

[MarcoRossignoli]

Did some investigation and you're suffering of 3 different issue:

1. you have to pass --collect:"XPlat Code Coverage" always on command line(and setup options through runsettings) or the in process collector won't be injected, if it's not injected you'll suffer again of msbuild known issue so no hit file will be persisted and with no hit file no translation of local file name to source link ones. It's on guide https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/VSTestIntegration.md let me know if we can be more clear on that
2. collector is not "fully supported" on .NET Framework because in process one is compiled as netcoreapp2.0 and when loaded inside a .NET Framework process for some assemblies will fail resolution(class for .NET Core are not in same assemblies of .NET Framework), consequence you'll suffer of same issue as msbuild because is thanks to in proc collector that we synch with vstest plat and avoid early process kill. Feel free to add a thumbs up or comment here [BUG] vstest.console.exe --collect:"XPlat Code Coverage" throw System.FormatException microsoft/vstest#2278 (comment) I'm waiting to know if it's ok compile for netstandard2.0(collectors was developed by vstest team in the beginning, so I want to be sure to not break anything if compiled for netstandard)
3. After we'll compile for netstandard2.0(the minimum supported by vstest engine) you should update System.Diagnostics.DiagnosticSource to <DiagnosticSourceVersion>4.7.1</DiagnosticSourceVersion> because with version 4.4.1 will fail resolution on netstandard2.0, for project Steeltoe.Common

At the end it works with collectors compiled for netstandard2.0 and with 4.7.1 for System.Diagnostics.DiagnosticSource

Now as temporary solution you can try my netstandard compilation on my unofficial "beta" feed https://f.feedz.io/marcorossignoli/coverletunofficial/nuget/index.json the package is coverlet.collector 1.3.1-preview.34 and with System.Diagnostics.DiagnosticSource version update(if possible).

[MarcoRossignoli]

related issue #705 (comment)

[TimHess]

Thanks for digging in @MarcoRossignoli!

1. Ah, I forgot that in the repro steps... for what it's worth, our build process has had that this whole time
2. I'm not seeing that version in that feed
3. I'll look into the version upgrade there

Did some investigation and you're suffering of 3 different issue:

1. you have to pass --collect:"XPlat Code Coverage" always on command line(and setup options through runsettings) or the in process collector won't be injected, if it's not injected you'll suffer again of msbuild known issue so no hit file will be persisted and with no hit file no translation of local file name to source link ones. It's on guide https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/VSTestIntegration.md let me know if we can be more clear on that
2. collector is not "fully supported" on .NET Framework because in process one is compiled as netcoreapp2.0 and when loaded inside a .NET Framework process for some assemblies will fail resolution(class for .NET Core are not in same assemblies of .NET Framework), consequence you'll suffer of same issue as msbuild because is thanks to in proc collector that we synch with vstest plat and avoid early process kill. Feel free to add a thumbs up or comment here microsoft/vstest#2278 (comment) I'm waiting to know if it's ok compile for netstandard2.0(collectors was developed by vstest team in the beginning, so I want to be sure to not break anything if compiled for netstandard)
3. After we'll compile for netstandard2.0(the minimum supported by vstest engine) you should update System.Diagnostics.DiagnosticSource to <DiagnosticSourceVersion>4.7.1</DiagnosticSourceVersion> because with version 4.4.1 will fail resolution on netstandard2.0, for project Steeltoe.Common

At the end it works with collectors compiled for netstandard2.0 and with 4.7.1 for System.Diagnostics.DiagnosticSource

Now as temporary solution you can try my netstandard compilation on my unofficial "beta" feed https://f.feedz.io/marcorossignoli/coverletunofficial/nuget/index.json the package is coverlet.collector 1.3.1-preview.34 and with System.Diagnostics.DiagnosticSource version update(if possible).

[MarcoRossignoli]

I'm not seeing that version in that feed

We merged today so will be in next coverlet release #970

For now try with custom build on unofficial and let me know.

[TimHess]

Ah, the tooling wasn't respecting the change I made the config file (so it wasn't actually looking at that feed when it should have been), that's why I couldn't find that version, sorry about that confusion. The custom build works, and we'll likely include the DiagnosticSource update in our next version, Thank you very much!

[MarcoRossignoli]

Glad to hear that 🤗!