From 5fc832f53971770ebebcd4e68d14e4e76fbdb0f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Mon, 20 May 2024 10:22:32 +0200 Subject: [PATCH] Add option to not share .NET Framework testhosts (#5018) * Add options to not share hosts and skip default adapters * Add tests * Fix conflated appdomaindisable and testhost sharing * Share by default but add opt-out * Use reactorable names --- .../TestPlatform.cs | 24 ++++++++- .../FeatureFlag/FeatureFlag.cs | 4 ++ .../PublicAPI/PublicAPI.Shipped.txt | 2 + .../RunSettings/RunConfiguration.cs | 51 +++++++++++++++++++ .../Hosting/DefaultTestHostManager.cs | 16 ++++-- .../DifferentTestFrameworkSimpleTests.cs | 2 +- .../Hosting/DefaultTestHostManagerTests.cs | 14 +++++ 7 files changed, 105 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.TestPlatform.Client/TestPlatform.cs b/src/Microsoft.TestPlatform.Client/TestPlatform.cs index 89c3d9d8bf..45ee1303a1 100644 --- a/src/Microsoft.TestPlatform.Client/TestPlatform.cs +++ b/src/Microsoft.TestPlatform.Client/TestPlatform.cs @@ -87,7 +87,7 @@ public IDiscoveryRequest CreateDiscoveryRequest( loggerManager.Initialize(discoveryCriteria.RunSettings); IProxyDiscoveryManager discoveryManager = _testEngine.GetDiscoveryManager(requestData, discoveryCriteria, sourceToSourceDetailMap, warningLogger); - discoveryManager.Initialize(options?.SkipDefaultAdapters ?? false); + discoveryManager.Initialize(GetSkipDefaultAdapters(options, discoveryCriteria.RunSettings)); return new DiscoveryRequest(requestData, discoveryCriteria, discoveryManager, loggerManager); } @@ -110,11 +110,31 @@ public ITestRunRequest CreateTestRunRequest( loggerManager.Initialize(testRunCriteria.TestRunSettings); IProxyExecutionManager executionManager = _testEngine.GetExecutionManager(requestData, testRunCriteria, sourceToSourceDetailMap, warningLogger); - executionManager.Initialize(options?.SkipDefaultAdapters ?? false); + executionManager.Initialize(GetSkipDefaultAdapters(options, testRunCriteria.TestRunSettings)); return new TestRunRequest(requestData, testRunCriteria, executionManager, loggerManager); } + private static bool GetSkipDefaultAdapters(TestPlatformOptions? options, string? runSettings) + { + if (options?.SkipDefaultAdapters ?? false) + { + EqtTrace.Verbose($"TestPlatform.GetSkipDefaultAdapters: Skipping default adapters because of TestPlatform options SkipDefaultAdapters."); + return true; + } + + RunConfiguration runConfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(runSettings); + var skipping = runConfiguration.SkipDefaultAdapters; + if (skipping) + { + EqtTrace.Verbose($"TestPlatform.GetSkipDefaultAdapters: Skipping default adapters because of RunConfiguration SkipDefaultAdapters."); + return true; + } + + EqtTrace.Verbose($"TestPlatform.GetSkipDefaultAdapters: Not skipping default adapters SkipDefaultAdapters was false."); + return false; + } + /// public bool StartTestSession( IRequestData requestData, diff --git a/src/Microsoft.TestPlatform.CoreUtilities/FeatureFlag/FeatureFlag.cs b/src/Microsoft.TestPlatform.CoreUtilities/FeatureFlag/FeatureFlag.cs index 86a7f4c85b..76f8ec1546 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/FeatureFlag/FeatureFlag.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/FeatureFlag/FeatureFlag.cs @@ -69,6 +69,10 @@ private FeatureFlag() { } // Disable forwarding standard output of testhost. public const string VSTEST_DISABLE_STANDARD_OUTPUT_FORWARDING = nameof(VSTEST_DISABLE_STANDARD_OUTPUT_FORWARDING); + // Disable not sharing .NET Framework testhosts. Which will return behavior to sharing testhosts when they are running .NET Framework dlls, and are not disabling appdomains or running in parallel. + public const string VSTEST_DISABLE_SHARING_NETFRAMEWORK_TESTHOST = nameof(VSTEST_DISABLE_SHARING_NETFRAMEWORK_TESTHOST); + + [Obsolete("Only use this in tests.")] internal static void Reset() { diff --git a/src/Microsoft.TestPlatform.ObjectModel/PublicAPI/PublicAPI.Shipped.txt b/src/Microsoft.TestPlatform.ObjectModel/PublicAPI/PublicAPI.Shipped.txt index 74364fb56a..221f78d7d1 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/PublicAPI/PublicAPI.Shipped.txt +++ b/src/Microsoft.TestPlatform.ObjectModel/PublicAPI/PublicAPI.Shipped.txt @@ -974,3 +974,5 @@ virtual Microsoft.VisualStudio.TestPlatform.ObjectModel.ValidateValueCallback.In Microsoft.VisualStudio.TestPlatform.ObjectModel.Architecture.RiscV64 = 8 -> Microsoft.VisualStudio.TestPlatform.ObjectModel.Architecture Microsoft.VisualStudio.TestPlatform.ObjectModel.RunConfiguration.CaptureStandardOutput.get -> bool Microsoft.VisualStudio.TestPlatform.ObjectModel.RunConfiguration.ForwardStandardOutput.get -> bool +Microsoft.VisualStudio.TestPlatform.ObjectModel.RunConfiguration.DisableSharedTestHost.get -> bool +Microsoft.VisualStudio.TestPlatform.ObjectModel.RunConfiguration.SkipDefaultAdapters.get -> bool diff --git a/src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs b/src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs index 57f198202e..1de0786730 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs @@ -95,6 +95,7 @@ public RunConfiguration() : base(Constants.RunConfigurationSettingsName) ExecutionThreadApartmentState = Constants.DefaultExecutionThreadApartmentState; CaptureStandardOutput = !FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_STANDARD_OUTPUT_CAPTURING); ForwardStandardOutput = !FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_STANDARD_OUTPUT_FORWARDING); + DisableSharedTestHost = FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_SHARING_NETFRAMEWORK_TESTHOST); } /// @@ -455,6 +456,16 @@ public bool ResultsDirectorySet /// public bool ForwardStandardOutput { get; private set; } + /// + /// Disables sharing of .NET Framework testhosts. + /// + public bool DisableSharedTestHost { get; private set; } + + /// + /// Skips passing VisualStudio built in adapters to the project. + /// + public bool SkipDefaultAdapters { get; private set; } + /// public override XmlElement ToXml() { @@ -578,6 +589,14 @@ public override XmlElement ToXml() forwardStandardOutput.InnerXml = ForwardStandardOutput.ToString(); root.AppendChild(forwardStandardOutput); + XmlElement disableSharedTesthost = doc.CreateElement(nameof(DisableSharedTestHost)); + disableSharedTesthost.InnerXml = DisableSharedTestHost.ToString(); + root.AppendChild(disableSharedTesthost); + + XmlElement skipDefaultAdapters = doc.CreateElement(nameof(SkipDefaultAdapters)); + skipDefaultAdapters.InnerXml = SkipDefaultAdapters.ToString(); + root.AppendChild(skipDefaultAdapters); + return root; } @@ -964,6 +983,38 @@ public static RunConfiguration FromXml(XmlReader reader) runConfiguration.ForwardStandardOutput = bForwardStandardOutput; break; + case nameof(DisableSharedTestHost): + { + XmlRunSettingsUtilities.ThrowOnHasAttributes(reader); + string element = reader.ReadElementContentAsString(); + + bool boolValue; + if (!bool.TryParse(element, out boolValue)) + { + throw new SettingsException(string.Format(CultureInfo.CurrentCulture, + Resources.Resources.InvalidSettingsIncorrectValue, Constants.RunConfigurationSettingsName, boolValue, elementName)); + } + + runConfiguration.DisableSharedTestHost = boolValue; + break; + } + + case nameof(SkipDefaultAdapters): + { + XmlRunSettingsUtilities.ThrowOnHasAttributes(reader); + string element = reader.ReadElementContentAsString(); + + bool boolValue; + if (!bool.TryParse(element, out boolValue)) + { + throw new SettingsException(string.Format(CultureInfo.CurrentCulture, + Resources.Resources.InvalidSettingsIncorrectValue, Constants.RunConfigurationSettingsName, boolValue, elementName)); + } + + runConfiguration.SkipDefaultAdapters = boolValue; + break; + } + default: // Ignore a runsettings element that we don't understand. It could occur in the case // the test runner is of a newer version, but the test host is of an earlier version. diff --git a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs index ca4e20243e..9d5fd5e4ef 100644 --- a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs +++ b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs @@ -58,7 +58,7 @@ public class DefaultTestHostManager : ITestRuntimeProvider2 private readonly IEnvironment _environment; private readonly IDotnetHostHelper _dotnetHostHelper; private readonly IEnvironmentVariableHelper _environmentVariableHelper; - + private bool _disableAppDomain; private Architecture _architecture; private Framework? _targetFramework; private ITestHostLauncher? _customTestHostLauncher; @@ -199,10 +199,10 @@ public virtual TestProcessStartInfo GetTestHostProcessStartInfo( testhostProcessPath = Path.Combine(currentWorkingDirectory, "..", testHostProcessName); } - if (!Shared) + if (_disableAppDomain) { - // Not sharing the host which means we need to pass the test assembly path as argument - // so that the test host can create an appdomain on startup (Main method) and set appbase + // When host appdomains are disabled (in that case host is not shared) we need to pass the test assembly path as argument + // so that the test host can create one appdomain on startup (Main method) and set appbase. argumentsString += " --testsourcepath " + sources.FirstOrDefault()?.AddDoubleQuote(); } @@ -379,7 +379,13 @@ public void Initialize(IMessageLogger? logger, string runsettingsXml) _targetFramework = runConfiguration.TargetFramework; _testHostProcess = null; - Shared = !runConfiguration.DisableAppDomain; + _disableAppDomain = runConfiguration.DisableAppDomain; + // If appdomains are disabled the host cannot be shared, because sharing means loading multiple assemblies + // into the same process, and without appdomains we cannot safely do that. + // + // The OPPOSITE is not true though, disabling testhost sharing does not mean that we should not load the + // dll into a separate appdomain in the host. It just means that we wish to run each dll in separate exe. + Shared = !_disableAppDomain && !runConfiguration.DisableSharedTestHost; _hostExitedEventRaised = false; IsInitialized = true; diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DifferentTestFrameworkSimpleTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DifferentTestFrameworkSimpleTests.cs index 0ab5326a1f..cde8909ee8 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DifferentTestFrameworkSimpleTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DifferentTestFrameworkSimpleTests.cs @@ -163,7 +163,7 @@ public void NUnitRunAllTestExecution(RunnerInfo runnerInfo) var arguments = PrepareArguments( GetAssetFullPath("NUTestProject.dll"), - GetTestAdapterPath(UnitTestFramework.NUnit), + null, // GetTestAdapterPath(UnitTestFramework.NUnit), string.Empty, FrameworkArgValue, runnerInfo.InIsolationValue, TempDirectory.Path); InvokeVsTest(arguments); diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs index c38169246a..9b36182524 100644 --- a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs @@ -394,6 +394,20 @@ public void PropertiesShouldReturnEmptyDictionary() Assert.AreEqual(0, _testHostManager.Properties.Count); } + [TestMethod] + public void DefaultTestHostManagerShouldNotBeSharedWhenOptedIn() + { + _testHostManager.Initialize(_mockMessageLogger.Object, $""" + + + + {true} + + + """); + Assert.IsFalse(_testHostManager.Shared); + } + [TestMethod] public void DefaultTestHostManagerShouldBeShared() {