microsoft
Debug.Assert in test method crashes full process
Describe the bug
Debug.Assert in test method crashes full process
Steps To Reproduce
[TestClass]
public sealed class Test1
{
[TestMethod]
public void TM()
{
Debug.Assert(false);
}
}
Output:
MSTest v3.6.1 (UTC 03/10/2024) [win-x64 - .NET 9.0.0-rc.2.24473.5]
Process terminated. Assertion Failed
false
at TestProject8.Test1.TM() in C:\src\temp\TestProject8\TestProject8\Test1.cs:line 11
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions.MethodInfoExtensions.InvokeAsSynchronousTask(MethodInfo methodInfo, Object classInstance, Object[] arguments) in /_/src/Adapter/MSTest.TestAdapter/Extensions/MethodInfoExtensions.cs:line 181
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodInfo.ExecuteInternal(Object[] arguments) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs:line 252
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodInfo.Invoke(Object[] arguments) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs:line 115
at Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute.Execute(ITestMethod testMethod) in /_/src/TestFramework/TestFramework/Attributes/TestMethod/TestMethodAttribute.cs:line 39
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodRunner.ExecuteTest(TestMethodInfo testMethodInfo) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs:line 418
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodRunner.RunTestMethod() in /_/src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs:line 185
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodRunner.<>c__DisplayClass4_0.<Execute>g__SafeRunTestMethod|0(String initializationLogs, String initializationErrorLogs, String initializationTrace, String initializationTestContextMessages) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs:line 120
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodRunner.Execute(String initializationLogs, String initializationErrorLogs, String initializationTrace, String initializationTestContextMessages) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs:line 110
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.UnitTestRunner.RunSingleTest(TestMethod testMethod, IDictionary`2 testContextProperties) in /_/src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs:line 180
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestExecutionManager.ExecuteTestsWithTestRunner(IEnumerable`1 tests, ITestExecutionRecorder testExecutionRecorder, String source, IDictionary`2 sourceLevelParameters, UnitTestRunner testRunner) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestExecutionManager.cs:line 438
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestExecutionManager.ExecuteTestsInSource(IEnumerable`1 tests, IRunContext runContext, IFrameworkHandle frameworkHandle, String source, Boolean isDeploymentDone) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestExecutionManager.cs:line 394
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestExecutionManager.ExecuteTests(IEnumerable`1 tests, IRunContext runContext, IFrameworkHandle frameworkHandle, Boolean isDeploymentDone) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestExecutionManager.cs:line 173
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestExecutionManager.RunTests(IEnumerable`1 sources, IRunContext runContext, IFrameworkHandle frameworkHandle, TestRunCancellationToken cancellationToken) in /_/src/Adapter/MSTest.TestAdapter/Execution/TestExecutionManager.cs:line 149
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestExecutor.<>c__DisplayClass9_0.<RunTests>b__0(TestRunCancellationToken testRunToken) in /_/src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs:line 72
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestExecutor.<>c__DisplayClass11_0.<RunTestsFromRightContext>g__DoRunTests|0() in /_/src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs:line 126
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestExecutor.RunTestsFromRightContext(IFrameworkHandle frameworkHandle, Action`1 runTestsAction) in /_/src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs:line 115
at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestExecutor.RunTests(IEnumerable`1 sources, IRunContext runContext, IFrameworkHandle frameworkHandle) in /_/src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs:line 72
at Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBridgedTestFramework.SynchronizedRunTestsAsync(VSTestRunTestExecutionRequest request, IMessageBus messageBus, CancellationToken cancellationToken) in /_/src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestBridgedTestFramework.cs:line 56
at Microsoft.Testing.Extensions.VSTestBridge.SynchronizedSingleSessionVSTestBridgedTestFramework.<>c__DisplayClass22_0.<<ExecuteRequestAsync>b__0>d.MoveNext() in /_/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/SynchronizedSingleSessionVSTestAndTestAnywhereAdapter.cs:line 124
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Extensions.VSTestBridge.SynchronizedSingleSessionVSTestBridgedTestFramework.<>c__DisplayClass22_0.<ExecuteRequestAsync>b__0()
at Microsoft.Testing.Extensions.VSTestBridge.SynchronizedSingleSessionVSTestBridgedTestFramework.ExecuteRequestWithRequestCountGuardAsync(Func`1 asyncFunc) in /_/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/SynchronizedSingleSessionVSTestAndTestAnywhereAdapter.cs:line 145
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Extensions.VSTestBridge.SynchronizedSingleSessionVSTestBridgedTestFramework.ExecuteRequestWithRequestCountGuardAsync(Func`1 asyncFunc)
at Microsoft.Testing.Extensions.VSTestBridge.SynchronizedSingleSessionVSTestBridgedTestFramework.ExecuteRequestAsync(TestExecutionRequest request, IMessageBus messageBus, CancellationToken cancellationToken) in /_/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/SynchronizedSingleSessionVSTestAndTestAnywhereAdapter.cs:line 108
at Microsoft.Testing.Extensions.VSTestBridge.VSTestBridgedTestFrameworkBase.ExecuteRequestAsync(ExecuteRequestContext context) in /_/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/VSTestBridgedTestFrameworkBase.cs:line 72
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Extensions.VSTestBridge.VSTestBridgedTestFrameworkBase.ExecuteRequestAsync(ExecuteRequestContext context)
at Microsoft.Testing.Platform.Requests.TestHostTestFrameworkInvoker.ExecuteRequestAsync(ITestFramework testFramework, TestExecutionRequest request, IMessageBus messageBus, CancellationToken cancellationToken) in /_/src/Platform/Microsoft.Testing.Platform/Requests/TestHostTestFrameworkInvoker.cs:line 73
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Requests.TestHostTestFrameworkInvoker.ExecuteRequestAsync(ITestFramework testFramework, TestExecutionRequest request, IMessageBus messageBus, CancellationToken cancellationToken)
at Microsoft.Testing.Platform.Requests.TestHostTestFrameworkInvoker.ExecuteAsync(ITestFramework testFramework, ClientInfo client, CancellationToken cancellationToken) in /_/src/Platform/Microsoft.Testing.Platform/Requests/TestHostTestFrameworkInvoker.cs:line 62
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Requests.TestHostTestFrameworkInvoker.ExecuteAsync(ITestFramework testFramework, ClientInfo client, CancellationToken cancellationToken)
at Microsoft.Testing.Platform.Hosts.CommonTestHost.ExecuteRequestAsync(IPlatformOutputDevice outputDevice, ITestSessionContext testSessionInfo, ServiceProvider serviceProvider, BaseMessageBus baseMessageBus, ITestFramework testFramework, ClientInfo client) in /_/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs:line 111
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Hosts.CommonTestHost.ExecuteRequestAsync(IPlatformOutputDevice outputDevice, ITestSessionContext testSessionInfo, ServiceProvider serviceProvider, BaseMessageBus baseMessageBus, ITestFramework testFramework, ClientInfo client)
at Microsoft.Testing.Platform.Hosts.ConsoleTestHost.InternalRunAsync() in /_/src/Platform/Microsoft.Testing.Platform/Hosts/ConsoleTestHost.cs:line 84
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Hosts.ConsoleTestHost.InternalRunAsync()
at Microsoft.Testing.Platform.Hosts.CommonTestHost.RunTestAppAsync(CancellationToken testApplicationCancellationToken) in /_/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs:line 85
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Hosts.CommonTestHost.RunTestAppAsync(CancellationToken testApplicationCancellationToken)
at Microsoft.Testing.Platform.Hosts.CommonTestHost.RunAsync() in /_/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs:line 33
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Hosts.CommonTestHost.RunAsync()
at Microsoft.Testing.Platform.Builder.TestApplication.RunAsync() in /_/src/Platform/Microsoft.Testing.Platform/Builder/TestApplication.cs:line 240
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Microsoft.Testing.Platform.Builder.TestApplication.RunAsync()
at TestingPlatformEntryPoint.Main(String[] args) in C:\src\temp\TestProject8\TestProject8\obj\Debug\net9.0\TestPlatformEntryPoint.cs:line 16
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at TestingPlatformEntryPoint.Main(String[] args)
at TestingPlatformEntryPoint.<Main>(String[] args)
Expected behavior
No issue
Actual behavior
Process crashes
This is by design, Debug.Assert behaves like this, if you are not debugging. I don't think it is great experience (especially for tests) because either:
- you are in interactive session and you get a pop-up (especially annoying if you run in CI with interactive mode where you block tests forever), or if 1000 tests fail on the assertion and you get 1000 pop-ups
- you are in non-interactive mode => crash
- you are debugging => condition is skipped
To prevent this you need to register a trace listener and handle the error there. I did that in VSTest previously, but admittedly my solution is not great because it will translate the assert to exception that can be handled.
Instead, it would be better, if the listener threw exception, but also told the test that it must fail, even if the underlying code handles the exception.
E.g. using async local storage to pick up the state of the current test.
Just a quick note that I simplified the customer problem for the sake of the repro. In their case, the Debug.Assert call is part of the main/production code so they cannot use assertions instead.
We use Debug.Assert in our code base to validate that the desired state was reached. If not, this Debug.Assert is thrown. This is used in both tests and when running code outside of tests.
But when our debug code throws this exception during tests, the test host is killed and our tests cannot continue. In NUnit (where we are migrating from), the test would be marked as failed and other tests continue.
I don't see any mention of NUnit handling the Assert calls, do you happen to know where they do it?
Running via nunit3-console it crashes the child dotnet.exe process as well.
I think you might be referring to running NUnit tests with VSTest (e.g. with dotnet test, in VS, or with vstest.console). In those cases the solution described in the comment above is used, which will prevent the process from crashing.
But it also does not prevent the assertion exception from being handled, which I think is incorrect. The test below will pass, but it should fail.
[Test]
public void Test1()
{
try
{
Debug.Fail("OOOOOOOOOOOOOOOOh!");
}
catch
{
}
}
@Evangelink I think this needs more design to solve properly, would you be willing to mark it as feature and move it to 3.7?
To solve this properly we need to establish some context for the test that is accessible globally (or based on async local storage) and put the data about failure there. If this is done, such storage is useful for other things as well, e.g. for soft assertions.
@Liamdoult if this is a blocker for you, you can simply grab the implementation from vstest, and just use Assembly initialize to put it in place. VSTest does just that, but built-in:
// file Test1.cs
using Microsoft.VisualStudio.TestPlatform.TestHost;
using System.Diagnostics;
namespace TestProject93
{
[TestClass]
public sealed class Test1
{
[TestMethod]
public void TestMethod1()
{
Debug.Fail("OOOOOh");
}
[AssemblyInitialize]
public static void Setup(TestContext _)
{
TestHostTraceListener.Setup();
}
}
}
// file DebugAssertListener.cs
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
namespace Microsoft.VisualStudio.TestPlatform.TestHost
{
#if NETCOREAPP
using System;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
internal class TestHostTraceListener : DefaultTraceListener
{
public static void Setup()
{
// in the majority of cases there will be only a single DefaultTraceListener in this collection
// and we will replace that with our listener, in case there are listeners of different types we keep
// them as is
for (var i = 0; i < Trace.Listeners.Count; i++)
{
var listener = Trace.Listeners[i];
if (listener is DefaultTraceListener)
{
Trace.Listeners[i] = new TestHostTraceListener();
}
}
}
public override void Fail(string message)
{
throw GetException(message);
}
public override void Fail(string message, string detailMessage)
{
throw GetException((message + Environment.NewLine + detailMessage));
}
public static void ShowDialog(string stackTrace, string message, string detailMessage, string _)
{
var text = !string.IsNullOrEmpty(message)
? !string.IsNullOrEmpty(detailMessage)
? (message + Environment.NewLine + detailMessage)
: message
: null;
throw GetException(text);
}
private static DebugAssertException GetException(string message)
{
var debugTypes = new Type[] { typeof(Debug), typeof(Trace) };
var stack = new StackTrace(true);
var debugMethodFound = false;
var frameCount = 0;
MethodBase method = null;
foreach (var f in stack.GetFrames())
{
var m = f.GetMethod();
var declaringType = m?.DeclaringType;
if (!debugMethodFound && (declaringType == typeof(Debug) || declaringType == typeof(Trace)))
{
method = m;
debugMethodFound = true;
}
if (debugMethodFound)
{
frameCount++;
}
}
var stackTrace = string.Join(Environment.NewLine, stack.ToString().Split(Environment.NewLine).TakeLast(frameCount));
var methodName = method != null ? $"{method.DeclaringType.Name}.{method.Name}" : "<method>";
var wholeMessage = $"Method {methodName} failed with '{message}', and was translated to {typeof(DebugAssertException).FullName} to avoid terminating the process hosting the test.";
return new DebugAssertException(wholeMessage, stackTrace);
}
}
internal sealed class DebugAssertException : Exception
{
public DebugAssertException(string message, string stackTrace) : base(message)
{
StackTrace = stackTrace;
}
public override string StackTrace { get; }
}
#endif
}
FWIW, I just added support for this in xUnit.net after going many years without realizing that VSTest overrides the DefaultTestListener. 😁
https://github.com/xunit/xunit/issues/3220 https://github.com/xunit/xunit/commit/4c931758375b071414f70423e810b8a83c2e3221
Changing this behavior is risky IMO. Debug.Assert explicitly terminates the process so that catches don't mark such assertions.
If anything to do here, I would like us to only change the DefaultTraceListener on .NET Framework so that it does the same as in .NET Core (crashes the full process instead of showing a UI, which can cause CI to remain "stuck").
Also, consider the case where Debug.Assert is happening in a different thread.
To me crashing the process is not great, users expressed many times that this behavior is unexpected to them, I still think that this is the right approach:
Instead, it would be better, if the listener threw exception, but also told the test that it must fail, even if the underlying code handles the exception. https://github.com/microsoft/testfx/issues/3955#issuecomment-2425919151
It would work across threads as well, if we are not able to directly relate the thread to a test, then we can at least notify the test run that there was an assertion failure and fail the test run at the and, rather than killing the process.
I don't see a good way to relate the failure to a test. Using async local can be good if tests are not run in parallel. But if we are run in parallel, we might not be able to relate if the assert failure happens in a thread that was started with UnsafeStart which doesn't propagate execution context AFAIK.
Also, even when not run in parallel, the assert failure could happen at a point when we really don't have an active test. Consider Test1 started running a thread, the test completed, but the thread is still running and caused an assertion failure. We could have started running Test2 (in that case our association might not be accurate), or we couldn't have yet started running Test2, in that case we don't have an active test.
The test below will pass, but it should fail.
[Test] public void Test1() { try { Debug.Fail("OOOOOOOOOOOOOOOOh!"); } catch { } }
I was just re-browsing this issue (since the recent activity popped it back onto my radar) and I actually want to disagree with this (pardon the pun) assertion.
Having Debug.Fail translated from "crashing process" to "throwing exception" makes sense to me (it's what we chose to do), which means an unbounded, do-nothing catch should eat it. Absolutely. It's just an exception.
Imagine if you said the same thing about this code:
[Test]
public void Test1()
{
try
{
Assert.Fail("OOOOOOOOOOOOOOOOh!");
}
catch
{
}
}
What makes Debug.Fail "more important" than Assert.Fail? For better or worse, ever since NUnit was ported from JUnit, the notion that assertion failures translated to exceptions has been a fundamental part of .NET unit testing.
You can make a reasonable argument for switching from a world where "first assertion failure throws" to "all assertion failures log and you keep running". Other testing frameworks do work like this. But MSTest doesn't, nor does NUnit or xUnit.net. Trying to categorize the crash => exception translation from Debug.Fail as being a "special" kind of exception just feel too magical, too special cased.
FWIW, not a single person has ever asked me in the nearly 20 years I've been working on xUnit.net why having a blind catch stopped assertions from working.
I don't see a good way to relate the failure to a test.
We don't try. We just translate it to an exception. If it happens in the context of the test, then it turns into a failure; if it happens in the context of a worker thread, then it behaves exactly like any other exception thrown by a worker thread.
Having Debug.Fail translated from "crashing process" to "throwing exception" makes sense to me (it's what we chose to do), which means an unbounded, do-nothing catch should eat it. Absolutely. It's just an exception.
I am not a heavy user of Debug.Assert, I don't see much value in failing in different way than in production, and I also don't like how the whole feature behaves: sometimes showing pop-up you have to dismiss, ignoring the assertion when you are debugging etc.
But I think for that reason people choose to use Debug.Assert over normal exception, to show an exceptional case that bubbles up through all try catches. This is unfortunately conflicting with tests, because they exercise multiple paths through the program and so we don't want to crash the process, but would still like to know about the exceptional case.
So to me the approach above which throws exception but also notifies the test framework makes sense because it brings the best of both worlds.
I don't see a good way to relate the failure to a test. Using async local can be good if tests are not run in parallel. But if we are run in parallel, we might not be able to relate if the assert failure happens in a thread that was started with
UnsafeStartwhich doesn't propagate execution context AFAIK.Also, even when not run in parallel, the assert failure could happen at a point when we really don't have an active test. Consider
Test1started running a thread, the test completed, but the thread is still running and caused an assertion failure. We could have started runningTest2(in that case our association might not be accurate), or we couldn't have yet started runningTest2, in that case we don't have an active test.
I think that if we cannot link it to a test then the failure should be associated with the run and reported as error. It will let the other tests finish and still report the assert failure.
The thing that's curious to be about wanting to continue to run rather than throwing an exception is:
- How can you trust the rest of the test result when something crucial has failed?
- How can you write a test for that failure, unless you throw? Are you forcing people who want to monitor for that failure to temporarily swap in their own trace listener? Given that the trace listener list is a static resource, doesn't that make test parallelism more challenging once you want to do that more than once?
To be clear, none of what you decide to do here affects xUnit.net so I don't really have a horse in the race. I just wanted to share some of the thinking that went into my exception thinking.
I am not sure where I said that I want to continue without exception, if you are responding to my comment above. :)
I do want to throw an exception. And also tell the framework that there was a Debug.Assert failure, to make the "exception" bubble up, even if handled by try catch in user code. Mimicking the behavior of Debug.Assert, but without its annoying properties like showing 1000 windows that you have to dismiss one by one.
So the overall experience for the user would be:
- Debug.Assert fails. Failure is translated to exception.
- Test that does not swallow the exception fails, like it does now.
- Test the does swallow the exception will see the test code complete, but will still be marked as failed, and the assertion exception will be shown.
- If we cannot associate the failure to a test (e.g. it runs in abandoned task) we associate it with the run, and fail the run.
User will see failure when there were assertion failures, but they will not get incomplete run, because the host process won't crash on the first Debug.Assert failure.