Merge timeout behaviors for .NET Core and .NET Framework

TestFx.sln

@@ -166,8 +166,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TimeoutTestProject", "test\
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DeploymentTestProjectNetCore", "test\E2ETests\TestAssets\DeploymentTestProjectNetCore\DeploymentTestProjectNetCore.csproj", "{26F0B8EF-92D4-4A23-ACB4-D1B662F0EEBE}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TimeoutTestProjectNetCore", "test\E2ETests\TestAssets\TimeoutTestProjectNetCore\TimeoutTestProjectNetCore.csproj", "{ED27A844-6870-4FE6-8FEF-3ABDD1ED6564}"
-EndProject
 Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "FSharpTestProject", "test\E2ETests\TestAssets\FSharpTestProject\FSharpTestProject.fsproj", "{E5E58613-82FC-44CD-B75F-4F1C7ED52D0D}"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DiscoveryAndExecutionTests", "test\E2ETests\DiscoveryAndExecutionTests\DiscoveryAndExecutionTests.csproj", "{EEE57613-6424-4A1C-9635-B73768F2146D}"
@@ -325,10 +323,6 @@ Global
 		{26F0B8EF-92D4-4A23-ACB4-D1B662F0EEBE}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{26F0B8EF-92D4-4A23-ACB4-D1B662F0EEBE}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{26F0B8EF-92D4-4A23-ACB4-D1B662F0EEBE}.Release|Any CPU.Build.0 = Release|Any CPU
-		{ED27A844-6870-4FE6-8FEF-3ABDD1ED6564}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
-		{ED27A844-6870-4FE6-8FEF-3ABDD1ED6564}.Debug|Any CPU.Build.0 = Debug|Any CPU
-		{ED27A844-6870-4FE6-8FEF-3ABDD1ED6564}.Release|Any CPU.ActiveCfg = Release|Any CPU
-		{ED27A844-6870-4FE6-8FEF-3ABDD1ED6564}.Release|Any CPU.Build.0 = Release|Any CPU
 		{E5E58613-82FC-44CD-B75F-4F1C7ED52D0D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{E5E58613-82FC-44CD-B75F-4F1C7ED52D0D}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{E5E58613-82FC-44CD-B75F-4F1C7ED52D0D}.Release|Any CPU.ActiveCfg = Release|Any CPU
@@ -408,7 +402,6 @@ Global
 		{7FB80AAB-7123-4416-B6CD-8D3D69AA83F1} = {D53BD452-F69F-4FB3-8B98-386EDA28A4C8}
 		{4F0B2ACF-1341-42AF-918C-669A6D5CEA2B} = {D53BD452-F69F-4FB3-8B98-386EDA28A4C8}
 		{26F0B8EF-92D4-4A23-ACB4-D1B662F0EEBE} = {D53BD452-F69F-4FB3-8B98-386EDA28A4C8}
-		{ED27A844-6870-4FE6-8FEF-3ABDD1ED6564} = {D53BD452-F69F-4FB3-8B98-386EDA28A4C8}
 		{E5E58613-82FC-44CD-B75F-4F1C7ED52D0D} = {D53BD452-F69F-4FB3-8B98-386EDA28A4C8}
 		{EEE57613-6424-4A1C-9635-B73768F2146D} = {F1A27537-78D1-4BBD-8E76-ADB31BC0C2B4}
 		{D11C6664-1C4E-48F0-AA92-7F5BADC6F82C} = {CA01DAF5-8D9D-496E-9AD3-94BB7FBB2D34}

src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs

@@ -269,8 +269,7 @@ private TestResult ExecuteInternal(object[] arguments)
                     if (RunTestInitializeMethod(classInstance, result))
                     {
                         hasTestInitializePassed = true;
-                        PlatformServiceProvider.Instance.ThreadOperations.ExecuteWithAbortSafety(
-                            () => TestMethod.InvokeAsSynchronousTask(classInstance, arguments));
+                        TestMethod.InvokeAsSynchronousTask(classInstance, arguments);
                         result.Outcome = UTF.UnitTestOutcome.Passed;
                     }
                 }

src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IThreadOperations.cs

@@ -19,10 +19,4 @@ public interface IThreadOperations
     /// <param name="cancelToken">Token to cancel the execution</param>
     /// <returns>Returns true if the action executed before the timeout. returns false otherwise.</returns>
     bool Execute(Action action, int timeout, CancellationToken cancelToken);
-
-    /// <summary>
-    /// Execute an action with handling for Thread Aborts (if possible) so the main thread of the adapter does not die.
-    /// </summary>
-    /// <param name="action"> The action to execute. </param>
-    void ExecuteWithAbortSafety(Action action);
 }

src/Adapter/MSTestAdapter.PlatformServices/Services/ThreadOperations.cs

@@ -4,7 +4,6 @@
 namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 
 using System;
-using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -24,47 +23,9 @@ public class ThreadOperations : IThreadOperations
     /// <returns>Returns true if the action executed before the timeout. returns false otherwise.</returns>
     public bool Execute(Action action, int timeout, CancellationToken cancelToken)
     {
-#if NETFRAMEWORK
-        bool executionAborted = false;
-        Thread executionThread = new(new ThreadStart(action))
-        {
-            IsBackground = true,
-            Name = "MSTestAdapter Thread"
-        };
-
-        executionThread.SetApartmentState(Thread.CurrentThread.GetApartmentState());
-        executionThread.Start();
-        cancelToken.Register(() =>
-        {
-            executionAborted = true;
-            AbortThread(executionThread);
-        });
-
-        if (JoinThread(timeout, executionThread))
-        {
-            if (executionAborted)
-            {
-                return false;
-            }
-
-            // Successfully completed
-            return true;
-        }
-        else if (executionAborted)
-        {
-            // Execution aborted due to user choice
-            return false;
-        }
-        else
-        {
-            // Timed out
-            AbortThread(executionThread);
-            return false;
-        }
-#else
-        var executionTask = Task.Factory.StartNew(action);
         try
         {
+            var executionTask = Task.Run(action, cancelToken);
             if (executionTask.Wait(timeout, cancelToken))
             {
                 return true;
@@ -80,65 +41,5 @@ public bool Execute(Action action, int timeout, CancellationToken cancelToken)
             // Task execution canceled.
             return false;
         }
-#endif
-    }
-
-    /// <summary>
-    /// Execute an action with handling for Thread Aborts (if possible) so the main thread of the adapter does not die.
-    /// </summary>
-    /// <param name="action"> The action to execute. </param>
-    public void ExecuteWithAbortSafety(Action action)
-    {
-#if NETFRAMEWORK
-        try
-        {
-            action.Invoke();
-        }
-        catch (ThreadAbortException exception)
-        {
-            Thread.ResetAbort();
-
-            // Throwing an exception so that the test is marked as failed.
-            // This is a TargetInvocation exception because we want just the ThreadAbort exception to be shown to the user and not something we create here.
-            // TargetInvocation exceptions are stripped off by the test failure handler surfacing the actual exception.
-            throw new TargetInvocationException(exception);
-        }
-#else
-        // There is no Thread abort scenarios yet in .Net Core. Once we move the core platform service to support Thread abort related API's
-        // then this logic would be similar to the desktop platform service. UWP would then be the only diverging platform service since it does not have Thread APIs exposed.
-        action.Invoke();
-#endif
-    }
-
-#if NETFRAMEWORK
-    private static bool JoinThread(int timeout, Thread executionThread)
-    {
-        try
-        {
-            return executionThread.Join(timeout);
-        }
-        catch (ThreadStateException)
-        {
-            // Join was called on a thread not started
-        }
-
-        return false;
-    }
-
-    private static void AbortThread(Thread executionThread)
-    {
-        try
-        {
-            // Abort test thread after timeout.
-            executionThread.Abort();
-        }
-        catch (ThreadStateException)
-        {
-            // Catch and discard ThreadStateException. If Abort is called on a thread that has been suspended,
-            // a ThreadStateException is thrown in the thread that called Abort,
-            // and AbortRequested is added to the ThreadState property of the thread being aborted.
-            // A ThreadAbortException is not thrown in the suspended thread until Resume is called.
-        }
     }
-#endif
 }

test/E2ETests/Automation.CLI/CLITestBase.e2e.cs

@@ -7,12 +7,13 @@ namespace Microsoft.MSTestV2.CLIAutomation;
 using System.IO;
 using System.Linq;
 using System.Runtime.CompilerServices;
-using System.Xml;
-using global::TestFramework.ForTestingMSTest;
+
 using Microsoft.TestPlatform.VsTestConsole.TranslationLayer;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+using TestFramework.ForTestingMSTest;
+
 public partial class CLITestBase : TestContainer
 {
     private static VsTestConsoleWrapper s_vsTestConsoleWrapper;

test/E2ETests/Automation.CLI/RunEventsHandler.cs

@@ -38,7 +38,7 @@ public class RunEventsHandler : ITestRunEventsHandler
 
     public void HandleLogMessage(TestMessageLevel level, string message)
     {
-        switch ((TestMessageLevel)level)
+        switch (level)
         {
             case TestMessageLevel.Informational:
                 EqtTrace.Info(message);

test/E2ETests/Smoke.E2E.Tests/TimeoutTests.cs

@@ -3,45 +3,48 @@
 
 namespace MSTestAdapter.Smoke.E2ETests;
 
+using System.Collections.Generic;
 using System.IO;
 
 using Microsoft.MSTestV2.CLIAutomation;
 
 public class TimeoutTests : CLITestBase
 {
     private const string TimeoutTestAssembly = "TimeoutTestProject.dll";
-    private const string TimeoutTestAssemblyNetCore = "netcoreapp3.1\\TimeoutTestProjectNetCore.dll";
-    private const int TestMethodWaitTimeInMs = 6000;
-    private const int OverheadTimeInMs = 2500;
-    private const string TimeoutFileToValidateNetCore = "netcoreapp3.1\\TimeoutTestOutputNetCore.txt";
-    private const string TimeoutFileToValidate = "TimeoutTestOutput.txt";
 
-    public void ValidateTimeoutTests()
+    public void ValidateTimeoutTests_net462()
     {
-        Validate(TimeoutTestAssembly, TimeoutFileToValidate);
+        ValidateTimeoutTests("net462");
     }
 
-    public void ValidateTimeoutTestsNetCore()
+    public void ValidateTimeoutTests_netcoreapp31()
     {
-        Validate(TimeoutTestAssemblyNetCore, TimeoutFileToValidateNetCore);
+        ValidateTimeoutTests("netcoreapp3.1");
     }
 
-    private void Validate(string testAssembly, string fileToValidate)
+    private void ValidateTimeoutTests(string targetFramework)
     {
-        InvokeVsTestForExecution(new string[] { testAssembly });
+        InvokeVsTestForExecution(new string[] { targetFramework + "\\" + TimeoutTestAssembly }, testCaseFilter: "TimeoutTest|RegularTest");
 
-        ValidateTestRunTime(TestMethodWaitTimeInMs + OverheadTimeInMs);
+        ValidateFailedTestsCount(targetFramework == "net462" ? 5 : 4);
 
-        ValidateFailedTestsCount(2);
+        var failedTests = new List<string>
+        {
+            "TimeoutTestProject.TimeoutTestClass.TimeoutTest_WhenUserCancelsTestContextToken_AbortTest",
+            "TimeoutTestProject.TimeoutTestClass.RegularTest_WhenUserCancelsTestContextToken_TestContinues",
+            "TimeoutTestProject.TimeoutTestClass.TimeoutTest_WhenTimeoutReached_CancelsTestContextToken",
+            "TimeoutTestProject.TimeoutTestClass.TimeoutTest_WhenTimeoutReached_ForcesTestAbort",
+        };
 
-        ValidateFailedTestsContain(
-            testAssembly,
-            false,
-            "TimeoutTestProject.TerminateLongRunningTasksUsingTokenTestClass.TerminateLongRunningTasksUsingToken",
-            "TimeoutTestProject.SelfTerminatingTestClass.SelfTerminatingTestMethod");
-        // Unable to locate the TimeoutTestOutput.txt file.
-        Verify(File.Exists(GetAssetFullPath(fileToValidate)));
-    }
+        if (targetFramework == "net462")
+        {
+            failedTests.Add("TimeoutTestProject.TimeoutTestClass.TimeoutTest_WhenUserCallsThreadAbort_AbortTest");
+        }
+
+        ValidateFailedTestsContain(TimeoutTestAssembly, false, failedTests.ToArray());
 
-    // TODO @haplois | @evangelink: We should add netcoreapp2.1 tests here.
+        // We should find the <TargetFramework>/TimeoutTestOutput.txt file, as it's our way to validate
+        // that when the timeout expires it cancels the test context token.
+        Verify(File.Exists(GetAssetFullPath(targetFramework + "\\" + "TimeoutTestOutput.txt")));
+    }
 }