Handshake from test host orchestrator in dotnet test pipe protocol (#6179)

docs/Changelog-Platform.md

@@ -34,6 +34,7 @@ See full log [of v4.2.3...v4.3.0](https://github.com/microsoft/testfx/compare/v4
 * Add --progress {auto|on|off} and deprecate --no-progress in MTP terminal reporter by @Evangelink in [#9145](https://github.com/microsoft/testfx/pull/9145)
 * Add silence-driven progress heartbeat for SimpleAnsi/NoAnsi output modes by @Evangelink in [#9147](https://github.com/microsoft/testfx/pull/9147)
 * Add Azure DevOps per-assembly log groups by @Evangelink in [#9177](https://github.com/microsoft/testfx/pull/9177)
+* Handshake from the test host orchestrator in the dotnet test pipe protocol, advertising the orchestration feature (e.g. retry) by @Copilot in [#9215](https://github.com/microsoft/testfx/pull/9215)
 
 ### Fixed
 

src/Platform/Microsoft.Testing.Extensions.TrxReport/Microsoft.Testing.Extensions.TrxReport.csproj

@@ -29,6 +29,7 @@
     <Compile Include="$(RepoRoot)src\Platform\Microsoft.Testing.Platform\OutputDevice\TargetFrameworkParser.cs" Link="Helpers\TargetFrameworkParser.cs" />
     <Compile Include="$(RepoRoot)src\Platform\SharedExtensionHelpers\ReportFileNameHelper.cs" Link="Helpers\ReportFileNameHelper.cs" />
     <Compile Include="$(RepoRoot)src\Platform\SharedExtensionHelpers\ReportFileNameSanitizer.cs" Link="Helpers\ReportFileNameSanitizer.cs" />
+    <Compile Include="$(RepoRoot)src\Platform\SharedExtensionHelpers\ReportGeneratorCommandLineBase.cs" Link="Helpers\ReportGeneratorCommandLineBase.cs" />
     <Compile Include="$(RepoRoot)src\Platform\SharedExtensionHelpers\ReportFileNameValidator.cs" Link="Helpers\ReportFileNameValidator.cs" />
     <Compile Include="$(RepoRoot)src\Platform\SharedExtensionHelpers\ReportFileWriterHelper.cs" Link="Helpers\ReportFileWriterHelper.cs" />
     <Compile Include="$(RepoRoot)src\Platform\SharedExtensionHelpers\TargetFrameworkMonikerHelper.cs" Link="Helpers\TargetFrameworkMonikerHelper.cs" />

src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxCommandLine.cs

@@ -2,46 +2,29 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using Microsoft.Testing.Extensions.TrxReport.Resources;
-using Microsoft.Testing.Platform.CommandLine;
-using Microsoft.Testing.Platform.Extensions;
-using Microsoft.Testing.Platform.Extensions.CommandLine;
 
 namespace Microsoft.Testing.Extensions.TrxReport.Abstractions;
 
-internal sealed class TrxReportGeneratorCommandLine : CommandLineOptionsProviderBase
+internal sealed class TrxReportGeneratorCommandLine : ReportGeneratorCommandLineBase
 {
     public const string TrxReportOptionName = "report-trx";
     public const string TrxReportFileNameOptionName = "report-trx-filename";
 
     public TrxReportGeneratorCommandLine()
         : base(
             nameof(TrxReportGeneratorCommandLine),
-            ExtensionVersion.DefaultSemVer,
             ExtensionResources.TrxReportGeneratorDisplayName,
             ExtensionResources.TrxReportGeneratorDescription,
-            [
-                new(TrxReportOptionName, ExtensionResources.TrxReportOptionDescription, ArgumentArity.Zero, false),
-                new(TrxReportFileNameOptionName, ExtensionResources.TrxReportFileNameOptionDescription, ArgumentArity.ExactlyOne, false)
-            ])
-    {
-    }
-
-    public override Task<ValidationResult> ValidateOptionArgumentsAsync(CommandLineOption commandOption, string[] arguments)
-        => commandOption.Name == TrxReportFileNameOptionName
-            ? ReportFileNameValidator.ValidateReportFileNameArgumentAsync(
-                arguments,
-                ".trx",
-                ExtensionResources.TrxReportFileNameMustNotBeEmpty,
-                ExtensionResources.TrxReportFileNameExtensionIsNotTrx,
-                ExtensionResources.TrxReportFileNameRelativePathMustStayUnderResultsDirectory)
-            : ValidationResult.ValidTask;
-
-    public override Task<ValidationResult> ValidateCommandLineOptionsAsync(ICommandLineOptions commandLineOptions)
-        => ReportFileNameValidator.ValidateReportCommandLineOptionsAsync(
-            commandLineOptions,
             TrxReportOptionName,
             TrxReportFileNameOptionName,
+            ExtensionResources.TrxReportOptionDescription,
+            ExtensionResources.TrxReportFileNameOptionDescription,
+            ".trx",
+            ExtensionResources.TrxReportFileNameMustNotBeEmpty,
+            ExtensionResources.TrxReportFileNameExtensionIsNotTrx,
+            ExtensionResources.TrxReportFileNameRelativePathMustStayUnderResultsDirectory,
             ExtensionResources.TrxReportFileNameRequiresTrxReport,
-            ExtensionResources.TrxReportIsNotValidForDiscovery,
-            PlatformCommandLineProvider.DiscoverTestsOptionKey);
+            ExtensionResources.TrxReportIsNotValidForDiscovery)
+    {
+    }
 }

src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs

@@ -5,6 +5,7 @@
 using Microsoft.Testing.Platform.Extensions.TestFramework;
 using Microsoft.Testing.Platform.Extensions.TestHost;
 using Microsoft.Testing.Platform.Helpers;
+using Microsoft.Testing.Platform.IPC;
 using Microsoft.Testing.Platform.Logging;
 using Microsoft.Testing.Platform.Messages;
 using Microsoft.Testing.Platform.OutputDevice;
@@ -111,15 +112,17 @@ public async Task<int> RunAsync()
     protected virtual string HostType
         => this switch
         {
-            ConsoleTestHost => "TestHost",
-            TestHostControllersTestHost => "TestHostController",
-            ServerTestHost => "ServerTestHost",
+            ConsoleTestHost => HandshakeMessageHostTypes.TestHost,
+            TestHostControllersTestHost => HandshakeMessageHostTypes.TestHostController,
+            ServerTestHost => HandshakeMessageHostTypes.ServerTestHost,
             _ => throw new InvalidOperationException($"Unknown host type '{GetType().FullName}'"),
         };
 
     private string GetHostType()
     {
-        // For now, we don't  inherit TestHostOrchestratorHost from CommonHost one so we don't connect when we orchestrate
+        // TestHostOrchestratorHost does not inherit from CommonHost, so the orchestrator handshake is
+        // performed there directly (see TestHostOrchestratorHost.RunAsync) rather than going through
+        // this path. This method only covers the test host and test host controller roles.
         string hostType = HostType;
         return hostType;
     }

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.Modes.cs

@@ -54,7 +54,7 @@ private async Task<IHost> BuildHostAsync(BuildContext context)
                 // detect mismatches such as --help being injected via RunArguments during a
                 // normal run. The handshake is needed for the SDK to know that the test host
                 // is in help mode, so it can ignore any incoming CommandLineOptionMessages.
-                bool isProtocolCompatible = await pushOnlyProtocol.IsCompatibleProtocolAsync("TestHost").ConfigureAwait(false);
+                bool isProtocolCompatible = await pushOnlyProtocol.IsCompatibleProtocolAsync(HandshakeMessageHostTypes.TestHost).ConfigureAwait(false);
                 if (!isProtocolCompatible)
                 {
                     CompleteBuilderActivity(context.BuilderActivity, nameof(InformativeCommandLineHost));

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostOchestratorHost.cs

@@ -3,7 +3,9 @@
 
 using Microsoft.Testing.Platform.Extensions.TestHostOrchestrator;
 using Microsoft.Testing.Platform.Helpers;
+using Microsoft.Testing.Platform.IPC;
 using Microsoft.Testing.Platform.Logging;
+using Microsoft.Testing.Platform.ServerMode;
 using Microsoft.Testing.Platform.Services;
 using Microsoft.Testing.Platform.Telemetry;
 using Microsoft.Testing.Platform.TestHostOrchestrator;
@@ -26,6 +28,26 @@ public async Task<int> RunAsync()
 
         ITestHostExecutionOrchestrator testHostOrchestrator = _testHostOrchestratorConfiguration.TestHostOrchestrators[0];
         ITestApplicationCancellationTokenSource applicationCancellationToken = _serviceProvider.GetTestApplicationCancellationTokenSource();
+
+        // When connected to dotnet test through the pipe protocol, handshake from the orchestrator too
+        // (test hosts and test host controllers already do). This lets the SDK know that an orchestrator
+        // (e.g. retry) is participating in the run, identified by the OrchestratorFeature property.
+        IPushOnlyProtocol? pushOnlyProtocol = _serviceProvider.GetService<IPushOnlyProtocol>();
+        if (pushOnlyProtocol is { IsServerMode: true })
+        {
+            Dictionary<byte, string> additionalHandshakeProperties = new()
+            {
+                [HandshakeMessagePropertyNames.OrchestratorFeature] = testHostOrchestrator.Uid,
+            };
+
+            bool isProtocolCompatible = await pushOnlyProtocol.IsCompatibleProtocolAsync(
+                HandshakeMessageHostTypes.TestHostOrchestrator, additionalHandshakeProperties).ConfigureAwait(false);
+            if (!isProtocolCompatible)
+            {
+                return (int)ExitCode.IncompatibleProtocolVersion;
+            }
+        }
+
         int exitCode;
         await logger.LogInformationAsync($"Running test orchestrator '{testHostOrchestrator.Uid}'").ConfigureAwait(false);
         try

src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs

@@ -87,12 +87,12 @@ public async Task HelpInvokedAsync()
 
     public bool IsIDE { get; private set; }
 
-    public async Task<bool> IsCompatibleProtocolAsync(string hostType)
+    public async Task<bool> IsCompatibleProtocolAsync(string hostType, IReadOnlyDictionary<byte, string>? additionalHandshakeProperties = null)
     {
         RoslynDebug.Assert(_dotnetTestPipeClient is not null);
 
         string supportedProtocolVersions = ProtocolConstants.Version;
-        HandshakeMessage handshakeMessage = new(new Dictionary<byte, string>
+        Dictionary<byte, string> properties = new()
         {
             { HandshakeMessagePropertyNames.PID, _environment.ProcessId.ToString(CultureInfo.InvariantCulture) },
             { HandshakeMessagePropertyNames.Architecture, RuntimeInformation.ProcessArchitecture.ToString() },
@@ -104,7 +104,17 @@ public async Task<bool> IsCompatibleProtocolAsync(string hostType)
             { HandshakeMessagePropertyNames.ExecutionId,  _environment.GetEnvironmentVariable(EnvironmentVariableConstants.TESTINGPLATFORM_DOTNETTEST_EXECUTIONID) ?? string.Empty },
             { HandshakeMessagePropertyNames.InstanceId, InstanceId },
             { HandshakeMessagePropertyNames.ExecutionMode, GetExecutionMode() },
-        });
+        };
+
+        if (additionalHandshakeProperties is not null)
+        {
+            foreach (KeyValuePair<byte, string> property in additionalHandshakeProperties)
+            {
+                properties[property.Key] = property.Value;
+            }
+        }
+
+        HandshakeMessage handshakeMessage = new(properties);
 
         HandshakeMessage response = await _dotnetTestPipeClient.RequestReplyAsync<HandshakeMessage, HandshakeMessage>(handshakeMessage, _cancellationTokenSource.CancellationToken).ConfigureAwait(false);
 

src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs

@@ -39,6 +39,28 @@ internal static class HandshakeMessagePropertyNames
     // as a help/list-tests option leaking from RunArguments into a normal run.
     // Values come from HandshakeMessageExecutionModes.
     internal const byte ExecutionMode = 10;
+
+    // Identifies the orchestration feature responsible for the orchestrator host
+    // (e.g. "retry"). Only sent by the test host orchestrator so consumers (e.g.
+    // dotnet test in the SDK) can understand why an orchestrator is participating
+    // in the run. The value is the orchestrator extension Uid.
+    internal const byte OrchestratorFeature = 11;
+}
+
+internal static class HandshakeMessageHostTypes
+{
+    // A regular (console or server) test host that actually runs tests.
+    internal const string TestHost = "TestHost";
+
+    // A test host controller process that restarts/monitors the test host.
+    internal const string TestHostController = "TestHostController";
+
+    // A test host running in server (JSON-RPC) mode.
+    internal const string ServerTestHost = "ServerTestHost";
+
+    // A test host orchestrator process (e.g. the retry orchestrator) that drives
+    // one or more test host executions.
+    internal const string TestHostOrchestrator = "TestHostOrchestrator";
 }
 
 internal static class HandshakeMessageExecutionModes

src/Platform/Microsoft.Testing.Platform/ServerMode/IPushOnlyProtocol.cs

@@ -11,7 +11,7 @@ internal interface IPushOnlyProtocol : IDisposable
 
     Task HelpInvokedAsync();
 
-    Task<bool> IsCompatibleProtocolAsync(string testHostType);
+    Task<bool> IsCompatibleProtocolAsync(string testHostType, IReadOnlyDictionary<byte, string>? additionalHandshakeProperties = null);
 
     Task<IPushOnlyProtocolConsumer> GetDataConsumerAsync();
 

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeOrchestratorHandshakeTests.cs

@@ -0,0 +1,123 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Microsoft.Testing.Platform.Acceptance.IntegrationTests.DotnetTestPipe;
+
+/// <summary>
+/// Verifies that, under the <c>--server dotnettestcli</c> pipe protocol, the test host orchestrator
+/// (here the retry orchestrator) performs the handshake as well — not only the test hosts it spawns.
+/// This locks down <a href="https://github.com/microsoft/testfx/issues/6179">microsoft/testfx#6179</a>:
+/// the orchestrator handshakes with <c>HostType=TestHostOrchestrator</c> and advertises the
+/// orchestration feature responsible for the run via the <c>OrchestratorFeature</c> property.
+/// </summary>
+[TestClass]
+public class DotnetTestPipeOrchestratorHandshakeTests : AcceptanceTestBase<DotnetTestPipeOrchestratorHandshakeTests.TestAssetFixture>
+{
+    private const string AssetName = "DotnetTestPipeOrchestratorHandshake";
+
+    // Mirrors HandshakeMessageHostTypes.TestHostOrchestrator on the testfx side. Kept as a literal
+    // here so the black-box harness stays decoupled from internal testfx types.
+    private const string TestHostOrchestratorHostType = "TestHostOrchestrator";
+    private const string TestHostHostType = "TestHost";
+
+    // The retry orchestrator extension Uid, sent as the OrchestratorFeature value.
+    private const string RetryOrchestratorFeature = "RetryOrchestrator";
+
+    public TestContext TestContext { get; set; } = null!;
+
+    [TestMethod]
+    public async Task DotnetTestPipe_Orchestrator_HandshakesWithFeature()
+    {
+        var testHost = TestInfrastructure.TestHost.LocateFrom(
+            AssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetCurrent);
+
+        string resultDirectory = Path.Combine(testHost.DirectoryName, Guid.NewGuid().ToString("N"));
+
+        FakeDotnetTestSdkMultiConnectionResult result = await FakeDotnetTestSdkMultiConnection.RunAsync(
+            testHost,
+            extraArguments: $"--retry-failed-tests 3 --results-directory {resultDirectory}",
+            environmentVariables: new() { { EnvironmentVariableConstants.TESTINGPLATFORM_TELEMETRY_OPTOUT, "1" } },
+            cancellationToken: TestContext.CancellationToken);
+
+        // The orchestrator process and the single (passing) test host it spawned should both have
+        // connected and handshaked.
+        Dictionary<byte, string> orchestratorHandshake = Assert.ContainsSingle(result.HandshakesWithHostType(TestHostOrchestratorHostType));
+
+        Assert.IsTrue(
+            orchestratorHandshake.TryGetValue(DotnetTestPipeProtocol.HandshakeProperties.OrchestratorFeature, out string? feature),
+            "Orchestrator handshake is missing the OrchestratorFeature property.");
+        Assert.AreEqual(RetryOrchestratorFeature, feature);
+
+        // The actual test host (spawned by the orchestrator) handshakes as a plain TestHost and must
+        // NOT carry an orchestrator feature.
+        Dictionary<byte, string> testHostHandshake = Assert.ContainsSingle(result.HandshakesWithHostType(TestHostHostType));
+        Assert.IsFalse(
+            testHostHandshake.ContainsKey(DotnetTestPipeProtocol.HandshakeProperties.OrchestratorFeature),
+            "Test host handshake should not carry the OrchestratorFeature property.");
+    }
+
+    public sealed class TestAssetFixture() : TestAssetFixtureBase()
+    {
+        private const string AssetCode = """
+#file DotnetTestPipeOrchestratorHandshake.csproj
+<Project Sdk="Microsoft.NET.Sdk">
+    <PropertyGroup>
+        <TargetFrameworks>$TargetFrameworks$</TargetFrameworks>
+        <ImplicitUsings>enable</ImplicitUsings>
+        <Nullable>enable</Nullable>
+        <OutputType>Exe</OutputType>
+        <UseAppHost>true</UseAppHost>
+        <LangVersion>preview</LangVersion>
+    </PropertyGroup>
+
+    <ItemGroup>
+        <PackageReference Include="Microsoft.Testing.Platform" Version="$MicrosoftTestingPlatformVersion$" />
+        <PackageReference Include="Microsoft.Testing.Extensions.Retry" Version="$MicrosoftTestingPlatformVersion$" />
+    </ItemGroup>
+</Project>
+
+#file Program.cs
+using Microsoft.Testing.Extensions;
+using Microsoft.Testing.Platform.Builder;
+using Microsoft.Testing.Platform.Capabilities.TestFramework;
+using Microsoft.Testing.Platform.Extensions.TestFramework;
+
+public class Program
+{
+    public static async Task<int> Main(string[] args)
+    {
+        ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args);
+        builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new DummyTestFramework());
+        builder.AddRetryProvider();
+        using ITestApplication app = await builder.BuildAsync();
+        return await app.RunAsync();
+    }
+}
+
+public class DummyTestFramework : ITestFramework
+{
+    public string Uid => nameof(DummyTestFramework);
+    public string Version => "2.0.0";
+    public string DisplayName => nameof(DummyTestFramework);
+    public string Description => nameof(DummyTestFramework);
+    public Task<bool> IsEnabledAsync() => Task.FromResult(true);
+    public Task<CreateTestSessionResult> CreateTestSessionAsync(CreateTestSessionContext context)
+        => Task.FromResult(new CreateTestSessionResult() { IsSuccess = true });
+    public Task<CloseTestSessionResult> CloseTestSessionAsync(CloseTestSessionContext context)
+        => Task.FromResult(new CloseTestSessionResult() { IsSuccess = true });
+    public Task ExecuteRequestAsync(ExecuteRequestContext context)
+    {
+        context.Complete();
+        return Task.CompletedTask;
+    }
+}
+""";
+
+        public string TargetAssetPath => GetAssetPath(AssetName);
+
+        public override (string ID, string Name, string Code) GetAssetsToGenerate() => (AssetName, AssetName,
+            AssetCode
+                .PatchTargetFrameworks(TargetFrameworks.NetCurrent)
+                .PatchCodeWithReplace("$MicrosoftTestingPlatformVersion$", MicrosoftTestingPlatformVersion));
+    }
+}

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs

@@ -55,6 +55,7 @@ public static class HandshakeProperties
         public const byte InstanceId = 8;
         public const byte IsIDE = 9;
         public const byte ExecutionMode = 10;
+        public const byte OrchestratorFeature = 11;
     }
 
     public static class SessionEventTypes

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs

@@ -104,7 +104,7 @@ public static async Task<FakeDotnetTestSdkResult> RunAsync(
     /// also in <paramref name="sdkSupportedVersions"/>. Returns <see cref="string.Empty"/> if none
     /// match.
     /// </summary>
-    private static string SelectHighestMutuallySupportedVersion(Dictionary<byte, string> handshakeProperties, string sdkSupportedVersions)
+    internal static string SelectHighestMutuallySupportedVersion(Dictionary<byte, string> handshakeProperties, string sdkSupportedVersions)
     {
         if (!handshakeProperties.TryGetValue(DotnetTestPipeProtocol.HandshakeProperties.SupportedProtocolVersions, out string? appVersions)
             || string.IsNullOrWhiteSpace(appVersions))