From efebd7f29dd2ca4dd481c17e73758ed28de06a4f Mon Sep 17 00:00:00 2001 From: Nolan Glore Date: Fri, 5 Jun 2020 00:53:27 -0700 Subject: [PATCH] Set IsClassInitializeExecuted=true after base class init to avoid repeated class init calls (#705) --- .../Execution/TestClassInfo.cs | 77 +++++++------------ .../Execution/TestClassInfoTests.cs | 46 ++++++++++- 2 files changed, 73 insertions(+), 50 deletions(-) diff --git a/src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs b/src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs index 59fa2f8dd7..433f4a8a68 100644 --- a/src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs +++ b/src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs @@ -246,53 +246,10 @@ public void RunClassInitialize(TestContext testContext) 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>( - 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 @@ public void RunClassInitialize(TestContext testContext) { 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>( + 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 @@ public string RunClassCleanup() return null; } - if (this.IsClassInitializeExecuted || this.ClassInitializeMethod is null || this.BaseClassCleanupMethodsStack.Any()) + if (this.IsClassInitializeExecuted || this.ClassInitializeMethod is null) { try { diff --git a/test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TestClassInfoTests.cs b/test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TestClassInfoTests.cs index 4ed8e3faed..aff4f3c6e6 100644 --- a/test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Execution/TestClassInfoTests.cs +++ b/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 @@ public TestClassInfoTests() this.testContext = new Mock().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 @@ public void RunClassInitializeShouldSetClassInitializeExecutedFlag() 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 @@ public void RunClassInitializeShouldNotExecuteBaseClassInitializeMethodIfClassIn 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); }