[rel/18.10] Disable the MTP testhost by default (#16337)

docs/Overview.md

@@ -89,7 +89,7 @@ Testhost receives the request to run tests, and runs them via an appropriate tes
 
 Datacollector observes the testhost to collect additional information about the run when data collection is enabled.
 
-Microsoft.Testing.Platform (MTP) test applications are an emerging model. For those applications, the application hosts itself and TestPlatform drives discovery and execution over the MTP protocol instead of launching a VSTest testhost.
+Microsoft.Testing.Platform (MTP) test applications are an emerging model. When the experimental testhost is enabled with `VSTEST_DISABLE_MTP_TESTHOST=0`, the application hosts itself and TestPlatform drives discovery and execution over the MTP protocol instead of launching a VSTest testhost.
 
 While the tests execute, the results are reported back to the runner, aggregated, and forwarded to the client.
 

docs/environment-variables.md

@@ -127,6 +127,12 @@ This document lists environment variables that are currently handled by VSTest s
 
 ## Feature Control Variables (Disable Features)
 
+### VSTEST_DISABLE_MTP_TESTHOST
+- **Description**: Disables the experimental capability to discover and run Microsoft.Testing.Platform (MTP) test applications under VSTest.
+- **Default**: `1` (disabled)
+- **Values**: Set to `0` to opt in to the experimental MTP testhost; any other value keeps it disabled
+- **Example**: `VSTEST_DISABLE_MTP_TESTHOST=0`
+
 ### VSTEST_DISABLE_ARTIFACTS_POSTPROCESSING
 - **Description**: Disables artifact post-processing functionality.
 - **Values**: Set to any non-zero value to disable

src/Microsoft.TestPlatform.CoreUtilities/FeatureFlag/FeatureFlag.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Concurrent;
+using System.Collections.Generic;
 
 namespace Microsoft.VisualStudio.TestPlatform.Utilities;
 
@@ -23,14 +24,19 @@ namespace Microsoft.VisualStudio.TestPlatform.Utilities;
 // !!! SDK USED FEATURE NAMES MUST BE KEPT IN SYNC IN https://github.com/dotnet/sdk/blob/main/src/Cli/dotnet/commands/dotnet-test/VSTestFeatureFlag.cs !!!
 internal partial class FeatureFlag : IFeatureFlag
 {
+    private static readonly IReadOnlyDictionary<string, bool> DefaultValues = new Dictionary<string, bool>
+    {
+        [VSTEST_DISABLE_MTP_TESTHOST] = true,
+    };
+
     private readonly ConcurrentDictionary<string, bool> _cache = new();
 
     public static IFeatureFlag Instance { get; private set; } = new FeatureFlag();
 
     private FeatureFlag() { }
 
-    // Only check the env variable once, when it is not set or is set to 0, consider it unset. When it is anything else, consider it set.
-    public bool IsSet(string featureFlag) => _cache.GetOrAdd(featureFlag, f => (Environment.GetEnvironmentVariable(f)?.Trim() ?? "0") != "0");
+    // Only check the env variable once. The environment value takes precedence over the default value.
+    public bool IsSet(string featureFlag) => _cache.GetOrAdd(featureFlag, GetValue);
 
     // Added for artifact post-processing, it enable/disable the post processing.
     // Added in 17.2-preview 7.0-preview
@@ -78,7 +84,20 @@ private FeatureFlag() { }
     // Disable turning dynamic code coverage for native code to OFF by default. Setting this to 1 will skip adding the setting.
     public const string VSTEST_DISABLE_DYNAMICNATIVE_CODECOVERAGE_DEFAULT_SETTING = nameof(VSTEST_DISABLE_DYNAMICNATIVE_CODECOVERAGE_DEFAULT_SETTING);
 
+    // Disable running Microsoft.Testing.Platform applications under vstest while the integration is experimental.
+    // This defaults to true. Set it to 0 to opt in to the feature.
+    public const string VSTEST_DISABLE_MTP_TESTHOST = nameof(VSTEST_DISABLE_MTP_TESTHOST);
 
+    private static bool GetValue(string featureFlag)
+    {
+        var environmentValue = Environment.GetEnvironmentVariable(featureFlag)?.Trim();
+        if (environmentValue is not null)
+        {
+            return environmentValue != "0";
+        }
+
+        return DefaultValues.TryGetValue(featureFlag, out var defaultValue) && defaultValue;
+    }
 
     [Obsolete("Only use this in tests.")]
     internal static void Reset()

src/Microsoft.TestPlatform.TestHostProvider/Hosting/MtpTestRuntimeProvider.cs

@@ -16,6 +16,7 @@
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Host;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
+using Microsoft.VisualStudio.TestPlatform.Utilities;
 
 namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Hosting;
 
@@ -29,9 +30,10 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Hosting;
 /// <see cref="NotSupportedException"/>. Instead it:
 /// <list type="bullet">
 ///   <item><description>
-///     claims MTP sources via <see cref="ISourceAwareTestRuntimeProvider"/> (source-aware detection using the
-///     build-time Microsoft.Testing.Platform marker), so it is selected ahead of the generic testhost providers
-///     that match only by target framework; and
+///     when <c>VSTEST_DISABLE_MTP_TESTHOST</c> is set to <c>0</c>, claims MTP sources via
+///     <see cref="ISourceAwareTestRuntimeProvider"/> (source-aware detection using the build-time
+///     Microsoft.Testing.Platform marker), so it is selected ahead of the generic testhost providers that match
+///     only by target framework; and
 ///   </description></item>
 ///   <item><description>
 ///     supplies its own discovery/execution proxy managers via <see cref="IProxyManagerFactory"/>, so the
@@ -78,7 +80,7 @@ void ITestRuntimeProvider.Initialize(IMessageLogger? logger, string runsettingsX
     // application. A mixed set (some MTP, some classic) is split into separate configurations upstream, so each
     // group asked here is homogeneous.
     bool ISourceAwareTestRuntimeProvider.CanExecuteCurrentRunConfiguration(string? runsettingsXml, IEnumerable<string> sources)
-        => AllSourcesAreMicrosoftTestingPlatform(sources);
+        => !FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_MTP_TESTHOST) && AllSourcesAreMicrosoftTestingPlatform(sources);
 
     void ITestRuntimeProvider.SetCustomLauncher(ITestHostLauncher customLauncher)
     {

test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs

@@ -21,6 +21,8 @@ namespace Microsoft.TestPlatform.AcceptanceTests;
 [TestClass]
 public class MtpUnderVstestTests : AcceptanceTestBase
 {
+    private const string MtpTestHostDisableFeatureFlag = "VSTEST_DISABLE_MTP_TESTHOST";
+
     // MtpMSTestProject is an MSTest project built as an MTP application (EnableMSTestRunner): three tests
     // pass, one fails, one is skipped.
     private const string MtpApp = "MtpMSTestProject.dll";
@@ -29,6 +31,30 @@ public class MtpUnderVstestTests : AcceptanceTestBase
     // fails, one is skipped.
     private const string ClassicApp = "MSTestProject1.dll";
 
+    [TestMethod]
+    [TestMatrix(testHost: Target.Net)]
+    public void MtpApplicationIsNotRunWhenMtpTestHostIsDisabled(RunnerInfo runnerInfo)
+    {
+        SetTestEnvironment(_testEnvironment, runnerInfo);
+
+        var arguments = PrepareArguments(
+            GetAssetFullPath(MtpApp),
+            testAdapterPath: null,
+            runSettings: string.Empty,
+            FrameworkArgValue,
+            runnerInfo.InIsolationValue,
+            resultsDirectory: TempDirectory.Path);
+        var environmentVariables = new Dictionary<string, string?>
+        {
+            [MtpTestHostDisableFeatureFlag] = "1",
+        };
+
+        InvokeVsTest(arguments, environmentVariables);
+
+        ExitCodeEquals(1);
+        StdErrorContains("Could not find testhost for test source");
+    }
+
     [TestMethod]
     // MTP apps are .NET (Core) applications. Pin the testhost axis to .NET so we drive the net11.0 MTP app
     // from both the .NET Framework and the .NET console (the .NET Framework console exercises the Jsonite
@@ -46,7 +72,7 @@ public void RunMtpApplicationExecutesTestsOverMtpProtocol(RunnerInfo runnerInfo)
             runnerInfo.InIsolationValue,
             resultsDirectory: TempDirectory.Path);
 
-        InvokeVsTest(arguments);
+        InvokeVsTestWithMtpTestHostEnabled(arguments);
 
         ValidateSummaryStatus(3, 1, 1);
     }
@@ -68,7 +94,7 @@ public void RunMixedClassicAndMtpApplicationsInSingleRun(RunnerInfo runnerInfo)
             runnerInfo.InIsolationValue,
             resultsDirectory: TempDirectory.Path);
 
-        InvokeVsTest(arguments);
+        InvokeVsTestWithMtpTestHostEnabled(arguments);
 
         // Classic 1/1/1 + MTP 3/1/1 aggregated into one run summary.
         ValidateSummaryStatus(4, 2, 2);
@@ -92,7 +118,7 @@ public void RunMixedClassicAndMtpApplicationsWritesSingleTrx(RunnerInfo runnerIn
             resultsDirectory: TempDirectory.Path);
         arguments = string.Concat(arguments, $" /logger:trx;LogFileName={trxFileName}");
 
-        InvokeVsTest(arguments);
+        InvokeVsTestWithMtpTestHostEnabled(arguments);
 
         ValidateSummaryStatus(4, 2, 2);
 
@@ -121,7 +147,7 @@ public void RunMtpApplicationWithBlameCompletesRun(RunnerInfo runnerInfo)
             resultsDirectory: TempDirectory.Path);
         arguments = string.Concat(arguments, " /Blame");
 
-        InvokeVsTest(arguments);
+        InvokeVsTestWithMtpTestHostEnabled(arguments);
 
         ValidateSummaryStatus(3, 1, 1);
     }
@@ -161,7 +187,7 @@ public void RunMtpApplicationInjectsRunSettingsEnvironmentVariables(RunnerInfo r
             ["CHECK_RUNSETTINGS_VAR"] = "1",
         };
 
-        InvokeVsTest(arguments, env);
+        InvokeVsTestWithMtpTestHostEnabled(arguments, env);
 
         // The guarded test passes only if MTP_FROM_RUNSETTINGS reached the host with the runsettings value.
         ValidateSummaryStatus(3, 1, 1);
@@ -187,7 +213,7 @@ public void RunMtpApplicationSurfacesPerTestStandardOutput(RunnerInfo runnerInfo
             resultsDirectory: TempDirectory.Path);
         arguments = string.Concat(arguments, $" /logger:trx;LogFileName={trxFileName}");
 
-        InvokeVsTest(arguments);
+        InvokeVsTestWithMtpTestHostEnabled(arguments);
 
         // MtpMSTestProject has five test cases: three pass, one fails, one is skipped.
         ValidateSummaryStatus(3, 1, 1);
@@ -234,7 +260,7 @@ public void RunMtpApplicationWithGenericOutOfProcDataCollectorCompletesRun(Runne
             ["TEST_ASSET_SAMPLE_COLLECTOR_PATH"] = collectorSourceDirectory,
         };
 
-        InvokeVsTest(arguments, env);
+        InvokeVsTestWithMtpTestHostEnabled(arguments, env);
 
         // The run must complete with the usual summary rather than hang at shutdown. MtpMSTestProject has
         // five test cases: three pass, one fails, one is skipped.
@@ -261,4 +287,11 @@ public void RunMtpApplicationWithGenericOutOfProcDataCollectorCompletesRun(Runne
             .ToList();
         Assert.HasCount(5, testCaseAttachments, "Expected one per-test-case attachment for each started MtpMSTestProject test case forwarded on the MTP path.");
     }
+
+    private void InvokeVsTestWithMtpTestHostEnabled(string arguments, Dictionary<string, string?>? environmentVariables = null)
+    {
+        environmentVariables ??= [];
+        environmentVariables[MtpTestHostDisableFeatureFlag] = "0";
+        InvokeVsTest(arguments, environmentVariables);
+    }
 }

test/Microsoft.TestPlatform.CoreUtilities.UnitTests/FeatureFlag/FeatureFlagTests.cs

@@ -1,17 +1,60 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
+
 using Microsoft.VisualStudio.TestPlatform.Utilities;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace Microsoft.TestPlatform.CoreUtilities.UnitTests;
 
 [TestClass]
+[DoNotParallelize]
 public class FeatureFlagTests
 {
     [TestMethod]
     public void SingletonAlwaysReturnsTheSameInstance()
     {
         Assert.IsTrue(ReferenceEquals(FeatureFlag.Instance, FeatureFlag.Instance));
     }
+
+    [TestMethod]
+    public void MtpTestHostIsDisabledByDefault()
+    {
+        AssertMtpTestHostDisableFlag(environmentValue: null, expected: true);
+    }
+
+    [TestMethod]
+    public void MtpTestHostCanBeEnabledBySettingDisableFlagToZero()
+    {
+        AssertMtpTestHostDisableFlag(environmentValue: "0", expected: false);
+    }
+
+    [TestMethod]
+    public void MtpTestHostRemainsDisabledWhenDisableFlagIsNonZero()
+    {
+        AssertMtpTestHostDisableFlag(environmentValue: "1", expected: true);
+    }
+
+    private static void AssertMtpTestHostDisableFlag(string? environmentValue, bool expected)
+    {
+        const string featureFlag = FeatureFlag.VSTEST_DISABLE_MTP_TESTHOST;
+        var originalValue = Environment.GetEnvironmentVariable(featureFlag);
+        try
+        {
+            Environment.SetEnvironmentVariable(featureFlag, environmentValue);
+            ResetFeatureFlag();
+
+            Assert.AreEqual(expected, FeatureFlag.Instance.IsSet(featureFlag));
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable(featureFlag, originalValue);
+            ResetFeatureFlag();
+        }
+    }
+
+#pragma warning disable CS0618 // FeatureFlag.Reset exists for tests.
+    private static void ResetFeatureFlag() => FeatureFlag.Reset();
+#pragma warning restore CS0618
 }