Rework assert.multiple behavior under debugger to throw on each failure (nunit#4749)

* rework assert.multiple behavior under debugger to throw on each failure

* address CR

* copy ThrowOnEachFailureUnderDebugger to child context

Author: MaceWindu

src/NUnitFramework/framework/Api/FrameworkPackageSettings.cs

@@ -88,9 +88,9 @@ public static class FrameworkPackageSettings
         public const string StopOnError = "StopOnError";
 
         /// <summary>
-        /// If true, asserts in multiple asserts block will throw immediately.
+        /// If true, asserts in multiple asserts block will throw first-chance exception on failure.
         /// </summary>
-        public const string DisableMultipleAssertsUnderDebugger = "DisableMultipleAssertsUnderDebugger";
+        public const string ThrowOnEachFailureUnderDebugger = "ThrowOnEachFailureUnderDebugger";
 
         /// <summary>
         /// If true, use of the event queue is suppressed and test events are synchronous.

src/NUnitFramework/framework/Api/NUnitTestAssemblyRunner.cs

@@ -317,8 +317,8 @@ private TestExecutionContext CreateTestExecutionContext(ITest loadedTest, ITestL
                 context.CurrentUICulture = new CultureInfo((string)uiCulture, false);
             if (Settings.TryGetValue(FrameworkPackageSettings.StopOnError, out var stopOnError))
                 context.StopOnError = (bool)stopOnError;
-            if (Settings.TryGetValue(FrameworkPackageSettings.DisableMultipleAssertsUnderDebugger, out var disableMultiple))
-                context.DisableMultipleAssertsUnderDebugger = (bool)disableMultiple;
+            if (Settings.TryGetValue(FrameworkPackageSettings.ThrowOnEachFailureUnderDebugger, out var throwOnEachFailure))
+                context.ThrowOnEachFailureUnderDebugger = (bool)throwOnEachFailure;
 
             // Apply attributes to the context
 

src/NUnitFramework/framework/Assert.cs

@@ -320,9 +320,22 @@ private static void ReportFailure(string message)
             result.RecordAssertion(AssertionStatus.Failed, message, GetStackTrace());
             result.RecordTestCompletion();
 
-            // If we are outside any multiple assert block or multiple asserts disabled, then throw
-            if (TestExecutionContext.CurrentContext.MultipleAssertLevel == 0 || (TestExecutionContext.CurrentContext.DisableMultipleAssertsUnderDebugger && Debugger.IsAttached))
+            // If multiple asserts disabled, then throw
+            if (TestExecutionContext.CurrentContext.MultipleAssertLevel == 0)
+            {
                 throw new AssertionException(result.Message);
+            }
+            else if (TestExecutionContext.CurrentContext.ThrowOnEachFailureUnderDebugger && Debugger.IsAttached)
+            {
+                try
+                {
+                    throw new AssertionException(result.Message);
+                }
+                catch (AssertionException)
+                {
+                    // we catch exception for multiple assert block to not change observed behavior but still allow user to break into debugger
+                }
+            }
         }
 
         private static void IssueWarning(string message)

src/NUnitFramework/framework/Internal/TestExecutionContext.cs

@@ -108,6 +108,7 @@ public TestExecutionContext(TestExecutionContext other)
             _listener = other._listener;
             StopOnError = other.StopOnError;
             TestCaseTimeout = other.TestCaseTimeout;
+            ThrowOnEachFailureUnderDebugger = other.ThrowOnEachFailureUnderDebugger;
             UseCancellation = other.UseCancellation;
             CancellationToken = other.CancellationToken;
             UpstreamActions = new List<ITestAction>(other.UpstreamActions);
@@ -314,9 +315,9 @@ public Randomizer RandomGenerator
         internal int MultipleAssertLevel { get; set; }
 
         /// <summary>
-        /// Gets or sets wether asserts in multiple assert block should throw immediately under debugger.
+        /// Gets or sets wether asserts in multiple assert block should throw first-change exceptions under debugger.
         /// </summary>
-        internal bool DisableMultipleAssertsUnderDebugger { get; set; }
+        internal bool ThrowOnEachFailureUnderDebugger { get; set; }
 
         /// <summary>
         /// Gets or sets the test case timeout value

src/NUnitFramework/tests/Internal/TestExecutionContextTests.cs

@@ -660,18 +660,18 @@ public async Task CanAccessTestCaseTimeout_Async()
         [Test]
         public void CanAccessDisableMultipleAssertsUnderDebugger()
         {
-            var value = _fixtureContext.DisableMultipleAssertsUnderDebugger;
-            Assert.That(_setupContext.DisableMultipleAssertsUnderDebugger, Is.EqualTo(value));
-            Assert.That(TestExecutionContext.CurrentContext.DisableMultipleAssertsUnderDebugger, Is.EqualTo(value));
+            var value = _fixtureContext.ThrowOnEachFailureUnderDebugger;
+            Assert.That(_setupContext.ThrowOnEachFailureUnderDebugger, Is.EqualTo(value));
+            Assert.That(TestExecutionContext.CurrentContext.ThrowOnEachFailureUnderDebugger, Is.EqualTo(value));
         }
 
         [Test]
         public async Task CanAccessDisableMultipleAssertsUnderDebugger_Async()
         {
-            var value = TestExecutionContext.CurrentContext.DisableMultipleAssertsUnderDebugger;
-            Assert.That(value, Is.EqualTo(_setupContext.DisableMultipleAssertsUnderDebugger));
+            var value = TestExecutionContext.CurrentContext.ThrowOnEachFailureUnderDebugger;
+            Assert.That(value, Is.EqualTo(_setupContext.ThrowOnEachFailureUnderDebugger));
             await YieldAsync();
-            Assert.That(TestExecutionContext.CurrentContext.DisableMultipleAssertsUnderDebugger, Is.EqualTo(value));
+            Assert.That(TestExecutionContext.CurrentContext.ThrowOnEachFailureUnderDebugger, Is.EqualTo(value));
         }
 
         #endregion