From 600624b65838dc3af12902f1215a66fe0f5c132a Mon Sep 17 00:00:00 2001 From: Evangelink Date: Tue, 28 Jul 2026 09:56:57 +0200 Subject: [PATCH 1/3] Include exception message in JUnit failure/error bodies .NET's Exception.StackTrace omits the leading ype: message header that Java's Throwable.printStackTrace() (and Exception.ToString()) include, so writing the stack trace alone into the / body dropped the single most useful piece of diagnostic information. Consumers that render the body rather than the message attribute -- GitLab CI and CircleCI most notably -- showed only a stack trace with no indication of why a test failed. The body now mirrors printStackTrace() shape, degrading gracefully when the exception type, message or stack trace is absent. The message and ype attributes are unchanged, so consumers reading them directly are unaffected. Also always emit the ype attribute. The Ant/windyroad JUnit.xsd marks it use="required" (Surefire relaxes it to optional), but MTP only supplies an exception type when the state property carried an actual Exception -- frameworks reporting via Explanation alone leave it null. Fall back to the element name so the document stays valid under the stricter schema. Fixes #10269 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a565c557-8c36-4279-ab7b-41311abcd836 --- docs/RFCs/016-JUnit-Report.md | 30 ++- .../InternalAPI/InternalAPI.Unshipped.txt | 1 + .../JUnitXmlWriter.cs | 59 ++++- .../JUnitReportTests.cs | 6 +- .../JUnitReportFailureBodyTests.cs | 204 ++++++++++++++++++ 5 files changed, 288 insertions(+), 12 deletions(-) create mode 100644 test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs diff --git a/docs/RFCs/016-JUnit-Report.md b/docs/RFCs/016-JUnit-Report.md index 3ecd5f5449..d63bcb0895 100644 --- a/docs/RFCs/016-JUnit-Report.md +++ b/docs/RFCs/016-JUnit-Report.md @@ -131,16 +131,37 @@ The root `` `name` attribute is the module file name without extensi | ---------------------------------------------- | ---------------------------------------------- | | `PassedTestNodeStateProperty` | *(no child element)* | | `SkippedTestNodeStateProperty` | `` + reason as text | -| `FailedTestNodeStateProperty` | `` + stack | -| `TimeoutTestNodeStateProperty` | `` + stack | -| `ErrorTestNodeStateProperty` | `` + stack | -| `CancelledTestNodeStateProperty` *(obsolete)* | `` + stack | +| `FailedTestNodeStateProperty` | `` + body | +| `TimeoutTestNodeStateProperty` | `` + body | +| `ErrorTestNodeStateProperty` | `` + body | +| `CancelledTestNodeStateProperty` *(obsolete)* | `` + body | | Other `WellKnownTestNodeTestRunOutcomeFailedProperties` | `` | | `DiscoveredTestNodeStateProperty` | *(filtered out — not emitted)* | | `InProgressTestNodeStateProperty` | *(filtered out — not emitted)* | `Cancelled` becomes `` rather than `` because cancellation indicates an interruption, not an assertion failure — `` is the schema-correct bucket for "the test could not be evaluated". +### Failure and error body format + +The body of ``/`` mirrors the shape Java's `Throwable.printStackTrace()` produces, which is what genuine Surefire reports contain: + +```text +System.InvalidOperationException: The expected condition was not met. + at MyProject.MyTests.MyTest() +``` + +That is, the exception type and message form a header line, followed by the stack trace. Neither the Ant/windyroad `JUnit.xsd` nor Surefire's `surefire-test-report.xsd` constrains this body — both declare it as free-form text — so the header line is schema-valid in every flavor. + +Writing the stack trace alone would **not** be equivalent to Java's output: .NET's `Exception.StackTrace` omits the leading `type: message` header that `Throwable.printStackTrace()` (and `Exception.ToString()`) include. Consumers that render the body rather than the `message` attribute — GitLab CI and CircleCI most notably — would then show a stack trace with no indication of *why* the test failed. This is especially damaging for fluent assertion libraries, whose stack traces are largely framework frames while the assertion message carries the actual diagnosis. + +The `message` and `type` attributes are still written, so consumers that read them directly are unaffected. The resulting duplication between the `message` attribute and the body is exactly what every Maven/Surefire report already exhibits, so consumers that render both have long handled it. + +Each part degrades gracefully: a missing exception type drops the header prefix, a missing message drops the `: ` separator, and a missing stack trace yields a header-only body. + +### The `type` attribute + +`type` is **always** emitted on ``/``. The Ant/windyroad `JUnit.xsd` marks it `use="required"` (Surefire relaxes it to optional), but MTP only supplies an exception type when the state property carried an actual `Exception` — frameworks that report a failure through `Explanation` alone leave it null. In that case the element name (`failure` or `error`) is used as the value, keeping the document valid under the stricter schema without inventing a bogus exception type name. + ## Per-testcase metadata The `` block (emitted **first** inside ``) carries: @@ -278,3 +299,4 @@ A fresh GUID is generated for the `` attribu - Parent-chain resolution with missing intermediate parents. - Counters at the suite and root level. - Outcome mapping (skipped/failed/error/timeout/cancelled). + - Failure/error body composition across every combination of present/absent exception type, message and stack trace, plus the `type` attribute fallback to the element name. diff --git a/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt index e79a69fb9d..3adb1ae467 100644 --- a/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt @@ -11,3 +11,4 @@ Microsoft.Testing.Extensions.MergeOutputFileHelper static Microsoft.Testing.Extensions.MergeOutputFileHelper.BuildCaseFoldedProbePath(string! directory, string! probeFileName) -> string! static Microsoft.Testing.Extensions.MergeOutputFileHelper.EnsureOutputDoesNotAliasInput(System.Collections.Generic.IReadOnlyList! inputPaths, string! outputPath) -> void static Microsoft.Testing.Extensions.MergeOutputFileHelper.WriteViaTemporarySiblingAsync(string! outputPath, System.Func! writeToTempAsync) -> System.Threading.Tasks.Task! +static Microsoft.Testing.Extensions.JUnitReport.JUnitXmlWriter.BuildFailureBody(Microsoft.Testing.Extensions.JUnitReport.CapturedTestResult! result) -> string? diff --git a/src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitXmlWriter.cs b/src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitXmlWriter.cs index b0dd7e5f4b..e1e775e46a 100644 --- a/src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitXmlWriter.cs +++ b/src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitXmlWriter.cs @@ -221,17 +221,66 @@ private static async Task WriteFailureOrErrorAsync(XmlWriter writer, string elem WriteAttribute(writer, "message", result.ErrorMessage!); } - if (!RoslynString.IsNullOrEmpty(result.ExceptionType)) + // The Ant/windyroad JUnit.xsd marks `type` as use="required" (Surefire relaxes it to + // optional). MTP only gives us an exception type when the state property carried an + // actual Exception; frameworks that report a failure through `Explanation` alone leave + // it null. Fall back to the element name so the document stays valid under the stricter + // schema without inventing a bogus exception type name. + WriteAttribute(writer, "type", RoslynString.IsNullOrEmpty(result.ExceptionType) ? elementName : result.ExceptionType!); + + string? body = BuildFailureBody(result); + if (!RoslynString.IsNullOrEmpty(body)) { - WriteAttribute(writer, "type", result.ExceptionType!); + await writer.WriteStringAsync(XmlSafeText(body)).ConfigureAwait(false); } - if (!RoslynString.IsNullOrEmpty(result.StackTrace)) + await writer.WriteEndElementAsync().ConfigureAwait(false); + } + + // Mirrors the shape Java's Throwable.printStackTrace() produces, which is what genuine + // Surefire reports put in the / body: + // + // com.example.AssertionError: expected:<1> but was:<2> + // at ... + // + // .NET's Exception.StackTrace omits that leading "type: message" header (only ToString() + // includes it), so writing StackTrace alone would drop the single most useful piece of + // diagnostic information. Consumers that render the body rather than the `message` + // attribute — GitLab and CircleCI most notably — would otherwise show only a stack trace. + // The `message` and `type` attributes are still written, so consumers reading those + // directly are unaffected by the duplication. + internal static string? BuildFailureBody(CapturedTestResult result) + { + bool hasExceptionType = !RoslynString.IsNullOrEmpty(result.ExceptionType); + bool hasErrorMessage = !RoslynString.IsNullOrEmpty(result.ErrorMessage); + bool hasStackTrace = !RoslynString.IsNullOrEmpty(result.StackTrace); + + if (!hasExceptionType && !hasErrorMessage) { - await writer.WriteStringAsync(XmlSafeText(result.StackTrace)).ConfigureAwait(false); + return hasStackTrace ? result.StackTrace : null; } - await writer.WriteEndElementAsync().ConfigureAwait(false); + var builder = new StringBuilder(); + if (hasExceptionType) + { + builder.Append(result.ExceptionType); + if (hasErrorMessage) + { + builder.Append(": "); + } + } + + if (hasErrorMessage) + { + builder.Append(result.ErrorMessage); + } + + if (hasStackTrace) + { + builder.Append('\n').Append(result.StackTrace); + } + + return builder.ToString(); } private static async Task WritePropertiesAsync(XmlWriter writer, TestCase tc) diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/JUnitReportTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/JUnitReportTests.cs index e305210f09..f5c37ce250 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/JUnitReportTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/JUnitReportTests.cs @@ -207,7 +207,7 @@ private static void AssertMixedOutcomesJUnitReportShape(string filePath) // fallback for the root passing test, then Container1 for its children). // Exit code is 2 (AtLeastOneTestFailed). The failure/error exceptions are // constructed but never thrown, so they have no stack trace and the - // / elements are self-closing with just message/type. + // / bodies contain only the "type: message" header line. const string expected = """ @@ -237,7 +237,7 @@ private static void AssertMixedOutcomesJUnitReportShape(string filePath) - + System.InvalidOperationException: boom @@ -251,7 +251,7 @@ private static void AssertMixedOutcomesJUnitReportShape(string filePath) - + System.InvalidProgramException: kaboom diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs new file mode 100644 index 0000000000..ac821eae7f --- /dev/null +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs @@ -0,0 +1,204 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Testing.Extensions.JUnitReport; +using Microsoft.Testing.Platform.Extensions.TestFramework; +using Microsoft.Testing.Platform.Helpers; + +using Moq; + +namespace Microsoft.Testing.Extensions.UnitTests; + +[TestClass] +public sealed class JUnitReportFailureBodyTests +{ + // DateTimeOffset.UnixEpoch is not available on .NET Framework, which this project also targets. + private static readonly DateTimeOffset Timestamp = new(1970, 1, 1, 0, 0, 0, TimeSpan.Zero); + + [TestMethod] + public void BuildFailureBody_WithExceptionTypeMessageAndStackTrace_MirrorsPrintStackTraceShape() + => Assert.AreEqual( + "System.InvalidOperationException: boom\n at Foo.Bar()", + JUnitXmlWriter.BuildFailureBody(CreateResult( + exceptionType: "System.InvalidOperationException", + errorMessage: "boom", + stackTrace: " at Foo.Bar()"))); + + [TestMethod] + public void BuildFailureBody_WithoutStackTrace_StillContainsTypeAndMessage() + => Assert.AreEqual( + "System.InvalidOperationException: boom", + JUnitXmlWriter.BuildFailureBody(CreateResult( + exceptionType: "System.InvalidOperationException", + errorMessage: "boom", + stackTrace: null))); + + [TestMethod] + public void BuildFailureBody_WithoutExceptionType_OmitsTheTypePrefix() + => Assert.AreEqual( + "Assert.AreEqual failed\n at Foo.Bar()", + JUnitXmlWriter.BuildFailureBody(CreateResult( + exceptionType: null, + errorMessage: "Assert.AreEqual failed", + stackTrace: " at Foo.Bar()"))); + + [TestMethod] + public void BuildFailureBody_WithoutErrorMessage_OmitsTheSeparator() + => Assert.AreEqual( + "System.InvalidOperationException\n at Foo.Bar()", + JUnitXmlWriter.BuildFailureBody(CreateResult( + exceptionType: "System.InvalidOperationException", + errorMessage: null, + stackTrace: " at Foo.Bar()"))); + + [TestMethod] + public void BuildFailureBody_WithStackTraceOnly_ReturnsStackTraceUnchanged() + => Assert.AreEqual( + " at Foo.Bar()", + JUnitXmlWriter.BuildFailureBody(CreateResult( + exceptionType: null, + errorMessage: null, + stackTrace: " at Foo.Bar()"))); + + [TestMethod] + public void BuildFailureBody_WithNoExceptionDetails_ReturnsNull() + => Assert.IsNull(JUnitXmlWriter.BuildFailureBody(CreateResult( + exceptionType: null, + errorMessage: null, + stackTrace: null))); + + [TestMethod] + public async Task WriteXmlAsync_FailedTest_WritesMessageIntoTheFailureBodyAndKeepsAttributes() + { + XElement failure = await WriteSingleTestCaseAndGetOutcomeElementAsync( + CreateResult( + exceptionType: "System.InvalidOperationException", + errorMessage: "boom", + stackTrace: " at Foo.Bar()")); + + Assert.AreEqual("failure", failure.Name.LocalName); + + // Attributes keep their existing meaning so consumers reading them directly are unaffected. + Assert.AreEqual("boom", failure.Attribute("message")!.Value); + Assert.AreEqual("System.InvalidOperationException", failure.Attribute("type")!.Value); + + // The body now leads with the "type: message" header, like Throwable.printStackTrace(). + Assert.AreEqual("System.InvalidOperationException: boom\n at Foo.Bar()", NormalizeLineEndings(failure.Value)); + } + + [TestMethod] + public async Task WriteXmlAsync_FailedTestWithoutException_FallsBackToElementNameForTypeAttribute() + { + // A framework that reports a failure through `Explanation` alone gives us no exception + // type, but the Ant/windyroad JUnit.xsd marks `type` as required. + XElement failure = await WriteSingleTestCaseAndGetOutcomeElementAsync( + CreateResult(exceptionType: null, errorMessage: "explanation only", stackTrace: null)); + + Assert.AreEqual("failure", failure.Attribute("type")!.Value); + Assert.AreEqual("explanation only", NormalizeLineEndings(failure.Value)); + } + + [TestMethod] + public async Task WriteXmlAsync_ErroredTestWithoutException_FallsBackToElementNameForTypeAttribute() + { + XElement error = await WriteSingleTestCaseAndGetOutcomeElementAsync( + CreateResult(exceptionType: null, errorMessage: "kaboom", stackTrace: null, outcome: "errored")); + + Assert.AreEqual("error", error.Name.LocalName); + Assert.AreEqual("error", error.Attribute("type")!.Value); + } + + private static async Task WriteSingleTestCaseAndGetOutcomeElementAsync(CapturedTestResult result) + { + string xml = await WriteXmlAsync(result); + XElement testCase = XDocument.Parse(xml).Descendants("testcase").Single(); + return testCase.Elements().Single(e => e.Name.LocalName is "failure" or "error" or "skipped"); + } + + private static async Task WriteXmlAsync(CapturedTestResult result) + { + using var stream = new MemoryFileStream(); + var fileSystemMock = new Mock(); + var environmentMock = new Mock(); + var testFrameworkMock = new Mock(); + + _ = fileSystemMock.Setup(x => x.NewFileStream(It.IsAny(), FileMode.Create)).Returns(stream); + _ = environmentMock.SetupGet(x => x.MachineName).Returns("test-host"); + _ = testFrameworkMock.SetupGet(x => x.Uid).Returns("fake-uid"); + _ = testFrameworkMock.SetupGet(x => x.Version).Returns("1.2.3"); + _ = testFrameworkMock.SetupGet(x => x.DisplayName).Returns("Fake"); + + var testCase = new TestCase + { + ClassName = "MyClass", + Name = "MyTest", + OriginalName = "MyTest", + TestPath = "MyTest", + Result = result, + DuplicateIndex = 0, + DuplicateOf = 0, + }; + + var suites = new SuiteSet + { + Name = "MyModule", + Suites = + [ + new Suite + { + Name = "MyClass", + Tests = [testCase], + Failures = result.Outcome == "failed" ? 1 : 0, + Errors = result.Outcome == "errored" ? 1 : 0, + Skipped = result.Outcome == "skipped" ? 1 : 0, + TotalDuration = TimeSpan.Zero, + Timestamp = Timestamp, + } + ], + TotalTests = 1, + TotalFailures = result.Outcome == "failed" ? 1 : 0, + TotalErrors = result.Outcome == "errored" ? 1 : 0, + TotalSkipped = result.Outcome == "skipped" ? 1 : 0, + TotalDuration = TimeSpan.Zero, + Timestamp = Timestamp, + }; + + await new JUnitXmlWriter(fileSystemMock.Object, environmentMock.Object, testFrameworkMock.Object, exitCode: 2, CancellationToken.None) + .WriteXmlAsync("report.xml", suites); + + // MemoryStream.ToArray() is still valid after the writer disposed the stream. + return new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetString(stream.Stream.ToArray()); + } + + private static CapturedTestResult CreateResult(string? exceptionType, string? errorMessage, string? stackTrace, string outcome = "failed") + => new() + { + RawUid = "uid-1", + Outcome = outcome, + ExceptionType = exceptionType, + ErrorMessage = errorMessage, + StackTrace = stackTrace, + }; + + // XML parsers normalize literal CRLF in text content to LF, but the writer's + // NewLineHandling can still round-trip escaped carriage returns, so normalize + // before comparing against the LF-based expectations above. + private static string NormalizeLineEndings(string value) => value.Replace("\r\n", "\n").Replace("\r", "\n"); + + private sealed class MemoryFileStream : IFileStream + { + public MemoryFileStream() => Stream = new MemoryStream(); + + public MemoryStream Stream { get; } + + Stream IFileStream.Stream => Stream; + + string IFileStream.Name => string.Empty; + + void IDisposable.Dispose() => Stream.Dispose(); + +#if NETCOREAPP + ValueTask IAsyncDisposable.DisposeAsync() => Stream.DisposeAsync(); +#endif + } +} From 01b563f21b1d08b96907baa92db5395e17d3ddfa Mon Sep 17 00:00:00 2001 From: Evangelink Date: Tue, 28 Jul 2026 14:11:35 +0200 Subject: [PATCH 2/3] Show explicit start/end tags for failure/error in RFC 016 The outcome-mapping table and the XML skeleton both rendered and as self-closing while the table also claimed a body, which is an impossible XML shape. Now that these elements always carry a body, show them with explicit start/end tags in both places, and correct the row: the writer emits only the message attribute, never a body. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a565c557-8c36-4279-ab7b-41311abcd836 --- docs/RFCs/016-JUnit-Report.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/RFCs/016-JUnit-Report.md b/docs/RFCs/016-JUnit-Report.md index d63bcb0895..d9bc44c208 100644 --- a/docs/RFCs/016-JUnit-Report.md +++ b/docs/RFCs/016-JUnit-Report.md @@ -63,11 +63,11 @@ We target the **Jenkins/Surefire** JUnit XML flavor (the schema published at `je - - - - ... - ... + + ... + ... + ... + ... ... ... @@ -130,15 +130,17 @@ The root `` `name` attribute is the module file name without extensi | MTP `TestNodeStateProperty` | JUnit element | | ---------------------------------------------- | ---------------------------------------------- | | `PassedTestNodeStateProperty` | *(no child element)* | -| `SkippedTestNodeStateProperty` | `` + reason as text | -| `FailedTestNodeStateProperty` | `` + body | -| `TimeoutTestNodeStateProperty` | `` + body | -| `ErrorTestNodeStateProperty` | `` + body | -| `CancelledTestNodeStateProperty` *(obsolete)* | `` + body | -| Other `WellKnownTestNodeTestRunOutcomeFailedProperties` | `` | +| `SkippedTestNodeStateProperty` | `` *(no body)* | +| `FailedTestNodeStateProperty` | `body` | +| `TimeoutTestNodeStateProperty` | `body` | +| `ErrorTestNodeStateProperty` | `body` | +| `CancelledTestNodeStateProperty` *(obsolete)* | `body` | +| Other `WellKnownTestNodeTestRunOutcomeFailedProperties` | `body` | | `DiscoveredTestNodeStateProperty` | *(filtered out — not emitted)* | | `InProgressTestNodeStateProperty` | *(filtered out — not emitted)* | +`body` is the composed failure text described in [Failure and error body format](#failure-and-error-body-format); the element is written with explicit start/end tags whenever that text is non-empty. + `Cancelled` becomes `` rather than `` because cancellation indicates an interruption, not an assertion failure — `` is the schema-correct bucket for "the test could not be evaluated". ### Failure and error body format From 2674e7a548abe6faa97f6d03fcd728e2b1e259c8 Mon Sep 17 00:00:00 2001 From: Evangelink Date: Tue, 28 Jul 2026 14:22:27 +0200 Subject: [PATCH 3/3] Assert message attribute and body in the error type-fallback test WriteXmlAsync_ErroredTestWithoutException_FallsBackToElementNameForTypeAttribute verified the element name and the type fallback but left the message attribute and the body content unasserted, so a regression in either would have gone undetected by this test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a565c557-8c36-4279-ab7b-41311abcd836 --- .../JUnitReportFailureBodyTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs index ac821eae7f..23db83fc29 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs @@ -106,6 +106,8 @@ public async Task WriteXmlAsync_ErroredTestWithoutException_FallsBackToElementNa Assert.AreEqual("error", error.Name.LocalName); Assert.AreEqual("error", error.Attribute("type")!.Value); + Assert.AreEqual("kaboom", error.Attribute("message")!.Value); + Assert.AreEqual("kaboom", NormalizeLineEndings(error.Value)); } private static async Task WriteSingleTestCaseAndGetOutcomeElementAsync(CapturedTestResult result)