Pass coverlet codebase in runsettings for inproc data collector initi…

…alization (#2288)

* remove some coverlet hardcoded code

* check null

* address PR feedback

* move get coverlet codebase path to init

* nit style

* rollback ArgumentProcessorPriority

* revert test

* fix test

* switch ArgumentProcessorPriority between RunSettings/TestAdapterPath

* fix CapabilitiesShouldReturnAppropriateProperties test

src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginCache.cs

@@ -295,6 +295,15 @@ public void ClearExtensions()
             this.TestExtensions?.InvalidateCache();
         }
 
+        /// <summary>
+        /// Add search directories to assembly resolver
+        /// </summary>
+        /// <param name="directories"></param>
+        public void AddResolverSearchDirectories(string[] directories)
+        {
+            assemblyResolver.AddSearchDirectories(directories);
+        }
+
         #endregion
 
         #region Utility methods
@@ -486,7 +495,7 @@ private Dictionary<string, TPluginInfo> GetTestExtensions<TPluginInfo, TExtensio
             return discoverer.GetTestExtensionsInformation<TPluginInfo, TExtension>(extensionPaths);
         }
 
-        private void SetupAssemblyResolver(string extensionAssembly)
+        protected void SetupAssemblyResolver(string extensionAssembly)
         {
             IList<string> resolutionPaths;
 

src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManager.cs

@@ -37,12 +37,6 @@ internal class ProxyExecutionManager : ProxyOperationManager, IProxyExecutionMan
         private bool skipDefaultAdapters;
         private readonly IFileHelper fileHelper;
 
-        // Only send coverlet inproc datacollector dll to be initialized via testhost, 
-        // ideally this should get initialized via InProcessDC Node in runsettings, but 
-        // somehow it is failing, hence putting this ugly HACK, to fix issues like
-        // https://developercommunity.visualstudio.com/content/problem/738856/could-not-load-file-or-assembly-microsoftintellitr.html
-        private const string CoverletDataCollector = "coverlet.collector.dll";
-
         /// <inheritdoc/>
         public bool IsInitialized { get; private set; } = false;
 
@@ -54,7 +48,7 @@ internal class ProxyExecutionManager : ProxyOperationManager, IProxyExecutionMan
         /// <param name="requestData">The Request Data for providing services and data for Run.</param>
         /// <param name="requestSender">Test request sender instance.</param>
         /// <param name="testHostManager">Test host manager for this proxy.</param>
-        public ProxyExecutionManager(IRequestData requestData, ITestRequestSender requestSender, ITestRuntimeProvider testHostManager) : 
+        public ProxyExecutionManager(IRequestData requestData, ITestRequestSender requestSender, ITestRuntimeProvider testHostManager) :
             this(requestData, requestSender, testHostManager, JsonDataSerializer.Instance, new FileHelper())
         {
         }
@@ -108,7 +102,7 @@ public virtual int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsH
                 {
                     EqtTrace.Verbose("ProxyExecutionManager: Test host is always Lazy initialize.");
                 }
-                
+
                 var testSources = new List<string>(testRunCriteria.HasSpecificSources ? testRunCriteria.Sources :
                                                     // If the test execution is with a test filter, group them by sources
                                                     testRunCriteria.Tests.GroupBy(tc => tc.Source).Select(g => g.Key));
@@ -118,7 +112,7 @@ public virtual int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsH
                 if (this.isCommunicationEstablished)
                 {
                     this.CancellationTokenSource.Token.ThrowTestPlatformExceptionIfCancellationRequested();
-                    
+
                     this.InitializeExtensions(testSources);
 
                     // This code should be in sync with InProcessProxyExecutionManager.StartTestRun executionContext
@@ -181,7 +175,7 @@ public virtual int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsH
         public virtual void Cancel(ITestRunEventsHandler eventHandler)
         {
             // Just in case ExecuteAsync isn't called yet, set the eventhandler
-            if(this.baseTestRunEventsHandler == null)
+            if (this.baseTestRunEventsHandler == null)
             {
                 this.baseTestRunEventsHandler = eventHandler;
             }
@@ -207,7 +201,7 @@ public virtual int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testPr
         public void Abort(ITestRunEventsHandler eventHandler)
         {
             // Just in case ExecuteAsync isn't called yet, set the eventhandler
-            if(this.baseTestRunEventsHandler == null)
+            if (this.baseTestRunEventsHandler == null)
             {
                 this.baseTestRunEventsHandler = eventHandler;
             }
@@ -238,7 +232,7 @@ public void HandleRawMessage(string rawMessage)
         {
             var message = this.dataSerializer.DeserializeMessage(rawMessage);
 
-            if(string.Equals(message.MessageType, MessageType.ExecutionComplete))
+            if (string.Equals(message.MessageType, MessageType.ExecutionComplete))
             {
                 this.Close();
             }
@@ -267,9 +261,6 @@ private void LogMessage(TestMessageLevel testMessageLevel, string message)
         private void InitializeExtensions(IEnumerable<string> sources)
         {
             var extensions = TestPluginCache.Instance.GetExtensionPaths(TestPlatformConstants.TestAdapterEndsWithPattern, this.skipDefaultAdapters);
-
-            // remove this line once we figure out why coverlet inproc DC is not initialized via runsetting inproc node.
-            extensions = extensions.Concat(TestPluginCache.Instance.GetExtensionPaths(ProxyExecutionManager.CoverletDataCollector, true)).ToList();
 
             // Filter out non existing extensions
             var nonExistingExtensions = extensions.Where(extension => !this.fileHelper.Exists(extension));

src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/InProcDataCollector.cs

@@ -4,10 +4,11 @@
 namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection
 {
     using System;
+    using System.Diagnostics;
     using System.IO;
     using System.Linq;
     using System.Reflection;
-
+    using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
     using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection;
@@ -45,7 +46,7 @@ public InProcDataCollector(
             string assemblyQualifiedName,
             TypeInfo interfaceTypeInfo,
             string configXml)
-            : this(codeBase, assemblyQualifiedName, interfaceTypeInfo, configXml, new PlatformAssemblyLoadContext())
+            : this(codeBase, assemblyQualifiedName, interfaceTypeInfo, configXml, new PlatformAssemblyLoadContext(), TestPluginCache.Instance)
         {
         }
 
@@ -62,7 +63,7 @@ public InProcDataCollector(
         /// </param>
         /// <param name="assemblyLoadContext">
         /// </param>
-        internal InProcDataCollector(string codeBase, string assemblyQualifiedName, TypeInfo interfaceTypeInfo, string configXml, IAssemblyLoadContext assemblyLoadContext)
+        internal InProcDataCollector(string codeBase, string assemblyQualifiedName, TypeInfo interfaceTypeInfo, string configXml, IAssemblyLoadContext assemblyLoadContext, TestPluginCache testPluginCache)
         {
             this.configXml = configXml;
             this.assemblyLoadContext = assemblyLoadContext;
@@ -75,6 +76,10 @@ internal InProcDataCollector(string codeBase, string assemblyQualifiedName, Type
                 // If we're loading coverlet collector we skip to check the version of assembly
                 // to allow upgrade throught nuget package
                 filterPredicate = (x) => x.FullName.Equals(Constants.CoverletDataCollectorTypeName) && interfaceTypeInfo.IsAssignableFrom(x.GetTypeInfo());
+
+                // Coverlet collector is consumed as nuget package we need to add assemblies directory to resolver to correctly load references.
+                Debug.Assert(Path.IsPathRooted(codeBase), "Absolute path expected");
+                testPluginCache.AddResolverSearchDirectories(new string[] { Path.GetDirectoryName(codeBase) });
             }
             else
             {

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

@@ -45,7 +45,6 @@ public class DotnetTestHostManager : ITestRuntimeProvider
         private const string DotnetTestHostUri = "HostProvider://DotnetTestHost";
         private const string DotnetTestHostFriendlyName = "DotnetTestHost";
         private const string TestAdapterRegexPattern = @"TestAdapter.dll";
-        private const string CoverletDataCollectorRegexPattern = @"coverlet.collector.dll";
 
         private IDotnetHostHelper dotnetHostHelper;
         private IEnvironment platformEnvironment;
@@ -316,11 +315,6 @@ public IEnumerable<string> GetTestPlatformExtensions(IEnumerable<string> sources
                 extensionPaths.AddRange(this.fileHelper.EnumerateFiles(sourceDirectory, SearchOption.TopDirectoryOnly, TestAdapterRegexPattern));
             }
 
-            if (extensions != null && extensions.Any())
-            {
-                extensionPaths.AddRange(extensions.Where(x => x.EndsWith(CoverletDataCollectorRegexPattern, StringComparison.OrdinalIgnoreCase)));
-            }
-
             return extensionPaths;
         }
 

src/vstest.console/Processors/CollectArgumentProcessor.cs

@@ -7,12 +7,15 @@ namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Processors
     using System.Collections.Generic;
 
     using System.Globalization;
+    using System.IO;
     using Microsoft.VisualStudio.TestPlatform.Common;
     using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
     using Microsoft.VisualStudio.TestPlatform.Utilities;
+    using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers;
+    using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;
     using CommandLineResources = Microsoft.VisualStudio.TestPlatform.CommandLine.Resources.Resources;
 
     /// <summary>
@@ -58,7 +61,7 @@ public Lazy<IArgumentExecutor> Executor
             {
                 if (this.executor == null)
                 {
-                    this.executor = new Lazy<IArgumentExecutor>(() => new CollectArgumentExecutor(RunSettingsManager.Instance));
+                    this.executor = new Lazy<IArgumentExecutor>(() => new CollectArgumentExecutor(RunSettingsManager.Instance, new FileHelper()));
                 }
 
                 return this.executor;
@@ -71,7 +74,7 @@ public Lazy<IArgumentExecutor> Executor
         }
     }
 
-    
+
     internal class CollectArgumentProcessorCapabilities : BaseArgumentProcessorCapabilities
     {
         public override string CommandName => CollectArgumentProcessor.CommandName;
@@ -90,11 +93,13 @@ internal class CollectArgumentProcessorCapabilities : BaseArgumentProcessorCapab
     /// <inheritdoc />
     internal class CollectArgumentExecutor : IArgumentExecutor
     {
-        private IRunSettingsProvider runSettingsManager;
+        private readonly IRunSettingsProvider runSettingsManager;
+        private readonly IFileHelper fileHelper;
         internal static List<string> EnabledDataCollectors = new List<string>();
-        internal CollectArgumentExecutor(IRunSettingsProvider runSettingsManager)
+        internal CollectArgumentExecutor(IRunSettingsProvider runSettingsManager, IFileHelper fileHelper)
         {
             this.runSettingsManager = runSettingsManager;
+            this.fileHelper = fileHelper;
         }
 
         /// <inheritdoc />
@@ -113,11 +118,29 @@ public void Initialize(string argument)
                     argument));
             }
 
-            if(InferRunSettingsHelper.IsTestSettingsEnabled(this.runSettingsManager.ActiveRunSettings.SettingsXml))
+            if (InferRunSettingsHelper.IsTestSettingsEnabled(this.runSettingsManager.ActiveRunSettings.SettingsXml))
             {
                 throw new SettingsException(string.Format(CommandLineResources.CollectWithTestSettingErrorMessage, argument));
             }
-            AddDataCollectorToRunSettings(argument, this.runSettingsManager);
+            AddDataCollectorToRunSettings(argument, this.runSettingsManager, this.fileHelper);
+        }
+
+        /// <summary>
+        /// Returns coverlet codebase searching coverlet.collector.dll assembly inside adaptersPaths
+        /// </summary>
+        private static string GetCoverletCodeBasePath(IRunSettingsProvider runSettingProvider, IFileHelper fileHelper)
+        {
+            foreach (string adapterPath in RunSettingsUtilities.GetTestAdaptersPaths(runSettingProvider.ActiveRunSettings.SettingsXml))
+            {
+                string collectorPath = Path.Combine(adapterPath, CoverletConstants.CoverletDataCollectorCodebase);
+                if (fileHelper.Exists(collectorPath))
+                {
+                    EqtTrace.Verbose("CoverletDataCollector in-process codeBase path '{0}'", collectorPath);
+                    return collectorPath;
+                }
+            }
+
+            return null;
         }
 
         /// <inheritdoc />
@@ -146,7 +169,7 @@ internal static void EnableDataCollectorUsingFriendlyName(string argument, DataC
         /// <summary>
         /// Enables coverlet inproc datacollector
         /// </summary>
-        internal static void EnableCoverletInProcDataCollector(string argument, DataCollectionRunSettings dataCollectionRunSettings)
+        internal static void EnableCoverletInProcDataCollector(string argument, DataCollectionRunSettings dataCollectionRunSettings, IRunSettingsProvider runSettingProvider, IFileHelper fileHelper)
         {
             DataCollectorSettings dataCollectorSettings = null;
 
@@ -156,7 +179,7 @@ internal static void EnableCoverletInProcDataCollector(string argument, DataColl
                 dataCollectorSettings = new DataCollectorSettings();
                 dataCollectorSettings.FriendlyName = argument;
                 dataCollectorSettings.AssemblyQualifiedName = CoverletConstants.CoverletDataCollectorAssemblyQualifiedName;
-                dataCollectorSettings.CodeBase = CoverletConstants.CoverletDataCollectorCodebase;
+                dataCollectorSettings.CodeBase = GetCoverletCodeBasePath(runSettingProvider, fileHelper) ?? CoverletConstants.CoverletDataCollectorCodebase;
                 dataCollectorSettings.IsEnabled = true;
                 dataCollectionRunSettings.DataCollectorSettingsList.Add(dataCollectorSettings);
             }
@@ -186,7 +209,7 @@ private static bool DoesDataCollectorSettingsExist(string friendlyName,
             return false;
         }
 
-        internal static void AddDataCollectorToRunSettings(string argument, IRunSettingsProvider runSettingsManager)
+        internal static void AddDataCollectorToRunSettings(string argument, IRunSettingsProvider runSettingsManager, IFileHelper fileHelper)
         {
             EnabledDataCollectors.Add(argument.ToLower());
 
@@ -198,7 +221,7 @@ internal static void AddDataCollectorToRunSettings(string argument, IRunSettings
             }
 
             var dataCollectionRunSettings = XmlRunSettingsUtilities.GetDataCollectionRunSettings(settings) ?? new DataCollectionRunSettings();
-            var inProcDataCollectionRunSettings = XmlRunSettingsUtilities.GetInProcDataCollectionRunSettings(settings) 
+            var inProcDataCollectionRunSettings = XmlRunSettingsUtilities.GetInProcDataCollectionRunSettings(settings)
                 ?? new DataCollectionRunSettings(
                     Constants.InProcDataCollectionRunSettingsName,
                     Constants.InProcDataCollectorsSettingName,
@@ -212,7 +235,7 @@ internal static void AddDataCollectorToRunSettings(string argument, IRunSettings
             if (string.Equals(argument, CoverletConstants.CoverletDataCollectorFriendlyName, StringComparison.OrdinalIgnoreCase))
             {
                 // Add inproc data collector to runsetings if coverlet code coverage is enabled
-                EnableCoverletInProcDataCollector(argument, inProcDataCollectionRunSettings);
+                EnableCoverletInProcDataCollector(argument, inProcDataCollectionRunSettings, runSettingsManager, fileHelper);
                 runSettingsManager.UpdateRunSettingsNodeInnerXml(Constants.InProcDataCollectionRunSettingsName, inProcDataCollectionRunSettings.ToXml().InnerXml);
             }
         }

src/vstest.console/Processors/EnableBlameArgumentProcessor.cs

@@ -18,7 +18,8 @@ namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Processors
     using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;
     using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.Utilities;
-
+    using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers;
+    using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;
     using CommandLineResources = Microsoft.VisualStudio.TestPlatform.CommandLine.Resources.Resources;
 
     internal class EnableBlameArgumentProcessor : IArgumentProcessor
@@ -61,7 +62,7 @@ public Lazy<IArgumentExecutor> Executor
             {
                 if (this.executor == null)
                 {
-                    this.executor = new Lazy<IArgumentExecutor>(() => new EnableBlameArgumentExecutor(RunSettingsManager.Instance, new PlatformEnvironment()));
+                    this.executor = new Lazy<IArgumentExecutor>(() => new EnableBlameArgumentExecutor(RunSettingsManager.Instance, new PlatformEnvironment(), new FileHelper()));
                 }
 
                 return this.executor;
@@ -111,13 +112,19 @@ internal class EnableBlameArgumentExecutor : IArgumentExecutor
         /// </summary>
         private IEnvironment environment;
 
+        /// <summary>
+        /// For file related operation
+        /// </summary>
+        private readonly IFileHelper fileHelper;
+
         #region Constructor
 
-        internal EnableBlameArgumentExecutor(IRunSettingsProvider runSettingsManager, IEnvironment environment)
+        internal EnableBlameArgumentExecutor(IRunSettingsProvider runSettingsManager, IEnvironment environment, IFileHelper fileHelper)
         {
             this.runSettingsManager = runSettingsManager;
             this.environment = environment;
             this.Output = ConsoleOutput.Instance;
+            this.fileHelper = fileHelper;
         }
 
         #endregion
@@ -182,7 +189,7 @@ private void InitializeBlame(bool enableDump, Dictionary<string, string> collect
             LoggerUtilities.AddLoggerToRunSettings(BlameFriendlyName, null, this.runSettingsManager);
 
             // Add Blame Data Collector
-            CollectArgumentExecutor.AddDataCollectorToRunSettings(BlameFriendlyName, this.runSettingsManager);
+            CollectArgumentExecutor.AddDataCollectorToRunSettings(BlameFriendlyName, this.runSettingsManager, this.fileHelper);
 
 
             // Add default run settings if required.