diff --git a/docs/RFCs/016-JUnit-Report.md b/docs/RFCs/016-JUnit-Report.md index 3ecd5f5449..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,17 +130,40 @@ The root `` `name` attribute is the module file name without extensi | MTP `TestNodeStateProperty` | JUnit element | | ---------------------------------------------- | ---------------------------------------------- | | `PassedTestNodeStateProperty` | *(no child element)* | -| `SkippedTestNodeStateProperty` | `` + reason as text | -| `FailedTestNodeStateProperty` | `` + stack | -| `TimeoutTestNodeStateProperty` | `` + stack | -| `ErrorTestNodeStateProperty` | `` + stack | -| `CancelledTestNodeStateProperty` *(obsolete)* | `` + stack | -| 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 + +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 +301,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..23db83fc29 --- /dev/null +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportFailureBodyTests.cs @@ -0,0 +1,206 @@ +// 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); + Assert.AreEqual("kaboom", error.Attribute("message")!.Value); + Assert.AreEqual("kaboom", NormalizeLineEndings(error.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 + } +}