Issue4658 timeout

.editorconfig

@@ -161,6 +161,11 @@ dotnet_diagnostic.IDE0044.severity = warning
 # IDE0055: Fix formatting  (Skipping this as it doesnt say WHAT is wrong. Rely on StyleCop instead)
 dotnet_diagnostic.IDE0055.severity = none
 
+# IDE0079: Remove unnecessary suppression
+# This rule implementation is flaky. It claims it can be removed,
+# but when the file is actually opened. The violation goes away.
+dotnet_diagnostic.IDE0079.severity = none
+
 # IDE1006: Naming Styles
 dotnet_diagnostic.IDE1006.severity = warning
 
@@ -177,3 +182,10 @@ dotnet_diagnostic.CSIsNull001.severity = warning
 
 # CSIsNull002: Use `is object` for non-null checks
 dotnet_diagnostic.CSIsNull002.severity = warning
+
+##################################################################################
+# NET8 (or later) Analyzers
+
+# SYSLIB1045: Convert to 'GeneratedRegexAttribute'.
+
+dotnet_diagnostic.SYSLIB1045.severity = silent

src/NUnitFramework/Directory.Build.props

@@ -7,13 +7,13 @@
     <AssemblyOriginatorKeyFile>..\..\nunit.snk</AssemblyOriginatorKeyFile>
     <DisableImplicitNuGetFallbackFolder>true</DisableImplicitNuGetFallbackFolder>
     <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
-    <NUnitLibraryFrameworks>net462;net6.0</NUnitLibraryFrameworks>
+    <NUnitLibraryFrameworks>net462;net6.0;net8.0</NUnitLibraryFrameworks>
     <NUnitRuntimeFrameworks>net462;net6.0;net8.0</NUnitRuntimeFrameworks>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
     <!--<OutputPath>..\..\..\bin\$(Configuration)\</OutputPath>-->
     <CheckEolTargetFramework>false</CheckEolTargetFramework>
     <Nullable>enable</Nullable>
-    <AnnotatedReferenceAssemblyVersion>7.0.0</AnnotatedReferenceAssemblyVersion>
+    <AnnotatedReferenceAssemblyVersion>8.0.0</AnnotatedReferenceAssemblyVersion>
     <GenerateNullableAttributes>false</GenerateNullableAttributes>
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
   </PropertyGroup>
@@ -24,7 +24,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <DefineConstants Condition="$(TargetFramework.StartsWith('net4'))">$(DefineConstants);THREAD_ABORT</DefineConstants>
+    <DefineConstants Condition="!$(TargetFramework.StartsWith('net6'))">$(DefineConstants);THREAD_ABORT</DefineConstants>
   </PropertyGroup>
 
   <!-- We always want a good debugging experience in tests -->

src/NUnitFramework/framework/Attributes/TimeoutAttribute.cs

@@ -12,8 +12,8 @@ namespace NUnit.Framework
     /// When applied to a class or assembly, the default timeout is set for all contained test methods.
     /// </summary>
     [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Assembly, AllowMultiple = false, Inherited = false)]
-#if !NETFRAMEWORK
-    [Obsolete(".NET No longer supports aborting threads as it is not a safe thing to do. Update your tests to use CancelAfterAttribute instead")]
+#if !THREAD_ABORT
+    [Obsolete("Current SDK does not supports aborting threads as it is not a safe thing to do. Update your tests to use CancelAfterAttribute instead")]
 #endif
     public class TimeoutAttribute : PropertyAttribute, IApplyToContext
     {

src/NUnitFramework/framework/Exceptions/AssertionException.cs

@@ -26,14 +26,6 @@ public AssertionException(string message, Exception? inner) : base(message, inne
         {
         }
 
-        /// <summary>
-        /// Serialization Constructor
-        /// </summary>
-        protected AssertionException(System.Runtime.Serialization.SerializationInfo info,
-            System.Runtime.Serialization.StreamingContext context) : base(info, context)
-        {
-        }
-
         /// <summary>
         /// Gets the ResultState provided by this exception
         /// </summary>

src/NUnitFramework/framework/Exceptions/IgnoreException.cs

@@ -25,14 +25,6 @@ public IgnoreException(string message, Exception? inner) : base(message, inner)
         {
         }
 
-        /// <summary>
-        /// Serialization Constructor
-        /// </summary>
-        protected IgnoreException(System.Runtime.Serialization.SerializationInfo info,
-            System.Runtime.Serialization.StreamingContext context) : base(info, context)
-        {
-        }
-
         /// <summary>
         /// Gets the ResultState provided by this exception
         /// </summary>

src/NUnitFramework/framework/Exceptions/InconclusiveException.cs

@@ -28,15 +28,6 @@ public InconclusiveException(string message, Exception? inner)
         {
         }
 
-        /// <summary>
-        /// Serialization Constructor
-        /// </summary>
-        protected InconclusiveException(System.Runtime.Serialization.SerializationInfo info,
-            System.Runtime.Serialization.StreamingContext context)
-            : base(info, context)
-        {
-        }
-
         /// <summary>
         /// Gets the ResultState provided by this exception
         /// </summary>

src/NUnitFramework/framework/Exceptions/MultipleAssertException.cs

@@ -28,16 +28,6 @@ public MultipleAssertException(ITestResult testResult)
             TestResult = testResult;
         }
 
-#pragma warning disable CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable.
-        /// <summary>
-        /// Serialization Constructor
-        /// </summary>
-        protected MultipleAssertException(System.Runtime.Serialization.SerializationInfo info,
-            System.Runtime.Serialization.StreamingContext context) : base(info, context)
-        {
-        }
-#pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable.
-
         /// <summary>
         /// Gets the <see cref="ResultState"/> provided by this exception.
         /// </summary>

src/NUnitFramework/framework/Exceptions/ResultStateException.cs

@@ -26,14 +26,6 @@ public ResultStateException(string message, Exception? inner) : base(message, in
         {
         }
 
-        /// <summary>
-        /// Serialization Constructor
-        /// </summary>
-        protected ResultStateException(System.Runtime.Serialization.SerializationInfo info,
-            System.Runtime.Serialization.StreamingContext context) : base(info, context)
-        {
-        }
-
         /// <summary>
         /// Gets the ResultState provided by this exception
         /// </summary>

src/NUnitFramework/framework/Exceptions/SuccessException.cs

@@ -27,15 +27,6 @@ public SuccessException(string message, Exception? inner)
         {
         }
 
-        /// <summary>
-        /// Serialization Constructor
-        /// </summary>
-        protected SuccessException(System.Runtime.Serialization.SerializationInfo info,
-            System.Runtime.Serialization.StreamingContext context)
-            : base(info, context)
-        {
-        }
-
         /// <summary>
         /// Gets the ResultState provided by this exception
         /// </summary>

src/NUnitFramework/framework/Internal/Commands/AfterTestActionCommand.cs

@@ -16,7 +16,7 @@ public class AfterTestActionCommand : AfterTestCommand
         public AfterTestActionCommand(TestCommand innerCommand, TestActionItem action)
             : base(innerCommand)
         {
-            Guard.ArgumentValid(innerCommand.Test is TestSuite, "BeforeTestActionCommand may only apply to a TestSuite", nameof(innerCommand));
+            Guard.ArgumentValid(innerCommand.Test is TestSuite, "AfterTestActionCommand may only apply to a TestSuite", nameof(innerCommand));
             Guard.ArgumentNotNull(action, nameof(action));
 
             AfterTest = context =>

src/NUnitFramework/framework/Internal/Commands/AfterTestCommand.cs

@@ -29,7 +29,10 @@ public override TestResult Execute(TestExecutionContext context)
                 context.CurrentResult = innerCommand.Execute(context);
             });
 
-            AfterTest(context);
+            RunTestMethodInThreadAbortSafeZone(context, () =>
+            {
+                AfterTest(context);
+            });
 
             return context.CurrentResult;
         }

src/NUnitFramework/framework/Internal/Commands/CancelAfterCommand.cs

@@ -17,7 +17,7 @@ public class CancelAfterCommand : DelegatingTestCommand
         private readonly IDebugger _debugger;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="TimeoutCommand"/> class.
+        /// Initializes a new instance of the <see cref="CancelAfterCommand"/> class.
         /// </summary>
         /// <param name="innerCommand">The inner command</param>
         /// <param name="timeout">Timeout value</param>
@@ -32,11 +32,7 @@ internal CancelAfterCommand(TestCommand innerCommand, int timeout, IDebugger deb
             Guard.ArgumentNotNull(debugger, nameof(debugger));
         }
 
-        /// <summary>
-        /// Runs the test, saving a TestResult in the supplied TestExecutionContext.
-        /// </summary>
-        /// <param name="context">The context in which the test should run.</param>
-        /// <returns>A TestResult</returns>
+        /// <inheritdoc/>
         public override TestResult Execute(TestExecutionContext context)
         {
             // Because of the debugger possibly attaching after the test method is started
@@ -54,7 +50,7 @@ public override TestResult Execute(TestExecutionContext context)
 
             try
             {
-                innerCommand.Execute(context);
+                ExecuteInnerCommand(context);
             }
             catch (OperationCanceledException ex)
             {
@@ -67,5 +63,14 @@ public override TestResult Execute(TestExecutionContext context)
 
             return context.CurrentResult;
         }
+
+        /// <summary>
+        /// Execute the 'inner command' using the <paramref name="context"/>.
+        /// </summary>
+        /// <param name="context">The TestExecutionContext to be used for running the test.</param>
+        protected virtual void ExecuteInnerCommand(TestExecutionContext context)
+        {
+            innerCommand.Execute(context);
+        }
     }
 }

src/NUnitFramework/framework/Internal/Commands/DelegatingTestCommand.cs

@@ -1,9 +1,6 @@
 // Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
 
 using System;
-#if THREAD_ABORT
-using System.Threading;
-#endif
 
 namespace NUnit.Framework.Internal.Commands
 {
@@ -42,10 +39,6 @@ protected static void RunTestMethodInThreadAbortSafeZone(TestExecutionContext co
             }
             catch (Exception ex)
             {
-#if THREAD_ABORT
-                if (ex is ThreadAbortException)
-                    Thread.ResetAbort();
-#endif
                 context.CurrentResult.RecordException(ex);
             }
         }

src/NUnitFramework/framework/Internal/Commands/TimeoutCommand.cs

@@ -1,130 +1,29 @@
 // Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
 
-#if THREAD_ABORT
-using System.Threading;
-#else
-using System;
-using System.Threading.Tasks;
-#endif
-using NUnit.Framework.Interfaces;
 using NUnit.Framework.Internal.Abstractions;
 
 namespace NUnit.Framework.Internal.Commands
 {
     /// <summary>
-    /// <see cref="TimeoutCommand"/> creates a timer in order to cancel
-    /// a test if it exceeds a specified time and adjusts
-    /// the test result if it did time out.
+    /// Unlike <see cref="CancelAfterCommand"/> this command will try to Abort the current executing Thread after the timeout.
     /// </summary>
-    public class TimeoutCommand : BeforeAndAfterTestCommand
+    public class TimeoutCommand : CancelAfterCommand
     {
-        private readonly int _timeout;
-        private readonly IDebugger _debugger;
-#if THREAD_ABORT
-        private Timer? _commandTimer;
-        private bool _commandTimedOut;
-#endif
-
         /// <summary>
         /// Initializes a new instance of the <see cref="TimeoutCommand"/> class.
         /// </summary>
         /// <param name="innerCommand">The inner command</param>
         /// <param name="timeout">Timeout value</param>
         /// <param name="debugger">An <see cref="IDebugger" /> instance</param>
-        internal TimeoutCommand(TestCommand innerCommand, int timeout, IDebugger debugger) : base(innerCommand)
+        internal TimeoutCommand(TestCommand innerCommand, int timeout, IDebugger debugger)
+            : base(innerCommand, timeout, debugger)
         {
-            _timeout = timeout;
-            _debugger = debugger;
-
-            Guard.ArgumentValid(innerCommand.Test is TestMethod, "TimeoutCommand may only apply to a TestMethod", nameof(innerCommand));
-            Guard.ArgumentValid(timeout > 0, "Timeout value must be greater than zero", nameof(timeout));
-            Guard.ArgumentNotNull(debugger, nameof(debugger));
-
-#if THREAD_ABORT
-            BeforeTest = _ =>
-            {
-                var testThread = Thread.CurrentThread;
-                var nativeThreadId = ThreadUtility.GetCurrentThreadNativeId();
-
-                // Create a timer to cancel the current thread
-                _commandTimer = new Timer(
-                    o =>
-                    {
-                        if (_debugger.IsAttached)
-                        {
-                            return;
-                        }
-
-                        _commandTimedOut = true;
-                        ThreadUtility.Abort(testThread, nativeThreadId);
-                        // No join here, since the thread doesn't really terminate
-                    },
-                    null,
-                    timeout,
-                    Timeout.Infinite);
-            };
-
-            AfterTest = (context) =>
-            {
-                _commandTimer?.Dispose();
-
-                // If the timer cancelled the current thread, change the result
-                if (_commandTimedOut)
-                {
-                    var message = $"Test exceeded Timeout value of {timeout}ms";
-
-                    context.CurrentResult.SetResult(
-                        ResultState.Failure,
-                        message);
-                }
-            };
-#else
-            BeforeTest = _ => { };
-            AfterTest = _ => { };
-#endif
-        }
-
-#if !THREAD_ABORT
-        /// <summary>
-        /// Runs the test, saving a TestResult in the supplied TestExecutionContext.
-        /// </summary>
-        /// <param name="context">The context in which the test should run.</param>
-        /// <returns>A TestResult</returns>
-        public override TestResult Execute(TestExecutionContext context)
-        {
-            try
-            {
-                var testExecution = RunTestOnSeparateThread(context);
-                if (Task.WaitAny(new Task[] { testExecution }, _timeout) != -1
-                    || _debugger.IsAttached)
-                {
-                    context.CurrentResult = testExecution.GetAwaiter().GetResult();
-                }
-                else
-                {
-                    string message = $"Test exceeded Timeout value of {_timeout}ms";
-
-                    context.CurrentResult.SetResult(
-                        ResultState.Failure,
-                        message);
-                }
-            }
-            catch (Exception exception)
-            {
-                context.CurrentResult.RecordException(exception, FailureSite.Test);
-            }
-
-            return context.CurrentResult;
         }
 
-        private Task<TestResult> RunTestOnSeparateThread(TestExecutionContext context)
+        /// <inheritdoc/>
+        protected override void ExecuteInnerCommand(TestExecutionContext context)
         {
-            var separateContext = new TestExecutionContext(context)
-            {
-                CurrentResult = context.CurrentTest.MakeTestResult()
-            };
-            return Task.Run(() => innerCommand.Execute(separateContext));
+            NUnitControlledExecution.Run(() => base.ExecuteInnerCommand(context), context.CancellationToken);
         }
-#endif
     }
 }