TerminalTestReporter: Use executionId as dictionary key (#50134)

Author: Youssef1313

src/Cli/dotnet/Commands/Test/Terminal/TerminalTestReporter.cs

@@ -45,6 +45,8 @@ internal event EventHandler OnProgressStopUpdate
 
     private readonly TestProgressStateAwareTerminal _terminalWithProgress;
 
+    private int _handshakeFailuresCount;
+
     private readonly uint? _originalConsoleMode;
     private bool _isDiscovery;
     private bool _isHelp;
@@ -59,6 +61,8 @@ internal event EventHandler OnProgressStopUpdate
 
     private bool? _shouldShowPassedTests;
 
+    public bool HasHandshakeFailure => _handshakeFailuresCount > 0;
+
 #if NET7_0_OR_GREATER
     // Specifying no timeout, the regex is linear. And the timeout does not measure the regex only, but measures also any
     // thread suspends, so the regex gets blamed incorrectly.
@@ -168,7 +172,7 @@ public void TestExecutionStarted(DateTimeOffset testStartTime, int workerCount,
         _terminalWithProgress.StartShowingProgress(workerCount);
     }
 
-    public void AssemblyRunStarted(string assembly, string? targetFramework, string? architecture, string? executionId)
+    public void AssemblyRunStarted(string assembly, string? targetFramework, string? architecture, string executionId)
     {
         var assemblyRun = GetOrAddAssemblyRun(assembly, targetFramework, architecture, executionId);
         assemblyRun.TryCount++;
@@ -197,10 +201,9 @@ public void AssemblyRunStarted(string assembly, string? targetFramework, string?
         }
     }
 
-    private TestProgressState GetOrAddAssemblyRun(string assembly, string? targetFramework, string? architecture, string? executionId)
+    private TestProgressState GetOrAddAssemblyRun(string assembly, string? targetFramework, string? architecture, string executionId)
     {
-        string key = $"{assembly}|{targetFramework}|{architecture}|{executionId}";
-        return _assemblies.GetOrAdd(key, _ =>
+        return _assemblies.GetOrAdd(executionId, _ =>
         {
             IStopwatch sw = CreateStopwatch();
             var assemblyRun = new TestProgressState(Interlocked.Increment(ref _counter), assembly, targetFramework, architecture, sw);
@@ -280,7 +283,7 @@ private void AppendTestRunSummary(ITerminal terminal, int? exitCode)
         bool notEnoughTests = totalTests < _options.MinimumExpectedTests;
         bool allTestsWereSkipped = totalTests == 0 || totalTests == totalSkippedTests;
         bool anyTestFailed = totalFailedTests > 0;
-        bool anyAssemblyFailed = _assemblies.Values.Any(a => !a.Success);
+        bool anyAssemblyFailed = _assemblies.Values.Any(a => !a.Success) || HasHandshakeFailure;
         bool runFailed = anyAssemblyFailed || anyTestFailed || notEnoughTests || allTestsWereSkipped || _wasCancelled;
         terminal.SetColor(runFailed ? TerminalColor.DarkRed : TerminalColor.DarkGreen);
 
@@ -335,7 +338,7 @@ private void AppendTestRunSummary(ITerminal terminal, int? exitCode)
         int passed = _assemblies.Values.Sum(t => t.PassedTests);
         int skipped = _assemblies.Values.Sum(t => t.SkippedTests);
         int retried = _assemblies.Values.Sum(t => t.RetriedFailedTests);
-        int error = _assemblies.Values.Sum(t => !t.Success && (t.TotalTests == 0 || t.FailedTests == 0) ? 1 : 0);
+        int error = _assemblies.Values.Sum(t => !t.Success && (t.TotalTests == 0 || t.FailedTests == 0) ? 1 : 0) + _handshakeFailuresCount;
         TimeSpan runDuration = _testExecutionStartTime != null && _testExecutionEndTime != null ? (_testExecutionEndTime - _testExecutionStartTime).Value : TimeSpan.Zero;
 
         bool colorizeFailed = failed > 0;
@@ -454,7 +457,7 @@ internal void TestCompleted(
         string assembly,
         string? targetFramework,
         string? architecture,
-        string? executionId,
+        string executionId,
         string instanceId,
         string testNodeUid,
         string displayName,
@@ -467,7 +470,7 @@ internal void TestCompleted(
         string? standardOutput,
         string? errorOutput)
     {
-        TestProgressState asm = _assemblies[$"{assembly}|{targetFramework}|{architecture}|{executionId}"];
+        TestProgressState asm = _assemblies[executionId];
         var attempt = asm.TryCount;
 
         if (_options.ShowActiveTests)
@@ -797,12 +800,12 @@ private static void AppendIndentedLine(ITerminal terminal, string? message, stri
         }
     }
 
-    internal void AssemblyRunCompleted(string assembly, string? targetFramework, string? architecture, string? executionId,
+    internal void AssemblyRunCompleted(string executionId,
         // These parameters are useful only for "remote" runs in dotnet test, where we are reporting on multiple processes.
         // In single process run, like with testing platform .exe we report these via messages, and run exit.
-        int? exitCode, string? outputData, string? errorData)
+        int exitCode, string? outputData, string? errorData)
     {
-        TestProgressState assemblyRun = GetOrAddAssemblyRun(assembly, targetFramework, architecture, executionId);
+        TestProgressState assemblyRun = _assemblies[executionId];
         assemblyRun.ExitCode = exitCode;
         assemblyRun.Success = exitCode == 0 && assemblyRun.FailedTests == 0;
         assemblyRun.Stopwatch.Stop();
@@ -814,7 +817,7 @@ internal void AssemblyRunCompleted(string assembly, string? targetFramework, str
             _terminalWithProgress.WriteToTerminal(terminal => AppendAssemblySummary(assemblyRun, terminal));
         }
 
-        if (exitCode is null or 0)
+        if (exitCode == 0)
         {
             // Report nothing, we don't want to report on success, because then we will also report on test-discovery etc.
             return;
@@ -826,6 +829,22 @@ internal void AssemblyRunCompleted(string assembly, string? targetFramework, str
         });
     }
 
+    internal void HandshakeFailure(string assemblyPath, string targetFramework, int exitCode, string outputData, string errorData)
+    {
+        Interlocked.Increment(ref _handshakeFailuresCount);
+        _terminalWithProgress.WriteToTerminal(terminal =>
+        {
+            terminal.ResetColor();
+            AppendAssemblyLinkTargetFrameworkAndArchitecture(terminal, assemblyPath, targetFramework, architecture: null);
+            terminal.Append(' ');
+            terminal.SetColor(TerminalColor.DarkRed);
+            terminal.Append(CliCommandStrings.ZeroTestsRan);
+            terminal.ResetColor();
+            terminal.AppendLine();
+            AppendExecutableSummary(terminal, exitCode, outputData, errorData);
+        });
+    }
+
     private static void AppendExecutableSummary(ITerminal terminal, int? exitCode, string? outputData, string? errorData)
     {
         terminal.Append(CliCommandStrings.ExitCode);
@@ -936,11 +955,11 @@ internal void TestDiscovered(
         string assembly,
         string? targetFramework,
         string? architecture,
-        string? executionId,
+        string executionId,
         string? displayName,
         string? uid)
     {
-        TestProgressState asm = _assemblies[$"{assembly}|{targetFramework}|{architecture}|{executionId}"];
+        TestProgressState asm = _assemblies[executionId];
 
         // TODO: add mode for discovered tests to the progress bar - jajares
         asm.DiscoverTest(displayName, uid);
@@ -1028,9 +1047,9 @@ public void TestInProgress(
         string testNodeUid,
         string instanceId,
         string displayName,
-        string? executionId)
+        string executionId)
     {
-        TestProgressState asm = _assemblies[$"{assembly}|{targetFramework}|{architecture}|{executionId}"];
+        TestProgressState asm = _assemblies[executionId];
 
         if (_options.ShowActiveTests)
         {

src/Cli/dotnet/Commands/Test/TestApplicationEventHandlers.cs

@@ -13,6 +13,8 @@ internal sealed class TestApplicationsEventHandlers(TerminalTestReporter output)
     private readonly ConcurrentDictionary<TestApplication, (string ModulePath, string TargetFramework, string Architecture, string ExecutionId)> _executions = new();
     private readonly TerminalTestReporter _output = output;
 
+    public bool HasHandshakeFailure => _output.HasHandshakeFailure;
+
     public void OnHandshakeReceived(object sender, HandshakeArgs args)
     {
         var hostType = args.Handshake.Properties[HandshakeMessagePropertyNames.HostType];
@@ -145,11 +147,11 @@ public void OnTestProcessExited(object sender, TestProcessExitEventArgs args)
 
         if (_executions.TryGetValue(testApplication, out var appInfo))
         {
-            _output.AssemblyRunCompleted(appInfo.ModulePath, appInfo.TargetFramework, appInfo.Architecture, appInfo.ExecutionId, args.ExitCode, string.Join(Environment.NewLine, args.OutputData), string.Join(Environment.NewLine, args.ErrorData));
+            _output.AssemblyRunCompleted(appInfo.ExecutionId, args.ExitCode, string.Join(Environment.NewLine, args.OutputData), string.Join(Environment.NewLine, args.ErrorData));
         }
         else
         {
-            _output.AssemblyRunCompleted(testApplication.Module.TargetPath ?? testApplication.Module.ProjectFullPath, testApplication.Module.TargetFramework, architecture: null, null, args.ExitCode, string.Join(Environment.NewLine, args.OutputData), string.Join(Environment.NewLine, args.ErrorData));
+            _output.HandshakeFailure(testApplication.Module.TargetPath ?? testApplication.Module.ProjectFullPath, testApplication.Module.TargetFramework, args.ExitCode, string.Join(Environment.NewLine, args.OutputData), string.Join(Environment.NewLine, args.ErrorData));
         }
 
         LogTestProcessExit(args);

src/Cli/dotnet/Commands/Test/TestingPlatformCommand.cs

@@ -80,7 +80,9 @@ private int RunInternal(ParseResult parseResult)
         }
 
         _actionQueue.EnqueueCompleted();
-        return _actionQueue.WaitAllActions();
+        var exitCode = _actionQueue.WaitAllActions();
+        // Don't inline exitCode variable. We want to always call WaitAllActions first.
+        return _eventHandlers.HasHandshakeFailure ? ExitCode.GenericFailure : exitCode;
     }
 
     private void PrepareEnvironment(ParseResult parseResult, out TestOptions testOptions, out int degreeOfParallelism)