Fix dependencies resolving logic for netcore assemblies

[svg2003]

based on discussion from here:
#1311

[dnfadmin]

All CLA requirements met.

[CharliePoole]

Is this a replacement for #1315? If so, that one should be closed.

[CharliePoole]

Suggestion... The successful CI build shows that this does everything that the existing implementation already did but not that it does anything more. To show that, you could add some tests that failed before but pass now. Could be unit tests or possibly integration (i.e. package) tests.

[CharliePoole]

@svg2003 Since you are working on the main branch, that implies that your fixes will not be part of any new Version 3 release but will wait for the major upgrade to version 4. Is that what you intended?

@nunit/engine-team Can somebody tell @svg2003 which branch you prefer that he work on? :-)

[svg2003]

Sorry guys. My intension was to merge this fix into version3 and probably to trunk later, but git and github itself (why it cannot clone repository with all branches by default???), it's actually worst source control system I've ever worked. Feel free to create you own PR and merge my changes, might be with your own changes.
As of tests - I'm not sure how to add it. Should it be the real MS's binaries? What is your test environment? Will it be able to executed at all on your CI pipeline?

[CharliePoole]

@svg2003 Cloning does give you all branches but you have to use git checkout to select the one you want to work with in your local directory. For V3, you should checkout version3 and then create a local branch from that. And, of course, you have to merge it back to version3. FYI version3 will never be literally merged back to main because development has been proceeding on both separately and they are too far apart. Important changes (this is probably one) get ported individually with whatever changes are needed to make them work.

If I were doing this, I'd create a failing package test. To see how that happens, look at the package test definitions in build-settings.cake and how the tests are specified for each package in package-definitions.cake. There is a bit of a learning curve to cake, but it's still just C# code. If you come up with one or more test assemblies to use for testing, I'll be glad to walk you through the process more exactly.

The test environment is currently only local (your machine) and AppVeyor. You run the CI locally yourself using the build command. Not sure what you mean by "the real MS binaries."

[svg2003]

@CharliePoole,

Seems "GitHub-desktop" app is more user friendly, and allowed me to clone repository with all branches, so I created one more PR for version3.0.

And after, I tried to rollback my changes and add some similar test that I have failed before.
But it turned out that test engine is actually does not use "nunit-console". Seems it produces some executable, that passes all tests. But, when I'm trying to test the same from nunit-console - I do see error

Here is my test:

namespace NUnit.Engine.Core.Tests.Internal
{
[TestFixture]
public class TestAssemblyDependenciesResolver
{
#if NET6_0_OR_GREATER
[Test]
[Platform("win")]
public void TestResolveDependenciesFromDepsJson()
{
#pragma warning disable CA1416 // Validate platform compatibility
// This test will fail, if dependency would be resolved not from deps.json, but is taken from file, next to the test assembly
var identity = new SecurityIdentifier(WellKnownSidType.WorldSid, null);

        var allowEveryoneRule = new EventWaitHandleAccessRule(identity,
            EventWaitHandleRights.FullControl,
            AccessControlType.Allow);

        Assert.IsNotNull(allowEveryoneRule);
        Assert.IsTrue(false, "test");

#pragma warning restore CA1416 // Validate platform compatibility
}
#endif
}
}

During the build (build.cmd -t Test), it fails in Assert.IsTrue(false, "test"); line

Errors, Failures and Warnings

1. Failed : NUnit.Engine.Core.Tests.Internal.TestAssemblyDependenciesResolver.TestResolveDependenciesFromDepsJson
   test
   Expected: True
   But was: False
   at NUnit.Engine.Core.Tests.Internal.TestAssemblyDependenciesResolver.TestResolveDependenciesFromDepsJson() in C:\Projects\nunit-console3\src\NUnitEngine\nunit.engine.core.tests\Internal\TestAssemblyDependenciesResolver.cs:line 30

But real issue is in 2 lines before, and when I'm using nunit-console.exe for that - I do see expected exception:

nunit3-console.exe C:\Projects\nunit-console3\src\NUnitEngine\nunit.engine.core.tests\bin\Release\net6.0\nunit.engine.core.tests.dll

Errors, Failures and Warnings

1. Error : NUnit.Engine.Core.Tests.Internal.TestAssemblyDependenciesResolver.TestResolveDependenciesFromDepsJson
   System.ArgumentNullException : Value cannot be null. (Parameter 'identity')
   at System.Security.AccessControl.AuthorizationRule..ctor(IdentityReference identity, Int32 accessMask, Boolean isInherited, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags)
   at System.Security.AccessControl.AccessRule..ctor(IdentityReference identity, Int32 accessMask, Boolean isInherited, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, AccessControlType type)
   at System.Security.AccessControl.EventWaitHandleAccessRule..ctor(IdentityReference identity, EventWaitHandleRights eventRights, AccessControlType type)
   at NUnit.Engine.Core.Tests.Internal.TestAssemblyDependenciesResolver.TestResolveDependenciesFromDepsJson() in C:\Projects\nunit-console3\src\NUnitEngine\nunit.engine.core.tests\Internal\TestAssemblyDependenciesResolver.cs:line 25

Run Settings
DisposeRunners: True
WorkDirectory: C:\Projects\nunit-console3\src\NUnitConsole\nunit3-console\bin\Release\net462
ImageRuntimeVersion: 4.0.30319
ImageTargetFrameworkName: .NETCoreApp,Version=v6.0
ImageRequiresX86: False
ImageRequiresDefaultAppDomainAssemblyResolver: False
TargetRuntimeFramework: netcore-6.0
NumberOfTestWorkers: 4

[SeanKilleen]

@svg2003 that target framework looks a little suspicious to me. The right target for .NET 6 is net6.0 -- could you try it with that framework specified?

[svg2003]

@SeanKilleen ,

Sorry, I don't get it, what do you mean "right framework".
I pushed changes in my test repository, so you can review them:
svg2003@4d152d2

The idea is - test itself is built against net6 and executed during the build (build.cmd -t Test), because I do see it's failed in Assert.IsTrue(false). But it cannot catch original issue.
But, if I'm using nunit-console (runner, targeted to net462), eventually it would be translated to net6 agent and after will fail in the right place (due to wrongly resolved assembly)

[SeanKilleen]

@svg2003 sorry, I'm coming late and my observation might be entirely unrelated. While quickly glancing, I saw you noted;

TargetRuntimeFramework: netcore-6.0

My observation was that netcore-6.0 is not a valid runtime framework as far as I know. I noted that you may want to ensure you're using net6.0.

Again, sorry for the drive-by comments; they were only a quick observation at-a-glance.

I see in your code you are indeed using net6.0. 👍

[CharliePoole]

@SeanKilleen @svg2003 "TargetRuntimeFramework netcore-6.0" is using NUnit's id for the runtime rather than the tfm. The id is used both internally and as an argument to the --framework option of the console runner.

The plan was for this to go away when the RuntimeFramework class was replaced, which has not yet happened in either version 3 or 4 code although it is extremely reduced in V4 right now. It would be pretty easy to translate these ids to tfms on output in order to avoid confusion.