diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs index cd166daa21..71df06f231 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs +++ b/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) { diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs index fde66bace1..061f4201c4 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs +++ b/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); diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.TestCompletion.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.TestCompletion.cs index 0c8c7b33c4..54bc468198 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.TestCompletion.cs +++ b/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( /// /// Orchestrator overload (dotnet test): 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; (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 ; assembly/tfm/arch are accepted for signature parity and the future + /// per-test assembly link. /// 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; } diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs index a12976b2c5..afce870335 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs +++ b/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 _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 _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; } + /// Gets the number of tests whose earlier-attempt failure was superseded by a later retry; rendered as the "/r{N}" segment. + public int RetriedFailedTests { get; private set; } - public int TotalTests { get; internal set; } + /// Gets the total number of tests: the discovered count in discovery mode, otherwise the live passed/skipped/failed tally. + 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 /// Gets or sets a value indicating whether the assembly run completed successfully (set by the orchestrator on completion). public bool Success { get; internal set; } + + /// Gets the number of attempts (handshakes) seen for this assembly; greater than 1 indicates retries. + 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++); + + /// + /// Registers a handshake for the given . A previously unseen instance id is a new + /// retry attempt and bumps ; re-seeing the current attempt is a no-op. + /// + 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 incrementTestNodeInfoEntry, + Action 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; + } } diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.SessionLifecycle.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.SessionLifecycle.cs index 823d11e523..10e88d8e92 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.SessionLifecycle.cs +++ b/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)) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/SilenceDrivenHeartbeatRendererTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/SilenceDrivenHeartbeatRendererTests.cs index 20736cc01f..13d554b9c0 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/SilenceDrivenHeartbeatRendererTests.cs +++ b/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) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs index 55a3d5f5ab..59127af9aa 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs +++ b/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() {