Share terminal retry counting model + port SDK oracle test (4/n)

src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs

@@ -19,9 +19,12 @@ public void TestExecutionStarted(DateTimeOffset testStartTime, int workerCount,
     }
 
     public void AssemblyRunStarted(string assembly, string? targetFramework, string? architecture, string executionId, string instanceId)
-        // instanceId: reserved for the SDK orchestrator's instanceId-based retry-counting logic (follow-up PR).
-        // It is not used by the in-process host path, which registers a single assembly per run.
-        => GetOrAddAssemblyRun(assembly, targetFramework, architecture, executionId);
+    {
+        // Each (re-)start registers its instance id as a retry attempt; the first attempt is the normal run. The
+        // in-process host starts a single assembly with a single fixed instance id (one attempt).
+        TestProgressState assemblyRun = GetOrAddAssemblyRun(assembly, targetFramework, architecture, executionId);
+        assemblyRun.NotifyHandshake(instanceId);
+    }
 
     private TestProgressState GetOrAddAssemblyRun(string assembly, string? targetFramework, string? architecture, string executionId)
     {

src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs

@@ -160,15 +160,10 @@ internal void TestDiscovered(string executionId, string displayName)
             throw ApplicationStateGuard.Unreachable();
         }
 
+        // In discovery mode TotalTests is computed from DiscoveredTests; in execution mode it is computed from the
+        // passed/skipped/failed tally as tests complete. So we only need to bump the discovered count here.
         asm.DiscoveredTests++;
 
-        if (_isDiscovery)
-        {
-            // In discovery mode we count discovered tests,
-            // but in execution mode the completion of test will increase the total tests count.
-            asm.TotalTests++;
-        }
-
         asm.DiscoveredTestDisplayNames.Add(MakeControlCharactersVisible(displayName, true));
 
         _terminalWithProgress.UpdateWorker(asm.SlotIndex);

src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.TestCompletion.cs

@@ -25,6 +25,8 @@ internal void TestCompleted(
         FlatException[] flatExceptions = ExceptionFlattener.Flatten(errorMessage, exception);
         TestCompleted(
             executionId,
+            // In-process host: a single attempt, so the instance id is the (fixed) execution id.
+            instanceId: executionId,
             testNodeUid,
             displayName,
             outcome,
@@ -39,9 +41,9 @@ internal void TestCompleted(
 
     /// <summary>
     /// Orchestrator overload (<c>dotnet test</c>): carries the assembly/target-framework/architecture and the
-    /// per-attempt instance id that the multi-process orchestrator knows. Counting and rendering currently delegate
-    /// to the shared per-execution-id path; <paramref name="instanceId"/> (retry attribution) and the per-test
-    /// assembly link are reserved for the retry-counting follow-up.
+    /// per-attempt instance id that the multi-process orchestrator knows. The instance id drives retry attribution
+    /// in <see cref="TestProgressState"/>; assembly/tfm/arch are accepted for signature parity and the future
+    /// per-test assembly link.
     /// </summary>
     internal void TestCompleted(
         string assembly,
@@ -59,11 +61,11 @@ internal void TestCompleted(
         string? actual,
         string? standardOutput,
         string? errorOutput)
-        // assembly / targetFramework / architecture / instanceId are intentionally not forwarded yet: counting and
-        // rendering still go through the shared per-execution-id path. They are reserved for the per-assembly link
-        // and instanceId-based retry attribution added in a follow-up slice (see the XML doc above). Not a bug.
+        // assembly / targetFramework / architecture are intentionally not forwarded yet: they are reserved for the
+        // per-test assembly link in a follow-up. The instance id IS forwarded — it drives retry attribution.
         => TestCompleted(
             executionId,
+            instanceId,
             testNodeUid,
             displayName,
             outcome,
@@ -77,6 +79,7 @@ internal void TestCompleted(
 
     private void TestCompleted(
         string executionId,
+        string instanceId,
         string testNodeUid,
         string displayName,
         TestOutcome outcome,
@@ -104,16 +107,13 @@ private void TestCompleted(
             case TestOutcome.Timeout:
             case TestOutcome.Canceled:
             case TestOutcome.Fail:
-                asm.FailedTests++;
-                asm.TotalTests++;
+                asm.ReportFailedTest(testNodeUid, instanceId);
                 break;
             case TestOutcome.Passed:
-                asm.PassedTests++;
-                asm.TotalTests++;
+                asm.ReportPassingTest(testNodeUid, instanceId);
                 break;
             case TestOutcome.Skipped:
-                asm.SkippedTests++;
-                asm.TotalTests++;
+                asm.ReportSkippedTest(testNodeUid, instanceId);
                 break;
         }
 

src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs

@@ -1,14 +1,31 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// 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.CodeAnalysis;
 using Microsoft.Testing.Platform.Helpers;
 
+using TestNodeInfoEntry = (int Passed, int Skipped, int Failed, int LastAttemptNumber);
+
 namespace Microsoft.Testing.Platform.OutputDevice.Terminal;
 
 [Embedded]
 internal sealed class TestProgressState
 {
+    // THREADING: this type is intentionally not internally synchronized. Each TestProgressState instance is owned by
+    // a single executionId; the reporter looks it up from a ConcurrentDictionary (_assemblies), but the Microsoft
+    // Testing Platform message pipeline delivers events for a given executionId on a single consumer, so the mutating
+    // members below (the dictionary/list and the Passed/Skipped/Failed/Retried/TryCount counters) are only ever
+    // touched by one thread at a time for a given instance. Do not call these members concurrently for the same
+    // assembly without adding synchronization.
+
+    // Tracks the per-test-node tally and the attempt it belongs to, so retries (which re-report the same test node
+    // uid under a new instance id) replace rather than double-count the earlier attempt's result.
+    private readonly Dictionary<string, TestNodeInfoEntry> _testUidToResults = [];
+
+    // Ordered list of instance ids seen for this assembly. Each new instance id is a retry attempt. In most runs
+    // there is exactly one (no retry).
+    private readonly List<string> _orderedInstanceIds = [];
+
     public TestProgressState(long id, string assembly, string? targetFramework, string? architecture, IStopwatch stopwatch, bool isDiscovery)
     {
         Id = id;
@@ -33,15 +50,17 @@ public TestProgressState(long id, string assembly, string? targetFramework, stri
 
     public int DiscoveredTests { get; internal set; }
 
-    public int FailedTests { get; internal set; }
+    public int FailedTests { get; private set; }
 
-    public int PassedTests { get; internal set; }
+    public int PassedTests { get; private set; }
 
-    public int SkippedTests { get; internal set; }
+    public int SkippedTests { get; private set; }
 
-    public int RetriedFailedTests { get; internal set; }
+    /// <summary>Gets the number of tests whose earlier-attempt failure was superseded by a later retry; rendered as the "/r{N}" segment.</summary>
+    public int RetriedFailedTests { get; private set; }
 
-    public int TotalTests { get; internal set; }
+    /// <summary>Gets the total number of tests: the discovered count in discovery mode, otherwise the live passed/skipped/failed tally.</summary>
+    public int TotalTests => IsDiscovery ? DiscoveredTests : PassedTests + SkippedTests + FailedTests;
 
     public TestNodeResultsState? TestNodeResultsState { get; internal set; }
 
@@ -57,4 +76,85 @@ public TestProgressState(long id, string assembly, string? targetFramework, stri
 
     /// <summary>Gets or sets a value indicating whether the assembly run completed successfully (set by the orchestrator on completion).</summary>
     public bool Success { get; internal set; }
+
+    /// <summary>Gets the number of attempts (handshakes) seen for this assembly; greater than 1 indicates retries.</summary>
+    public int TryCount { get; private set; }
+
+    public void ReportPassingTest(string testNodeUid, string instanceId)
+        => ReportGenericTestResult(testNodeUid, instanceId, static entry => entry with { Passed = entry.Passed + 1 }, static @this => @this.PassedTests++);
+
+    public void ReportSkippedTest(string testNodeUid, string instanceId)
+        => ReportGenericTestResult(testNodeUid, instanceId, static entry => entry with { Skipped = entry.Skipped + 1 }, static @this => @this.SkippedTests++);
+
+    public void ReportFailedTest(string testNodeUid, string instanceId)
+        => ReportGenericTestResult(testNodeUid, instanceId, static entry => entry with { Failed = entry.Failed + 1 }, static @this => @this.FailedTests++);
+
+    /// <summary>
+    /// Registers a handshake for the given <paramref name="instanceId"/>. A previously unseen instance id is a new
+    /// retry attempt and bumps <see cref="TryCount"/>; re-seeing the current attempt is a no-op.
+    /// </summary>
+    internal void NotifyHandshake(string instanceId)
+    {
+        int index = _orderedInstanceIds.IndexOf(instanceId);
+        if (index < 0)
+        {
+            _orderedInstanceIds.Add(instanceId);
+            TryCount++;
+        }
+        else if (index != _orderedInstanceIds.Count - 1)
+        {
+            // We received a handshake for an instance id that is not the most recent one — unexpected ordering.
+            throw ApplicationStateGuard.Unreachable();
+        }
+    }
+
+    private void ReportGenericTestResult(
+        string testNodeUid,
+        string instanceId,
+        Func<TestNodeInfoEntry, TestNodeInfoEntry> incrementTestNodeInfoEntry,
+        Action<TestProgressState> incrementCountAction)
+    {
+        int currentAttemptNumber = GetAttemptNumberFromInstanceId(instanceId);
+
+        if (_testUidToResults.TryGetValue(testNodeUid, out TestNodeInfoEntry value))
+        {
+            if (value.LastAttemptNumber == currentAttemptNumber)
+            {
+                // Another result for the same test node in the same attempt — just increment.
+                _testUidToResults[testNodeUid] = incrementTestNodeInfoEntry(value);
+            }
+            else if (currentAttemptNumber > value.LastAttemptNumber)
+            {
+                // Retry: discard the previous attempt's contribution to the live tally and re-count this attempt.
+                RetriedFailedTests += value.Failed;
+                PassedTests -= value.Passed;
+                SkippedTests -= value.Skipped;
+                FailedTests -= value.Failed;
+                _testUidToResults[testNodeUid] = incrementTestNodeInfoEntry((Passed: 0, Skipped: 0, Failed: 0, LastAttemptNumber: currentAttemptNumber));
+            }
+            else
+            {
+                // A result for an attempt older than the latest one we saw — unexpected ordering.
+                throw ApplicationStateGuard.Unreachable();
+            }
+        }
+        else
+        {
+            _testUidToResults.Add(testNodeUid, incrementTestNodeInfoEntry((Passed: 0, Skipped: 0, Failed: 0, LastAttemptNumber: currentAttemptNumber)));
+        }
+
+        incrementCountAction(this);
+    }
+
+    private int GetAttemptNumberFromInstanceId(string instanceId)
+    {
+        int index = _orderedInstanceIds.IndexOf(instanceId);
+        if (index < 0)
+        {
+            throw ApplicationStateGuard.Unreachable();
+        }
+
+        // Attempt numbers are 1-based.
+        return index + 1;
+    }
 }

src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.SessionLifecycle.cs

@@ -87,6 +87,10 @@ public async Task DisplayBeforeSessionStartAsync(CancellationToken cancellationT
         // Start test execution here, rather than in ShowBanner, because then we know
         // if we are a testHost controller or not, and if we should show progress bar.
         _terminalTestReporter.TestExecutionStarted(_clock.UtcNow, workerCount: 1, isDiscovery: _isListTests, isHelp: false, isRetry: false);
+
+        // In-process host contract: pass instanceId == executionId (both InProcessExecutionId). The in-process
+        // TestCompleted overload forwards the executionId as the instanceId (a single fixed attempt), so the
+        // attempt-number lookup only resolves if AssemblyRunStarted registered that same instance id here.
         _terminalTestReporter.AssemblyRunStarted(_assemblyName, _targetFramework, _shortArchitecture, InProcessExecutionId, InProcessExecutionId);
         if (_logger is not null && _logger.IsEnabled(LogLevel.Trace))
         {

test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/SilenceDrivenHeartbeatRendererTests.cs

@@ -224,11 +224,20 @@ public void OnWrite_DoesNotEraseOrRenderProgress_JustWrites()
 
     private static TestProgressState CreateAssembly(FakeClock clock, int totalTests, int failedTests, string? activeTestName)
     {
-        var asm = new TestProgressState(1, "MyAcceptance.dll", "net9.0", "x64", clock.CreateStopwatch(), isDiscovery: false)
+        var asm = new TestProgressState(1, "MyAcceptance.dll", "net9.0", "x64", clock.CreateStopwatch(), isDiscovery: false);
+
+        // Counts are now derived from reported per-test results (to support retry de-duplication), so seed them by
+        // reporting the requested number of failed + passed tests under a single attempt.
+        asm.NotifyHandshake("inst-1");
+        for (int i = 0; i < failedTests; i++)
         {
-            TotalTests = totalTests,
-            FailedTests = failedTests,
-        };
+            asm.ReportFailedTest($"fail-{i}", "inst-1");
+        }
+
+        for (int i = 0; i < totalTests - failedTests; i++)
+        {
+            asm.ReportPassingTest($"pass-{i}", "inst-1");
+        }
 
         if (activeTestName is not null)
         {

test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs

@@ -1426,6 +1426,41 @@ public void TestExecutionCompleted_WithMultipleAssemblies_PrintsPerAssemblyCount
         Assert.Contains(ExpectedCounts(5, 0, 2), GetAssemblySummaryLine(output, assemblyB));
     }
 
+    // Ported from the dotnet/sdk TerminalTestReporterTests: when an assembly's tests were retried, the per-assembly
+    // summary appends a "/r{N}" segment so the user can tell the final counts came from retries. Attempt 1 fails the
+    // test; attempt 2 (a new instance id under the same execution id) passes it, so the final tally is 1 passed with
+    // 1 retried.
+    [TestMethod]
+    public void AssemblyRunCompleted_WhenTestsWereRetried_ShowsRetriedCount()
+    {
+        var stringBuilderConsole = new StringBuilderConsole();
+        var terminalReporter = new TerminalTestReporter(stringBuilderConsole, new TerminalTestReporterOptions
+        {
+            AnsiMode = AnsiMode.NoAnsi,
+            ShowProgress = () => false,
+            ShowAssembly = true,
+            ShowAssemblyStartAndComplete = true,
+        });
+
+        terminalReporter.TestExecutionStarted(DateTimeOffset.MinValue, workerCount: 1, isDiscovery: false, isHelp: false, isRetry: true);
+
+        string assembly = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? @"C:\repo\Flaky.Tests.dll" : "/repo/Flaky.Tests.dll";
+        const string executionId = "exec-flaky";
+
+        // Attempt 1: register the first instance and report a failure.
+        terminalReporter.AssemblyRunStarted(assembly, "net9.0", "x64", executionId, instanceId: "inst-1");
+        ReportOrchestratorTest(terminalReporter, assembly, executionId, instanceId: "inst-1", testUid: "flaky-1", TestOutcome.Fail);
+
+        // Attempt 2: a new instance id triggers a retry; the failing test now passes.
+        terminalReporter.AssemblyRunStarted(assembly, "net9.0", "x64", executionId, instanceId: "inst-2");
+        ReportOrchestratorTest(terminalReporter, assembly, executionId, instanceId: "inst-2", testUid: "flaky-1", TestOutcome.Passed);
+
+        terminalReporter.AssemblyRunCompleted(executionId, exitCode: 0, outputData: null, errorData: null);
+
+        string assemblyLine = GetAssemblySummaryLine(stringBuilderConsole.Output, assembly);
+        Assert.Contains(ExpectedCounts(1, 0, 0, retried: 1), assemblyLine);
+    }
+
     private static void ReportOrchestratorTest(TerminalTestReporter reporter, string assembly, string executionId, string instanceId, string testUid, TestOutcome outcome)
         => reporter.TestCompleted(
             assembly,
@@ -1456,6 +1491,9 @@ private static string GetAssemblySummaryLine(string output, string assemblyPath)
     private static string ExpectedCounts(int passed, int failed, int skipped)
         => $"[+{passed.ToString(CultureInfo.CurrentCulture)}/x{failed.ToString(CultureInfo.CurrentCulture)}/?{skipped.ToString(CultureInfo.CurrentCulture)}]";
 
+    private static string ExpectedCounts(int passed, int failed, int skipped, int retried)
+        => $"[+{passed.ToString(CultureInfo.CurrentCulture)}/x{failed.ToString(CultureInfo.CurrentCulture)}/?{skipped.ToString(CultureInfo.CurrentCulture)}/r{retried.ToString(CultureInfo.CurrentCulture)}]";
+
     [TestMethod]
     public void TerminalTestReporter_WhenReusedAcrossSessions_DoesNotLeakArtifactsOrCancelledState()
     {