From c7288818a8b18dacb309cd159fd7cb52e53e82b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 28 May 2026 12:35:42 +0200 Subject: [PATCH 1/2] Show currently running tests in dotnet test MTP output (#49712) Adds a new TestInProgressMessages IPC message type (serializer id 10) so that the test platform can notify the SDK when a test starts. The SDK already has rendering machinery for in-progress tests (TestProgressState, TestNodeResultsState, AnsiTerminalTestProgressFrame, including the '... and X more' trimming) but it was dead because the protocol carried no 'test started' event and ShowActiveTests was never enabled. Changes: * IPC layer: new TestInProgressMessages model + TLV serializer, registered with id 10. Back-compat is preserved by the TLV format (unknown field ids are skipped) and NamedPipeServer skipUnknownMessages, so old SDKs paired with new MTPs continue to work and vice-versa. * Handler: TestApplicationHandler.OnTestInProgressReceived forwards each event to TerminalTestReporter.TestInProgress(assembly, tfm, arch, executionId, instanceId, uid, displayName). * State: TestNodeResultsState now keys running tests by (instanceId, uid) and tracks a completed set, so a stale in-progress notification arriving after the matching test result cannot resurrect a 'running' entry. Retries (new instanceId) are still surfaced correctly. * Default-on: ShowActiveTests is enabled when AnsiMode == AnsiIfPossible and progress is not suppressed, so it stays off in CI, --no-ansi, --no-progress and LLM environments. * Tests: round-trip + serializer id assertion for the new serializer, plus stale-add suppression / retry handling for TestNodeResultsState. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../MTP/IPC/Models/TestInProgressMessages.cs | 8 + .../Commands/Test/MTP/IPC/ObjectFieldIds.cs | 15 ++ .../IPC/Serializers/RegisterSerializers.cs | 2 + .../TestInProgressMessagesSerializer.cs | 164 ++++++++++++++++++ .../MicrosoftTestingPlatformTestCommand.cs | 1 + .../Test/MTP/Terminal/TerminalTestReporter.cs | 12 +- .../Test/MTP/Terminal/TestNodeResultsState.cs | 25 ++- .../Commands/Test/MTP/TestApplication.cs | 7 + .../Test/MTP/TestApplicationHandler.cs | 54 ++++++ .../TestInProgressMessagesSerializerTests.cs | 66 +++++++ .../Test/TestNodeResultsStateTests.cs | 67 +++++++ 11 files changed, 415 insertions(+), 6 deletions(-) create mode 100644 src/Cli/dotnet/Commands/Test/MTP/IPC/Models/TestInProgressMessages.cs create mode 100644 src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/TestInProgressMessagesSerializer.cs create mode 100644 test/dotnet.Tests/CommandTests/Test/TestInProgressMessagesSerializerTests.cs create mode 100644 test/dotnet.Tests/CommandTests/Test/TestNodeResultsStateTests.cs diff --git a/src/Cli/dotnet/Commands/Test/MTP/IPC/Models/TestInProgressMessages.cs b/src/Cli/dotnet/Commands/Test/MTP/IPC/Models/TestInProgressMessages.cs new file mode 100644 index 000000000000..4ac698cd40f4 --- /dev/null +++ b/src/Cli/dotnet/Commands/Test/MTP/IPC/Models/TestInProgressMessages.cs @@ -0,0 +1,8 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.DotNet.Cli.Commands.Test.IPC.Models; + +internal sealed record TestInProgressMessage(string? Uid, string? DisplayName); + +internal sealed record TestInProgressMessages(string? ExecutionId, string? InstanceId, TestInProgressMessage[] InProgressMessages) : IRequest; diff --git a/src/Cli/dotnet/Commands/Test/MTP/IPC/ObjectFieldIds.cs b/src/Cli/dotnet/Commands/Test/MTP/IPC/ObjectFieldIds.cs index 9ac5ba9943ab..ab35b986166b 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/IPC/ObjectFieldIds.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/IPC/ObjectFieldIds.cs @@ -133,3 +133,18 @@ internal static class HandshakeMessageFieldsId { public const int MessagesSerializerId = 9; } + +internal static class TestInProgressMessagesFieldsId +{ + public const int MessagesSerializerId = 10; + + public const ushort ExecutionId = 1; + public const ushort InstanceId = 2; + public const ushort TestInProgressMessageList = 3; +} + +internal static class TestInProgressMessageFieldsId +{ + public const ushort Uid = 1; + public const ushort DisplayName = 2; +} diff --git a/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/RegisterSerializers.cs b/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/RegisterSerializers.cs index bb0ae35f03c5..7e9240e76c50 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/RegisterSerializers.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/RegisterSerializers.cs @@ -18,6 +18,7 @@ namespace Microsoft.DotNet.Cli.Commands.Test.IPC.Serializers; * FileArtifactMessageSerializer: 7 * TestSessionEventSerializer: 8 * HandshakeMessageSerializer: 9 + * TestInProgressMessagesSerializer: 10 */ internal static class RegisterSerializers @@ -31,5 +32,6 @@ public static void RegisterAllSerializers(this NamedPipeBase namedPipeBase) namedPipeBase.RegisterSerializer(new FileArtifactMessagesSerializer(), typeof(FileArtifactMessages)); namedPipeBase.RegisterSerializer(new TestSessionEventSerializer(), typeof(TestSessionEvent)); namedPipeBase.RegisterSerializer(new HandshakeMessageSerializer(), typeof(HandshakeMessage)); + namedPipeBase.RegisterSerializer(new TestInProgressMessagesSerializer(), typeof(TestInProgressMessages)); } } diff --git a/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/TestInProgressMessagesSerializer.cs b/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/TestInProgressMessagesSerializer.cs new file mode 100644 index 000000000000..868e45a226cd --- /dev/null +++ b/src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/TestInProgressMessagesSerializer.cs @@ -0,0 +1,164 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using Microsoft.DotNet.Cli.Commands.Test.IPC.Models; + +namespace Microsoft.DotNet.Cli.Commands.Test.IPC.Serializers; + +/* +|---FieldCount---| 2 bytes + +|---ExecutionId Id---| (2 bytes) +|---ExecutionId Size---| (4 bytes) +|---ExecutionId Value---| (n bytes) + +|---InstanceId Id---| (2 bytes) +|---InstanceId Size---| (4 bytes) +|---InstanceId Value---| (n bytes) + +|---TestInProgressMessageList Id---| (2 bytes) +|---TestInProgressMessageList Size---| (4 bytes) +|---TestInProgressMessageList Value---| (n bytes) + |---TestInProgressMessageList Length---| (4 bytes) + + |---TestInProgressMessageList[0] FieldCount---| 2 bytes + + |---TestInProgressMessageList[0].Uid Id---| (2 bytes) + |---TestInProgressMessageList[0].Uid Size---| (4 bytes) + |---TestInProgressMessageList[0].Uid Value---| (n bytes) + + |---TestInProgressMessageList[0].DisplayName Id---| (2 bytes) + |---TestInProgressMessageList[0].DisplayName Size---| (4 bytes) + |---TestInProgressMessageList[0].DisplayName Value---| (n bytes) +*/ + +internal sealed class TestInProgressMessagesSerializer : BaseSerializer, INamedPipeSerializer +{ + public int Id => TestInProgressMessagesFieldsId.MessagesSerializerId; + + public object Deserialize(Stream stream) + { + string? executionId = null; + string? instanceId = null; + List? inProgressMessages = null; + + ushort fieldCount = ReadUShort(stream); + + for (int i = 0; i < fieldCount; i++) + { + int fieldId = ReadUShort(stream); + int fieldSize = ReadInt(stream); + + switch (fieldId) + { + case TestInProgressMessagesFieldsId.ExecutionId: + executionId = ReadStringValue(stream, fieldSize); + break; + + case TestInProgressMessagesFieldsId.InstanceId: + instanceId = ReadStringValue(stream, fieldSize); + break; + + case TestInProgressMessagesFieldsId.TestInProgressMessageList: + inProgressMessages = ReadInProgressMessagesPayload(stream); + break; + + default: + // If we don't recognize the field id, skip the payload corresponding to that field + SetPosition(stream, stream.Position + fieldSize); + break; + } + } + + return new TestInProgressMessages(executionId, instanceId, inProgressMessages is null ? [] : [.. inProgressMessages]); + } + + private static List ReadInProgressMessagesPayload(Stream stream) + { + List inProgressMessages = []; + + int length = ReadInt(stream); + for (int i = 0; i < length; i++) + { + string? uid = null, displayName = null; + + int fieldCount = ReadUShort(stream); + + for (int j = 0; j < fieldCount; j++) + { + int fieldId = ReadUShort(stream); + int fieldSize = ReadInt(stream); + + switch (fieldId) + { + case TestInProgressMessageFieldsId.Uid: + uid = ReadStringValue(stream, fieldSize); + break; + + case TestInProgressMessageFieldsId.DisplayName: + displayName = ReadStringValue(stream, fieldSize); + break; + + default: + SetPosition(stream, stream.Position + fieldSize); + break; + } + } + + inProgressMessages.Add(new TestInProgressMessage(uid, displayName)); + } + + return inProgressMessages; + } + + public void Serialize(object objectToSerialize, Stream stream) + { + Debug.Assert(stream.CanSeek, "We expect a seekable stream."); + + var inProgressMessages = (TestInProgressMessages)objectToSerialize; + + WriteUShort(stream, GetFieldCount(inProgressMessages)); + + WriteField(stream, TestInProgressMessagesFieldsId.ExecutionId, inProgressMessages.ExecutionId); + WriteField(stream, TestInProgressMessagesFieldsId.InstanceId, inProgressMessages.InstanceId); + WriteInProgressMessagesPayload(stream, inProgressMessages.InProgressMessages); + } + + private static void WriteInProgressMessagesPayload(Stream stream, TestInProgressMessage[]? inProgressMessageList) + { + if (inProgressMessageList is null || inProgressMessageList.Length == 0) + { + return; + } + + WriteUShort(stream, TestInProgressMessagesFieldsId.TestInProgressMessageList); + + // We will reserve an int (4 bytes) + // so that we fill the size later, once we write the payload + WriteInt(stream, 0); + + long before = stream.Position; + WriteInt(stream, inProgressMessageList.Length); + foreach (TestInProgressMessage inProgressMessage in inProgressMessageList) + { + WriteUShort(stream, GetFieldCount(inProgressMessage)); + + WriteField(stream, TestInProgressMessageFieldsId.Uid, inProgressMessage.Uid); + WriteField(stream, TestInProgressMessageFieldsId.DisplayName, inProgressMessage.DisplayName); + } + + // NOTE: We are able to seek only if we are using a MemoryStream + // thus, the seek operation is fast as we are only changing the value of a property + WriteAtPosition(stream, (int)(stream.Position - before), before - sizeof(int)); + } + + private static ushort GetFieldCount(TestInProgressMessages inProgressMessages) => + (ushort)((inProgressMessages.ExecutionId is null ? 0 : 1) + + (inProgressMessages.InstanceId is null ? 0 : 1) + + (IsNullOrEmpty(inProgressMessages.InProgressMessages) ? 0 : 1)); + + private static ushort GetFieldCount(TestInProgressMessage inProgressMessage) => + (ushort)((inProgressMessage.Uid is null ? 0 : 1) + + (inProgressMessage.DisplayName is null ? 0 : 1)); +} diff --git a/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs b/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs index 9f253581c518..ca0a8abfc6f0 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs @@ -94,6 +94,7 @@ private static TerminalTestReporter InitializeOutput(int degreeOfParallelism, Pa { ShowPassedTests = showPassedTests, ShowProgress = !noProgress, + ShowActiveTests = !noProgress && ansiMode == AnsiMode.AnsiIfPossible, AnsiMode = ansiMode, ShowAssembly = true, ShowAssemblyStartAndComplete = true, diff --git a/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs b/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs index f560e340fa24..bcdcce26264d 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs @@ -409,7 +409,7 @@ internal void TestCompleted( if (_options.ShowActiveTests) { - asm.TestNodeResultsState?.RemoveRunningTestNode(testNodeUid); + asm.TestNodeResultsState?.RemoveRunningTestNode(instanceId, testNodeUid); } switch (outcome) @@ -950,9 +950,13 @@ private static TerminalColor ToTerminalColor(ConsoleColor consoleColor) }; public void TestInProgress( + string assembly, + string? targetFramework, + string? architecture, + string executionId, + string instanceId, string testNodeUid, - string displayName, - string executionId) + string displayName) { TestProgressState asm = _assemblies[executionId]; @@ -960,7 +964,7 @@ public void TestInProgress( { asm.TestNodeResultsState ??= new(Interlocked.Increment(ref _counter)); asm.TestNodeResultsState.AddRunningTestNode( - Interlocked.Increment(ref _counter), testNodeUid, displayName, CreateStopwatch()); + Interlocked.Increment(ref _counter), instanceId, testNodeUid, displayName, CreateStopwatch()); } _terminalWithProgress.UpdateWorker(asm.SlotIndex); diff --git a/src/Cli/dotnet/Commands/Test/MTP/Terminal/TestNodeResultsState.cs b/src/Cli/dotnet/Commands/Test/MTP/Terminal/TestNodeResultsState.cs index 268225e0f192..9f1ea9db140e 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/Terminal/TestNodeResultsState.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/Terminal/TestNodeResultsState.cs @@ -12,12 +12,33 @@ internal sealed class TestNodeResultsState(long id) private readonly TestDetailState _summaryDetail = new(id, stopwatch: null, text: string.Empty); private readonly ConcurrentDictionary _testNodeProgressStates = new(); + private readonly ConcurrentDictionary _completed = new(); public int Count => _testNodeProgressStates.Count; - public void AddRunningTestNode(int id, string uid, string name, IStopwatch stopwatch) => _testNodeProgressStates[uid] = new TestDetailState(id, stopwatch, name); + public void AddRunningTestNode(int id, string instanceId, string uid, string name, IStopwatch stopwatch) + { + string key = MakeKey(instanceId, uid); + + // Guard against stale "in-progress" notifications that arrive after the + // test already completed. Without this we could surface a "running" + // entry that will never be removed. + if (_completed.ContainsKey(key)) + { + return; + } + + _testNodeProgressStates[key] = new TestDetailState(id, stopwatch, name); + } + + public void RemoveRunningTestNode(string instanceId, string uid) + { + string key = MakeKey(instanceId, uid); + _completed[key] = 0; + _testNodeProgressStates.TryRemove(key, out _); + } - public void RemoveRunningTestNode(string uid) => _testNodeProgressStates.TryRemove(uid, out _); + private static string MakeKey(string instanceId, string uid) => $"{instanceId}\u0000{uid}"; public IEnumerable GetRunningTasks(int maxCount) { diff --git a/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs b/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs index 645e86d57782..d7128790c0e7 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs @@ -273,6 +273,10 @@ private Task OnRequest(NamedPipeServer server, IRequest request) OnFileArtifactMessages(fileArtifactMessages); break; + case TestInProgressMessages testInProgressMessages: + OnTestInProgressMessages(testInProgressMessages); + break; + case TestSessionEvent sessionEvent: OnSessionEvent(sessionEvent); break; @@ -363,6 +367,9 @@ private void OnTestResultMessages(TestResultMessages testResultMessage) private void OnFileArtifactMessages(FileArtifactMessages fileArtifactMessages) => _handler.OnFileArtifactsReceived(fileArtifactMessages); + private void OnTestInProgressMessages(TestInProgressMessages testInProgressMessages) + => _handler.OnTestInProgressReceived(testInProgressMessages); + private void OnSessionEvent(TestSessionEvent sessionEvent) => _handler.OnSessionEventReceived(sessionEvent); diff --git a/src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs b/src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs index 778bb269aee2..7e412e92db15 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs @@ -169,6 +169,40 @@ internal void OnTestResultsReceived(TestResultMessages testResultMessage) } } + internal void OnTestInProgressReceived(TestInProgressMessages testInProgressMessages) + { + LogTestInProgress(testInProgressMessages); + + if (_options.IsHelp) + { + throw new InvalidOperationException(string.Format(CliCommandStrings.UnexpectedMessageInHelpMode, nameof(TestInProgressMessages))); + } + + if (!_handshakeInfo.HasValue) + { + throw new InvalidOperationException(string.Format(CliCommandStrings.UnexpectedMessageWithoutHandshake, nameof(TestInProgressMessages))); + } + + if (testInProgressMessages.ExecutionId != _handshakeInfo.Value.ExecutionId) + { + // Received 'ExecutionId' of value '{0}' for message '{1}' while the 'ExecutionId' received of the handshake message was '{2}'. + throw new InvalidOperationException(string.Format(CliCommandStrings.DotnetTestMismatchingExecutionId, testInProgressMessages.ExecutionId, nameof(TestInProgressMessages), _handshakeInfo.Value.ExecutionId)); + } + + var handshakeInfo = _handshakeInfo.Value; + foreach (TestInProgressMessage inProgressMessage in testInProgressMessages.InProgressMessages) + { + _output.TestInProgress( + _module.TargetPath, + handshakeInfo.TargetFramework, + handshakeInfo.Architecture, + handshakeInfo.ExecutionId, + testInProgressMessages.InstanceId!, + inProgressMessage.Uid!, + inProgressMessage.DisplayName!); + } + } + internal void OnFileArtifactsReceived(FileArtifactMessages fileArtifactMessages) { LogFileArtifacts(fileArtifactMessages); @@ -360,6 +394,26 @@ private static void LogTestResults(TestResultMessages testResultMessages) Logger.LogTrace(logMessageBuilder, static logMessageBuilder => logMessageBuilder.ToString()); } + private static void LogTestInProgress(TestInProgressMessages testInProgressMessages) + { + if (!Logger.TraceEnabled) + { + return; + } + + var logMessageBuilder = new StringBuilder(); + + logMessageBuilder.AppendLine($"TestInProgress Execution Id: {testInProgressMessages.ExecutionId}"); + logMessageBuilder.AppendLine($"TestInProgress Instance Id: {testInProgressMessages.InstanceId}"); + + foreach (TestInProgressMessage inProgressMessage in testInProgressMessages.InProgressMessages) + { + logMessageBuilder.AppendLine($"TestInProgress: {inProgressMessage.Uid}, {inProgressMessage.DisplayName}"); + } + + Logger.LogTrace(logMessageBuilder, static logMessageBuilder => logMessageBuilder.ToString()); + } + private static void LogFileArtifacts(FileArtifactMessages fileArtifactMessages) { if (!Logger.TraceEnabled) diff --git a/test/dotnet.Tests/CommandTests/Test/TestInProgressMessagesSerializerTests.cs b/test/dotnet.Tests/CommandTests/Test/TestInProgressMessagesSerializerTests.cs new file mode 100644 index 000000000000..3457ab783789 --- /dev/null +++ b/test/dotnet.Tests/CommandTests/Test/TestInProgressMessagesSerializerTests.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.DotNet.Cli.Commands.Test.IPC.Models; +using Microsoft.DotNet.Cli.Commands.Test.IPC.Serializers; + +namespace dotnet.Tests.CommandTests.Test; + +public class TestInProgressMessagesSerializerTests +{ + [Fact] + public void RoundTrips_WithPopulatedMessages() + { + var serializer = new TestInProgressMessagesSerializer(); + var original = new TestInProgressMessages( + ExecutionId: "exec-123", + InstanceId: "inst-456", + InProgressMessages: + [ + new TestInProgressMessage("uid-1", "DisplayName1"), + new TestInProgressMessage("uid-2", "DisplayName2"), + ]); + + using var stream = new MemoryStream(); + serializer.Serialize(original, stream); + stream.Position = 0; + + var deserialized = (TestInProgressMessages)serializer.Deserialize(stream); + + Assert.Equal(original.ExecutionId, deserialized.ExecutionId); + Assert.Equal(original.InstanceId, deserialized.InstanceId); + Assert.Equal(2, deserialized.InProgressMessages.Length); + Assert.Equal("uid-1", deserialized.InProgressMessages[0].Uid); + Assert.Equal("DisplayName1", deserialized.InProgressMessages[0].DisplayName); + Assert.Equal("uid-2", deserialized.InProgressMessages[1].Uid); + Assert.Equal("DisplayName2", deserialized.InProgressMessages[1].DisplayName); + } + + [Fact] + public void RoundTrips_WithEmptyMessagesList() + { + var serializer = new TestInProgressMessagesSerializer(); + var original = new TestInProgressMessages( + ExecutionId: "exec-123", + InstanceId: "inst-456", + InProgressMessages: []); + + using var stream = new MemoryStream(); + serializer.Serialize(original, stream); + stream.Position = 0; + + var deserialized = (TestInProgressMessages)serializer.Deserialize(stream); + + Assert.Equal(original.ExecutionId, deserialized.ExecutionId); + Assert.Equal(original.InstanceId, deserialized.InstanceId); + Assert.Empty(deserialized.InProgressMessages); + } + + [Fact] + public void SerializerId_IsTen() + { + // The IPC protocol reserves serializer IDs across SDK and MTP. + // ID 10 is the contract — keep this assertion to prevent accidental changes. + Assert.Equal(10, new TestInProgressMessagesSerializer().Id); + } +} diff --git a/test/dotnet.Tests/CommandTests/Test/TestNodeResultsStateTests.cs b/test/dotnet.Tests/CommandTests/Test/TestNodeResultsStateTests.cs new file mode 100644 index 000000000000..04aa1be6e7c6 --- /dev/null +++ b/test/dotnet.Tests/CommandTests/Test/TestNodeResultsStateTests.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.DotNet.Cli.Commands.Test.Terminal; + +namespace dotnet.Tests.CommandTests.Test; + +public class TestNodeResultsStateTests +{ + [Fact] + public void AddRunningTestNode_AfterRemove_IsSuppressed() + { + // Simulates the stale-add race: the producer may emit an in-progress + // notification for a test that already completed. The state must not + // resurrect the test in the "running" list. + var state = new TestNodeResultsState(id: 1); + const string instanceId = "instance-A"; + const string uid = "Foo"; + + state.AddRunningTestNode(id: 100, instanceId, uid, "Foo", new FakeStopwatch()); + Assert.Equal(1, state.Count); + + state.RemoveRunningTestNode(instanceId, uid); + Assert.Equal(0, state.Count); + + // Stale in-progress arriving after completion must be ignored. + state.AddRunningTestNode(id: 101, instanceId, uid, "Foo", new FakeStopwatch()); + Assert.Equal(0, state.Count); + } + + [Fact] + public void AddRunningTestNode_DifferentInstance_SameUid_NotSuppressed() + { + // Retries use a new instanceId. A previous instance completing must + // not prevent the new instance from showing as running. + var state = new TestNodeResultsState(id: 1); + const string uid = "Foo"; + + state.AddRunningTestNode(id: 100, "instance-A", uid, "Foo", new FakeStopwatch()); + state.RemoveRunningTestNode("instance-A", uid); + Assert.Equal(0, state.Count); + + state.AddRunningTestNode(id: 200, "instance-B", uid, "Foo", new FakeStopwatch()); + Assert.Equal(1, state.Count); + } + + [Fact] + public void AddRunningTestNode_DistinctTests_AllTracked() + { + var state = new TestNodeResultsState(id: 1); + + state.AddRunningTestNode(id: 100, "instance-A", "Test1", "Test1", new FakeStopwatch()); + state.AddRunningTestNode(id: 101, "instance-A", "Test2", "Test2", new FakeStopwatch()); + state.AddRunningTestNode(id: 102, "instance-B", "Test1", "Test1", new FakeStopwatch()); + + Assert.Equal(3, state.Count); + } + + private sealed class FakeStopwatch : IStopwatch + { + public TimeSpan Elapsed => TimeSpan.Zero; + + public void Start() { } + + public void Stop() { } + } +} From 8b09e8f62487511ab8ace7583902d7d98e47e8d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 29 May 2026 09:11:17 +0200 Subject: [PATCH 2/2] Address PR review: gate active-test tracking on effective progress capability The ShowActiveTests option is set based on the requested AnsiMode, but TerminalTestReporter may decide at runtime to disable progress (e.g. when stdout is redirected, console is non-TTY, or ANSI codes aren't accepted). In that case the incoming TestInProgress events would still allocate and update TestNodeResultsState even though nothing can ever be rendered. Compute an effective _showActiveTests in the ctor as _options.ShowActiveTests && showProgress and use it to gate both the AddRunningTestNode and RemoveRunningTestNode paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Test/MTP/Terminal/TerminalTestReporter.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs b/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs index bcdcce26264d..2200b3c4d2e2 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs @@ -36,6 +36,14 @@ internal sealed partial class TerminalTestReporter : IDisposable private readonly TestProgressStateAwareTerminal _terminalWithProgress; + /// + /// Whether to track and render currently running tests. Gated on both the caller-requested + /// and the effective progress + /// capability of the console: if progress cannot actually be rendered (e.g. redirected stdout, + /// non-TTY, or ANSI not supported) there is no point allocating per-test running-state. + /// + private readonly bool _showActiveTests; + private int _handshakeFailuresCount; private readonly uint? _originalConsoleMode; @@ -94,6 +102,7 @@ public TerminalTestReporter(IConsole console, TerminalTestReporterOptions option } _terminalWithProgress = new TestProgressStateAwareTerminal(terminal, showProgress); + _showActiveTests = _options.ShowActiveTests && showProgress; } public void TestExecutionStarted(DateTimeOffset testStartTime, int workerCount, bool isDiscovery, bool isHelp, bool isRetry) @@ -407,7 +416,7 @@ internal void TestCompleted( TestProgressState asm = _assemblies[executionId]; var attempt = asm.TryCount; - if (_options.ShowActiveTests) + if (_showActiveTests) { asm.TestNodeResultsState?.RemoveRunningTestNode(instanceId, testNodeUid); } @@ -960,7 +969,7 @@ public void TestInProgress( { TestProgressState asm = _assemblies[executionId]; - if (_options.ShowActiveTests) + if (_showActiveTests) { asm.TestNodeResultsState ??= new(Interlocked.Increment(ref _counter)); asm.TestNodeResultsState.AddRunningTestNode(