Run multiple target frameworks and architectures in single vstest.console

src/Microsoft.TestPlatform.Client/TestPlatform.cs

@@ -85,20 +85,16 @@ protected internal TestPlatform(
     public IDiscoveryRequest CreateDiscoveryRequest(
         IRequestData requestData,
         DiscoveryCriteria discoveryCriteria!!,
-        TestPlatformOptions options)
+        TestPlatformOptions options,
+        Dictionary<string, SourceDetail> sourceToSourceDetailMap)
     {
         PopulateExtensions(discoveryCriteria.RunSettings, discoveryCriteria.Sources);
 
         // Initialize loggers.
         ITestLoggerManager loggerManager = TestEngine.GetLoggerManager(requestData);
         loggerManager.Initialize(discoveryCriteria.RunSettings);
 
-        ITestRuntimeProvider testHostManager = _testHostProviderManager.GetTestHostManagerByRunConfiguration(discoveryCriteria.RunSettings);
-        TestPlatform.ThrowExceptionIfTestHostManagerIsNull(testHostManager, discoveryCriteria.RunSettings);
-
-        testHostManager.Initialize(TestSessionMessageLogger.Instance, discoveryCriteria.RunSettings);
-
-        IProxyDiscoveryManager discoveryManager = TestEngine.GetDiscoveryManager(requestData, testHostManager, discoveryCriteria);
+        IProxyDiscoveryManager discoveryManager = TestEngine.GetDiscoveryManager(requestData,  discoveryCriteria, sourceToSourceDetailMap);
         discoveryManager.Initialize(options?.SkipDefaultAdapters ?? false);
 
         return new DiscoveryRequest(requestData, discoveryCriteria, discoveryManager, loggerManager);
@@ -108,7 +104,8 @@ public IDiscoveryRequest CreateDiscoveryRequest(
     public ITestRunRequest CreateTestRunRequest(
         IRequestData requestData,
         TestRunCriteria testRunCriteria!!,
-        TestPlatformOptions options)
+        TestPlatformOptions options,
+        Dictionary<string, SourceDetail> sourceToSourceDetailMap)
     {
         IEnumerable<string> sources = GetSources(testRunCriteria);
         PopulateExtensions(testRunCriteria.TestRunSettings, sources);
@@ -117,23 +114,7 @@ public ITestRunRequest CreateTestRunRequest(
         ITestLoggerManager loggerManager = TestEngine.GetLoggerManager(requestData);
         loggerManager.Initialize(testRunCriteria.TestRunSettings);
 
-        // TODO: PERF: this will create a testhost manager, and then it will pass that to GetExecutionManager, where it will
-        // be used only when we will run in-process. If we don't run in process, we will throw away the manager we just
-        // created and let the proxy parallel callbacks to create a new one. This seems to be very easy to move to the GetExecutionManager,
-        // and safe as well, so we create the manager only once.
-        // TODO: Of course TestEngine.GetExecutionManager is public api...
-        ITestRuntimeProvider testHostManager = _testHostProviderManager.GetTestHostManagerByRunConfiguration(testRunCriteria.TestRunSettings);
-        TestPlatform.ThrowExceptionIfTestHostManagerIsNull(testHostManager, testRunCriteria.TestRunSettings);
-
-        testHostManager.Initialize(TestSessionMessageLogger.Instance, testRunCriteria.TestRunSettings);
-
-        // NOTE: The custom launcher should not be set when we have test session info available.
-        if (testRunCriteria.TestHostLauncher != null)
-        {
-            testHostManager.SetCustomLauncher(testRunCriteria.TestHostLauncher);
-        }
-
-        IProxyExecutionManager executionManager = TestEngine.GetExecutionManager(requestData, testHostManager, testRunCriteria);
+        IProxyExecutionManager executionManager = TestEngine.GetExecutionManager(requestData, testRunCriteria, sourceToSourceDetailMap);
         executionManager.Initialize(options?.SkipDefaultAdapters ?? false);
 
         return new TestRunRequest(requestData, testRunCriteria, executionManager, loggerManager);

src/Microsoft.TestPlatform.Common/Interfaces/Engine/ClientProtocol/ITestEngine.cs

@@ -5,6 +5,8 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
 
+using System.Collections.Generic;
+
 using Client;
 
 using Host;
@@ -28,8 +30,8 @@ public interface ITestEngine
     /// <returns>An IProxyDiscoveryManager object that can do discovery.</returns>
     IProxyDiscoveryManager GetDiscoveryManager(
         IRequestData requestData,
-        ITestRuntimeProvider testHostManager,
-        DiscoveryCriteria discoveryCriteria);
+        DiscoveryCriteria discoveryCriteria,
+        Dictionary<string, SourceDetail> sourceToSourceDetailMap);
 
     /// <summary>
     /// Fetches the ExecutionManager for this engine. This manager would provide all
@@ -45,8 +47,8 @@ IProxyDiscoveryManager GetDiscoveryManager(
     /// <returns>An IProxyExecutionManager object that can do execution.</returns>
     IProxyExecutionManager GetExecutionManager(
         IRequestData requestData,
-        ITestRuntimeProvider testHostManager,
-        TestRunCriteria testRunCriteria);
+        TestRunCriteria testRunCriteria,
+        Dictionary<string, SourceDetail> sourceToSourceDetailMap);
 
     /// <summary>
     /// Fetches the TestSessionManager for this engine. This manager would provide all

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

@@ -9,7 +9,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Utilities;
 using System.Collections.Generic;
 
 // !!! FEATURES MUST BE KEPT IN SYNC WITH https://github.com/dotnet/sdk/blob/main/src/Cli/dotnet/commands/dotnet-test/VSTestFeatureFlag.cs !!!
-internal partial class FeatureFlag : IFeatureFlag
+internal sealed class FeatureFlag : IFeatureFlag
 {
     private static readonly Dictionary<string, bool> FeatureFlags = new();
 
@@ -19,18 +19,40 @@ internal partial class FeatureFlag : IFeatureFlag
 
     static FeatureFlag()
     {
+#pragma warning disable CS0618 // Type or member is obsolete
+        Reset();
+#pragma warning restore CS0618 // Type or member is obsolete
+    }
+
+    [Obsolete("Use this only from tests, and ctor.")]
+    internal static void Reset()
+    {
+        FeatureFlags.Clear();
         FeatureFlags.Add(ARTIFACTS_POSTPROCESSING, true);
         FeatureFlags.Add(ARTIFACTS_POSTPROCESSING_SDK_KEEP_OLD_UX, false);
+        FeatureFlags.Add(MULTI_TFM_RUN, true);
     }
 
-    // Added for artifact porst-processing, it enable/disable the post processing.
+    [Obsolete("Use this only from tests.")]
+    internal static void SetFlag(string name, bool value)
+    {
+        if (!FeatureFlags.ContainsKey(name))
+            throw new ArgumentException($"Feature flag {name} is a known feature flag.");
+
+        FeatureFlags[name] = value;
+    }
+
+    // Added for artifact post-processing, it enable/disable the post processing.
     // Added in 17.2-preview 7.0-preview
-    public static string ARTIFACTS_POSTPROCESSING = VSTEST_FEATURE + "_" + "ARTIFACTS_POSTPROCESSING";
+    public static string ARTIFACTS_POSTPROCESSING = VSTEST_FEATURE + "_" + nameof(ARTIFACTS_POSTPROCESSING);
 
-    // Added for artifact porst-processing, it will show old output for dotnet sdk scenario.
+    // Added for artifact post-processing, it will show old output for dotnet sdk scenario.
     // It can be useful if we need to restore old UX in case users are parsing the console output.
     // Added in 17.2-preview 7.0-preview
-    public static string ARTIFACTS_POSTPROCESSING_SDK_KEEP_OLD_UX = VSTEST_FEATURE + "_" + "ARTIFACTS_POSTPROCESSING_SDK_KEEP_OLD_UX";
+    public static string ARTIFACTS_POSTPROCESSING_SDK_KEEP_OLD_UX = VSTEST_FEATURE + "_" + nameof(ARTIFACTS_POSTPROCESSING_SDK_KEEP_OLD_UX);
+
+    // Allow vstest.console to sources from multiple TFMs
+    public static string MULTI_TFM_RUN = VSTEST_FEATURE + "_" + nameof(MULTI_TFM_RUN);
 
     // For now we're checking env var.
     // We could add it also to some section inside the runsettings.

src/Microsoft.TestPlatform.CoreUtilities/Friends.cs

@@ -4,3 +4,6 @@
 using System.Runtime.CompilerServices;
 
 [assembly: InternalsVisibleTo("vstest.console, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")]
+
+
+[assembly: InternalsVisibleTo("vstest.ProgrammerTests, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")]

src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs

@@ -16,31 +16,32 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client;
 using ObjectModel.Engine;
 
 /// <summary>
-/// Abstract class having common parallel manager implementation
+/// Common parallel manager functionality.
 /// </summary>
-internal abstract class ParallelOperationManager<T, TU> : IParallelOperationManager, IDisposable
+internal abstract class ParallelOperationManager<TParallelManager, TEventHandler, TCreationContext> : IParallelOperationManager, IDisposable
 {
     #region ConcurrentManagerInstanceData
 
-    protected Func<T> CreateNewConcurrentManager { get; set; }
+    protected Func<TCreationContext, TParallelManager> CreateNewConcurrentManager { get; }
 
     /// <summary>
-    /// Gets a value indicating whether hosts are shared.
+    /// Holds all active managers, so we can do actions on all of them, like initialize, run, cancel or close.
     /// </summary>
-    protected bool SharedHosts { get; private set; }
-
-    private IDictionary<T, TU> _concurrentManagerHandlerMap;
+    // TODO: make this ConcurrentDictionary and use it's concurrent api, if we have the need.
+    private readonly IDictionary<TParallelManager, TEventHandler> _concurrentManagerHandlerMap = new ConcurrentDictionary<TParallelManager, TEventHandler>();
 
     /// <summary>
     /// Singleton Instance of this class
     /// </summary>
-    protected static T s_instance;
+    protected static TParallelManager s_instance;
 
     /// <summary>
     /// Default number of Processes
     /// </summary>
     private int _currentParallelLevel;
 
+    protected int MaxParallelLevel { get; private set; }
+
     #endregion
 
     #region Concurrency Keeper Objects
@@ -53,20 +54,27 @@ internal abstract class ParallelOperationManager<T, TU> : IParallelOperationMana
 
     #endregion
 
-    protected ParallelOperationManager(Func<T> createNewManager, int parallelLevel, bool sharedHosts)
+    protected ParallelOperationManager(Func<TCreationContext, TParallelManager> createNewManager, int parallelLevel)
     {
         CreateNewConcurrentManager = createNewManager;
-        SharedHosts = sharedHosts;
 
         // Update Parallel Level
-        UpdateParallelLevel(parallelLevel);
+        // REVIEW: this "pre-starts" testhosts so we have a pool of them, this is the reason the number or
+        // parallel hosts is capped to the amount of sources so we don't "pre-start" too many of them
+        // instead we should take each source, look if it can be run by shared host, and if so try to
+        // grab a free host, run new one if we are below parallel level, or wait if we are at parallel
+        // level and everyone is busy if we have non-shared host we do just the two last options, run new
+        // one if current count is under parallel level, or wait till we can run new one.
+        // this.UpdateParallelLevel(parallelLevel);
+
+        MaxParallelLevel = parallelLevel;
     }
 
     /// <summary>
     /// Remove and dispose a manager from concurrent list of manager.
     /// </summary>
     /// <param name="manager">Manager to remove</param>
-    public void RemoveManager(T manager)
+    public void RemoveManager(TParallelManager manager)
     {
         _concurrentManagerHandlerMap.Remove(manager);
     }
@@ -76,7 +84,7 @@ public void RemoveManager(T manager)
     /// </summary>
     /// <param name="manager">Manager to add</param>
     /// <param name="handler">eventHandler of the manager</param>
-    public void AddManager(T manager, TU handler)
+    public void AddManager(TParallelManager manager, TEventHandler handler)
     {
         _concurrentManagerHandlerMap.Add(manager, handler);
     }
@@ -87,7 +95,7 @@ public void AddManager(T manager, TU handler)
     /// </summary>
     /// <param name="manager">Manager to update</param>
     /// <param name="handler">event handler to update for manager</param>
-    public void UpdateHandlerForManager(T manager, TU handler)
+    public void UpdateHandlerForManager(TParallelManager manager, TEventHandler handler)
     {
         if (_concurrentManagerHandlerMap.ContainsKey(manager))
         {
@@ -103,7 +111,7 @@ public void UpdateHandlerForManager(T manager, TU handler)
     /// Get the event handler associated with the manager.
     /// </summary>
     /// <param name="manager">Manager</param>
-    public TU GetHandlerForGivenManager(T manager)
+    public TEventHandler GetHandlerForGivenManager(TParallelManager manager)
     {
         return _concurrentManagerHandlerMap[manager];
     }
@@ -119,7 +127,7 @@ public int GetConcurrentManagersCount()
     /// <summary>
     /// Get instances of all active concurrent manager
     /// </summary>
-    public IEnumerable<T> GetConcurrentManagerInstances()
+    public IEnumerable<TParallelManager> GetConcurrentManagerInstances()
     {
         return _concurrentManagerHandlerMap.Keys.ToList();
     }
@@ -133,24 +141,17 @@ public void UpdateParallelLevel(int newParallelLevel)
     {
         if (_concurrentManagerHandlerMap == null)
         {
-            // not initialized yet
-            // create rest of concurrent clients other than default one
-            _concurrentManagerHandlerMap = new ConcurrentDictionary<T, TU>();
-            for (int i = 0; i < newParallelLevel; i++)
-            {
-                AddManager(CreateNewConcurrentManager(), default);
-            }
+            throw new Exception("ParallelOperationManager.UpdateParallelLevel: This should not be used anymore, to pre-start hosts.");
         }
         else if (_currentParallelLevel != newParallelLevel)
         {
             // If number of concurrent clients is less than the new level
             // Create more concurrent clients and update the list
             if (_currentParallelLevel < newParallelLevel)
             {
-                for (int i = 0; i < newParallelLevel - _currentParallelLevel; i++)
-                {
-                    AddManager(CreateNewConcurrentManager(), default);
-                }
+                // This path does not even seem to be used anywhere.
+
+                throw new Exception("ParallelOperationManager.UpdateParallelLevel: This should not be used anymore, to ensure we add more hosts.");
             }
             else
             {
@@ -190,7 +191,7 @@ public void Dispose()
         s_instance = default;
     }
 
-    protected void DoActionOnAllManagers(Action<T> action, bool doActionsInParallel = false)
+    protected void DoActionOnAllManagers(Action<TParallelManager> action, bool doActionsInParallel = false)
     {
         if (_concurrentManagerHandlerMap != null && _concurrentManagerHandlerMap.Count > 0)
         {
@@ -237,15 +238,16 @@ private void DoManagerAction(Action action)
     /// </summary>
     /// <param name="source">source data to work on - source file or testCaseList</param>
     /// <returns>True, if data exists. False otherwise</returns>
-    protected bool TryFetchNextSource<TY>(IEnumerator enumerator, out TY source)
+    protected bool TryFetchNextSource<TSource>(IEnumerator enumerator, out TSource source)
     {
+        // TODO: If only something like a concurrent queue existed.
         source = default;
         var hasNext = false;
         lock (_sourceEnumeratorLockObject)
         {
             if (enumerator != null && enumerator.MoveNext())
             {
-                source = (TY)enumerator.Current;
+                source = (TSource)enumerator.Current;
                 hasNext = source != null;
             }
         }