AssemblyCleanup in generic base class throws exception

[pi3k14]

Describe the bug

MSTest.TestFramework 3.0.4

AssemblyCleanup in generic base class throws exception

System.InvalidOperationException: Late bound operations cannot be performed on types or methods for which ContainsGenericParameters is true..
StackTrace:
at System.Reflection.RuntimeMethodInfo.ThrowNoInvokeException()
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)

[TestClass]
public class BaseClass<T>
{
    [AssemblyCleanup] 
    public static void AssemblyCleanup()
    {
        Console.WriteLine(nameof(AssemblyCleanup));
    }
}

[TestClass]
public class UnitTest1 : BaseClass<int>
{
    [TestMethod]
    public void TestMethod1()
    {
        Console.WriteLine(nameof(TestMethod1));
    }
}

AB#1950772

[Evangelink]

Hi @pi3k14,

I confirm that I can reproduce the bug. We will investigate it and fix it asap.

[nohwnd]

This is happening because we are trying to call a method on an open generic type BaseClass<T> rather than on its realized type BaseClass<int>.

The assembly cleanup and assembly initialization is not tied to any particular class so we cannot really know what T should be.

This is probably not a problem for most of cleanups, because they are probably not referring to the T type at all. But we cannot know that.

What we can do is that we:

1. say that assembly initialize / cleanup in generic class is not supported
2. force user to specify the types to use in the attribute [AssemblyCleanup(genericParams: new [] { typeof(int))]
3. force the user to define a non-generic derived class for the initialize method
4. scan for all realizations of the type, and if they are not all the same, we fallback to the attribute types, and if they are not there we throw

Option 2 is probably most performant for us, if we want to allow this functionality, but not very logical for the user I am afraid.

[Evangelink]

Thanks for the investigation @nohwnd ❤️

Doing a quick look this seems to be unsupported scenario for xUnit so I would probably favor option 1, add an analyzer for such case and create a feature request ticket for option 2 to see if we get some thumbs up and plan implementing or not based on that.

WDYT?

[pi3k14]

If going for option 1 I would strongly urge you to fix #1889

[nohwnd]

I agree that making it unsupported is the right call here. It is both performant and easy to workaround, because you simply need to add another class that will hold the assembly level setups and cleanups. Which to me even seems cleaner.

[pi3k14]

you simply need to add another class that will hold the assembly level setups and cleanups. Which to me even seems cleaner.

If doing operations on the generic type, e.g. a specific DbContext, that would neither be simple or cleaner.

[nohwnd]

In that case you would need to run AssemblyInitialize / Cleanup for every type that is mentioned as T right?

e.g. following the example above:

public class UnitTest1 : BaseClass<int>

public class UnitTest2 : BaseClass<double>

There would be 2 invocations for AssemblyCleanup one for T = int and one for T = double.

That seems doable, but I am not sure if that is not a little unexpected, if the cleanup is not the reason why there is a T.

public class  FileManagerBaseClass<TInMemoryStorageType> { 
  // Assembly initialize deletes the file that is always the same
  // but TInMemoryStorageType decides into which in memory storage 
  // we load the data, but the file is the same, and the cleanup is not concerned
  // with it. 
} 
public class MyTests : FileManagerBaseClass<TInMemoryStorageType> 

Not the best example I admit, but the only one I can think of right now.

[Evangelink]

@nohwnd Assigning you this issue for milestone 3.3. The idea is to add a a code check and report a runtime issue as we do for other invalid layout. @cvpoienaru will add a code analyzer for this.

[testplatform-bot]

✅ Successfully linked to Azure Boards work item(s):

• User Story 1950772

[Evangelink]

Self-assigning so you have more time for native AOT. I already know what to fix and how to fix so will be able to do a quick PR.