Fix resolving assemblies from frameworks not referenced by coverlet itself

[lg2de]

Instead of fixed referencing aspnetcore directories according current runtime (previously done in AspNetCoreSharedFrameworkResolver), the frameworks listed in the generated *.runtimeconfig.json file should be evaluated (now in NetCoreSharedFrameworkResolver). Then e.g. also net6 WPF assemblies can be loaded.

Therefore the existing test was moved to a new project really references aspnetcore5 project.

I've changed the order of referenced resolvers in CecilAssemblyResolver because ReferenceAssemblyPathResolver is quite aggressive and throws an exception even the called method TryResolveAssemblyPaths suggests a "try pattern".

This fixes #1221.

Further I commited (separately) two changes to make the project compilable with newer installed IDEs resp. SDKs. If you agree at all I can also move them to a different PR.

[lg2de]

Unfortunately I do not understand why the builds are failing.
Any advice welcome.

[daveMueller]

Thanks for the contribution. Could you try to adapt coverlet.tests.projectsample.aspnet5.csproj to

<Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <IsPackable>false</IsPackable>
    <IsTestProject>false</IsTestProject>
    <WarnOnPackingNonPackableProject>false</WarnOnPackingNonPackableProject>
  </PropertyGroup>
</Project>

I think this is some default behavior of ASP.NET 5.0.

[lg2de]

Could you try to adapt coverlet.tests.projectsample.aspnet5.csproj to ...

I did, and the build gets successful!

[MarcoRossignoli]

Can you add a test with WPF like you did for asp.net?

[MarcoRossignoli]

I think it's good...maybe add a sample also for WPF framework before to merge for the dogfooding.

[lg2de]

I think it's good...maybe add a sample also for WPF framework before to merge for the dogfooding.

I did, but (as expected) the builds fails for Linux and MacOS.
So far, I had no good idea how to exclude them there, @MarcoRossignoli .

[lg2de]

I started introducing separate solution configurations to build test "Windows Desktop" on "Windows" only.
Now, the MacOS build is successful but Linux and Windows are failing even all create the same error/warning.
I do not know how to fix that.

[daveMueller]

@lg2de thanks a lot for this PR. This tackles some issues I had planned to work on in the last months but never found the time to (#1221; #1432; #1231; #1459).
Regarding the WPF test where the build is failing. There currently isn't any possibility to build a WPF application targeting NET6 on Linux or Mac. Thus the only solution would really be some different build configurations. I did it with this branch her in the PR #1468 but I don't really like it because the different build configurations generate much noise in the solution file.
@MarcoRossignoli I would suggest to drop the WPF integration test for now.

I still want to do some manual tests and have some questions. First of all why did you decide to even search in the specific version directory instead of just search all over C:\Program Files\dotnet\shared? For instance Altcover is doing it like this https://github.com/SteveGilham/altcover/blob/master/AltCover.Engine/CecilEx.fs#L129-L131.

Another issue I'm facing is that I still get an Failed to resove assembly exception. For me it feels like the TryResoveAssemblyPaths doesn't get executed at all. Any idea what this could be?

Sorry I didn't find much more time to analyse the PR yet. 🙏

[lg2de]

Regarding the WPF test where the build is failing.

The problem is not WPF anymore. With commit "Introduce separate solution configurations for Windows and non-Windows [ed0e4e9] I already separated WPF test on Windows only. See, macOS builds were recently successful.

The current problem is that the build is not reliable. As written on 18 march there are two builds with the same output (warning/error due to missing header) with different outcome. I have no idea what the problem is.

First of all why did you decide to even search in the specific version directory instead of just search all over C:\Program Files\dotnet\shared?

I introduced evaluating the JSON file because it defines, which runtime version should be used.
Why should I search in more folders than on runtime?
Further, with just searching for files according name without respecting the (major) version could (and should) create unexpected behavior.
Why are you asking? Do you think this is too complicated? Or do you have unexpected behavior?

Another issue I'm facing is that I still get an Failed to resove assembly exception. For me it feels like the TryResoveAssemblyPaths doesn't get executed at all. Any idea what this could be?

I'm not sure what do you mean. When debugging you should see wether it get called or not.
If you find other constellations not yet working, I need an detailed example for further analysis.
In the example you shared a screenshot the assembly Microsoft.AspNetCore.Mvc.Razor was failed to load. Do you found it on your PC? Where?

Sorry I didn't find much more time to analyse the PR yet. 🙏

I'm a bit confused. What is the status of the project? It is referenced by the official dotnet sdk test project templates. So, I guess it is in the focus of Microsoft. This PR is now 2 month old and fixes problems existing and known for long time. And it is still not completed...
The coverlet code reproduces behavior already existing in the dotnet runtime without using it or sharing code or ideas. I think, my fix is more a short to mid term solution. dotnet should integrate coverlet to align the behavior.

[bitbonk]

@daveMueller

There currently isn't any possibility to build a WPF application targeting NET6 on Linux or Mac

Just a side note: On my my machine (macOS Ventura) I can build (and publish) a WPF apps when I use EnableWindowsTargeting.

This is how my csproj looks like:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <OutputType>WinExe</OutputType>
        <TargetFramework>net6.0-windows</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <UseWpf>true</UseWpf>
        <EnableWindowsTargeting>true</EnableWindowsTargeting>
    </PropertyGroup>

</Project>

[MarcoRossignoli]

I'm a bit confused. What is the status of the project? It is referenced by the official dotnet sdk test project templates. So, I guess it is in the focus of Microsoft.

Hi @lg2de coverlet is not under Microsoft focus in the sense of "ownership", it's been historically the first open source MIT solution to have code-coverage cross platform and added to the template for that reason. As said coverlet is open and maintained by the community(MS devs are contributors too of coverlet), at the moment we're mainly in 3 me(at the moment on deploy to nuget), @daveMueller for the code and @petli helping with code review, plus sometime the original creator @tonerdo . Sorry for the late answers last year it's been hard for me dedicate a lot of time to the project and @daveMueller is doing a tremendous amount of work to keep coverlet in a good shape, for that reason I want to really thank @daveMueller and @petli 🙇

Due to the amount of devs we don't have tight schedule, but if you're interested into join the coverlet dev support let me know and we can have an offline chat for it.

Microsoft is shipping a free non open source cross platform solution for code coverage built-in in the sdk together with coverlet. You can try it using the flag --collect "Code Coverage" and you can read the documentation inside the official MS pages https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-test.
The repo to open issue and ask question is this one https://github.com/microsoft/codecoverage and the owner is @jakubch1

[daveMueller]

@bitbonk Thanks I didn't know the EnableWindowsTargeting property yet. What I figured out yesterday is that this only seems to work reliable with net6.0.

When you build with net5.0 sdk it doesn't seem to work.

@lg2de @MarcoRossignoli as coverlet is still on net5.0 I think we currently can't get this wpf project build in our pipeline. Yesterday I tried to migrate this branch here to net6.0 and the pipline successfully build all the projects (also the wpf) but unfortunately there seems to be another issue with some tests after migration. Thats why #1465 is also still open.

So I would suggest we skip this wpf project and test for now and create another issue for the missing test that we resolve after the migration to net6.0 was done. I suspect building the sample project will be much easier for our pipeline then. What do you think?

[daveMueller]

Why should I search in more folders than on runtime?
Further, with just searching for files according name without respecting the (major) version could (and should) create unexpected behavior.

Sure nevermind, I was just curious. I was just thinking we could search for CompilationLibrary.Name and CompilationLibrary.Version instead and then maybe wouldn't need the trimming of the major version etc.

If you find other constellations not yet working, I need an detailed example for further analysis.
In the example you shared a screenshot the assembly Microsoft.AspNetCore.Mvc.Razor was failed to load. Do you found it on your PC? Where?

Sorry on that day I just had a couple of minutes to try out your PR and went through some other issues to see if they are also solved by this (must have been one of those #1221, #1432, #1231, #1459). Thus I can't give you any more details yet if there is another constellation that might not work or maybe it was just me that made an error when compiling your branch locally. I plan on doing this more in detail as soon as I find a bit more time in the next days.

[lg2de]

Due to the amount of devs we don't have tight schedule, but if you're interested into join the coverlet dev support let me know and we can have an offline chat for it.

Sure, we can have a discussion on joining, @MarcoRossignoli. I do not know whether I can really help. Here I just followed the error message and identified the mismatch regarding the expectation.

Microsoft is shipping a free non open source cross platform solution for code coverage built-in in the sdk together with coverlet.

Why two solutions in parallel? What are the advantages of the solutions? (I know this is off-topic...)

EnableWindowsTargeting

This feature may help in compilation. But the WPF test will not be executable on Linux or macOS, isn't it?

Yesterday I tried to migrate this branch here to net6.0 and the pipline successfully build all the projects

The build is already successful on macOS, @MarcoRossignoli, @daveMueller!
The problem is currently the following message:

src\coverlet.console\ConsoleTables\ConsoleTable.cs(1,1):
error IDE0073: A source file contains a header that does not match the required text
[src\coverlet.msbuild.tasks\coverlet.msbuild.tasks.csproj]

There is no relation to the platform!
Anyway, I can separate the WPF test from this branch to another if you like.

I was just thinking we could search for CompilationLibrary.Name and CompilationLibrary.Version instead and then maybe wouldn't need the trimming of the major version etc.

Sure, the current solution is not fully identical to the search logic in the dotnet runtime. But it is at least close enough and faster than searching for all files and checking the file version.

[daveMueller]

@lg2de Really sorry for the late response on this. Meanwhile we migrated coverlet to net6.0 and now the EnableWindowsTargeting property should work properly. Thus I think we can get the WPF test project build without different build configurations now.

This feature may help in compilation. But the WPF test will not be executable on Linux or macOS, isn't it?

Yes but therefore coverlet has an attribute to skip tests on specific operating systems -> https://github.com/coverlet-coverage/coverlet/blob/master/test/coverlet.tests.xunit.extensions/SkipOnOS.cs

[lg2de]

Yes but therefore coverlet has an attribute to skip tests on specific operating systems -> https://github.com/coverlet-coverage/coverlet/blob/master/test/coverlet.tests.xunit.extensions/SkipOnOS.cs

This seams not to work. I guess it is just not possible to load the test assembly.

[daveMueller]

Yes but therefore coverlet has an attribute to skip tests on specific operating systems -> https://github.com/coverlet-coverage/coverlet/blob/master/test/coverlet.tests.xunit.extensions/SkipOnOS.cs

This seams not to work. I guess it is just not possible to load the test assembly.

Yes you right, sorry for that. I worked a bit on this myself and moved the wpf test to an integration test. In that case the assembly can be loaded only when the test is executed. You can see this here -> #1468

I also could confirm now that your changes also fix #1459.

A bit odd is that I had to move the registration of the NetCoreSharedFrameworkResolver to another location inside the ICompilationAssemblyResolver array. Otherwise the NetCoreSharedFrameworkResolver wasn't used at all. I've seen you have done something similar, did you figure out some details on that?

Nevertheless, would you mind to take over my changes to get rid of the different build configurations? I think we then can finally merge this PR.

[lg2de]

Nevertheless, would you mind to take over my changes to get rid of the different build configurations? I think we then can finally merge this PR.

I did. Hope it can be released soon.

A bit odd is that I had to move the registration of the NetCoreSharedFrameworkResolver to another location inside the ICompilationAssemblyResolver array. Otherwise the NetCoreSharedFrameworkResolver wasn't used at all. I've seen you have done something similar, did you figure out some details on that?

At the beginning of this PR I already wrote: "I've changed the order of referenced resolvers in CecilAssemblyResolver because ReferenceAssemblyPathResolver is quite aggressive and throws an exception even the called method TryResolveAssemblyPaths suggests a 'try pattern'."
This is the behavior of ReferenceAssemblyPathResolver which we have to live with, I guess.

[daveMueller]

@lg2de thanks a lot for this 🙏. @MarcoRossignoli @petli this can be merged now.

[lg2de]

@MarcoRossignoli , @daveMueller , since 3 months I'm waiting for a release with this fix.
When do you plan the next release?