diff --git a/src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs b/src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs index 475d51bd00..32c7570ec3 100644 --- a/src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs +++ b/src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs @@ -28,6 +28,10 @@ public class TestMethodInfo : ITestMethod /// public const int TimeoutWhenNotSet = 0; + private object? _classInstance; + private bool _isTestContextSet; + private bool _isTestCleanupInvoked; + internal TestMethodInfo( MethodInfo testMethod, TestClassInfo parent, @@ -219,33 +223,33 @@ private TestResult ExecuteInternal(object?[]? arguments) var result = new TestResult(); // TODO remove dry violation with TestMethodRunner - object? classInstance = CreateTestClassInstance(result); - bool testContextSetup = false; + _classInstance = CreateTestClassInstance(result); bool isExceptionThrown = false; bool hasTestInitializePassed = false; Exception? testRunnerException = null; + _isTestCleanupInvoked = false; try { try { - if (classInstance != null && SetTestContext(classInstance, result)) + if (_classInstance != null && SetTestContext(_classInstance, result)) { // For any failure after this point, we must run TestCleanup - testContextSetup = true; + _isTestContextSet = true; - if (RunTestInitializeMethod(classInstance, result)) + if (RunTestInitializeMethod(_classInstance, result)) { hasTestInitializePassed = true; if (IsTimeoutSet) { ExecutionContextService.RunActionOnContext( - () => TestMethod.InvokeAsSynchronousTask(classInstance, arguments), - new InstanceExecutionContextScope(classInstance, Parent.ClassType)); + () => TestMethod.InvokeAsSynchronousTask(_classInstance, arguments), + new InstanceExecutionContextScope(_classInstance, Parent.ClassType)); } else { - TestMethod.InvokeAsSynchronousTask(classInstance, arguments); + TestMethod.InvokeAsSynchronousTask(_classInstance, arguments); } result.Outcome = UTF.UnitTestOutcome.Passed; @@ -301,10 +305,7 @@ private TestResult ExecuteInternal(object?[]? arguments) // Pulling it out so extension writers can abort custom cleanups if need be. Having this in a finally block // does not allow a thread abort exception to be raised within the block but throws one after finally is executed // crashing the process. This was blocking writing an extension for Dynamic Timeout in VSO. - if (classInstance != null && testContextSetup) - { - RunTestCleanupMethod(classInstance, result); - } + RunTestCleanupMethod(result); return testRunnerException != null ? throw testRunnerException : result; } @@ -445,14 +446,18 @@ private static TestFailedException HandleMethodException(Exception ex, Exception /// /// Runs TestCleanup methods of parent TestClass and base classes. /// - /// Instance of TestClass. /// Instance of TestResult. [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")] - private void RunTestCleanupMethod(object classInstance, TestResult result) + private void RunTestCleanupMethod(TestResult result) { - DebugEx.Assert(classInstance != null, "classInstance != null"); DebugEx.Assert(result != null, "result != null"); + if (_classInstance is null || !_isTestContextSet || _isTestCleanupInvoked) + { + return; + } + + _isTestCleanupInvoked = true; MethodInfo? testCleanupMethod = Parent.TestCleanupMethod; Exception? testCleanupException; try @@ -462,22 +467,22 @@ private void RunTestCleanupMethod(object classInstance, TestResult result) // Test cleanups are called in the order of discovery // Current TestClass -> Parent -> Grandparent testCleanupException = testCleanupMethod is not null - ? InvokeCleanupMethod(testCleanupMethod, classInstance, Parent.BaseTestCleanupMethodsQueue.Count) + ? InvokeCleanupMethod(testCleanupMethod, _classInstance, Parent.BaseTestCleanupMethodsQueue.Count) : null; var baseTestCleanupQueue = new Queue(Parent.BaseTestCleanupMethodsQueue); while (baseTestCleanupQueue.Count > 0 && testCleanupException is null) { testCleanupMethod = baseTestCleanupQueue.Dequeue(); - testCleanupException = InvokeCleanupMethod(testCleanupMethod, classInstance, baseTestCleanupQueue.Count); + testCleanupException = InvokeCleanupMethod(testCleanupMethod, _classInstance, baseTestCleanupQueue.Count); } } finally { #if NET6_0_OR_GREATER // If you implement IAsyncDisposable without calling the DisposeAsync this would result a resource leak. - (classInstance as IAsyncDisposable)?.DisposeAsync().AsTask().Wait(); + (_classInstance as IAsyncDisposable)?.DisposeAsync().AsTask().Wait(); #endif - (classInstance as IDisposable)?.Dispose(); + (_classInstance as IDisposable)?.Dispose(); } } catch (Exception ex) @@ -810,24 +815,30 @@ void ExecuteAsyncAction() } DebugEx.Assert(result is not null, "result is not null"); + + // It's possible that some failures happened and that the cleanup wasn't executed, so we need to run it here. + // The method already checks if the cleanup was already executed. + RunTestCleanupMethod(result); return result; } + + // Timed out or canceled + string errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName); + if (TestMethodOptions.TestContext.Context.CancellationTokenSource.IsCancellationRequested) + { + errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Cancelled, TestMethodName); + } else { - // Timed out or canceled - string errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName); - if (TestMethodOptions.TestContext.Context.CancellationTokenSource.IsCancellationRequested) - { - errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Cancelled, TestMethodName); - } - else - { - // Cancel the token source as test has timed out - TestMethodOptions.TestContext.Context.CancellationTokenSource.Cancel(); - } - - TestResult timeoutResult = new() { Outcome = UTF.UnitTestOutcome.Timeout, TestFailureException = new TestFailedException(ObjectModelUnitTestOutcome.Timeout, errorMessage) }; - return timeoutResult; + // Cancel the token source as test has timed out + TestMethodOptions.TestContext.Context.CancellationTokenSource.Cancel(); } + + TestResult timeoutResult = new() { Outcome = UTF.UnitTestOutcome.Timeout, TestFailureException = new TestFailedException(ObjectModelUnitTestOutcome.Timeout, errorMessage) }; + + // We don't know when the cancellation happened so it's possible that the cleanup wasn't executed, so we need to run it here. + // The method already checks if the cleanup was already executed. + RunTestCleanupMethod(timeoutResult); + return timeoutResult; } }