Surface exit-code verdict and per-assembly clarity in GitHubActionsReport

src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs

@@ -6,9 +6,11 @@
 using Microsoft.Testing.Platform.Extensions;
 using Microsoft.Testing.Platform.Extensions.Messages;
 using Microsoft.Testing.Platform.Extensions.OutputDevice;
+using Microsoft.Testing.Platform.Extensions.TestHost;
 using Microsoft.Testing.Platform.Helpers;
 using Microsoft.Testing.Platform.Logging;
 using Microsoft.Testing.Platform.OutputDevice;
+using Microsoft.Testing.Platform.Services;
 
 namespace Microsoft.Testing.Extensions.GitHubActionsReport;
 
@@ -18,13 +20,21 @@ namespace Microsoft.Testing.Extensions.GitHubActionsReport;
 /// pull request's "Files changed" diff gutter. Skipped tests are surfaced as title-only <c>::warning</c>
 /// workflow commands so they are visible in the Annotations tab too.
 /// </summary>
+/// <remarks>
+/// It also implements <see cref="ITestSessionLifetimeHandler"/> so that, at session end, it can emit one
+/// extra <c>::error</c> for a non-test-result failure exit code (e.g. a <c>--minimum-expected-tests</c>
+/// violation or a run that discovered zero tests). Those outcomes carry no failing <see cref="TestNode"/>,
+/// so without this they would leave the Annotations tab empty despite the run failing.
+/// </remarks>
 internal sealed class GitHubActionsAnnotationReporter :
     IDataConsumer,
+    ITestSessionLifetimeHandler,
     IOutputDeviceDataProducer
 {
     private readonly IEnvironment _environment;
     private readonly IFileSystem _fileSystem;
     private readonly IOutputDevice _outputDisplay;
+    private readonly ITestApplicationProcessExitCode _testApplicationProcessExitCode;
     private readonly ILogger _logger;
     private readonly bool _isEnabled;
 
@@ -33,11 +43,13 @@ public GitHubActionsAnnotationReporter(
         IEnvironment environment,
         IFileSystem fileSystem,
         IOutputDevice outputDisplay,
+        ITestApplicationProcessExitCode testApplicationProcessExitCode,
         ILoggerFactory loggerFactory)
     {
         _environment = environment;
         _fileSystem = fileSystem;
         _outputDisplay = outputDisplay;
+        _testApplicationProcessExitCode = testApplicationProcessExitCode;
         _logger = loggerFactory.CreateLogger<GitHubActionsAnnotationReporter>();
         _isEnabled = GitHubActionsFeature.IsEnabled(commandLine, environment, GitHubActionsCommandLineOptions.GitHubActionsAnnotations);
     }
@@ -117,6 +129,68 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella
         }
     }
 
+    public Task OnTestSessionStartingAsync(ITestSessionContext testSessionContext) => Task.CompletedTask;
+
+    public async Task OnTestSessionFinishingAsync(ITestSessionContext testSessionContext)
+    {
+        try
+        {
+            testSessionContext.CancellationToken.ThrowIfCancellationRequested();
+
+            if (!_isEnabled)
+            {
+                return;
+            }
+
+            int exitCode = _testApplicationProcessExitCode.GetProcessExitCode();
+
+            // Only emit the run-level annotation for a non-test-result failure. Plain "at least one test failed"
+            // (and success) are already conveyed by the per-test annotations above, so we would otherwise add a
+            // redundant, location-less error on top of the real ones.
+            // NOTE: a hard abort/cancellation (ExitCode.TestSessionAborted) never reaches this handler — the host
+            // swallows the OperationCanceledException before the end-of-session notification completes — so this
+            // path only surfaces session-derived outcomes such as ZeroTests, MinimumExpectedTestsPolicyViolation,
+            // TestAdapterTestSessionFailure and TestExecutionStoppedForMaxFailedTests.
+            if (GitHubActionsExitCode.IsTestResultOutcome(exitCode))
+            {
+                return;
+            }
+
+            string line = GetExitCodeAnnotation(exitCode);
+
+            if (_logger.IsEnabled(LogLevel.Trace))
+            {
+                _logger.LogTrace($"Showing exit-code annotation '{line}'.");
+            }
+
+            await DisplayAnnotationLineAsync(line, testSessionContext.CancellationToken).ConfigureAwait(false);
+        }
+        catch (OperationCanceledException)
+        {
+            throw;
+        }
+        catch (Exception ex)
+        {
+            _logger.LogUnexpectedException(nameof(OnTestSessionFinishingAsync), ex);
+        }
+    }
+
+    internal static /* for testing */ string GetExitCodeAnnotation(int exitCode)
+    {
+        string title = string.Format(
+            CultureInfo.InvariantCulture,
+            GitHubActionsResources.ExitCodeAnnotationTitle,
+            exitCode.ToString(CultureInfo.InvariantCulture),
+            GitHubActionsExitCode.GetName(exitCode));
+        string message = GitHubActionsExitCode.GetReason(exitCode);
+
+        return string.Format(
+            CultureInfo.InvariantCulture,
+            "::error title={0}::{1}",
+            GitHubActionsEscaper.EscapeProperty(title),
+            GitHubActionsEscaper.EscapeData(message));
+    }
+
     private Task WriteAnnotationAsync(string testName, string? explanation, Exception? exception, CancellationToken cancellationToken)
     {
         if (_logger.IsEnabled(LogLevel.Trace))

src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsExitCode.cs

@@ -0,0 +1,64 @@
+// 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.GitHubActionsReport.Resources;
+using Microsoft.Testing.Platform.Helpers;
+
+namespace Microsoft.Testing.Extensions.GitHubActionsReport;
+
+/// <summary>
+/// Helpers for turning a Microsoft.Testing.Platform process exit code into a GitHub-friendly verdict.
+/// Used by the step-summary and annotation reporters so that non-test-result failures observable once the
+/// test session has finished — e.g. a <c>--minimum-expected-tests</c> violation, a run that discovered zero
+/// tests, a <c>--maximum-failed-tests</c> stop or a test-adapter session failure — are surfaced instead of
+/// silently looking like a passing run. Outcomes that occur outside the end-of-session path (a hard
+/// abort/cancellation, or host failures raised before/after the session) are not reachable here.
+/// See <see href="https://learn.microsoft.com/dotnet/core/testing/microsoft-testing-platform-troubleshooting#exit-codes"/>.
+/// </summary>
+internal static class GitHubActionsExitCode
+{
+    /// <summary>
+    /// Returns <see langword="true"/> when the exit code is a normal test-result outcome (everything passed,
+    /// or at least one test failed). Those outcomes are already reflected by the passed/failed totals and the
+    /// per-test failure annotations, so callers do not surface a separate exit-code callout for them.
+    /// </summary>
+    public static bool IsTestResultOutcome(int exitCode)
+        => exitCode is (int)ExitCode.Success or (int)ExitCode.AtLeastOneTestFailed;
+
+    /// <summary>
+    /// Returns <see langword="true"/> when the run did not succeed (any non-zero exit code).
+    /// </summary>
+    public static bool IndicatesFailure(int exitCode)
+        => exitCode != (int)ExitCode.Success;
+
+    /// <summary>
+    /// Returns the enum name for a known exit code (e.g. <c>MinimumExpectedTestsPolicyViolation</c>) or
+    /// <c>Unknown</c> for a value outside the documented set.
+    /// </summary>
+    public static string GetName(int exitCode)
+        => Enum.IsDefined(typeof(ExitCode), exitCode)
+            ? ((ExitCode)exitCode).ToString()
+            : "Unknown";
+
+    /// <summary>
+    /// Returns a short, human-readable, localized explanation of the exit code.
+    /// </summary>
+    public static string GetReason(int exitCode)
+        => exitCode switch
+        {
+            (int)ExitCode.Success => GitHubActionsResources.ExitCodeReasonSuccess,
+            (int)ExitCode.GenericFailure => GitHubActionsResources.ExitCodeReasonGenericFailure,
+            (int)ExitCode.AtLeastOneTestFailed => GitHubActionsResources.ExitCodeReasonAtLeastOneTestFailed,
+            (int)ExitCode.TestSessionAborted => GitHubActionsResources.ExitCodeReasonTestSessionAborted,
+            (int)ExitCode.InvalidPlatformSetup => GitHubActionsResources.ExitCodeReasonInvalidPlatformSetup,
+            (int)ExitCode.InvalidCommandLine => GitHubActionsResources.ExitCodeReasonInvalidCommandLine,
+            (int)ExitCode.TestHostProcessExitedNonGracefully => GitHubActionsResources.ExitCodeReasonTestHostProcessExitedNonGracefully,
+            (int)ExitCode.ZeroTests => GitHubActionsResources.ExitCodeReasonZeroTests,
+            (int)ExitCode.MinimumExpectedTestsPolicyViolation => GitHubActionsResources.ExitCodeReasonMinimumExpectedTestsPolicyViolation,
+            (int)ExitCode.TestAdapterTestSessionFailure => GitHubActionsResources.ExitCodeReasonTestAdapterTestSessionFailure,
+            (int)ExitCode.DependentProcessExited => GitHubActionsResources.ExitCodeReasonDependentProcessExited,
+            (int)ExitCode.IncompatibleProtocolVersion => GitHubActionsResources.ExitCodeReasonIncompatibleProtocolVersion,
+            (int)ExitCode.TestExecutionStoppedForMaxFailedTests => GitHubActionsResources.ExitCodeReasonTestExecutionStoppedForMaxFailedTests,
+            _ => GitHubActionsResources.ExitCodeReasonUnknown,
+        };
+}

src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsExtensions.cs

@@ -27,6 +27,7 @@ public static void AddGitHubActionsProvider(this ITestApplicationBuilder builder
                 serviceProvider.GetFileSystem(),
                 serviceProvider.GetOutputDevice(),
                 serviceProvider.GetTestApplicationModuleInfo(),
+                serviceProvider.GetTestApplicationProcessExitCode(),
                 serviceProvider.GetLoggerFactory()));
 
         var compositeSlowTestReporter = new CompositeExtensionFactory<GitHubActionsSlowTestReporter>(serviceProvider =>
@@ -46,14 +47,18 @@ public static void AddGitHubActionsProvider(this ITestApplicationBuilder builder
                 serviceProvider.GetTestApplicationModuleInfo(),
                 serviceProvider.GetLoggerFactory()));
 
-        builder.TestHost.AddDataConsumer(serviceProvider =>
+        var compositeAnnotationReporter = new CompositeExtensionFactory<GitHubActionsAnnotationReporter>(serviceProvider =>
             new GitHubActionsAnnotationReporter(
                 serviceProvider.GetCommandLineOptions(),
                 serviceProvider.GetEnvironment(),
                 serviceProvider.GetFileSystem(),
                 serviceProvider.GetOutputDevice(),
+                serviceProvider.GetTestApplicationProcessExitCode(),
                 serviceProvider.GetLoggerFactory()));
 
+        builder.TestHost.AddDataConsumer(compositeAnnotationReporter);
+        builder.TestHost.AddTestSessionLifetimeHandler(compositeAnnotationReporter);
+
         builder.TestHost.AddDataConsumer(compositeSummaryReporter);
         builder.TestHost.AddTestSessionLifetimeHandler(compositeSummaryReporter);
         builder.TestHost.AddDataConsumer(compositeSlowTestReporter);

src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsSummaryReporter.cs

@@ -30,10 +30,19 @@ internal sealed class GitHubActionsSummaryReporter :
     private const int MaxFailures = 20;
     private const int MaxSlowestTests = 10;
 
+    // GITHUB_STEP_SUMMARY is a single shared file that every test-host process appends to. Under a
+    // concurrent multi-assembly `dotnet test` run, contention is resolved by an exclusive-append retry loop
+    // (see AppendStepSummaryWithRetryAsync). Twenty attempts at 50 ms bound the wait to ~1s, which is ample
+    // to serialize the tiny per-assembly writes while still failing fast (into a best-effort warning) on a
+    // genuinely unwritable path.
+    private const int StepSummaryMaxWriteAttempts = 20;
+    private static readonly TimeSpan StepSummaryRetryDelay = TimeSpan.FromMilliseconds(50);
+
     private readonly IEnvironment _environment;
     private readonly IFileSystem _fileSystem;
     private readonly IOutputDevice _outputDevice;
     private readonly ITestApplicationModuleInfo _testApplicationModuleInfo;
+    private readonly ITestApplicationProcessExitCode _testApplicationProcessExitCode;
     private readonly ILogger _logger;
     private readonly Lazy<string> _targetFrameworkMoniker;
     private readonly bool _isEnabled;
@@ -53,12 +62,14 @@ public GitHubActionsSummaryReporter(
         IFileSystem fileSystem,
         IOutputDevice outputDevice,
         ITestApplicationModuleInfo testApplicationModuleInfo,
+        ITestApplicationProcessExitCode testApplicationProcessExitCode,
         ILoggerFactory loggerFactory)
     {
         _environment = environment;
         _fileSystem = fileSystem;
         _outputDevice = outputDevice;
         _testApplicationModuleInfo = testApplicationModuleInfo;
+        _testApplicationProcessExitCode = testApplicationProcessExitCode;
         _logger = loggerFactory.CreateLogger<GitHubActionsSummaryReporter>();
         _targetFrameworkMoniker = new(TargetFrameworkMonikerHelper.GetTargetFrameworkMonikerIncludingPlatform);
         _isEnabled = GitHubActionsFeature.IsEnabled(commandLineOptions, environment, GitHubActionsCommandLineOptions.GitHubActionsStepSummary);
@@ -171,13 +182,11 @@ public async Task OnTestSessionFinishingAsync(ITestSessionContext testSessionCon
                 snapshot = [.. _records.Values];
             }
 
-            string markdown = BuildMarkdown(snapshot, _testApplicationModuleInfo.TryGetAssemblyName() ?? "unknown assembly name", _targetFrameworkMoniker.Value);
+            string markdown = BuildMarkdown(snapshot, _testApplicationModuleInfo.TryGetAssemblyName() ?? "unknown assembly name", _targetFrameworkMoniker.Value, _testApplicationProcessExitCode.GetProcessExitCode());
 
             try
             {
-                using IFileStream stream = _fileSystem.NewFileStream(path!, FileMode.Append, FileAccess.Write, FileShare.ReadWrite);
-                using var writer = new StreamWriter(stream.Stream, new UTF8Encoding(false));
-                await writer.WriteAsync(markdown).ConfigureAwait(false);
+                await AppendStepSummaryWithRetryAsync(_fileSystem, path!, markdown, StepSummaryMaxWriteAttempts, StepSummaryRetryDelay, testSessionContext.CancellationToken).ConfigureAwait(false);
             }
             catch (Exception ex) when (ex is not OperationCanceledException)
             {
@@ -200,7 +209,67 @@ public async Task OnTestSessionFinishingAsync(ITestSessionContext testSessionCon
         }
     }
 
-    internal static /* for testing */ string BuildMarkdown(IReadOnlyList<TestRecord> records, string assemblyName, string targetFrameworkMoniker)
+    /// <summary>
+    /// Appends <paramref name="content"/> to the shared <c>GITHUB_STEP_SUMMARY</c> file in a way that is safe
+    /// when multiple test-host processes (one per assembly / target framework in a <c>dotnet test</c> run) write
+    /// concurrently.
+    /// </summary>
+    /// <remarks>
+    /// <see cref="FileMode.Append"/> only seeks to the end of the file once, at open time, and performs no
+    /// atomic OS-level append. Opening with <see cref="FileShare.ReadWrite"/> would therefore let two processes
+    /// position at the same offset and interleave or overwrite each other's section. We instead open with
+    /// <see cref="FileShare.Read"/> — which denies other writers — so at most one process appends at a time, and
+    /// retry on the resulting sharing violation (an <see cref="IOException"/>) until the holder releases the file.
+    /// Each write is a single small section, so contention clears almost immediately; the bounded attempt count
+    /// still lets a genuinely unlockable file surface as the caller's best-effort warning rather than looping
+    /// forever.
+    /// <para>
+    /// Retries are scoped to <em>acquiring</em> the exclusive append handle only. Once the handle is acquired the
+    /// process appends alone, so contention can no longer occur; a failure that happens <em>during</em> the write
+    /// (e.g. disk full) may already have appended a partial section, and retrying would re-append the full section
+    /// on top of it and corrupt the summary. Such a mid-write failure is therefore propagated straight to the
+    /// caller's best-effort warning path instead of being retried.
+    /// </para>
+    /// </remarks>
+    internal static /* for testing */ async Task AppendStepSummaryWithRetryAsync(
+        IFileSystem fileSystem,
+        string path,
+        string content,
+        int maxAttempts,
+        TimeSpan retryDelay,
+        CancellationToken cancellationToken)
+    {
+        for (int attempt = 1; ; attempt++)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+
+            IFileStream stream;
+            try
+            {
+                stream = fileSystem.NewFileStream(path, FileMode.Append, FileAccess.Write, FileShare.Read);
+            }
+            catch (IOException) when (attempt < maxAttempts)
+            {
+                // Another test-host process currently holds the summary file open for writing. Back off briefly
+                // and retry so this assembly's section is appended intact once the holder releases the file.
+                await Task.Delay(retryDelay, cancellationToken).ConfigureAwait(false);
+                continue;
+            }
+
+            // The exclusive append handle is acquired: from here on we append alone, so any failure is a genuine
+            // write error (not contention) and must not be retried — a partial append followed by a full re-append
+            // would corrupt the summary. Let it propagate to the caller's best-effort warning path.
+            using (stream)
+            using (var writer = new StreamWriter(stream.Stream, new UTF8Encoding(false)))
+            {
+                await writer.WriteAsync(content).ConfigureAwait(false);
+            }
+
+            return;
+        }
+    }
+
+    internal static /* for testing */ string BuildMarkdown(IReadOnlyList<TestRecord> records, string assemblyName, string targetFrameworkMoniker, int exitCode)
     {
         int total = records.Count;
         int passed = 0;
@@ -231,7 +300,10 @@ public async Task OnTestSessionFinishingAsync(ITestSessionContext testSessionCon
             }
         }
 
-        string statusIcon = failed > 0 ? "❌" : "✅";
+        // Reflect the process verdict, not just the failed-test count: a run can end in failure with zero failed
+        // tests (e.g. zero tests discovered or a --minimum-expected-tests violation), which must not show ✅.
+        bool runFailed = failed > 0 || GitHubActionsExitCode.IndicatesFailure(exitCode);
+        string statusIcon = runFailed ? "❌" : "✅";
 
         var builder = new StringBuilder();
         builder.Append("## ").Append(statusIcon).Append(" Test Run Summary — ").Append(assemblyName).Append(" (").Append(targetFrameworkMoniker).Append(")\n\n");
@@ -243,6 +315,21 @@ public async Task OnTestSessionFinishingAsync(ITestSessionContext testSessionCon
             .Append(" | ").Append(skipped.ToString(CultureInfo.InvariantCulture))
             .Append(" | ").Append(FormatDuration(totalDuration)).Append(" |\n\n");
 
+        // Surface a non-test-result failure that this reporter can observe once the session has finished
+        // (zero tests, --minimum-expected-tests, --maximum-failed-tests, test-adapter session failure) as a
+        // GitHub alert callout. Plain pass / at-least-one-failed outcomes are already conveyed by the totals
+        // table and the failures section, so no callout is added for them.
+        if (!GitHubActionsExitCode.IsTestResultOutcome(exitCode))
+        {
+            string calloutText = string.Format(
+                CultureInfo.InvariantCulture,
+                GitHubActionsResources.ExitCodeCallout,
+                exitCode.ToString(CultureInfo.InvariantCulture),
+                GitHubActionsExitCode.GetName(exitCode),
+                GitHubActionsExitCode.GetReason(exitCode));
+            builder.Append("> [!WARNING]\n> ").Append(EscapeInlineCode(calloutText)).Append("\n\n");
+        }
+
         if (failures.Count > 0)
         {
             builder.Append("### ❌ Failures (").Append(failed.ToString(CultureInfo.InvariantCulture)).Append(")\n\n");