More 'dotnet test' cleanups (#50596)

Author: Youssef1313

src/Cli/dotnet/Commands/CliCommandStrings.resx

@@ -2663,4 +2663,7 @@ Proceed?</value>
   <data name="DotnetTestPipeFailureWithoutHandshake" xml:space="preserve">
     <value>Error disposing 'NamedPipeServer', and no handshake was found.</value>
   </data>
-</root>
+  <data name="DotnetTestIncompatibleHandshakeVersion" xml:space="preserve">
+    <value>Supported protocol versions sent by Microsoft.Testing.Platform are '{0}'. The SDK supports '{1}', which is incompatible.</value>
+  </data>
+</root>

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

@@ -65,7 +65,10 @@ internal static class HandshakeMessagePropertyNames
 
 internal static class ProtocolConstants
 {
-    internal const string Version = "1.0.0";
+    /// <summary>
+    /// The protocol versions that are supported by the current SDK. Multiple versions can be present and be semicolon separated.
+    /// </summary>
+    internal const string SupportedVersions = "1.0.0";
 }
 
 internal static class ProjectProperties

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

@@ -5,19 +5,20 @@
 
 namespace Microsoft.DotNet.Cli.Commands.Test;
 
-internal class HandshakeArgs : EventArgs
+internal sealed class HandshakeArgs : EventArgs
 {
     public Handshake Handshake { get; set; }
+    public bool GotSupportedVersion { get; set; }
 }
 
-internal class HelpEventArgs : EventArgs
+internal sealed class HelpEventArgs : EventArgs
 {
     public string ModulePath { get; set; }
 
     public CommandLineOption[] CommandLineOptions { get; set; }
 }
 
-internal class DiscoveredTestEventArgs : EventArgs
+internal sealed class DiscoveredTestEventArgs : EventArgs
 {
     public string ExecutionId { get; set; }
 
@@ -26,7 +27,7 @@ internal class DiscoveredTestEventArgs : EventArgs
     public DiscoveredTest[] DiscoveredTests { get; set; }
 }
 
-internal class TestResultEventArgs : EventArgs
+internal sealed class TestResultEventArgs : EventArgs
 {
     public string ExecutionId { get; set; }
 
@@ -37,7 +38,7 @@ internal class TestResultEventArgs : EventArgs
     public FailedTestResult[] FailedTestResults { get; set; }
 }
 
-internal class FileArtifactEventArgs : EventArgs
+internal sealed class FileArtifactEventArgs : EventArgs
 {
     public string ExecutionId { get; set; }
 
@@ -46,25 +47,19 @@ internal class FileArtifactEventArgs : EventArgs
     public FileArtifact[] FileArtifacts { get; set; }
 }
 
-internal class SessionEventArgs : EventArgs
+internal sealed class SessionEventArgs : EventArgs
 {
     public TestSession SessionEvent { get; set; }
 }
 
-internal class ErrorEventArgs : EventArgs
+internal sealed class ErrorEventArgs : EventArgs
 {
     public string ErrorMessage { get; set; }
 }
 
-internal class TestProcessExitEventArgs : EventArgs
+internal sealed class TestProcessExitEventArgs : EventArgs
 {
     public List<string> OutputData { get; set; }
     public List<string> ErrorData { get; set; }
     public int ExitCode { get; set; }
 }
-
-internal class ExecutionEventArgs : EventArgs
-{
-    public string ModulePath { get; set; }
-    public string ExecutionId { get; set; }
-}

src/Cli/dotnet/Commands/Test/IPC/NamedPipeServer.cs

@@ -30,7 +30,7 @@ public NamedPipeServer(
         CancellationToken cancellationToken,
         bool skipUnknownMessages)
     {
-        _namedPipeServerStream = new((PipeName = pipeNameDescription).Name, PipeDirection.InOut, maxNumberOfServerInstances, PipeTransmissionMode.Byte, PipeOptions.Asynchronous);
+        _namedPipeServerStream = new((PipeName = pipeNameDescription).Name, PipeDirection.InOut, maxNumberOfServerInstances, PipeTransmissionMode.Byte, PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly);
         _callback = callback;
         _cancellationToken = cancellationToken;
         _skipUnknownMessages = skipUnknownMessages;

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

@@ -184,8 +184,9 @@ private Task<IResponse> OnRequest(NamedPipeServer server, IRequest request)
             {
                 case HandshakeMessage handshakeMessage:
                     _handshakes.Add(server, handshakeMessage);
-                    OnHandshakeMessage(handshakeMessage);
-                    return Task.FromResult((IResponse)CreateHandshakeMessage(GetSupportedProtocolVersion(handshakeMessage)));
+                    string negotiatedVersion = GetSupportedProtocolVersion(handshakeMessage);
+                    OnHandshakeMessage(handshakeMessage, negotiatedVersion.Length > 0);
+                    return Task.FromResult((IResponse)CreateHandshakeMessage(negotiatedVersion));
 
                 case CommandLineOptionMessages commandLineOptionMessages:
                     OnCommandLineOptionMessages(commandLineOptionMessages);
@@ -235,15 +236,26 @@ private Task<IResponse> OnRequest(NamedPipeServer server, IRequest request)
 
     private static string GetSupportedProtocolVersion(HandshakeMessage handshakeMessage)
     {
-        handshakeMessage.Properties.TryGetValue(HandshakeMessagePropertyNames.SupportedProtocolVersions, out string protocolVersions);
+        if (!handshakeMessage.Properties.TryGetValue(HandshakeMessagePropertyNames.SupportedProtocolVersions, out string protocolVersions) ||
+            protocolVersions is null)
+        {
+            // It's not expected we hit this.
+            // TODO: Maybe we should fail more hard?
+            return string.Empty;
+        }
 
-        string version = string.Empty;
-        if (protocolVersions is not null && protocolVersions.Split(";").Contains(ProtocolConstants.Version))
+        // NOTE: Today, ProtocolConstants.Version is only 1.0.0 (i.e, SDK supports only a single version).
+        // Whenever we support multiple versions in SDK, we should do intersection
+        // between protocolVersions given by MTP, and the versions supported by SDK.
+        // Then we return the "highest" version from the intersection.
+        // The current logic **assumes** that ProtocolConstants.SupportedVersions is a single version.
+        if (protocolVersions.Split(";").Contains(ProtocolConstants.SupportedVersions))
         {
-            version = ProtocolConstants.Version;
+            return ProtocolConstants.SupportedVersions;
         }
 
-        return version;
+        // The version given by MTP is not supported by SDK.
+        return string.Empty;
     }
 
     private static HandshakeMessage CreateHandshakeMessage(string version) =>
@@ -304,9 +316,9 @@ private bool ModulePathExists()
         return true;
     }
 
-    public void OnHandshakeMessage(HandshakeMessage handshakeMessage)
+    public void OnHandshakeMessage(HandshakeMessage handshakeMessage, bool gotSupportedVersion)
     {
-        HandshakeReceived?.Invoke(this, new HandshakeArgs { Handshake = new Handshake(handshakeMessage.Properties) });
+        HandshakeReceived?.Invoke(this, new HandshakeArgs { Handshake = new Handshake(handshakeMessage.Properties), GotSupportedVersion = gotSupportedVersion });
     }
 
     public void OnCommandLineOptionMessages(CommandLineOptionMessages commandLineOptionMessages)

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

@@ -18,12 +18,17 @@ internal sealed class TestApplicationsEventHandlers(TerminalTestReporter output)
     public void OnHandshakeReceived(object sender, HandshakeArgs args)
     {
         var testApplication = (TestApplication)sender;
-        // Today, it's 1.0.0 in MTP.
-        // https://github.com/microsoft/testfx/blob/516eebb3c9b7e81eb2677c00b3d0b7867d8acb33/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs#L40
-        var supportedProtocolVersions = args.Handshake.Properties[HandshakeMessagePropertyNames.SupportedProtocolVersions];
-        if (supportedProtocolVersions != "1.0.0" && !supportedProtocolVersions.Split(';').Contains("1.0.0"))
+        if (!args.GotSupportedVersion)
         {
-            _output.HandshakeFailure(testApplication.Module.TargetPath, string.Empty, ExitCode.GenericFailure, $"Supported protocol versions '{supportedProtocolVersions}' doesn't include '1.0.0' which is not supported by the current .NET SDK.", string.Empty);
+            _output.HandshakeFailure(
+                testApplication.Module.TargetPath,
+                string.Empty,
+                ExitCode.GenericFailure,
+                string.Format(
+                    CliCommandStrings.DotnetTestIncompatibleHandshakeVersion,
+                    args.Handshake.Properties[HandshakeMessagePropertyNames.SupportedProtocolVersions],
+                    ProtocolConstants.SupportedVersions),
+                string.Empty);
         }
 
         var hostType = args.Handshake.Properties[HandshakeMessagePropertyNames.HostType];