Fix cleanup inheritance calls (#1475)

Evangelink · web-flow · commit 37d6b05ff9af · 2022-12-19T11:09:10.000+01:00
diff --git a/src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs b/src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs
@@ -10,7 +10,6 @@
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
-using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 using ObjectModelUnitTestOutcome = Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel.UnitTestOutcome;
@@ -155,19 +154,19 @@ public bool HasExecutableCleanupMethod
     {
         get
         {
-            if (BaseClassCleanupMethodsStack.Any())
+            // If class has a cleanup method, then it is executable.
+            if (ClassCleanupMethod is not null)
             {
-                // 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 (ClassCleanupMethod == null)
+            // Otherwise, if any base cleanups were pushed to the stack we need to run them
+            if (BaseClassCleanupMethodsStack.Any())
             {
-                return false;
+                return true;
             }
 
-            return true;
+            return false;
         }
     }
 
diff --git a/src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs b/src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs
@@ -1,11 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-#nullable enable
-
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq;
@@ -487,14 +484,30 @@ internal virtual TAttribute[] GetCustomAttributes<TAttribute>(MemberInfo attribu
     /// <returns>Returns <see cref="ClassCleanupBehavior"/> if provided, otherwise <c>null</c>.</returns>
     internal virtual ClassCleanupBehavior? GetClassCleanupBehavior(TestClassInfo classInfo)
     {
-        if (classInfo.ClassCleanupMethod == null)
+        if (!classInfo.HasExecutableCleanupMethod)
         {
             return null;
         }
 
-        var sequencingAttribute = GetCustomAttributes<ClassCleanupAttribute>(classInfo.ClassCleanupMethod, true)?.FirstOrDefault();
+        var cleanupBehaviors =
+            new HashSet<ClassCleanupBehavior?>(
+                classInfo.BaseClassCleanupMethodsStack
+                .Select(x => x.GetCustomAttribute<ClassCleanupAttribute>(true)?.CleanupBehavior))
+            {
+                classInfo.ClassCleanupMethod?.GetCustomAttribute<ClassCleanupAttribute>(true)?.CleanupBehavior,
+            };
+
+        if (cleanupBehaviors.Contains(ClassCleanupBehavior.EndOfClass))
+        {
+            return ClassCleanupBehavior.EndOfClass;
+        }
 
-        return sequencingAttribute?.CleanupBehavior;
+        if (cleanupBehaviors.Contains(ClassCleanupBehavior.EndOfAssembly))
+        {
+            return ClassCleanupBehavior.EndOfAssembly;
+        }
+
+        return null;
     }
 
     /// <summary>
diff --git a/test/E2ETests/Smoke.E2E.Tests/SuiteLifeCycleTests.cs b/test/E2ETests/Smoke.E2E.Tests/SuiteLifeCycleTests.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
 
 using FluentAssertions;
@@ -24,9 +23,134 @@ public void ValidateTestRunLifecycle_net462()
         ValidateTestRunLifecycle("net462");
     }
 
+    public void ValidateInheritanceBehavior()
+    {
+        InvokeVsTestForExecution(
+            new[] { "net462\\" + Assembly },
+            testCaseFilter: "FullyQualifiedName~LifecycleInheritance",
+            targetFramework: "net462");
+
+        RunEventsHandler.PassedTests.Should().HaveCount(10);
+
+        var testMethod1 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerived_EndOfClass.TestMethod"));
+        testMethod1.Messages[0].Text.Should().Be(
+            """
+            Console: AssemblyInit was called
+            TestClassBaseEndOfClass: ClassInitialize
+            TestClassDerived_EndOfClass: TestMethod
+            TestClassBaseEndOfClass: ClassCleanup
+            
+            """);
+
+        var testMethod2 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerived_EndOfAssembly.TestMethod"));
+        testMethod2.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfAssembly: ClassInitialize
+            TestClassDerived_EndOfAssembly: TestMethod
+            
+            """);
+
+        var testMethod3 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerivedEndOfClass_EndOfClassEndOfClass.TestMethod"));
+        testMethod3.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfClass: ClassInitialize
+            TestClassIntermediateEndOfClassBaseEndOfClass: ClassInitialize
+            TestClassDerivedEndOfClass_EndOfClassEndOfClass: TestMethod
+            TestClassDerivedEndOfClass_EndOfClassEndOfClass: ClassCleanup
+            TestClassIntermediateEndOfClassBaseEndOfClass: ClassCleanup
+            TestClassBaseEndOfClass: ClassCleanup
+            
+            """);
+
+        var testMethod4 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerived_EndOfClassEndOfClass.TestMethod"));
+        testMethod4.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfClass: ClassInitialize
+            TestClassIntermediateEndOfClassBaseEndOfClass: ClassInitialize
+            TestClassDerived_EndOfClassEndOfClass: TestMethod
+            TestClassIntermediateEndOfClassBaseEndOfClass: ClassCleanup
+            TestClassBaseEndOfClass: ClassCleanup
+            
+            """);
+
+        var testMethod5 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerivedEndOfClass_EndOfClassEndOfAssembly.TestMethod"));
+        testMethod5.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfAssembly: ClassInitialize
+            TestClassIntermediateEndOfClassBaseEndOfAssembly: ClassInitialize
+            TestClassDerivedEndOfClass_EndOfClassEndOfAssembly: TestMethod
+            TestClassDerivedEndOfClass_EndOfClassEndOfAssembly: ClassCleanup
+            TestClassIntermediateEndOfClassBaseEndOfAssembly: ClassCleanup
+            TestClassBaseEndOfAssembly: ClassCleanup
+            
+            """);
+
+        var testMethod6 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerived_EndOfClassEndOfAssembly.TestMethod"));
+        testMethod6.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfAssembly: ClassInitialize
+            TestClassIntermediateEndOfClassBaseEndOfAssembly: ClassInitialize
+            TestClassDerived_EndOfClassEndOfAssembly: TestMethod
+            TestClassIntermediateEndOfClassBaseEndOfAssembly: ClassCleanup
+            TestClassBaseEndOfAssembly: ClassCleanup
+            
+            """);
+
+        var testMethod7 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerivedEndOfClass_EndOfAssemblyEndOfClass.TestMethod"));
+        testMethod7.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfClass: ClassInitialize
+            TestClassIntermediateEndOfAssemblyBaseEndOfClass: ClassInitialize
+            TestClassDerivedEndOfClass_EndOfAssemblyEndOfClass: TestMethod
+            TestClassDerivedEndOfClass_EndOfAssemblyEndOfClass: ClassCleanup
+            TestClassIntermediateEndOfAssemblyBaseEndOfClass: ClassCleanup
+            TestClassBaseEndOfClass: ClassCleanup
+            
+            """);
+
+        var testMethod8 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerived_EndOfAssemblyEndOfClass.TestMethod"));
+        testMethod8.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfClass: ClassInitialize
+            TestClassIntermediateEndOfAssemblyBaseEndOfClass: ClassInitialize
+            TestClassDerived_EndOfAssemblyEndOfClass: TestMethod
+            TestClassIntermediateEndOfAssemblyBaseEndOfClass: ClassCleanup
+            TestClassBaseEndOfClass: ClassCleanup
+            
+            """);
+
+        var testMethod9 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerivedEndOfClass_EndOfAssemblyEndOfAssembly.TestMethod"));
+        testMethod9.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfAssembly: ClassInitialize
+            TestClassIntermediateEndOfAssemblyBaseEndOfAssembly: ClassInitialize
+            TestClassDerivedEndOfClass_EndOfAssemblyEndOfAssembly: TestMethod
+            TestClassDerivedEndOfClass_EndOfAssemblyEndOfAssembly: ClassCleanup
+            TestClassIntermediateEndOfAssemblyBaseEndOfAssembly: ClassCleanup
+            TestClassBaseEndOfAssembly: ClassCleanup
+            
+            """);
+
+        var testMethod10 = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.EndsWith("TestClassDerived_EndOfAssemblyEndOfAssembly.TestMethod"));
+        testMethod10.Messages[0].Text.Should().Be(
+            """
+            TestClassBaseEndOfAssembly: ClassInitialize
+            TestClassIntermediateEndOfAssemblyBaseEndOfAssembly: ClassInitialize
+            TestClassDerived_EndOfAssemblyEndOfAssembly: TestMethod
+            TestClassIntermediateEndOfAssemblyBaseEndOfAssembly: ClassCleanup
+            TestClassBaseEndOfAssembly: ClassCleanup
+            TestClassBaseEndOfAssembly: ClassCleanup
+            Console: AssemblyCleanup was called
+            
+            """);
+    }
+
     private void ValidateTestRunLifecycle(string targetFramework)
     {
-        InvokeVsTestForExecution(new[] { targetFramework + "\\" + Assembly }, targetFramework: targetFramework);
+        InvokeVsTestForExecution(
+            new[] { targetFramework + "\\" + Assembly },
+            testCaseFilter: "FullyQualifiedName~SuiteLifeCycleTestProject",
+            targetFramework: targetFramework);
         RunEventsHandler.PassedTests.Should().HaveCount(27);  // The inherit class tests are called twice.
 
         var caseClassCleanup = RunEventsHandler.PassedTests.Single(x => x.TestCase.FullyQualifiedName.Contains("LifeCycleClassCleanup.TestMethod"));
diff --git a/test/E2ETests/TestAssets/SuiteLifeCycleTestProject/LifecycleInheritance.cs b/test/E2ETests/TestAssets/SuiteLifeCycleTestProject/LifecycleInheritance.cs

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,6 @@`
`10`	`10`
`11`	`11`	`using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;`
`12`	`12`	`using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;`
`13`		`-using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;`
`14`	`13`	`using Microsoft.VisualStudio.TestTools.UnitTesting;`
`15`	`14`
`16`	`15`	`using ObjectModelUnitTestOutcome = Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel.UnitTestOutcome;`
`@@ -155,19 +154,19 @@ public bool HasExecutableCleanupMethod`
`155`	`154`	`{`
`156`	`155`	`get`
`157`	`156`	`{`
`158`		`- if (BaseClassCleanupMethodsStack.Any())`
	`157`	`+ // If class has a cleanup method, then it is executable.`
	`158`	`+ if (ClassCleanupMethod is not null)`
`159`	`159`	`{`
`160`		`- // If any base cleanups were pushed to the stack we need to run them`
`161`	`160`	`return true;`
`162`	`161`	`}`
`163`	`162`
`164`		`- // If no class cleanup, then continue with the next one.`
`165`		`- if (ClassCleanupMethod == null)`
	`163`	`+ // Otherwise, if any base cleanups were pushed to the stack we need to run them`
	`164`	`+ if (BaseClassCleanupMethodsStack.Any())`
`166`	`165`	`{`
`167`		`- return false;`
	`166`	`+ return true;`
`168`	`167`	`}`
`169`	`168`
`170`		`- return true;`
	`169`	`+ return false;`
`171`	`170`	`}`
`172`	`171`	`}`
`173`	`172`