From 374097cc42ff0b28b541f78cd7ea9b695b940b04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 25 Jun 2026 03:37:42 +0200 Subject: [PATCH 1/2] Fix exception wrapping bug in BaseRunTests.RunTests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When RunTestsInternal throws, the catch block was creating: new Exception(ex.Message, ex.InnerException) This discards the original exception's type and stack trace — callers see a generic Exception with a broken inner-exception chain. The inner exception should be ex itself, not ex.InnerException. Fixes #16161 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Execution/BaseRunTests.cs | 2 +- .../Execution/BaseRunTestsTests.cs | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs index 99a570dbf3..eea2784706 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs @@ -218,7 +218,7 @@ public void RunTests() { EqtTrace.Error("BaseRunTests.RunTests: Failed to run the tests. Reason: {0}.", ex); - exception = new Exception(ex.Message, ex.InnerException); + exception = new Exception(ex.Message, ex); isAborted = true; } finally diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs index b72c2abb67..da6e6a5211 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs @@ -159,6 +159,37 @@ public void RunTestsShouldRaiseTestRunCompleteWithAbortedAsTrueOnException() Assert.IsTrue(receivedCompleteArgs.IsAborted); } + [TestMethod] + public void RunTestsShouldPreserveOriginalExceptionAsInnerException() + { + TestRunCompleteEventArgs? receivedCompleteArgs = null; + var originalException = new NotImplementedException("original message"); + + // Setup mocks. + _runTestsInstance.GetExecutorUriExtensionMapCallback = (fh, rc) => throw originalException; + _mockTestRunEventsHandler.Setup( + treh => + treh.HandleTestRunComplete( + It.IsAny(), + It.IsAny(), + It.IsAny>(), + It.IsAny>())) + .Callback( + ( + TestRunCompleteEventArgs complete, + TestRunChangedEventArgs stats, + ICollection attachments, + ICollection executorUris) => receivedCompleteArgs = complete); + + _runTestsInstance.RunTests(); + + Assert.IsNotNull(receivedCompleteArgs); + Assert.IsTrue(receivedCompleteArgs.IsAborted); + Assert.IsNotNull(receivedCompleteArgs.Error); + Assert.AreSame(originalException, receivedCompleteArgs.Error!.InnerException, + "The original exception should be preserved as the inner exception of the wrapper."); + } + [TestMethod] public void RunTestsShouldNotThrowIfExceptionIsAFileNotFoundException() { From c249f42493e48e477218d4f60152affec670bf28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Fri, 26 Jun 2026 10:26:43 +0200 Subject: [PATCH 2/2] Unwrap TargetInvocationException to the real exception on run abort RunTestsInternal can surface a TargetInvocationException when a test extension is created via reflection and its constructor throws. Surfacing the reflection wrapper is unhelpful, so unwrap it to the inner exception. Every other exception is preserved as-is, keeping its concrete type and stack trace (the original bug this PR fixes). Added a test for the unwrap path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Execution/BaseRunTests.cs | 10 +++++- .../Execution/BaseRunTestsTests.cs | 36 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs index eea2784706..9e640745d7 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/BaseRunTests.cs @@ -9,6 +9,7 @@ using System.Globalization; using System.IO; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.VisualStudio.TestPlatform.Common.ExtensionDecorators; @@ -218,7 +219,14 @@ public void RunTests() { EqtTrace.Error("BaseRunTests.RunTests: Failed to run the tests. Reason: {0}.", ex); - exception = new Exception(ex.Message, ex); + // RunTestsInternal can surface a TargetInvocationException when a test extension is + // instantiated via reflection and its constructor throws. Unwrap that wrapper to the + // real exception so callers don't see the reflection noise. Any other exception is + // preserved as-is so its concrete type and stack trace are not lost on the way out. + Exception realException = ex is TargetInvocationException tie && tie.InnerException is not null + ? tie.InnerException + : ex; + exception = new Exception(realException.Message, realException); isAborted = true; } finally diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs index da6e6a5211..bf6a959eac 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs @@ -7,6 +7,7 @@ using System.Globalization; using System.IO; using System.Linq; +using System.Reflection; using Microsoft.TestPlatform.TestUtilities; using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework; @@ -190,6 +191,41 @@ public void RunTestsShouldPreserveOriginalExceptionAsInnerException() "The original exception should be preserved as the inner exception of the wrapper."); } + [TestMethod] + public void RunTestsShouldUnwrapTargetInvocationExceptionToTheRealException() + { + TestRunCompleteEventArgs? receivedCompleteArgs = null; + var realException = new NotImplementedException("real message"); + + // A TargetInvocationException is what reflection-based extension instantiation throws when + // a constructor fails; the wrapper itself is noise, the real exception is the inner one. + var reflectionWrapper = new TargetInvocationException(realException); + + // Setup mocks. + _runTestsInstance.GetExecutorUriExtensionMapCallback = (fh, rc) => throw reflectionWrapper; + _mockTestRunEventsHandler.Setup( + treh => + treh.HandleTestRunComplete( + It.IsAny(), + It.IsAny(), + It.IsAny>(), + It.IsAny>())) + .Callback( + ( + TestRunCompleteEventArgs complete, + TestRunChangedEventArgs stats, + ICollection attachments, + ICollection executorUris) => receivedCompleteArgs = complete); + + _runTestsInstance.RunTests(); + + Assert.IsNotNull(receivedCompleteArgs); + Assert.IsTrue(receivedCompleteArgs.IsAborted); + Assert.IsNotNull(receivedCompleteArgs.Error); + Assert.AreSame(realException, receivedCompleteArgs.Error!.InnerException, + "A TargetInvocationException should be unwrapped to its real inner exception, not surfaced as-is."); + } + [TestMethod] public void RunTestsShouldNotThrowIfExceptionIsAFileNotFoundException() {