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.

Sign up for GitHub

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

Fix class initialization/cleanup calls #3362

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Fix class initialization/cleanup calls #3362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 59 additions & 51 deletions src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
/// </summary>
public class TestClassInfo
{
private readonly object _testClassExecuteSyncObject;
private readonly object _testClassExecuteSyncObject = new();
private MethodInfo? _classCleanupMethod;
private MethodInfo? _classInitializeMethod;
private MethodInfo? _testCleanupMethod;
Expand All @@ -48,17 +48,8 @@ internal TestClassInfo(
Constructor = constructor;
IsParameterlessConstructor = isParameterlessConstructor;
TestContextProperty = testContextProperty;
BaseClassCleanupMethodsStack = new Stack<MethodInfo>();
BaseClassInitAndCleanupMethods = new Queue<Tuple<MethodInfo?, MethodInfo?>>();
ClassInitializeMethodTimeoutMilliseconds = new Dictionary<MethodInfo, TimeoutInfo>();
ClassCleanupMethodTimeoutMilliseconds = new Dictionary<MethodInfo, TimeoutInfo>();
BaseTestInitializeMethodsQueue = new Queue<MethodInfo>();
BaseTestCleanupMethodsQueue = new Queue<MethodInfo>();
TestInitializeMethodTimeoutMilliseconds = new Dictionary<MethodInfo, TimeoutInfo>();
TestCleanupMethodTimeoutMilliseconds = new Dictionary<MethodInfo, TimeoutInfo>();
Parent = parent;
ClassAttribute = classAttribute;
_testClassExecuteSyncObject = new object();
}

/// <summary>
Expand Down Expand Up @@ -111,25 +102,25 @@ internal set
/// Gets the timeout for the class initialize methods.
/// We can use a dictionary because the MethodInfo is unique in an inheritance hierarchy.
/// </summary>
internal Dictionary<MethodInfo, TimeoutInfo> ClassInitializeMethodTimeoutMilliseconds { get; }
internal Dictionary<MethodInfo, TimeoutInfo> ClassInitializeMethodTimeoutMilliseconds { get; } = new();

/// <summary>
/// Gets the timeout for the class cleanup methods.
/// We can use a dictionary because the MethodInfo is unique in an inheritance hierarchy.
/// </summary>
internal Dictionary<MethodInfo, TimeoutInfo> ClassCleanupMethodTimeoutMilliseconds { get; }
internal Dictionary<MethodInfo, TimeoutInfo> ClassCleanupMethodTimeoutMilliseconds { get; } = new();

/// <summary>
/// Gets the timeout for the test initialize methods.
/// We can use a dictionary because the MethodInfo is unique in an inheritance hierarchy.
/// </summary>
internal Dictionary<MethodInfo, TimeoutInfo> TestInitializeMethodTimeoutMilliseconds { get; }
internal Dictionary<MethodInfo, TimeoutInfo> TestInitializeMethodTimeoutMilliseconds { get; } = new();

/// <summary>
/// Gets the timeout for the test cleanup methods.
/// We can use a dictionary because the MethodInfo is unique in an inheritance hierarchy.
/// </summary>
internal Dictionary<MethodInfo, TimeoutInfo> TestCleanupMethodTimeoutMilliseconds { get; }
internal Dictionary<MethodInfo, TimeoutInfo> TestCleanupMethodTimeoutMilliseconds { get; } = new();

/// <summary>
/// Gets a value indicating whether class initialize has executed.
Expand All @@ -144,7 +135,12 @@ internal set
/// <summary>
/// Gets a stack of class cleanup methods to be executed.
/// </summary>
public Stack<MethodInfo> BaseClassCleanupMethodsStack { get; internal set; }
[Obsolete("API will be dropped in v4")]
public Stack<MethodInfo> BaseClassCleanupMethodsStack { get; } = new();

internal List<MethodInfo> BaseClassInitMethods { get; } = new();

internal List<MethodInfo> BaseClassCleanupMethods { get; } = new();

/// <summary>
/// Gets the exception thrown during <see cref="ClassInitializeAttribute"/> method invocation.
Expand Down Expand Up @@ -189,14 +185,15 @@ public bool HasExecutableCleanupMethod
}

// Otherwise, if any base cleanups were pushed to the stack we need to run them
return BaseClassCleanupMethodsStack.Count != 0;
return BaseClassCleanupMethods.Count != 0;
}
}

/// <summary>
/// Gets a tuples' queue of class initialize/cleanup methods to call for this type.
/// </summary>
public Queue<Tuple<MethodInfo?, MethodInfo?>> BaseClassInitAndCleanupMethods { get; }
[Obsolete("API will be dropped in v4")]
public Queue<Tuple<MethodInfo?, MethodInfo?>> BaseClassInitAndCleanupMethods { get; } = new();

/// <summary>
/// Gets the test initialize method.
Expand Down Expand Up @@ -239,12 +236,12 @@ internal set
/// <summary>
/// Gets a queue of test initialize methods to call for this type.
/// </summary>
public Queue<MethodInfo> BaseTestInitializeMethodsQueue { get; }
public Queue<MethodInfo> BaseTestInitializeMethodsQueue { get; } = new();

/// <summary>
/// Gets a queue of test cleanup methods to call for this type.
/// </summary>
public Queue<MethodInfo> BaseTestCleanupMethodsQueue { get; }
public Queue<MethodInfo> BaseTestCleanupMethodsQueue { get; } = new();

/// <summary>
/// Runs the class initialize method.
Expand All @@ -255,7 +252,7 @@ internal set
public void RunClassInitialize(TestContext testContext)
{
// If no class initialize and no base class initialize, return
if (ClassInitializeMethod is null && !BaseClassInitAndCleanupMethods.Any(p => p.Item1 != null))
if (ClassInitializeMethod is null && BaseClassInitMethods.Count == 0)
{
return;
}
Expand Down Expand Up @@ -283,31 +280,22 @@ public void RunClassInitialize(TestContext testContext)
{
try
{
// ClassInitialize methods for base classes are called in reverse order of discovery
// Base -> Child TestClass
var baseClassInitializeStack = new Stack<Tuple<MethodInfo?, MethodInfo?>>(BaseClassInitAndCleanupMethods.Where(p => p.Item1 != null));

while (baseClassInitializeStack.Count > 0)
// We have discovered the methods from bottom (most derived) to top (less derived) but we want to execute
// from top to bottom.
for (int i = BaseClassInitMethods.Count - 1; i >= 0; i--)
{
Tuple<MethodInfo?, MethodInfo?> baseInitCleanupMethods = baseClassInitializeStack.Pop();
initializeMethod = baseInitCleanupMethods.Item1;

ClassInitializationException = initializeMethod is not null ? InvokeInitializeMethod(initializeMethod, testContext) : null;
initializeMethod = BaseClassInitMethods[i];
ClassInitializationException = InvokeInitializeMethod(initializeMethod, testContext);
if (ClassInitializationException is not null)
{
break;
}

if (baseInitCleanupMethods.Item2 != null)
{
BaseClassCleanupMethodsStack.Push(baseInitCleanupMethods.Item2);
}
}

if (ClassInitializationException is null)
{
initializeMethod = null;
ClassInitializationException = ClassInitializeMethod is not null ? InvokeInitializeMethod(ClassInitializeMethod, testContext) : null;
initializeMethod = ClassInitializeMethod;
ClassInitializationException = InvokeInitializeMethod(ClassInitializeMethod, testContext);
}
}
catch (Exception ex)
Expand Down Expand Up @@ -367,7 +355,7 @@ internal UnitTestResult GetResultOrRunClassInitialize(ITestContext testContext,
{
// For optimization purposes, we duplicate some of the logic of RunClassInitialize here so we don't need to start
// a thread for nothing.
if ((ClassInitializeMethod is null && !BaseClassInitAndCleanupMethods.Any(p => p.Item1 != null))
if ((ClassInitializeMethod is null && BaseClassInitMethods.Count == 0)
|| IsClassInitializeExecuted)
{
return DoRun();
Expand Down Expand Up @@ -446,8 +434,13 @@ UnitTestResult DoRun()
}
}

private TestFailedException? InvokeInitializeMethod(MethodInfo methodInfo, TestContext testContext)
private TestFailedException? InvokeInitializeMethod(MethodInfo? methodInfo, TestContext testContext)
{
if (methodInfo is null)
{
return null;
}

TimeoutInfo? timeout = null;
if (ClassInitializeMethodTimeoutMilliseconds.TryGetValue(methodInfo, out TimeoutInfo localTimeout))
{
Expand All @@ -471,9 +464,10 @@ UnitTestResult DoRun()
/// <returns>
/// Any exception that can be thrown as part of a class cleanup as warning messages.
/// </returns>
[Obsolete("API will be dropped in v4")]
public string? RunClassCleanup(ClassCleanupBehavior classCleanupLifecycle = ClassCleanupBehavior.EndOfAssembly)
{
if (ClassCleanupMethod is null && BaseClassInitAndCleanupMethods.All(p => p.Item2 == null))
if (ClassCleanupMethod is null && BaseClassCleanupMethods.Count == 0)
{
return null;
}
Expand All @@ -496,8 +490,8 @@ UnitTestResult DoRun()
try
{
classCleanupMethod = ClassCleanupMethod;
ClassCleanupException = classCleanupMethod is not null ? InvokeCleanupMethod(classCleanupMethod, BaseClassCleanupMethodsStack.Count) : null;
var baseClassCleanupQueue = new Queue<MethodInfo>(BaseClassCleanupMethodsStack);
ClassCleanupException = classCleanupMethod is not null ? InvokeCleanupMethod(classCleanupMethod, BaseClassCleanupMethods.Count) : null;
var baseClassCleanupQueue = new Queue<MethodInfo>(BaseClassCleanupMethods);
while (baseClassCleanupQueue.Count > 0 && ClassCleanupException is null)
{
classCleanupMethod = baseClassCleanupQueue.Dequeue();
Expand Down Expand Up @@ -555,13 +549,13 @@ UnitTestResult DoRun()
/// </remarks>
internal void ExecuteClassCleanup()
{
if ((ClassCleanupMethod is null && BaseClassInitAndCleanupMethods.All(p => p.Item2 == null))
if ((ClassCleanupMethod is null && BaseClassCleanupMethods.Count == 0)
|| IsClassCleanupExecuted)
{
return;
}

MethodInfo? classCleanupMethod = null;
MethodInfo? classCleanupMethod = ClassCleanupMethod;

lock (_testClassExecuteSyncObject)
{
Expand All @@ -574,24 +568,38 @@ internal void ExecuteClassCleanup()

try
{
IEnumerable<MethodInfo> cleanupMethods = (ClassCleanupMethod is null ? Array.Empty<MethodInfo>() : [ClassCleanupMethod]).Union(BaseClassCleanupMethodsStack);
var classCleanupQueue = new Queue<MethodInfo>(cleanupMethods);

while (classCleanupQueue.Count > 0 && ClassCleanupException is null)
if (classCleanupMethod is not null)
{
classCleanupMethod = classCleanupQueue.Dequeue();
if (!ReflectHelper.Instance.IsNonDerivedAttributeDefined<IgnoreAttribute>(classCleanupMethod.DeclaringType!, false))
{
ClassCleanupException = InvokeCleanupMethod(classCleanupMethod, classCleanupQueue.Count);
ClassCleanupException = InvokeCleanupMethod(classCleanupMethod, remainingCleanupCount: BaseClassCleanupMethods.Count);
}
}

IsClassCleanupExecuted = ClassCleanupException is null;
if (ClassCleanupException is null)
{
for (int i = 0; i < BaseClassCleanupMethods.Count; i++)
{
classCleanupMethod = BaseClassCleanupMethods[i];
if (!ReflectHelper.Instance.IsNonDerivedAttributeDefined<IgnoreAttribute>(classCleanupMethod.DeclaringType!, false))
{
ClassCleanupException = InvokeCleanupMethod(classCleanupMethod, remainingCleanupCount: BaseClassCleanupMethods.Count - 1 - i);
if (ClassCleanupException is not null)
{
break;
}
}
}
}
}
catch (Exception exception)
{
ClassCleanupException = exception;
}
finally
{
IsClassCleanupExecuted = true;
}
}

// If ClassCleanup was successful, then don't do anything
Expand Down Expand Up @@ -649,7 +657,7 @@ internal void RunClassCleanup(ITestContext testContext, ClassCleanupManager clas
{
// For optimization purposes, we duplicate some of the logic of ExecuteClassCleanup here so we don't need to start
// a thread for nothing.
if ((ClassCleanupMethod is null && BaseClassInitAndCleanupMethods.All(p => p.Item1 is null))
if ((ClassCleanupMethod is null && BaseClassCleanupMethods.Count == 0)
|| IsClassCleanupExecuted)
{
DoRun();
Expand Down
35 changes: 28 additions & 7 deletions src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ private TestClassInfo CreateClassInfo(Type classType, TestMethod testMethod)

// List holding the instance of the initialize/cleanup methods
// to be passed into the tuples' queue when updating the class info.
var initAndCleanupMethods = new MethodInfo[2];
var initAndCleanupMethods = new MethodInfo?[2];

// List of instance methods present in the type as well its base type
// which is used to decide whether TestInitialize/TestCleanup methods
Expand Down Expand Up @@ -533,14 +533,35 @@ private bool IsAssemblyOrClassCleanupMethod<TCleanupAttribute>(MethodInfo method
/// <param name="initAndCleanupMethods"> An array with the Initialize and Cleanup Methods Info. </param>
private static void UpdateInfoWithInitializeAndCleanupMethods(
TestClassInfo classInfo,
ref MethodInfo[] initAndCleanupMethods)
ref MethodInfo?[] initAndCleanupMethods)
{
if (initAndCleanupMethods.Any(x => x != null))
DebugEx.Assert(initAndCleanupMethods.Length == 2, "initAndCleanupMethods.Length == 2");

MethodInfo? initMethod = initAndCleanupMethods[0];
MethodInfo? cleanupMethod = initAndCleanupMethods[1];

if (initMethod is not null)
{
classInfo.BaseClassInitMethods.Add(initMethod);
}

if (cleanupMethod is not null)
{
classInfo.BaseClassCleanupMethods.Add(cleanupMethod);

#pragma warning disable CS0618 // Type or member is obsolete
classInfo.BaseClassCleanupMethodsStack.Push(cleanupMethod);
#pragma warning restore CS0618 // Type or member is obsolete
}

if (initMethod is not null || cleanupMethod is not null)
{
#pragma warning disable CS0618 // Type or member is obsolete - kept in case someone is using it
classInfo.BaseClassInitAndCleanupMethods.Enqueue(
new Tuple<MethodInfo?, MethodInfo?>(
initAndCleanupMethods.FirstOrDefault(),
initAndCleanupMethods.LastOrDefault()));
new Tuple<MethodInfo?, MethodInfo?>(
initMethod,
initAndCleanupMethods.LastOrDefault()));
#pragma warning restore CS0618 // Type or member is obsolete
}

initAndCleanupMethods = new MethodInfo[2];
Expand All @@ -557,7 +578,7 @@ private void UpdateInfoIfClassInitializeOrCleanupMethod(
TestClassInfo classInfo,
MethodInfo methodInfo,
bool isBase,
ref MethodInfo[] initAndCleanupMethods)
ref MethodInfo?[] initAndCleanupMethods)
{
bool isInitializeMethod = IsAssemblyOrClassInitializeMethod<ClassInitializeAttribute>(methodInfo);
bool isCleanupMethod = IsAssemblyOrClassCleanupMethod<ClassCleanupAttribute>(methodInfo);
Expand Down
2 changes: 1 addition & 1 deletion src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ internal static bool IsDoNotParallelizeSet(Assembly assembly)

var cleanupBehaviors =
new HashSet<ClassCleanupBehavior?>(
classInfo.BaseClassCleanupMethodsStack
classInfo.BaseClassCleanupMethods
.Select(x => GetFirstDerivedAttributeOrDefault<ClassCleanupAttribute>(x, inherit: true)?.CleanupBehavior))
{
classInfo.ClassCleanupMethod == null ? null : GetFirstDerivedAttributeOrDefault<ClassCleanupAttribute>(classInfo.ClassCleanupMethod, inherit: true)?.CleanupBehavior,
Expand Down
Loading