Skip to content

Commit

Permalink
Fix bugs in parent class init/cleanup logic (#660)
Browse files Browse the repository at this point in the history 
* Adding UT to catch issue where base class cleanup isn't called if not class cleanup on top level class

* Fix issue where base class cleanup isn't called when there is no top level class cleanup

* Add test to catch null ref ex when grandparent test class has class init/cleanup methods

* Fix null ref ex when grandparent test class has class init/cleanup methods
  • Loading branch information
@NGloreous @nohwnd
NGloreous authored and nohwnd committed Dec 20, 2019
1 parent 6c1ee30 commit 664ac7c
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 26 deletions.
14 changes: 12 additions & 2 deletions src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ public bool HasExecutableCleanupMethod
{
get
{
if (this.BaseClassCleanupMethodsStack.Any())
{
// If any base cleanups were pushed to the stack we need to run them
return true;
}

// If no class cleanup, then continue with the next one.
if (this.ClassCleanupMethod == null)
{
Expand Down Expand Up @@ -274,7 +280,11 @@ public void RunClassInitialize(TestContext testContext)
var baseInitCleanupMethods = baseClassInitializeStack.Pop();
initializeMethod = baseInitCleanupMethods.Item1;
initializeMethod?.InvokeAsSynchronousTask(null, testContext);
this.BaseClassCleanupMethodsStack.Push(baseInitCleanupMethods.Item2);

if (baseInitCleanupMethods.Item2 != null)
{
this.BaseClassCleanupMethodsStack.Push(baseInitCleanupMethods.Item2);
}
}
}
catch (Exception ex)
Expand Down Expand Up @@ -362,9 +372,9 @@ public string RunClassCleanup()

if (this.IsClassInitializeExecuted || this.ClassInitializeMethod is null || this.BaseClassCleanupMethodsStack.Any())
{
var classCleanupMethod = this.ClassCleanupMethod;
try
{
var classCleanupMethod = this.ClassCleanupMethod;
classCleanupMethod?.InvokeAsSynchronousTask(null);
var baseClassCleanupQueue = new Queue<MethodInfo>(this.BaseClassCleanupMethodsStack);
while (baseClassCleanupQueue.Count > 0)
Expand Down
14 changes: 6 additions & 8 deletions src/Adapter/MSTest.CoreAdapter/Execution/TypeCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,17 +488,15 @@ private void UpdateInfoWithInitializeAndCleanupMethods(
TestClassInfo classInfo,
ref MethodInfo[] initAndCleanupMethods)
{
if (initAndCleanupMethods is null)
if (initAndCleanupMethods.Any(x => x != null))
{
return;
classInfo.BaseClassInitAndCleanupMethods.Enqueue(
new Tuple<MethodInfo, MethodInfo>(
initAndCleanupMethods.FirstOrDefault(),
initAndCleanupMethods.LastOrDefault()));
}

classInfo.BaseClassInitAndCleanupMethods.Enqueue(
new Tuple<MethodInfo, MethodInfo>(
initAndCleanupMethods.FirstOrDefault(),
initAndCleanupMethods.LastOrDefault()));

initAndCleanupMethods = null;
initAndCleanupMethods = new MethodInfo[2];
}

/// <summary>
Expand Down
36 changes: 26 additions & 10 deletions test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TestClassInfoTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,16 +431,6 @@ public void RunClassInitializeShouldThrowForAlreadyExecutedTestClassInitWithExce
exception.Message);
}

[TestMethod]
public void RunClassCleanupShouldInvokeIfClassCleanupMethod()
{
var classcleanupCallCount = 0;
DummyTestClass.ClassCleanupMethodBody = () => classcleanupCallCount++;
this.testClassInfo.ClassCleanupMethod = typeof(DummyTestClass).GetMethod("ClassCleanupMethod");
Assert.IsNull(this.testClassInfo.RunClassCleanup());
Assert.AreEqual(1, classcleanupCallCount);
}

[TestMethod]
public void RunAssemblyInitializeShouldPassOnTheTestContextToAssemblyInitMethod()
{
Expand All @@ -454,6 +444,16 @@ public void RunAssemblyInitializeShouldPassOnTheTestContextToAssemblyInitMethod(

#region Run Class Cleanup tests

[TestMethod]
public void RunClassCleanupShouldInvokeIfClassCleanupMethod()
{
var classcleanupCallCount = 0;
DummyTestClass.ClassCleanupMethodBody = () => classcleanupCallCount++;
this.testClassInfo.ClassCleanupMethod = typeof(DummyTestClass).GetMethod("ClassCleanupMethod");
Assert.IsNull(this.testClassInfo.RunClassCleanup());
Assert.AreEqual(1, classcleanupCallCount);
}

[TestMethod]
public void RunClassCleanupShouldNotInvokeIfClassCleanupIsNull()
{
Expand Down Expand Up @@ -500,6 +500,22 @@ public void RunClassCleanupShouldReturnExceptionDetailsOfNonAssertExceptions()
"Class Cleanup method DummyTestClass.ClassCleanupMethod failed. Error Message: System.ArgumentException: Argument Exception. Stack Trace: at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.Execution.TestClassInfoTests.<>c.<RunClassCleanupShouldReturnExceptionDetailsOfNonAssertExceptions>");
}

[TestMethod]
public void RunBaseClassCleanupEvenIfThereIsNoDerivedClassCleanup()
{
var classcleanupCallCount = 0;
DummyBaseTestClass.ClassCleanupMethodBody = () => classcleanupCallCount++;

this.testClassInfo.ClassCleanupMethod = null;
this.testClassInfo.BaseClassInitAndCleanupMethods.Enqueue(
Tuple.Create((MethodInfo)null, typeof(DummyBaseTestClass).GetMethod("CleanupClassMethod")));
this.testClassInfo.BaseClassCleanupMethodsStack.Push(typeof(DummyBaseTestClass).GetMethod("CleanupClassMethod"));

Assert.IsTrue(this.testClassInfo.HasExecutableCleanupMethod);
Assert.IsNull(this.testClassInfo.RunClassCleanup());
Assert.AreEqual(1, classcleanupCallCount, "DummyBaseTestClass.CleanupClassMethod call count");
}

#endregion

[DummyTestClass]
Expand Down
100 changes: 94 additions & 6 deletions test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TypeCacheTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,8 @@ public void GetTestMethodInfoShouldCacheClassInitializeAttribute()
false);

Assert.AreEqual(1, this.typeCache.ClassInfoCache.Count());
Assert.AreEqual(type.GetMethod("AssemblyInit"), this.typeCache.ClassInfoCache.ToArray()[0].ClassInitializeMethod);
Assert.AreEqual(0, this.typeCache.ClassInfoCache.First().BaseClassInitAndCleanupMethods.Count);
Assert.AreEqual(type.GetMethod("AssemblyInit"), this.typeCache.ClassInfoCache.First().ClassInitializeMethod);
}

[TestMethodV1]
Expand Down Expand Up @@ -515,8 +516,8 @@ public void GetTestMethodInfoShouldCacheBaseClassInitializeAttributes()

Assert.AreEqual(1, this.typeCache.ClassInfoCache.Count());
Assert.AreEqual(1, this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.Count);
Assert.AreEqual(null, this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.First().Item2);
Assert.AreEqual(baseType.GetMethod("AssemblyInit"), this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.First().Item1);
Assert.IsNull(this.typeCache.ClassInfoCache.First().BaseClassInitAndCleanupMethods.First().Item2, "No base class cleanup");
Assert.AreEqual(baseType.GetMethod("AssemblyInit"), this.typeCache.ClassInfoCache.First().BaseClassInitAndCleanupMethods.First().Item1);
}

[TestMethodV1]
Expand Down Expand Up @@ -564,9 +565,9 @@ public void GetTestMethodInfoShouldCacheBaseClassCleanupAttributes()
false);

Assert.AreEqual(1, this.typeCache.ClassInfoCache.Count());
Assert.AreEqual(1, this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.Count);
Assert.AreEqual(null, this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.First().Item1);
Assert.AreEqual(baseType.GetMethod("AssemblyCleanup"), this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.First().Item2);
Assert.AreEqual(1, this.typeCache.ClassInfoCache.First().BaseClassInitAndCleanupMethods.Count);
Assert.IsNull(this.typeCache.ClassInfoCache.First().BaseClassInitAndCleanupMethods.First().Item1, "No base class init");
Assert.AreEqual(baseType.GetMethod("AssemblyCleanup"), this.typeCache.ClassInfoCache.First().BaseClassInitAndCleanupMethods.First().Item2);
}

[TestMethodV1]
Expand Down Expand Up @@ -638,6 +639,72 @@ public void GetTestMethodInfoShouldCacheBaseClassInitAndCleanupAttributes()
Assert.AreEqual(baseCleanupMethod, this.typeCache.ClassInfoCache.ToArray()[0].BaseClassInitAndCleanupMethods.First().Item2);
}

[TestMethodV1]
public void GetTestMethodInfoShouldCacheParentAndGrandparentClassInitAndCleanupAttributes()
{
var grandparentType = typeof(DummyBaseTestClassWithInitAndCleanupMethods);
var parentType = typeof(DummyChildBaseTestClassWithInitAndCleanupMethods);
var type = typeof(DummyTestClassWithParentAndGrandparentInitAndCleanupMeethods);

var grandparentInitMethod = grandparentType.GetMethod("ClassInit");
var grandparentCleanupMethod = grandparentType.GetMethod("ClassCleanup");
var parentInitMethod = parentType.GetMethod("ChildClassInit");
var parentCleanupMethod = parentType.GetMethod("ChildClassCleanup");

this.mockReflectHelper
.Setup(rh => rh.IsAttributeDefined(type, typeof(UTF.TestClassAttribute), true))
.Returns(true);

// Setup grandparent class init/cleanup methods
this.mockReflectHelper
.Setup(rh => rh.IsAttributeDefined(grandparentInitMethod, typeof(UTF.ClassInitializeAttribute), false))
.Returns(true);
this.mockReflectHelper
.Setup(rh => rh.GetCustomAttribute(grandparentInitMethod, typeof(UTF.ClassInitializeAttribute)))
.Returns(new UTF.ClassInitializeAttribute(UTF.InheritanceBehavior.BeforeEachDerivedClass));
this.mockReflectHelper
.Setup(rh => rh.IsAttributeDefined(grandparentCleanupMethod, typeof(UTF.ClassCleanupAttribute), false))
.Returns(true);
this.mockReflectHelper
.Setup(rh => rh.GetCustomAttribute(grandparentCleanupMethod, typeof(UTF.ClassCleanupAttribute)))
.Returns(new UTF.ClassCleanupAttribute(UTF.InheritanceBehavior.BeforeEachDerivedClass));

// Setup parent class init/cleanup methods
this.mockReflectHelper
.Setup(rh => rh.IsAttributeDefined(parentInitMethod, typeof(UTF.ClassInitializeAttribute), false))
.Returns(true);
this.mockReflectHelper
.Setup(rh => rh.GetCustomAttribute(parentInitMethod, typeof(UTF.ClassInitializeAttribute)))
.Returns(new UTF.ClassInitializeAttribute(UTF.InheritanceBehavior.BeforeEachDerivedClass));
this.mockReflectHelper
.Setup(rh => rh.IsAttributeDefined(parentCleanupMethod, typeof(UTF.ClassCleanupAttribute), false))
.Returns(true);
this.mockReflectHelper
.Setup(rh => rh.GetCustomAttribute(parentCleanupMethod, typeof(UTF.ClassCleanupAttribute)))
.Returns(new UTF.ClassCleanupAttribute(UTF.InheritanceBehavior.BeforeEachDerivedClass));

var testMethod = new TestMethod("TestMethod", type.FullName, "A", isAsync: false);
this.typeCache.GetTestMethodInfo(
testMethod,
new TestContextImplementation(testMethod, null, new Dictionary<string, object>()),
false);

var classInfo = this.typeCache.ClassInfoCache.FirstOrDefault();
Assert.AreEqual(1, this.typeCache.ClassInfoCache.Count());
Assert.IsNull(classInfo.ClassInitializeMethod);
Assert.IsNull(classInfo.ClassCleanupMethod);

Assert.AreEqual(2, classInfo.BaseClassInitAndCleanupMethods.Count);

var parentInitAndCleanup = classInfo.BaseClassInitAndCleanupMethods.Dequeue();
Assert.AreEqual(parentInitMethod, parentInitAndCleanup.Item1);
Assert.AreEqual(parentCleanupMethod, parentInitAndCleanup.Item2);

var grandparentInitAndCleanup = classInfo.BaseClassInitAndCleanupMethods.Dequeue();
Assert.AreEqual(grandparentInitMethod, grandparentInitAndCleanup.Item1);
Assert.AreEqual(grandparentCleanupMethod, grandparentInitAndCleanup.Item2);
}

[TestMethodV1]
public void GetTestMethodInfoShouldThrowIfClassInitHasIncorrectSignature()
{
Expand Down Expand Up @@ -1630,6 +1697,27 @@ public void TestInitOrCleanup()
}
}

[DummyTestClass]
private class DummyChildBaseTestClassWithInitAndCleanupMethods : DummyBaseTestClassWithInitAndCleanupMethods
{
[UTF.ClassInitialize(UTF.InheritanceBehavior.BeforeEachDerivedClass)]
public static void ChildClassInit(UTFExtension.TestContext tc)
{
}

public static void ChildClassCleanup()
{
}
}

[DummyTestClass]
private class DummyTestClassWithParentAndGrandparentInitAndCleanupMeethods : DummyChildBaseTestClassWithInitAndCleanupMethods
{
public void TestMethod()
{
}
}

[DummyTestClass]
private class DummyTestClassWithIncorrectInitializeMethods
{
Expand Down

0 comments on commit 664ac7c

Please # to comment.