From aee3ae61dfba31e073773078ea020729d61de1f0 Mon Sep 17 00:00:00 2001 From: Nullpointer Date: Mon, 3 Jun 2024 21:22:58 +0200 Subject: [PATCH] GD-104: Fix exception failure message propagation (#108) # Why see https://github.com/MikeSchulze/gdUnit4Net/issues/106 # What fixing the extraction of inner exception to catch the right exception message --- api/src/core/execution/ExecutionStage.cs | 24 ++++----- .../core/execution/TestSuiteExecutionStage.cs | 1 - test/.runsettings | 3 ++ test/src/core/ExecutorTest.cs | 49 +++++++++++++++++++ .../TestSuiteAllTestsFailWithExceptions.cs | 30 ++++++++++++ 5 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 test/src/core/resources/testsuites/mono/TestSuiteAllTestsFailWithExceptions.cs diff --git a/api/src/core/execution/ExecutionStage.cs b/api/src/core/execution/ExecutionStage.cs index e95f39c6..ec07f3d4 100644 --- a/api/src/core/execution/ExecutionStage.cs +++ b/api/src/core/execution/ExecutionStage.cs @@ -64,7 +64,7 @@ public virtual async Task Execute(ExecutionContext context) { ReportAsFailure(context, e); } - catch (TargetInvocationException e) + catch (Exception e) { if (e.GetBaseException() is TestFailedException ex) { @@ -76,11 +76,6 @@ public virtual async Task Execute(ExecutionContext context) ReportUnexpectedException(context, e); } } - // unexpected exceptions - catch (Exception e) - { - ReportUnexpectedException(context, e); - } } private static void ReportAsFailure(ExecutionContext context, TestFailedException e) @@ -91,10 +86,17 @@ private static void ReportAsFailure(ExecutionContext context, TestFailedExceptio private static void ReportUnexpectedException(ExecutionContext context, Exception exception) { - var ei = ExceptionDispatchInfo.Capture(exception.InnerException ?? exception); - var stack = new StackTrace(ei.SourceException, true); - var lineNumber = ScanFailureLineNumber(stack); - context.ReportCollector.Consume(new TestReport(ReportType.FAILURE, lineNumber, ei.SourceException.Message, TrimStackTrace(stack.ToString()))); + if (exception is TargetInvocationException) + { + var ei = ExceptionDispatchInfo.Capture(exception.InnerException ?? exception); + ReportUnexpectedException(context, ei.SourceException); + } + else + { + var stack = new StackTrace(exception, true); + var lineNumber = ScanFailureLineNumber(stack); + context.ReportCollector.Consume(new TestReport(ReportType.FAILURE, lineNumber, exception.Message, TrimStackTrace(stack.ToString()))); + } } private static int ScanFailureLineNumber(StackTrace stack) @@ -106,7 +108,7 @@ private static int ScanFailureLineNumber(StackTrace stack) if (frame.GetMethod()?.IsDefined(typeof(TestCaseAttribute)) ?? false) return frame.GetFileLineNumber(); } - return stack.FrameCount > 1 ? stack.GetFrame(1)!.GetFileLineNumber() : -1; + return stack.FrameCount > 1 ? stack.GetFrame(0)!.GetFileLineNumber() : -1; } internal static string TrimStackTrace(string stackTrace) diff --git a/api/src/core/execution/TestSuiteExecutionStage.cs b/api/src/core/execution/TestSuiteExecutionStage.cs index 8f94ecdd..5cfba90f 100644 --- a/api/src/core/execution/TestSuiteExecutionStage.cs +++ b/api/src/core/execution/TestSuiteExecutionStage.cs @@ -1,7 +1,6 @@ namespace GdUnit4.Executions; using System.Threading.Tasks; -using System.Linq; internal sealed class TestSuiteExecutionStage : IExecutionStage { diff --git a/test/.runsettings b/test/.runsettings index e043f836..c356f8cc 100644 --- a/test/.runsettings +++ b/test/.runsettings @@ -7,6 +7,9 @@ net7.0;net8.0 300000 true + + d:\development\Godot_v4.2.1-stable_mono_win64\Godot_v4.2.1-stable_mono_win64.exe + diff --git a/test/src/core/ExecutorTest.cs b/test/src/core/ExecutorTest.cs index ac73c7ba..f237db34 100644 --- a/test/src/core/ExecutorTest.cs +++ b/test/src/core/ExecutorTest.cs @@ -728,4 +728,53 @@ public async Task ExecuteParameterizedTest() Tuple(TESTSUITE_AFTER, "After", new List()) ); } + + [TestCase(Description = "Verifies the exceptions are catches the right message as failure.")] + public async Task ExecuteTestWithExceptions() + { + var testSuite = LoadTestSuite("src/core/resources/testsuites/mono/TestSuiteAllTestsFailWithExceptions.cs"); + AssertArray(testSuite.TestCases).Extract("Name").ContainsExactly(new string[] { "ExceptionIsThrownOnSceneInvoke", "ExceptionAtAsyncMethod", "ExceptionAtSyncMethod" }); + + var events = await ExecuteTestSuite(testSuite); + + AssertTestCaseNames(events) + .ContainsExactly(ExpectedEvents("TestSuiteAllTestsFailWithExceptions", "ExceptionIsThrownOnSceneInvoke", "ExceptionAtAsyncMethod", "ExceptionAtSyncMethod")); + + // we expect all tests are failing and commits failures + AssertEventCounters(events).ContainsExactly( + Tuple(TESTSUITE_BEFORE, "Before", 0, 0, 0), + Tuple(TESTCASE_BEFORE, "ExceptionIsThrownOnSceneInvoke", 0, 0, 0), + Tuple(TESTCASE_AFTER, "ExceptionIsThrownOnSceneInvoke", 0, 1, 0), + Tuple(TESTCASE_BEFORE, "ExceptionAtAsyncMethod", 0, 0, 0), + Tuple(TESTCASE_AFTER, "ExceptionAtAsyncMethod", 0, 1, 0), + Tuple(TESTCASE_BEFORE, "ExceptionAtSyncMethod", 0, 0, 0), + Tuple(TESTCASE_AFTER, "ExceptionAtSyncMethod", 0, 1, 0), + Tuple(TESTSUITE_AFTER, "After", 0, 0, 0) + ); + AssertEventStates(events).ContainsExactly( + Tuple(TESTSUITE_BEFORE, "Before", true, false, false, false), + Tuple(TESTCASE_BEFORE, "ExceptionIsThrownOnSceneInvoke", true, false, false, false), + Tuple(TESTCASE_AFTER, "ExceptionIsThrownOnSceneInvoke", false, false, true, false), + Tuple(TESTCASE_BEFORE, "ExceptionAtAsyncMethod", true, false, false, false), + Tuple(TESTCASE_AFTER, "ExceptionAtAsyncMethod", false, false, true, false), + Tuple(TESTCASE_BEFORE, "ExceptionAtSyncMethod", true, false, false, false), + Tuple(TESTCASE_AFTER, "ExceptionAtSyncMethod", false, false, true, false), + // report suite is not success, is failed + Tuple(TESTSUITE_AFTER, "After", false, false, true, false) + ); + // check for failure reports + AssertReports(events).Contains( + Tuple(TESTSUITE_BEFORE, "Before", new List()), + Tuple(TESTCASE_AFTER, "ExceptionIsThrownOnSceneInvoke", new List() { new(FAILURE, 12, """ + Test Exception + """) }), + Tuple(TESTCASE_AFTER, "ExceptionAtAsyncMethod", new List() { new(FAILURE, 24, """ + outer exception + """) }), + Tuple(TESTCASE_AFTER, "ExceptionAtSyncMethod", new List() { new(FAILURE, 28, """ + outer exception + """) }), + Tuple(TESTSUITE_AFTER, "After", new List())); + } + } diff --git a/test/src/core/resources/testsuites/mono/TestSuiteAllTestsFailWithExceptions.cs b/test/src/core/resources/testsuites/mono/TestSuiteAllTestsFailWithExceptions.cs new file mode 100644 index 00000000..c2b2f86e --- /dev/null +++ b/test/src/core/resources/testsuites/mono/TestSuiteAllTestsFailWithExceptions.cs @@ -0,0 +1,30 @@ +namespace GdUnit4.Tests.Core; + +using System; +using System.Threading.Tasks; + + +// will be ignored because of missing `[TestSuite]` annotation +// used by executor integration test +public class TestSuiteAllTestsFailWithExceptions +{ + + [TestCase] + public void ExceptionIsThrownOnSceneInvoke() + { + var runner = ISceneRunner.Load("res://src/core/resources/scenes/TestSceneWithExceptionTest.tscn"); + + runner.Invoke("SomeMethodThatThrowsException"); + } + + [TestCase] +#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously + public async Task ExceptionAtAsyncMethod() +#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously + => throw new ArgumentException("outer exception", new ArgumentNullException("inner exception")); + + [TestCase] + public void ExceptionAtSyncMethod() + => throw new ArgumentException("outer exception", new ArgumentNullException("inner exception")); + +}