From ad71b4e8348c4b73dc9f0e5b8d6ecf9865d7d533 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 14:36:19 +0000 Subject: [PATCH 1/4] Initial plan From 65bd75e6c5f4a397d34af2a1a472b4c20f9116e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 14:46:01 +0000 Subject: [PATCH 2/4] Remove all line number references from comments Co-authored-by: nohwnd <5735905+nohwnd@users.noreply.github.com> --- .../Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs | 2 +- .../Microsoft.Testing.Platform/IPC/NamedPipeServer.cs | 4 ++-- .../Messages/ChannelConsumerDataProcessor.cs | 2 +- .../Messages/ConsumingEnumerableConsumerDataProcessor.cs | 2 +- .../TestFramework.UnitTests/Assertions/AssertTests.That.cs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs index a1c54583bb..df7db6d961 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs @@ -214,7 +214,7 @@ public async Task OrchestrateTestHostExecutionAsync(CancellationToken cance #if NETCOREAPP await retryFailedTestsPipeServer.WaitForConnectionAsync(linkedToken2.Token).ConfigureAwait(false); #else - // We don't know why but if the cancellation is called quickly in line 171 for netfx we stuck sometime here, like if + // We don't know why but if the cancellation is called quickly after adding the pipe name to finalArguments for netfx we stuck sometime here, like if // the token we pass to the named pipe is not "correctly" verified inside the pipe implementation self. // We fallback with our custom agnostic cancellation mechanism in that case. // We see it happen only in .NET FX and not in .NET Core so for now we don't do it for core. diff --git a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs index 3cd3559467..f4825c0c75 100644 --- a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs +++ b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs @@ -309,7 +309,7 @@ public void Dispose() // This is unexpected and we throw an exception. ApplicationStateGuard.Ensure(_loopTask is not null); - // To close gracefully we need to ensure that the client closed the stream line 103. + // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method. if (!_loopTask.Wait(TimeoutHelper.DefaultHangTimeSpanTimeout)) { throw new InvalidOperationException(string.Format( @@ -340,7 +340,7 @@ public async ValueTask DisposeAsync() try { - // To close gracefully we need to ensure that the client closed the stream line 103. + // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method. await _loopTask.WaitAsync(TimeoutHelper.DefaultHangTimeSpanTimeout, _cancellationToken).ConfigureAwait(false); } catch (TimeoutException) diff --git a/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs b/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs index 4e453b5e0d..698cc237ee 100644 --- a/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs +++ b/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs @@ -77,7 +77,7 @@ private async Task ConsumeAsync() catch (Exception ex) when (ex is not OperationCanceledException) { // If we're draining before to increment the _totalPayloadProcessed we need to signal that we should throw because - // it's possible we have a race condition where the payload check at line 106 return false and the current task is not yet in a + // it's possible we have a race condition where the CompleteAddingAsync payload check returns false and the current task is not yet in a // "faulted state". _consumerState.SetException(ex); diff --git a/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs b/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs index 37f997e7c1..3c4827f5fc 100644 --- a/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs +++ b/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs @@ -66,7 +66,7 @@ private async Task ConsumeAsync() catch (Exception ex) when (ex is not OperationCanceledException) { // If we're draining before to increment the _totalPayloadProcessed we need to signal that we should throw because - // it's possible we have a race condition where the payload check at line 106 return false and the current task is not yet in a + // it's possible we have a race condition where the IsAddingCompleted payload check returns false and the current task is not yet in a // "faulted state". _consumerState.SetException(ex); diff --git a/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs b/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs index 00377cbfda..f6f28145c2 100644 --- a/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs +++ b/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs @@ -667,7 +667,7 @@ public void That_NonGenericCollection_FormatsCorrectly() public void That_NonGenericCollectionInComparison_FormatsCorrectly() { - // Arrange - Using ArrayList in a comparison to trigger FormatValue on line 421 + // Arrange - Using ArrayList in a comparison to trigger FormatValue method var arrayList = new ArrayList { 1, 2, 3 }; int[] expectedItems = [1, 2, 3, 4]; From 5b80b9d753a82206f8893386a9837c1b1dd58608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Fri, 7 Nov 2025 10:43:44 +0100 Subject: [PATCH 3/4] Apply suggestions from code review --- .../Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs | 2 +- .../Microsoft.Testing.Platform/IPC/NamedPipeServer.cs | 4 ++-- .../Messages/ChannelConsumerDataProcessor.cs | 2 +- .../Messages/ConsumingEnumerableConsumerDataProcessor.cs | 2 +- .../TestFramework.UnitTests/Assertions/AssertTests.That.cs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs index df7db6d961..668f0b4cb7 100644 --- a/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs +++ b/src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs @@ -214,7 +214,7 @@ public async Task OrchestrateTestHostExecutionAsync(CancellationToken cance #if NETCOREAPP await retryFailedTestsPipeServer.WaitForConnectionAsync(linkedToken2.Token).ConfigureAwait(false); #else - // We don't know why but if the cancellation is called quickly after adding the pipe name to finalArguments for netfx we stuck sometime here, like if + // We don't know why but if the cancellation is called quickly in `testHostProcess.Exited`: `processExitedCancellationToken.Cancel();` for netfx we stuck sometime here, like if // the token we pass to the named pipe is not "correctly" verified inside the pipe implementation self. // We fallback with our custom agnostic cancellation mechanism in that case. // We see it happen only in .NET FX and not in .NET Core so for now we don't do it for core. diff --git a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs index f4825c0c75..095082034b 100644 --- a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs +++ b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs @@ -309,7 +309,7 @@ public void Dispose() // This is unexpected and we throw an exception. ApplicationStateGuard.Ensure(_loopTask is not null); - // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method. + // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method (there is comment `// The client has disconnected`). if (!_loopTask.Wait(TimeoutHelper.DefaultHangTimeSpanTimeout)) { throw new InvalidOperationException(string.Format( @@ -340,7 +340,7 @@ public async ValueTask DisposeAsync() try { - // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method. + // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method (there is comment `// The client has disconnected`). await _loopTask.WaitAsync(TimeoutHelper.DefaultHangTimeSpanTimeout, _cancellationToken).ConfigureAwait(false); } catch (TimeoutException) diff --git a/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs b/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs index 698cc237ee..2a2378420e 100644 --- a/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs +++ b/src/Platform/Microsoft.Testing.Platform/Messages/ChannelConsumerDataProcessor.cs @@ -77,7 +77,7 @@ private async Task ConsumeAsync() catch (Exception ex) when (ex is not OperationCanceledException) { // If we're draining before to increment the _totalPayloadProcessed we need to signal that we should throw because - // it's possible we have a race condition where the CompleteAddingAsync payload check returns false and the current task is not yet in a + // it's possible we have a race condition where the payload counting in DrainDataAsync returns false and the current task is not yet in a // "faulted state". _consumerState.SetException(ex); diff --git a/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs b/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs index 3c4827f5fc..13f580a8da 100644 --- a/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs +++ b/src/Platform/Microsoft.Testing.Platform/Messages/ConsumingEnumerableConsumerDataProcessor.cs @@ -66,7 +66,7 @@ private async Task ConsumeAsync() catch (Exception ex) when (ex is not OperationCanceledException) { // If we're draining before to increment the _totalPayloadProcessed we need to signal that we should throw because - // it's possible we have a race condition where the IsAddingCompleted payload check returns false and the current task is not yet in a + // it's possible we have a race condition where the payload counting in DrainDataAsync returns false and the current task is not yet in a // "faulted state". _consumerState.SetException(ex); diff --git a/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs b/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs index f6f28145c2..180aa357c3 100644 --- a/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs +++ b/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs @@ -667,7 +667,7 @@ public void That_NonGenericCollection_FormatsCorrectly() public void That_NonGenericCollectionInComparison_FormatsCorrectly() { - // Arrange - Using ArrayList in a comparison to trigger FormatValue method + // Arrange - Using ArrayList in a comparison to trigger FormatValue that formats enumerables var arrayList = new ArrayList { 1, 2, 3 }; int[] expectedItems = [1, 2, 3, 4]; From e51cb73f6d4f07b8a136449d25f883717229ec18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Fri, 7 Nov 2025 14:25:55 +0100 Subject: [PATCH 4/4] Apply suggestion from @nohwnd --- src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs index 095082034b..9552011de3 100644 --- a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs +++ b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs @@ -340,7 +340,7 @@ public async ValueTask DisposeAsync() try { - // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method (there is comment `// The client has disconnected`). + // To close gracefully we need to ensure that the client closed the stream in the InternalLoopAsync method (there is comment `// The client has disconnected`). await _loopTask.WaitAsync(TimeoutHelper.DefaultHangTimeSpanTimeout, _cancellationToken).ConfigureAwait(false); } catch (TimeoutException)