nunit / teamcity-event-listener

NUnit Engine extension that helps integration with teamcity
MIT License
11 stars 10 forks source link

Failed to load netstandard.dll on net462 environment #76

Open duck-dot-net opened 3 years ago

duck-dot-net commented 3 years ago

After 1.0.8 release nunit console fails with "Could not load file or assembly 'netstandard, Version=2.0.0.0, ..." on Windows 2012R2 build agent with .net Framework 4.6.2. Fuslog shows that calling assembly is teamcity-event-listener 1.0.8.0 and that it's not possibile to locate netstandard.dll file in ConsoleRunner nor EventListener directories.

Using ConsoleRunner 3.11.1 called from cake.

NikolayPianikov commented 3 years ago

@duck-dot-net please try updating the file like "NUnit.Extension.TeamCityEventListener\1.0.8\tools.addins":

net20/
netstandard2.0/
duck-dot-net commented 3 years ago

@NikolayPianikov still the same error

NikolayPianikov commented 3 years ago

@duck-dot-net please remove netstandard2.0/

duck-dot-net commented 3 years ago

@NikolayPianikov with .addins file:

net20/

works fine.

NikolayPianikov commented 3 years ago

@ChrisMaddock looks like ExtensionSelector.IsBetterVersionOf works wrong for this case.

There are 2 assemblies net20 and netstandard2.0 and ExtensionSelector.IsBetterVersionOf selects a version by TargetFramework.FrameworkVersion. For net20 it is 2 and for netstandard2.0 it is 4. Thus ExtensionSelector.IsBetterVersionOf selects netstandard2.0.

Microsoft recommends to update .NET 4 to .NET 4.7.2 for netstandard2.0.

I could add netstandard.dll to NUnit.Extension.TeamCityEventListener nuget package for this case to support previous versions of nunit-console. But I am not sure it will work for NET 4.0, 4.5, 4.5.1 ...

@ChrisMaddock do you have any ideas?

ChrisMaddock commented 3 years ago

Hi @NikolayPianikov - I'll need to have a look into what's going on here. Looking back at the pull requests from the time, turns out the original implementation was only written to handle multiple version of the .NET Framework, which is why we're seeing these issues now. Sorry, I'd forgotten that as it was so long ago!

Thanks for filing the issues in the engine repo. Will take a look when I can.

ChrisMaddock commented 3 years ago

Thinking about this - whilst the engine logic isn't exactly right here, the end result is correct. Between these two options, we'd expect the the engine to choose the .NET Standard 2.0 package. This issue here that .NET 4.6.2 doesn't quite support .NET Standard properly as Microsoft says it does - do you agree? I presume including the NETStandard.Library NuGet package acts as a bit of a filler for that - but I'm not familiar with what's necessary to make .NET Standard work on .NET 4.6.X.

Going back to the problem in have - Nickolay, am I right that the behaviour you want is for the net20 to be used on all .NET Framework platforms, and the netstandard20 build on .NET Core? And you're not looking to support any other .NET Standard platforms at this point in time? (e.g. Xamarin, UWP)

In that case, maybe it would be better to have the two version of the extension target net20 and netcoreapp2.1, rather than bringing .NET Standard into it at all. That should ensure that all runs on .NET Framework still use the net20 build. (Pending possible fixes to the other issues you've filed on the engine repo.)

ChrisMaddock commented 3 years ago

cc @CharliePoole - this discussion is very relevant to our emails earlier today. ☝️

CharliePoole commented 3 years ago

Seems like I have to test extensions under multiple versions of the framework as well!

CharliePoole commented 3 years ago

@ChrisMaddock Shouldn't the extension service be checking compatibility against the target runtime of the engine rather than the execution runtime?

NikolayPianikov commented 3 years ago

Going back to the problem in have - Nickolay, am I right that the behaviour you want is for the net20 to be used on all .NET Framework platforms, and the netstandard20 build on .NET Core? And you're not looking to support any other .NET Standard platforms at this point in time? (e.g. Xamarin, UWP)

@ChrisMaddock yes I supposed it

In that case, maybe it would be better to have the two version of the extension target net20 and netcoreapp2.1, rather than bringing .NET Standard into it at all. That should ensure that all runs on .NET Framework still use the net20 build. (Pending possible fixes to the other issues you've filed on the engine repo.)

Yes, it would be a possible scenario. Should we create an extensions assembly for all app core versions? I mean 2, 3, 5. Is it ok?

ChrisMaddock commented 3 years ago

I think just one netcore version would be enough? An engine running on .NET Core 3.1 should just load the netcoreapp21 version of the extension.

ChrisMaddock commented 3 years ago

@CharliePoole - not sure I understand your last question, sorry?