[BUG] vstest.console.exe --collect:"XPlat Code Coverage" throw System.FormatException

[MarcoRossignoli]

vstest.console.exe --collect:"XPlat Code Coverage" throw System.FormatException

vstest.console.exe --collect:"XPlat Code Coverage" C:\Users\Marco\Downloads\test\XUnitTestProject1\bin\Debug\net472\XUnitTestProject1.dll
...
Data collection : Data collector 'XPlat Code Coverage' threw an exception during type loading, construction, or initialization: System.FormatException: Input string was not in a correct format.
   at System.Text.StringBuilder.FormatError()
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.Format(IFormatProvider provider, String format, Object[] args)
   at Microsoft.VisualStudio.TestPlatform.Common.DataCollector.DataCollectionManager.LoadAndInitialize(DataCollectorSettings dataCollectorSettings, String settingsXml).
...

We had a request on coverlet repo coverlet-coverage/coverlet#652 to use "Visual Studio Test task" https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/test/vstest?view=azure-devops

cc: @vagisha-nidhi

[MarcoRossignoli]

I did a check actually the bug was solved in #2221 the real warning is

Data collection : Unable to find a datacollector with friendly name 'XPlat Code Coverage'.
Data collection : Could not find data collector 'XPlat Code Coverage'

Because /testadapterpath is injected through https://github.com/tonerdo/coverlet/blob/master/src/coverlet.collector/build/netstandard1.0/coverlet.collector.targets for dotnet test.

It works if we provide collector path

D:\git\coverletissue\dotnet\dotnet-sdk-3.1.100-win-x64\dotnet.exe D:\git\vstest\artifacts\Debug\netcoreapp2.1\vstest.console.dll --collect:"XPlat Code Coverage" D:\git\coverletissue\vstestissue2275\XUnitTestProject1\bin\Debug\netcoreapp3.1\XUnitTestProject1.dll /testadapterpath:....\.nuget\packages\coverlet.collector\1.0.1\build\netstandard1.0

[Adanaran]

Adding the test adapter path to the "pathtoCustomTestAdapters" parameter of VSTest v2 did not change the error message on our end. We're using the "old" full .NET Framework tasks (NuGet, MSBuild, VSTest Installer, VSTest).

[MarcoRossignoli]

I think because plain vstest.console.exe(VsTest task) run follow a different workflow.

[Adanaran]

Is there any chance of getting this looked at? If not we can change to another coverage solution instead.

[MarcoRossignoli]

I did some test and find the issue, the problem is that coverlet collector is build for netcoreapp2.1 but packed for netstandard1.0 and vstest.console.exe try to load collector for .NET Framework and correctly fails to load .net core reference https://github.com/microsoft/vstest/blob/master/src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginDiscoverer.cs#L184
If I try to build coverlet collectors for netstandard2.0 it works, btw passing testadapter path param in case of vstest.console.exe(on dotnet test is passed through msbuild import props file):

vstest.console.exe  --collect:"XPlat Code Coverage" D:\git\coverletissue\vstestfull\bin\Debug\net47\vstestfull.dll /testadapterpath:...\.nuget\packages\coverlet.collector\1.2.0\build\netstandard1.0`

Now I've got some question for vstest team on coverlet collector they did:

1. Why compile for netcoreapp2.1 and pack for netstandard1.0?
2. Why not compile for netstandard2.0 and keep pack for netstandard1.0?

I think that pack for netstandard1.0 was done to "try to load collector for every possible plat" and let it fail for non compat cases.

Working on other related issue found this statement coverlet-coverage/coverlet#466 (comment) btw we've user that work on netfx and we could release netstandard2.0 or should we "announce" that collector will work properly only for core app?

cc: @vagisha-nidhi @nohwnd @jakubch1 @AbhitejJohn @singhsarab

[MarcoRossignoli]

@nohwnd @jakubch1 can someone please take a look at this and tell me if we can compile for netstandard?
We want to allow to .NET Framework apps to use collectors to avoid "early process kill" msbuild issue.
Some more info coverlet-coverage/coverlet#705 (comment)

Feel free to reach me off line also, thanks guys!

[saucecontrol]

As the netcore and netfx platforms diverge, it is becoming ever more important to be able to test on both and merge coverage results to make sure everything is covered. I really hope this can be fixed.

[nohwnd]

Why compile for netcoreapp2.1 and pack for netstandard1.0?

I don't know why it is done that way, if coverlet is targetting netcoreapp2.1 it should probably distribute as such. When I look at coverlet nuget package it lists no dependencies. I don't know what the minimal requirements are for coverlet, but Microsoft.NET.Test.SDK targets netcoreapp1.0, netcoreapp2.1 and net45. So if you want coverlet to run everywhere where test.sdk runs you should target that probably. This will also avoid problems with templates because they are already including test.sdk so if coverlet would go to netstandard2.0 (which is not net45 compatible) it would fail to restore when user manually changes the TFM to net45.

We actually don't do anything for .net45 in the package, it's imho targetting that just to avoid this multitargetting issue. So you might choose the actual framework you are building against, I don't know which apis you need to be able to provide the coverlet functionality.

Why not compile for netstandard2.0 and keep pack for netstandard1.0?

Would that help? You still would be "lying" about compatibility. We do a bit as well the test.sdk lists net45 as the minimum, but we build against net451, and ship those bits in SDK, so to run you'd need net451 not net45.

[MarcoRossignoli]

Minimum for coverlet core features is netstandard2.0(coverlet.core.dll shared between 'drivers' .NET tool, msbuild tasks, collectors) so we're ok to be usable on runtime that support >= netstandard2.0 I know we won't support net45 but is better than nothing support >=net461.
Today we don't support any version of .NET Framework.

Would that help? You still would be "lying" about compatibility

I know, I don't know why was done as is, by the way we can go on with that for now, at the moment we have only some issue with .NET Framework version >= net45.

So from you answer I think we can compile for our possible minimum to support collectors(in process one) for .NET Framework app >= net461

For the future the idea is to remove in-proc collector at all and use shared memory to gather hits, in that case we'll only use out of process one.

[MarcoRossignoli]

closed in coverlet-coverage/coverlet#970

Thanks to all for the support!

[nohwnd]

ping us when you release it, we should get it into templates in early stage, to detect errors while we can still fix them.

[MarcoRossignoli]

Got it, we don't have an ETA atm we have one fix on queue and after we can go, I'll let you asap.

[Adanaran]

@MarcoRossignoli Any news on a release on this?

[MarcoRossignoli]

@nohwnd @Adanaran we released version 3.0.1 and in a week we'll release 3.0.2 due to some regression.