From 816c2fab5a5d54cdd51e6da3138d76c6ce9f9f7b Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:00:28 +0100 Subject: [PATCH 1/4] feat(reporters): group GitHub summary failures by exception type (#5484) Replace the flat failure list in the GitHub Actions step summary with failures grouped by exception type. Each group is a collapsible section (or bold header in Full mode) containing a markdown table of affected tests and a "Common error" block from the first failure in the group. - Add FailureEntry record and GetExceptionTypeName helper - Group failures by exception type, ordered by count descending - Cap at 50 tests per group with overflow indicator - Include timeouts in Quick Diagnosis section - Keep other non-passing tests (cancelled, in-progress) in separate table - Add 8 new tests covering grouping, ordering, styles, and edge cases --- TUnit.Engine.Tests/GitHubReporterTests.cs | 236 +++++++++++++++++++++- TUnit.Engine/Reporters/GitHubReporter.cs | 118 ++++++----- 2 files changed, 306 insertions(+), 48 deletions(-) diff --git a/TUnit.Engine.Tests/GitHubReporterTests.cs b/TUnit.Engine.Tests/GitHubReporterTests.cs index 3b0f32c6b0..0580fd40f3 100644 --- a/TUnit.Engine.Tests/GitHubReporterTests.cs +++ b/TUnit.Engine.Tests/GitHubReporterTests.cs @@ -1,3 +1,7 @@ +#pragma warning disable TPEXP + +using Microsoft.Testing.Platform.Extensions.Messages; +using Microsoft.Testing.Platform.TestHost; using Shouldly; using TUnit.Engine.Reporters; @@ -6,15 +10,24 @@ namespace TUnit.Engine.Tests; [NotInParallel] public class GitHubReporterTests { + private readonly List _tempFiles = []; + [After(Test)] public void CleanupAfterTest() { - // Reset all environment variables Environment.SetEnvironmentVariable("TUNIT_DISABLE_GITHUB_REPORTER", null); Environment.SetEnvironmentVariable("DISABLE_GITHUB_REPORTER", null); Environment.SetEnvironmentVariable("TUNIT_GITHUB_REPORTER_STYLE", null); Environment.SetEnvironmentVariable("GITHUB_ACTIONS", null); Environment.SetEnvironmentVariable("GITHUB_STEP_SUMMARY", null); + Environment.SetEnvironmentVariable("GITHUB_REPOSITORY", null); + Environment.SetEnvironmentVariable("GITHUB_SHA", null); + + foreach (var file in _tempFiles) + { + try { File.Delete(file); } catch { /* best-effort cleanup */ } + } + _tempFiles.Clear(); } [Test] @@ -91,8 +104,227 @@ public async Task IsEnabledAsync_Should_Return_False_When_GITHUB_ACTIONS_Is_Not_ result.ShouldBeFalse(); } + [Test] + public async Task AfterRunAsync_Groups_Failures_By_Exception_Type() + { + var (reporter, outputFile) = await SetupReporter(); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "TestA", "MyService", new NullReferenceException("obj was null")), + CreateFailedTestMessage("2", "TestB", "MyService", new NullReferenceException("another null")), + CreateFailedTestMessage("3", "TestC", "OtherService", new ArgumentException("bad arg")) + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + output.ShouldContain("Failures by Cause"); + output.ShouldContain("NullReferenceException (2 tests)"); + output.ShouldContain("ArgumentException (1 test)"); + output.ShouldContain("`MyService.TestA`"); + output.ShouldContain("`MyService.TestB`"); + output.ShouldContain("`OtherService.TestC`"); + } + + [Test] + public async Task AfterRunAsync_Orders_Groups_By_Count_Descending() + { + var (reporter, outputFile) = await SetupReporter(); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "T1", "Svc", new ArgumentException("a")), + CreateFailedTestMessage("2", "T2", "Svc", new NullReferenceException("n1")), + CreateFailedTestMessage("3", "T3", "Svc", new NullReferenceException("n2")), + CreateFailedTestMessage("4", "T4", "Svc", new NullReferenceException("n3")) + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + var nreIndex = output.IndexOf("NullReferenceException (3 tests)", StringComparison.Ordinal); + var argIndex = output.IndexOf("ArgumentException (1 test)", StringComparison.Ordinal); + nreIndex.ShouldBeLessThan(argIndex, "NullReferenceException group (3) should appear before ArgumentException group (1)"); + } + + [Test] + public async Task AfterRunAsync_Groups_Timeouts_As_Timeout() + { + var (reporter, outputFile) = await SetupReporter(); + + await FeedTestMessages(reporter, + CreateTimeoutTestMessage("1", "SlowTest1", "MyService"), + CreateTimeoutTestMessage("2", "SlowTest2", "MyService") + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + output.ShouldContain("Timeout (2 tests)"); + output.ShouldContain("`MyService.SlowTest1`"); + output.ShouldContain("`MyService.SlowTest2`"); + } + + [Test] + public async Task AfterRunAsync_Collapsible_Style_Wraps_Groups_In_Details() + { + var (reporter, outputFile) = await SetupReporter(GitHubReporterStyle.Collapsible); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "T1", "Svc", new InvalidOperationException("oops")) + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + output.ShouldContain("
"); + output.ShouldContain("InvalidOperationException (1 test)"); + output.ShouldContain("
"); + } + + [Test] + public async Task AfterRunAsync_Full_Style_Renders_Groups_Expanded() + { + var (reporter, outputFile) = await SetupReporter(GitHubReporterStyle.Full); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "T1", "Svc", new InvalidOperationException("oops")) + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + output.ShouldContain("**InvalidOperationException (1 test)**"); + // Full mode should not wrap failure groups in
+ // The output contains
for other sections, but the failure group itself should use **bold** + output.ShouldContain("| `Svc.T1`"); + } + + [Test] + public async Task AfterRunAsync_Shows_Common_Error_For_Each_Group() + { + var (reporter, outputFile) = await SetupReporter(); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "T1", "Svc", new NullReferenceException("Object reference not set")), + CreateFailedTestMessage("2", "T2", "Svc", new NullReferenceException("Different message")) + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + output.ShouldContain("**Common error:**"); + // The common error is from the first entry in the group (order not guaranteed), + // so check that at least one of the messages appears + (output.Contains("Object reference not set") || output.Contains("Different message")) + .ShouldBeTrue("Common error should contain one of the exception messages"); + } + + [Test] + public async Task AfterRunAsync_Quick_Diagnosis_Includes_Timeouts() + { + var (reporter, outputFile) = await SetupReporter(); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "T1", "Svc", new NullReferenceException("n")), + CreateTimeoutTestMessage("2", "SlowTest", "Svc") + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + output.ShouldContain("Quick diagnosis:"); + output.ShouldContain("Timeout"); + } + + [Test] + public async Task AfterRunAsync_Other_NonPassing_Tests_Remain_Separate() + { + var (reporter, outputFile) = await SetupReporter(); + + await FeedTestMessages(reporter, + CreateFailedTestMessage("1", "FailedTest", "Svc", new Exception("err")), + CreatePassedTestMessage("2", "PassedTest", "Svc"), + CreateCancelledTestMessage("3", "CancelledTest", "Svc") + ); + + await reporter.AfterRunAsync(1, CancellationToken.None); + + var output = await File.ReadAllTextAsync(outputFile); + // Failures in grouped section + output.ShouldContain("Failures by Cause"); + output.ShouldContain("`Svc.FailedTest`"); + // Cancelled test in the other table + output.ShouldContain("Other non-passing tests"); + output.ShouldContain("CancelledTest"); + } + private string CreateTempFile() { - return Path.GetTempFileName(); + var path = Path.GetTempFileName(); + _tempFiles.Add(path); + return path; + } + + private async Task<(GitHubReporter Reporter, string OutputFile)> SetupReporter( + GitHubReporterStyle style = GitHubReporterStyle.Collapsible) + { + var outputFile = CreateTempFile(); + Environment.SetEnvironmentVariable("GITHUB_ACTIONS", "true"); + Environment.SetEnvironmentVariable("GITHUB_STEP_SUMMARY", outputFile); + + var reporter = new GitHubReporter(new MockExtension()); + await reporter.IsEnabledAsync(); + reporter.SetReporterStyle(style); + await reporter.BeforeRunAsync(CancellationToken.None); + + return (reporter, outputFile); + } + + private static async Task FeedTestMessages(GitHubReporter reporter, params TestNodeUpdateMessage[] messages) + { + foreach (var message in messages) + { + await reporter.ConsumeAsync(null!, message, CancellationToken.None); + } + } + + private static TestNodeUpdateMessage CreateTestMessage( + string testId, string displayName, string typeName, IProperty stateProperty) + { + return new TestNodeUpdateMessage( + sessionUid: new SessionUid("test-session"), + testNode: new TestNode + { + Uid = new TestNodeUid(testId), + DisplayName = displayName, + Properties = new PropertyBag( + stateProperty, + new TestMethodIdentifierProperty( + @namespace: "TestNamespace", + assemblyFullName: "TestAssembly", + typeName: typeName, + methodName: displayName, + parameterTypeFullNames: [], + returnTypeFullName: "System.Void", + methodArity: 0)) + }); } + + private static TestNodeUpdateMessage CreateFailedTestMessage( + string testId, string displayName, string typeName, Exception exception) => + CreateTestMessage(testId, displayName, typeName, new FailedTestNodeStateProperty(exception, exception.Message)); + + private static TestNodeUpdateMessage CreateTimeoutTestMessage( + string testId, string displayName, string typeName) => + CreateTestMessage(testId, displayName, typeName, new TimeoutTestNodeStateProperty("Test timed out after 30s")); + + private static TestNodeUpdateMessage CreatePassedTestMessage( + string testId, string displayName, string typeName) => + CreateTestMessage(testId, displayName, typeName, PassedTestNodeStateProperty.CachedInstance); + +#pragma warning disable CS0618 + private static TestNodeUpdateMessage CreateCancelledTestMessage( + string testId, string displayName, string typeName) => + CreateTestMessage(testId, displayName, typeName, new CancelledTestNodeStateProperty()); +#pragma warning restore CS0618 } diff --git a/TUnit.Engine/Reporters/GitHubReporter.cs b/TUnit.Engine/Reporters/GitHubReporter.cs index 7d5b968e39..2f37c72205 100644 --- a/TUnit.Engine/Reporters/GitHubReporter.cs +++ b/TUnit.Engine/Reporters/GitHubReporter.cs @@ -262,18 +262,13 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) stringBuilder.AppendLine("> **Tip:** You can have HTML reports uploaded automatically as artifacts. [Learn more](https://tunit.dev/docs/guides/html-report#enabling-automatic-artifact-upload)"); } - if (failed.Length > 0) + if (failed.Length > 0 || timeout.Length > 0) { - var failureGroups = failed + var failureGroups = failed.Concat(timeout) .Select(x => { var state = x.Value.TestNode.Properties.AsEnumerable().FirstOrDefault(p => p is TestNodeStateProperty); - var exceptionType = state switch - { - FailedTestNodeStateProperty f => f.Exception?.GetType().Name ?? "Unknown", - ErrorTestNodeStateProperty e => e.Exception?.GetType().Name ?? "Unknown", - _ => "Unknown" - }; + var exceptionType = GetExceptionTypeName(state); var method = x.Value.TestNode.Properties.AsEnumerable() .OfType().FirstOrDefault(); return (ExceptionType: exceptionType, ClassName: method?.TypeName ?? "Unknown"); @@ -306,7 +301,7 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) var githubServerUrl = Environment.GetEnvironmentVariable("GITHUB_SERVER_URL") ?? "https://github.com"; // Separate failures from other non-passing tests - var failureMessages = new List<(string Name, string? SourceLink, string Details, string Duration)>(); + var failureMessages = new List(); var otherMessages = new List<(string Name, string Status, string Details, string Duration)>(); foreach (var testNodeUpdateMessage in last.Values) @@ -325,8 +320,9 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) if (isFailed) { var sourceLink = GetSourceLink(testNodeUpdateMessage.TestNode, githubRepo, githubSha, githubWorkspace, githubServerUrl); - var details = GetDetails(stateProperty, testNodeUpdateMessage.TestNode.Properties); - failureMessages.Add((name, sourceLink, details, duration)); + var exceptionType = GetExceptionTypeName(stateProperty); + var commonError = GetError(stateProperty); + failureMessages.Add(new FailureEntry(name, sourceLink, duration, exceptionType, commonError)); } else { @@ -336,41 +332,70 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) } } - // Show top failures inline - const int maxInlineFailures = 5; + const int maxTestsPerGroup = 50; if (failureMessages.Count > 0) { stringBuilder.AppendLine(); - stringBuilder.AppendLine("#### Failures"); + stringBuilder.AppendLine("#### Failures by Cause"); stringBuilder.AppendLine(); - var inlineCount = Math.Min(failureMessages.Count, maxInlineFailures); - for (int i = 0; i < inlineCount; i++) + var grouped = failureMessages + .GroupBy(f => f.ExceptionType) + .OrderByDescending(g => g.Count()); + + foreach (var group in grouped) { - var (name, sourceLink, details, duration) = failureMessages[i]; - var sourcePart = sourceLink is not null ? $" \u2014 {sourceLink}" : ""; - stringBuilder.AppendLine("
"); - stringBuilder.AppendLine($"{name} ({duration}){sourcePart}"); - stringBuilder.AppendLine(); - stringBuilder.AppendLine(details); + var entries = group.ToList(); + var count = entries.Count; + var label = $"{group.Key} ({count} {(count == 1 ? "test" : "tests")})"; + + if (_reporterStyle == GitHubReporterStyle.Collapsible) + { + stringBuilder.AppendLine("
"); + stringBuilder.AppendLine($"{label}"); + } + else + { + stringBuilder.AppendLine($"**{label}**"); + } + stringBuilder.AppendLine(); - stringBuilder.AppendLine("
"); - } + stringBuilder.AppendLine("| Test | Duration |"); + stringBuilder.AppendLine("| --- | --- |"); + + var displayCount = Math.Min(count, maxTestsPerGroup); + for (int i = 0; i < displayCount; i++) + { + var entry = entries[i]; + var sourcePart = entry.SourceLink is not null ? $" {entry.SourceLink}" : ""; + stringBuilder.AppendLine($"| `{entry.Name}`{sourcePart} | {entry.Duration} |"); + } + + if (count > maxTestsPerGroup) + { + stringBuilder.AppendLine($"| *...and {count - maxTestsPerGroup} more* | |"); + } + + var firstError = entries[0].CommonError; + if (!string.IsNullOrWhiteSpace(firstError)) + { + stringBuilder.AppendLine(); + stringBuilder.AppendLine("**Common error:**"); + stringBuilder.AppendLine($"
{firstError}
"); + } + + if (_reporterStyle == GitHubReporterStyle.Collapsible) + { + stringBuilder.AppendLine(); + stringBuilder.AppendLine("
"); + } - if (failureMessages.Count > maxInlineFailures) - { stringBuilder.AppendLine(); - stringBuilder.AppendLine($"*...and {failureMessages.Count - maxInlineFailures} more failures*"); } } - // Build the full details table for remaining items - var remainingFailures = failureMessages.Count > maxInlineFailures - ? failureMessages.Skip(maxInlineFailures).ToList() - : new List<(string Name, string? SourceLink, string Details, string Duration)>(); - var hasRemainingDetails = remainingFailures.Count > 0 || otherMessages.Count > 0; - - if (hasRemainingDetails) + // Build the details table for other non-passing tests (cancelled, in-progress, etc.) + if (otherMessages.Count > 0) { var detailsBuilder = new StringBuilder(); detailsBuilder.AppendLine(); @@ -378,16 +403,6 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) detailsBuilder.AppendLine("TestStatusDetailsDuration"); detailsBuilder.AppendLine(""); - foreach (var (name, sourceLink, details, duration) in remainingFailures) - { - detailsBuilder.AppendLine(""); - detailsBuilder.AppendLine($"{name}"); - detailsBuilder.AppendLine("Failed"); - detailsBuilder.AppendLine($"{details}"); - detailsBuilder.AppendLine($"{duration}"); - detailsBuilder.AppendLine(""); - } - foreach (var (name, status, details, duration) in otherMessages) { detailsBuilder.AppendLine(""); @@ -402,10 +417,9 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) if (_reporterStyle == GitHubReporterStyle.Collapsible) { - var totalNonPassing = remainingFailures.Count + otherMessages.Count; stringBuilder.AppendLine(); stringBuilder.AppendLine("
"); - stringBuilder.AppendLine($"All non-passing tests ({totalNonPassing} total)"); + stringBuilder.AppendLine($"Other non-passing tests ({otherMessages.Count} total)"); stringBuilder.Append(detailsBuilder.ToString()); stringBuilder.AppendLine(); stringBuilder.AppendLine("
"); @@ -610,4 +624,16 @@ internal void SetReporterStyle(GitHubReporterStyle style) { TotalHours: < 1 } d => $"{d.Minutes}m {d.Seconds}s", var d => $"{(int)d.Value.TotalHours}h {d.Value.Minutes}m" }; + + private static string GetExceptionTypeName(IProperty? stateProperty) => stateProperty switch + { + FailedTestNodeStateProperty f => f.Exception?.GetType().Name ?? "Unknown", + ErrorTestNodeStateProperty e => e.Exception?.GetType().Name ?? "Unknown", + TimeoutTestNodeStateProperty => "Timeout", + _ => "Unknown" + }; + + private record FailureEntry( + string Name, string? SourceLink, string Duration, + string ExceptionType, string? CommonError); } From a9a8faf281ba29d7cb2639fc2a1edf627a0ad901 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:06:03 +0100 Subject: [PATCH 2/4] fix: address review feedback on failure grouping - HTML-encode common error in
 block to prevent injection from
  exception messages containing < and > characters
- Select most frequent error message in each group instead of arbitrary
  first entry, making "Common error" semantically accurate and stable
- Add test for the 50-test-per-group cap with overflow indicator
---
 TUnit.Engine.Tests/GitHubReporterTests.cs | 25 ++++++++++++++++++-----
 TUnit.Engine/Reporters/GitHubReporter.cs  | 12 ++++++++---
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/TUnit.Engine.Tests/GitHubReporterTests.cs b/TUnit.Engine.Tests/GitHubReporterTests.cs
index 0580fd40f3..278d6fe596 100644
--- a/TUnit.Engine.Tests/GitHubReporterTests.cs
+++ b/TUnit.Engine.Tests/GitHubReporterTests.cs
@@ -206,17 +206,32 @@ public async Task AfterRunAsync_Shows_Common_Error_For_Each_Group()
 
         await FeedTestMessages(reporter,
             CreateFailedTestMessage("1", "T1", "Svc", new NullReferenceException("Object reference not set")),
-            CreateFailedTestMessage("2", "T2", "Svc", new NullReferenceException("Different message"))
+            CreateFailedTestMessage("2", "T2", "Svc", new NullReferenceException("Object reference not set")),
+            CreateFailedTestMessage("3", "T3", "Svc", new NullReferenceException("Different message"))
         );
 
         await reporter.AfterRunAsync(1, CancellationToken.None);
 
         var output = await File.ReadAllTextAsync(outputFile);
         output.ShouldContain("**Common error:**");
-        // The common error is from the first entry in the group (order not guaranteed),
-        // so check that at least one of the messages appears
-        (output.Contains("Object reference not set") || output.Contains("Different message"))
-            .ShouldBeTrue("Common error should contain one of the exception messages");
+        // Most frequent error message in the group wins
+        output.ShouldContain("Object reference not set");
+    }
+
+    [Test]
+    public async Task AfterRunAsync_Caps_Group_At_50_Tests()
+    {
+        var (reporter, outputFile) = await SetupReporter();
+
+        var messages = Enumerable.Range(1, 55)
+            .Select(i => CreateFailedTestMessage(i.ToString(), $"T{i}", "Svc", new NullReferenceException("n")))
+            .ToArray();
+        await FeedTestMessages(reporter, messages);
+
+        await reporter.AfterRunAsync(1, CancellationToken.None);
+
+        var output = await File.ReadAllTextAsync(outputFile);
+        output.ShouldContain("...and 5 more");
     }
 
     [Test]
diff --git a/TUnit.Engine/Reporters/GitHubReporter.cs b/TUnit.Engine/Reporters/GitHubReporter.cs
index 2f37c72205..1765873c83 100644
--- a/TUnit.Engine/Reporters/GitHubReporter.cs
+++ b/TUnit.Engine/Reporters/GitHubReporter.cs
@@ -376,12 +376,18 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation)
                     stringBuilder.AppendLine($"| *...and {count - maxTestsPerGroup} more* | |");
                 }
 
-                var firstError = entries[0].CommonError;
-                if (!string.IsNullOrWhiteSpace(firstError))
+                var commonError = entries
+                    .Where(e => !string.IsNullOrWhiteSpace(e.CommonError))
+                    .GroupBy(e => e.CommonError)
+                    .OrderByDescending(g => g.Count())
+                    .FirstOrDefault()
+                    ?.Key;
+
+                if (commonError is not null)
                 {
                     stringBuilder.AppendLine();
                     stringBuilder.AppendLine("**Common error:**");
-                    stringBuilder.AppendLine($"
{firstError}
"); + stringBuilder.AppendLine($"
{System.Net.WebUtility.HtmlEncode(commonError)}
"); } if (_reporterStyle == GitHubReporterStyle.Collapsible) From 29d4d4d317e311527cf89e5e90c88bce0d9fc781 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:11:18 +0100 Subject: [PATCH 3/4] test: assert IsEnabledAsync returns true in SetupReporter Fail loudly if env vars are misconfigured instead of silently producing empty output. --- TUnit.Engine.Tests/GitHubReporterTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/TUnit.Engine.Tests/GitHubReporterTests.cs b/TUnit.Engine.Tests/GitHubReporterTests.cs index 278d6fe596..01864a3e68 100644 --- a/TUnit.Engine.Tests/GitHubReporterTests.cs +++ b/TUnit.Engine.Tests/GitHubReporterTests.cs @@ -288,7 +288,8 @@ private string CreateTempFile() Environment.SetEnvironmentVariable("GITHUB_STEP_SUMMARY", outputFile); var reporter = new GitHubReporter(new MockExtension()); - await reporter.IsEnabledAsync(); + var enabled = await reporter.IsEnabledAsync(); + enabled.ShouldBeTrue("Reporter should be enabled — check env var setup"); reporter.SetReporterStyle(style); await reporter.BeforeRunAsync(CancellationToken.None); From 40fbc04f64de293a826d3c3a860ddf1bb6a2459d Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:40:56 +0100 Subject: [PATCH 4/4] fix: address remaining review feedback on failure grouping - Eliminate double-iteration: build failureMessages before quick diagnosis block and derive quick diagnosis from it, instead of iterating failed.Concat(timeout) separately with duplicate GetExceptionTypeName/grouping logic - Make GetError static (no instance state accessed) - Remove unused #pragma warning disable TPEXP (no warnings triggered) - Add comment explaining maxTestsPerGroup = 50 rationale --- TUnit.Engine.Tests/GitHubReporterTests.cs | 2 - TUnit.Engine/Reporters/GitHubReporter.cs | 67 +++++++++++------------ 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/TUnit.Engine.Tests/GitHubReporterTests.cs b/TUnit.Engine.Tests/GitHubReporterTests.cs index 01864a3e68..78864df814 100644 --- a/TUnit.Engine.Tests/GitHubReporterTests.cs +++ b/TUnit.Engine.Tests/GitHubReporterTests.cs @@ -1,5 +1,3 @@ -#pragma warning disable TPEXP - using Microsoft.Testing.Platform.Extensions.Messages; using Microsoft.Testing.Platform.TestHost; using Shouldly; diff --git a/TUnit.Engine/Reporters/GitHubReporter.cs b/TUnit.Engine/Reporters/GitHubReporter.cs index 1765873c83..e1d91446f2 100644 --- a/TUnit.Engine/Reporters/GitHubReporter.cs +++ b/TUnit.Engine/Reporters/GitHubReporter.cs @@ -262,45 +262,13 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) stringBuilder.AppendLine("> **Tip:** You can have HTML reports uploaded automatically as artifacts. [Learn more](https://tunit.dev/docs/guides/html-report#enabling-automatic-artifact-upload)"); } - if (failed.Length > 0 || timeout.Length > 0) - { - var failureGroups = failed.Concat(timeout) - .Select(x => - { - var state = x.Value.TestNode.Properties.AsEnumerable().FirstOrDefault(p => p is TestNodeStateProperty); - var exceptionType = GetExceptionTypeName(state); - var method = x.Value.TestNode.Properties.AsEnumerable() - .OfType().FirstOrDefault(); - return (ExceptionType: exceptionType, ClassName: method?.TypeName ?? "Unknown"); - }) - .GroupBy(x => x.ExceptionType) - .OrderByDescending(g => g.Count()) - .Take(3); - - var diagParts = failureGroups.Select(g => - { - var topClass = g.GroupBy(x => x.ClassName).OrderByDescending(c => c.Count()).First(); - return $"{g.Count()} \u00d7 `{g.Key}` in `{topClass.Key}`"; - }); - - stringBuilder.AppendLine(); - stringBuilder.AppendLine($"> **Quick diagnosis:** {string.Join(", ", diagParts)}"); - } - - if (passedCount == last.Count) - { - stringBuilder.AppendLine(); - stringBuilder.AppendLine("---"); - return WriteFile(stringBuilder.ToString()); - } - // Cache env vars for source links (read once, not per test) var githubRepo = Environment.GetEnvironmentVariable(EnvironmentConstants.GitHubRepository); var githubSha = Environment.GetEnvironmentVariable(EnvironmentConstants.GitHubSha); var githubWorkspace = Environment.GetEnvironmentVariable("GITHUB_WORKSPACE")?.Replace('\\', '/'); var githubServerUrl = Environment.GetEnvironmentVariable("GITHUB_SERVER_URL") ?? "https://github.com"; - // Separate failures from other non-passing tests + // Separate failures from other non-passing tests (built once, used by both quick diagnosis and full rendering) var failureMessages = new List(); var otherMessages = new List<(string Name, string Status, string Details, string Duration)>(); @@ -322,7 +290,9 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) var sourceLink = GetSourceLink(testNodeUpdateMessage.TestNode, githubRepo, githubSha, githubWorkspace, githubServerUrl); var exceptionType = GetExceptionTypeName(stateProperty); var commonError = GetError(stateProperty); - failureMessages.Add(new FailureEntry(name, sourceLink, duration, exceptionType, commonError)); + var method = props.OfType().FirstOrDefault(); + var className = method?.TypeName ?? "Unknown"; + failureMessages.Add(new FailureEntry(name, sourceLink, duration, exceptionType, commonError, className)); } else { @@ -332,6 +302,31 @@ public Task AfterRunAsync(int exitCode, CancellationToken cancellation) } } + if (failureMessages.Count > 0) + { + var failureGroups = failureMessages + .GroupBy(f => f.ExceptionType) + .OrderByDescending(g => g.Count()) + .Take(3); + + var diagParts = failureGroups.Select(g => + { + var topClass = g.GroupBy(x => x.ClassName).OrderByDescending(c => c.Count()).First(); + return $"{g.Count()} \u00d7 `{g.Key}` in `{topClass.Key}`"; + }); + + stringBuilder.AppendLine(); + stringBuilder.AppendLine($"> **Quick diagnosis:** {string.Join(", ", diagParts)}"); + } + + if (passedCount == last.Count) + { + stringBuilder.AppendLine(); + stringBuilder.AppendLine("---"); + return WriteFile(stringBuilder.ToString()); + } + + // Cap per group to keep the GitHub step summary within the 1 MB file-size limit const int maxTestsPerGroup = 50; if (failureMessages.Count > 0) { @@ -519,7 +514,7 @@ or TimeoutTestNodeStateProperty return "Unknown Test State"; } - private string? GetError(IProperty? stateProperty) + private static string? GetError(IProperty? stateProperty) { return stateProperty switch { @@ -641,5 +636,5 @@ internal void SetReporterStyle(GitHubReporterStyle style) private record FailureEntry( string Name, string? SourceLink, string Duration, - string ExceptionType, string? CommonError); + string ExceptionType, string? CommonError, string ClassName); }