Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NUnit.Engine 3.12.0 .NET Core and TypeDescriptor.GetConverter([...]) dependency loading problems #916

Closed
Learfin opened this issue Mar 9, 2021 · 7 comments · Fixed by #942

Comments

@Learfin
Copy link

Learfin commented Mar 9, 2021

In .NET Core, NUnit.Engine 3.12.0 causes some problems when the tested code calls TypeDescriptor.GetConverter([...]).
For info, TypeDescriptor.GetConverter([...]) is used by JSonConvert.SerializeObject([...]) (from NewtonSoft.Json).

I use the following code for my tests:

using System;
using System.ComponentModel;
using System.Globalization;
using NUnit.Framework;

namespace MyNUnitTestProject
{
    [TestFixture]
    public class Tests
    {
        private sealed class MyClassTypeConverter : TypeConverter
        {
            public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)
            {
                if (destinationType == typeof(string))
                {
                    var toSerialize = (MyClass)value;
                    return toSerialize.Value;
                }
                throw new NotImplementedException();
            }
        }
        [TypeConverter(typeof(MyClassTypeConverter))]
        private sealed class MyClass
        {
            public MyClass()
            {
                Value = Guid.NewGuid().ToString();
            }
            public string Value { get; }
        }
        [Test] public void ShouldPass()
        {
            var myClass = new MyClass();
            var converted = TypeDescriptor.GetConverter(typeof(MyClass)).ConvertTo(null, null, myClass, typeof(string));
            Assert.AreEqual(myClass.Value, converted as string);
        }
    }
}

Problem 1 - Failure in NUnit.ConsoleRunner.NetCore (version 3.12.0-beta2)

For this test, the code above is compiled with the following csproj (created with dotnet test):

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
   <TargetFramework>netcoreapp3.1</TargetFramework>
   <IsPackable>false</IsPackable>
  </PropertyGroup>
  <ItemGroup>
   <PackageReference Include="NUnit" Version="3.12.0" />
   <PackageReference Include="NUnit3TestAdapter" Version="3.17.0" />
   <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.4.0" />
  </ItemGroup>
</Project>

The test passes in Visual Studio, Rider and dotnet test, but fails in NUnit.ConsoleRunner.NetCore (version 3.12.0-beta2):

NUnit Console Runner 3.12.0-beta2
Copyright (c) 2021 Charlie Poole, Rob Prouse
Tuesday 23 February 2021 10:33:11
 
Runtime Environment
   OS Version: Microsoft Windows 10.0.17134
  Runtime: .NET Core 3.1.12
 
Test Files
    D:\ARD\Temp\TestNUnit\MyNUnitTestProject\bin\Debug\netcoreapp3.1\MyNUnitTestProject.dll
 
Errors, Failures and Warnings
 
1) Failed : MyNUnitTestProject.Tests.ShouldPass
  Expected string length 36 but was 32. Strings differ at index 0.
  Expected: "e4ee8d1a-2171-470c-bbdb-a74780e9d0d5"
  But was:  "MyNUnitTestProject.Tests+MyClass"
  -----------^
   at MyNUnitTestProject.Tests.ShouldPass() in D:\ARD\Temp\TestNUnit\MyNUnitTestProject\UnitTest1.cs:line 41
 
Run Settings
    DisposeRunners: True
    WorkDirectory: D:\ARD\Temp\TestNUnit\MyNUnitTestProject\bin\Debug\netcoreapp3.1
    NumberOfTestWorkers: 8
 
Test Run Summary
  Overall result: Failed
  Test Count: 1, Passed: 0, Failed: 1, Warnings: 0, Inconclusive: 0, Skipped: 0
    Failed Tests - Failures: 1, Errors: 0, Invalid: 0
  Start time: 2021-02-23 09:33:11Z
    End time: 2021-02-23 09:33:12Z
    Duration: 0.250 seconds

Problem 2 - Failure when the tested code depends on NUnit.Engine 3.12.0

For this test, the code above is compiled with the following csproj (same as problem 1 with a new dependency to NUnit.Engine 3.12.0):

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
   <TargetFramework>netcoreapp3.1</TargetFramework>
   <IsPackable>false</IsPackable>
  </PropertyGroup>
  <ItemGroup>
   <PackageReference Include="NUnit" Version="3.12.0" />
   <PackageReference Include="NUnit.Engine" Version="3.12.0" />
   <PackageReference Include="NUnit3TestAdapter" Version="3.17.0" />
   <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.4.0" />
  </ItemGroup>
</Project>

The test fails in Visual Studio, Rider and dotnet test:

System.InvalidCastException : [A]MyClass cannot be cast to [B]MyClass. Type A originates from 'MyNUnitTestProject, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' in the context 'Default' at location 'D:\ARD\Temp\TestNUnit\MyNUnitTestProject\bin\Debug\netcoreapp3.1\MyNUnitTestProject.dll'. Type B originates from 'MyNUnitTestProject, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' in the context 'Default' at location 'D:\ARD\Temp\TestNUnit\MyNUnitTestProject\bin\Debug\netcoreapp3.1\MyNUnitTestProject.dll'.
    at MyNUnitTestProject.Tests.MyClassTypeConverter.ConvertTo(ITypeDescriptorContext context, CultureInfo culture, Object value, Type destinationType) in D:\ARD\Temp\TestNUnit\MyNUnitTestProject\UnitTest1.cs:line 20
    at MyNUnitTestProject.Tests.ShouldPass() in D:\ARD\Temp\TestNUnit\MyNUnitTestProject\UnitTest1.cs:line 40

The failure is linked to the usage of AssemblyLoadContext in NUnit.Engine 3.12.0: MyClass type's assembly is loaded in the Default AssemblyLoadContext and in the NUnit custom AssemblyLoadContext. At runtime, the type of the parameter value passed to MyClassTypeConverter.ConvertTo([...]) comes from the assembly loaded in the NUnit custom AssemblyLoadContext whereas the (MyClass) type used to cast comes from the assembly loaded in the Default AssemblLoadContext. The cast fails because types loaded in two AssemblyLoadContext are not the same.

Forcing the usage of the NUnit custom AssemblyLoadContext by calling the following line before the test execution makes it pass:

AssemblyLoadContext.All.Single(c => c.ToString().Contains("NUnit.Engine.Internal.CustomAssemblyLoadContext")).EnterContextualReflection();

I can add this line in the static constructor of a SetUpFixture class to setup it once for all, but I have to do this hack in each test project, and this is not very clean.

Note that with NUnit.ConsoleRunner.NetCore (version 3.12.0-beta2), the cast works (without the hack mentioned above) but there is the same failure as described in problem 1.

For info, we depend on NUnit.Engine 3.12.0 in our homemade unit test runner to be able to launch .NET Core unit tests (we need this fix).

@joeltankam
Copy link

Any update on this please ?

@ChrisMaddock
Copy link
Member

Hi @Learfin, @joeltankam, @VoltanFr, @bouchraRekhadda - believe you're all seeing manifestations of #828. That issue essentially is just waiting for someone to pick it up and investigate - we'd love some help, if you'd be willing? 🙂

@bouchraRekhadda
Copy link

Hello @ChrisMaddock thank you for your feedback. It seems like I've missed the issue you are referring to 😅.
I will shortly upload a small reproduction repository where we had issues around .NET Core 3.1 assemblies dynamic load (our workaround is to force assemblies load in the custom NUnt.Engine AssemblyLoadContext as mentioned above by @Learfin).

@ChrisMaddock
Copy link
Member

Great - thanks @bouchraRekhadda! Just digging into this now myself too...actually think #937 may help too, which is good news cc @CharliePoole

@joeltankam
Copy link

@ChrisMaddock Thanks a lot for #942
I was thinking that making the use of a custom assembly load context optional (the same way that the dedicated AppDomain can be disabled through --domain argument for .NET Framework) could also help.
In our particular case, if there was only the Default load context we would have avoided many issues.
(I do agree that properly using dedicated load contexts is the way to go; but giving more options can also help)

@ChrisMaddock
Copy link
Member

Hi @joeltankam - I'm not sure on this, but I believe the Custom Assembly Load Context is currently required to allow engine to locate the correct version of the framework assembly that a test assembly depends on. By all means feel free to dig in - if there's a way of it working without this and sufficient justification to do so, more than happy to consider it.

In our particular case, if there was only the Default load context we would have avoided many issues.

Out of interest - what case do you have where using the default load context is preferable?

@ChrisMaddock ChrisMaddock self-assigned this May 9, 2021
@ChrisMaddock ChrisMaddock changed the title NUnit.Engine 3.12.0 .NET Core and TypeDescriptor.GetConverter([...]) NUnit.Engine 3.12.0 .NET Core and TypeDescriptor.GetConverter([...]) dependency loading problems May 9, 2021
@CharliePoole
Copy link
Collaborator

@Learfin
Since you use NUnit.ConsoleRunner.NetCore I'd like to ask for your comments on #1045

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants