Set IsClassInitializeExecuted=true after base class init to avoid repeated class init calls (#705)

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

@@ -246,53 +246,10 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution
             MethodInfo initializeMethod = null;
             string failedClassInitializeMethodName = string.Empty;
 
-            // If class initialization is done, just return
-            if (this.IsClassInitializeExecuted)
+            // If class initialization is not done, then do it.
+            if (!this.IsClassInitializeExecuted)
             {
-                return;
-            }
-
-            // Acquiring a lock is usually a costly operation which does not need to be
-            // performed every time if the class init is already executed.
-            lock (this.testClassExecuteSyncObject)
-            {
-                // Perform a check again.
-                if (this.IsClassInitializeExecuted)
-                {
-                    return;
-                }
-
-                try
-                {
-                    // ClassInitialize methods for base classes are called in reverse order of discovery
-                    // Base -> Child TestClass
-                    var baseClassInitializeStack = new Stack<Tuple<MethodInfo, MethodInfo>>(
-                            this.BaseClassInitAndCleanupMethods.Where(p => p.Item1 != null));
-
-                    while (baseClassInitializeStack.Count > 0)
-                    {
-                        var baseInitCleanupMethods = baseClassInitializeStack.Pop();
-                        initializeMethod = baseInitCleanupMethods.Item1;
-                        initializeMethod?.InvokeAsSynchronousTask(null, testContext);
-
-                        if (baseInitCleanupMethods.Item2 != null)
-                        {
-                            this.BaseClassCleanupMethodsStack.Push(baseInitCleanupMethods.Item2);
-                        }
-                    }
-                }
-                catch (Exception ex)
-                {
-                    this.ClassInitializationException = ex;
-                    failedClassInitializeMethodName = initializeMethod.Name;
-                }
-            }
-
-            // If class initialization is not done and class initialize method is not null,
-            // and class initialization exception is null, then do it.
-            if (!this.IsClassInitializeExecuted && this.classInitializeMethod != null && this.ClassInitializationException == null)
-            {
-                // Acquiring a lock is usually a costly operation which does not need to be
+                // Aquiring a lock is usually a costly operation which does not need to be
                 // performed every time if the class init is already executed.
                 lock (this.testClassExecuteSyncObject)
                 {
@@ -301,12 +258,34 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution
                     {
                         try
                         {
-                            this.ClassInitializeMethod.InvokeAsSynchronousTask(null, testContext);
+                            // ClassInitialize methods for base classes are called in reverse order of discovery
+                            // Base -> Child TestClass
+                            var baseClassInitializeStack = new Stack<Tuple<MethodInfo, MethodInfo>>(
+                                    this.BaseClassInitAndCleanupMethods.Where(p => p.Item1 != null));
+
+                            while (baseClassInitializeStack.Count > 0)
+                            {
+                                var baseInitCleanupMethods = baseClassInitializeStack.Pop();
+                                initializeMethod = baseInitCleanupMethods.Item1;
+                                initializeMethod?.InvokeAsSynchronousTask(null, testContext);
+
+                                if (baseInitCleanupMethods.Item2 != null)
+                                {
+                                    this.BaseClassCleanupMethodsStack.Push(baseInitCleanupMethods.Item2);
+                                }
+                            }
+
+                            initializeMethod = null;
+
+                            if (this.classInitializeMethod != null)
+                            {
+                                this.ClassInitializeMethod.InvokeAsSynchronousTask(null, testContext);
+                            }
                         }
                         catch (Exception ex)
                         {
                             this.ClassInitializationException = ex;
-                            failedClassInitializeMethodName = this.ClassInitializeMethod.Name;
+                            failedClassInitializeMethodName = initializeMethod?.Name ?? this.ClassInitializeMethod.Name;
                         }
                         finally
                         {
@@ -364,7 +343,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution
                 return null;
             }
 
-            if (this.IsClassInitializeExecuted || this.ClassInitializeMethod is null || this.BaseClassCleanupMethodsStack.Any())
+            if (this.IsClassInitializeExecuted || this.ClassInitializeMethod is null)
             {
                 try
                 {

test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TestClassInfoTests.cs

@@ -17,6 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.Execution
     using Assert = FrameworkV1::Microsoft.VisualStudio.TestTools.UnitTesting.Assert;
     using StringAssert = FrameworkV1::Microsoft.VisualStudio.TestTools.UnitTesting.StringAssert;
     using TestClass = FrameworkV1::Microsoft.VisualStudio.TestTools.UnitTesting.TestClassAttribute;
+    using TestInitialize = FrameworkV1::Microsoft.VisualStudio.TestTools.UnitTesting.TestInitializeAttribute;
     using TestMethod = FrameworkV1::Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute;
     using UnitTestOutcome = Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel.UnitTestOutcome;
     using UTF = FrameworkV2::Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -57,6 +58,20 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.Execution
             this.testContext = new Mock<UTFExtension.TestContext>().Object;
         }
 
+        [TestInitialize]
+        public void TestInitialize()
+        {
+            // Prevent leaking init/cleanup methods between classes
+            DummyGrandParentTestClass.ClassInitMethodBody = null;
+            DummyGrandParentTestClass.CleanupClassMethodBody = null;
+            DummyBaseTestClass.ClassInitializeMethodBody = null;
+            DummyBaseTestClass.ClassCleanupMethodBody = null;
+            DummyDerivedTestClass.DerivedClassInitializeMethodBody = null;
+            DummyDerivedTestClass.DerivedClassCleanupMethodBody = null;
+            DummyTestClass.ClassInitializeMethodBody = null;
+            DummyTestClass.ClassCleanupMethodBody = null;
+        }
+
         [TestMethod]
         public void TestClassInfoClassAttributeGetsAReferenceToTheTestClassAttribute()
         {
@@ -265,6 +280,34 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.Execution
             Assert.IsTrue(this.testClassInfo.IsClassInitializeExecuted);
         }
 
+        [TestMethod]
+        public void RunClassInitializeShouldOnlyRunOnce()
+        {
+            var classInitCallCount = 0;
+            DummyTestClass.ClassInitializeMethodBody = (tc) => classInitCallCount++;
+            this.testClassInfo.ClassInitializeMethod = typeof(DummyTestClass).GetMethod("ClassInitializeMethod");
+
+            this.testClassInfo.RunClassInitialize(this.testContext);
+            this.testClassInfo.RunClassInitialize(this.testContext);
+
+            Assert.AreEqual(1, classInitCallCount, "Class Initialize called only once");
+        }
+
+        [TestMethod]
+        public void RunClassInitializeShouldRunOnlyOnceIfThereIsNoDerivedClassInitializeAndSetClassInitializeExecutedFlag()
+        {
+            var classInitCallCount = 0;
+            DummyBaseTestClass.ClassInitializeMethodBody = (tc) => classInitCallCount++;
+            this.testClassInfo.BaseClassInitAndCleanupMethods.Enqueue(
+                Tuple.Create(typeof(DummyBaseTestClass).GetMethod("InitBaseClassMethod"), (MethodInfo)null));
+
+            this.testClassInfo.RunClassInitialize(this.testContext);
+            Assert.IsTrue(this.testClassInfo.IsClassInitializeExecuted);
+
+            this.testClassInfo.RunClassInitialize(this.testContext);
+            Assert.AreEqual(1, classInitCallCount);
+        }
+
         [TestMethod]
         public void RunClassInitializeShouldSetClassInitializationExceptionOnException()
         {
@@ -309,8 +352,9 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.Execution
             this.testClassInfo.ClassInitializeMethod = typeof(DummyDerivedTestClass).GetMethod("InitDerivedClassMethod");
 
             this.testClassInfo.RunClassInitialize(this.testContext);
-            this.testClassInfo.RunClassInitialize(this.testContext); // this one shouldn't run
+            Assert.IsTrue(this.testClassInfo.IsClassInitializeExecuted);
 
+            this.testClassInfo.RunClassInitialize(this.testContext); // this one shouldn't run
             Assert.AreEqual(3, classInitCallCount);
         }