Filtering sources based on assembly type (#1537)

* Filtering sources based on assembly type * * Renaming PEReader -> AssemblyProperties. * Correcting Tests * Ignoring EventDataCollector Tests * change to IList
microsoft · Apr 11, 2018 · 2f6e1db · 2f6e1db
1 parent 534026f
commit 2f6e1db
Show file tree

Hide file tree

Showing 7 changed files with 326 additions and 68 deletions.
diff --git a/src/Microsoft.TestPlatform.Common/Interfaces/IAssemblyProperties.cs b/src/Microsoft.TestPlatform.Common/Interfaces/IAssemblyProperties.cs
@@ -0,0 +1,18 @@
+// 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.VisualStudio.TestPlatform.Common.Interfaces
+{
+    using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
+
+    /// <summary>
+    /// Metadata that is available for input test source, e.g. Whether it is native or managed dll, etc..
+    /// </summary>
+    public interface IAssemblyProperties
+    {
+        /// <summary>
+        /// Determines assembly type from file.
+        /// </summary>
+        AssemblyType GetAssemblyType(string filePath);
+    }
+}
diff --git a/...atform.Common/Utilities/PEReaderHelper.cs → ...rm.Common/Utilities/AssemblyProperties.cs b/...atform.Common/Utilities/PEReaderHelper.cs → ...rm.Common/Utilities/AssemblyProperties.cs
@@ -7,26 +7,27 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.Utilities
     using System.IO;
     using System.Reflection.PortableExecutable;
 
+    using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers;
     using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;
-
-    public class PEReaderHelper
+    
+    public class AssemblyProperties : IAssemblyProperties
     {
         private readonly IFileHelper fileHelper;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="PEReaderHelper"/> class.
+        /// Initializes a new instance of the <see cref="AssemblyProperties"/> class.
         /// </summary>
-        public PEReaderHelper() : this(new FileHelper())
+        public AssemblyProperties() : this(new FileHelper())
         {
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PEReaderHelper"/> class.
         /// </summary>
         /// <param name="fileHelper">File helper.</param>
-        public PEReaderHelper(IFileHelper fileHelper)
+        public AssemblyProperties(IFileHelper fileHelper)
         {
             this.fileHelper = fileHelper;
         }

diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs
@@ -16,6 +16,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Discovery
     using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.Common.Logging;
     using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
+    using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
     using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Tracing;
     using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Tracing.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
@@ -33,6 +34,7 @@ internal class DiscovererEnumerator
         private DiscoveryResultCache discoveryResultCache;
         private ITestPlatformEventSource testPlatformEventSource;
         private IRequestData requestData;
+        private IAssemblyProperties assemblyProperties;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DiscovererEnumerator"/> class.
@@ -51,8 +53,22 @@ internal DiscovererEnumerator(IRequestData requestData,
             this.discoveryResultCache = discoveryResultCache;
             this.testPlatformEventSource = testPlatformEventSource;
             this.requestData = requestData;
+            this.assemblyProperties = new AssemblyProperties();
         }
 
+        // Added this to make class testable, needed a PEHeader mocked Instance
+        internal DiscovererEnumerator(IRequestData requestData,
+            DiscoveryResultCache discoveryResultCache,
+            ITestPlatformEventSource testPlatformEventSource,
+            IAssemblyProperties assemblyProperties)
+        {
+            this.discoveryResultCache = discoveryResultCache;
+            this.testPlatformEventSource = testPlatformEventSource;
+            this.requestData = requestData;
+            this.assemblyProperties = assemblyProperties;
+        }
+
+
         /// <summary>
         /// Discovers tests from the sources.
         /// </summary>
@@ -89,7 +105,7 @@ private void LoadTestsFromAnExtension(string extensionAssembly, IEnumerable<stri
             // Stopwatch to collect metrics
             var timeStart = DateTime.UtcNow;
 
-            var discovererToSourcesMap = GetDiscovererToSourcesMap(extensionAssembly, sources, logger);
+            var discovererToSourcesMap = GetDiscovererToSourcesMap(extensionAssembly, sources, logger, this.assemblyProperties);
             var totalAdapterLoadTIme = DateTime.UtcNow - timeStart;
 
             // Collecting Data Point for TimeTaken to Load Adapters
@@ -222,7 +238,8 @@ private void SetAdapterLoggingSettings(IMessageLogger messageLogger, IRunSetting
         internal static Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabilities>, IEnumerable<string>> GetDiscovererToSourcesMap(
             string extensionAssembly,
             IEnumerable<string> sources,
-            IMessageLogger logger)
+            IMessageLogger logger,
+            IAssemblyProperties assemblyProperties)
         {
             var allDiscoverers = GetDiscoverers(extensionAssembly, throwOnError: true);
 
@@ -237,14 +254,24 @@ internal static Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabil
                 return null;
             }
 
+            IDictionary<AssemblyType, IList<string>> assemblyTypeToSoucesMap = null;
             var result = new Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabilities>, IEnumerable<string>>();
             var sourcesForWhichNoDiscovererIsAvailable = new List<string>(sources);
 
             foreach (var discoverer in allDiscoverers)
             {
+                var sourcesToCheck = sources;
+
+                if (discoverer.Metadata.AssemblyType == AssemblyType.Native ||
+                    discoverer.Metadata.AssemblyType == AssemblyType.Managed)
+                {
+                    assemblyTypeToSoucesMap = assemblyTypeToSoucesMap ?? GetAssemblyTypeToSoucesMap(sources, assemblyProperties);
+                    sourcesToCheck = assemblyTypeToSoucesMap[AssemblyType.None].Concat(assemblyTypeToSoucesMap[discoverer.Metadata.AssemblyType]);
+                }
+
                 // Find the sources which this discoverer can look at. 
                 // Based on whether it is registered for a matching file extension or no file extensions at all.
-                var matchingSources = (from source in sources
+                var matchingSources = (from source in sourcesToCheck
                                        where
                                            (discoverer.Metadata.FileExtension == null
                                             || discoverer.Metadata.FileExtension.Contains(
@@ -277,6 +304,49 @@ internal static Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabil
             return result;
         }
 
+        /// <summary>
+        /// Get assembly type to sources map.
+        /// </summary>
+        /// <param name="sources">Sources.</param>
+        /// <param name="assemblyType">Assembly type.</param>
+        /// <returns>Sources with mathcing assembly type.</returns>
+        private static IDictionary<AssemblyType, IList<string>> GetAssemblyTypeToSoucesMap(IEnumerable<string> sources, IAssemblyProperties assemblyProperties)
+        {
+            var assemblyTypeToSoucesMap = new Dictionary<AssemblyType, IList<string>>()
+            {
+                { AssemblyType.Native, new List<string>()},
+                { AssemblyType.Managed, new List<string>()},
+                { AssemblyType.None, new List<string>()}
+            };
+
+            if (sources != null && sources.Any())
+            {
+                foreach (string source in sources)
+                {
+                    var sourcesList = IsAssembly(source) ?
+                        assemblyTypeToSoucesMap[assemblyProperties.GetAssemblyType(source)] :
+                        assemblyTypeToSoucesMap[AssemblyType.None];
+
+                    sourcesList.Add(source);
+                }
+            }
+
+            return assemblyTypeToSoucesMap;
+        }
+
+        /// <summary>
+        /// Finds if a file is an assembly or not.
+        /// </summary>
+        /// <param name="filePath">File path.</param>
+        /// <returns>True if file is an assembly.</returns>
+        private static bool IsAssembly(string filePath)
+        {
+            var fileExtension = Path.GetExtension(filePath);
+
+            return ".dll".Equals(fileExtension, StringComparison.OrdinalIgnoreCase) ||
+                ".exe".Equals(fileExtension, StringComparison.OrdinalIgnoreCase);
+        }
+
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes")]
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design",
             "CA1006:DoNotNestGenericTypesInMemberSignatures")]

diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/RunTestsWithSources.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Execution/RunTestsWithSources.cs
@@ -11,6 +11,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution
     using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities;
     using Microsoft.VisualStudio.TestPlatform.Common.Filtering;
     using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
+    using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
     using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Tracing;
     using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Adapter;
     using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Discovery;
@@ -127,7 +128,8 @@ private Dictionary<Tuple<Uri, string>, IEnumerable<string>> GetExecutorVsSources
                     discovererToSourcesMap = DiscovererEnumerator.GetDiscovererToSourcesMap(
                         kvp.Key,
                         kvp.Value,
-                        logger);
+                        logger,
+                        new AssemblyProperties());
 
                 // Warning is logged by the inner layer
                 if (discovererToSourcesMap == null || discovererToSourcesMap.Count == 0)

diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/EventLogCollectorTests.cs b/test/Microsoft.TestPlatform.AcceptanceTests/EventLogCollectorTests.cs
@@ -20,6 +20,9 @@ public EventLogCollectorTests()
             this.resultsDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
         }
 
+        // Fails randomly https://ci.dot.net/job/Microsoft_vstest/job/master/job/Windows_NT_Release_prtest/2084/console
+        // https://ci.dot.net/job/Microsoft_vstest/job/master/job/Windows_NT_Debug_prtest/2085/console
+        [Ignore]
         [TestMethod]
         [NetFullTargetFrameworkDataSource]
         public void EventLogDataCollectorShoudCreateLogFileHavingEvents(RunnerInfo runnerInfo)

diff --git a/...mmon.PlatformTests/PEReaderHelperTests.cs → ....PlatformTests/AssemblyPropertiesTests.cs b/...mmon.PlatformTests/PEReaderHelperTests.cs → ....PlatformTests/AssemblyPropertiesTests.cs
@@ -4,17 +4,18 @@
 namespace TestPlatform.Common.UnitTests.Utilities
 {
     using Microsoft.TestPlatform.TestUtilities;
+    using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
     using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
 
     [TestClass]
-    public class PEReaderHelperTests : IntegrationTestBase
+    public class AssemblyPropertiesTests : IntegrationTestBase
     {
-        private PEReaderHelper peReaderHelper;
+        private IAssemblyProperties assemblyProperties;
 
-        public PEReaderHelperTests()
+        public AssemblyPropertiesTests()
         {
-            this.peReaderHelper = new PEReaderHelper();
+            this.assemblyProperties = new AssemblyProperties();
         }
 
         [TestMethod]
@@ -24,7 +25,7 @@ public PEReaderHelperTests()
         public void GetAssemblyTypeForManagedDll(string framework)
         {
             var assemblyPath = this.testEnvironment.GetTestAsset("SimpleTestProject3.dll", framework);
-            var assemblyType = this.peReaderHelper.GetAssemblyType(assemblyPath);
+            var assemblyType = this.assemblyProperties.GetAssemblyType(assemblyPath);
 
             Assert.AreEqual(AssemblyType.Managed, assemblyType);
         }
@@ -33,7 +34,7 @@ public void GetAssemblyTypeForManagedDll(string framework)
         public void GetAssemblyTypeForNativeDll()
         {
             var assemblyPath = $@"{this.testEnvironment.PackageDirectory}\microsoft.testplatform.testasset.nativecpp\2.0.0\contentFiles\any\any\Microsoft.TestPlatform.TestAsset.NativeCPP.dll";
-            var assemblyType = this.peReaderHelper.GetAssemblyType(assemblyPath);
+            var assemblyType = this.assemblyProperties.GetAssemblyType(assemblyPath);
 
             Assert.AreEqual(AssemblyType.Native, assemblyType);
         }
@@ -42,7 +43,7 @@ public void GetAssemblyTypeForNativeDll()
         public void GetAssemblyTypeForManagedExe()
         {
             var assemblyPath = this.testEnvironment.GetTestAsset("ConsoleManagedApp.exe", "net451");
-            var assemblyType = this.peReaderHelper.GetAssemblyType(assemblyPath);
+            var assemblyType = this.assemblyProperties.GetAssemblyType(assemblyPath);
 
             Assert.AreEqual(AssemblyType.Managed, assemblyType);
         }
@@ -53,7 +54,7 @@ public void GetAssemblyTypeForManagedExe()
         public void GetAssemblyTypeForNetCoreManagedExe(string framework)
         {
             var assemblyPath = this.testEnvironment.GetTestAsset("ConsoleManagedApp.dll", framework);
-            var assemblyType = this.peReaderHelper.GetAssemblyType(assemblyPath);
+            var assemblyType = this.assemblyProperties.GetAssemblyType(assemblyPath);
 
             Assert.AreEqual(AssemblyType.Managed, assemblyType);
         }
@@ -62,15 +63,15 @@ public void GetAssemblyTypeForNetCoreManagedExe(string framework)
         public void GetAssemblyTypeForNativeExe()
         {
             var assemblyPath = $@"{this.testEnvironment.PackageDirectory}\microsoft.testplatform.testasset.nativecpp\2.0.0\contentFiles\any\any\Microsoft.TestPlatform.TestAsset.ConsoleNativeApp.exe";
-            var assemblyType = this.peReaderHelper.GetAssemblyType(assemblyPath);
+            var assemblyType = this.assemblyProperties.GetAssemblyType(assemblyPath);
 
             Assert.AreEqual(AssemblyType.Native, assemblyType);
         }
 
         [TestMethod]
         public void GetAssemblyTypeShouldReturnNoneInCaseOfError()
         {
-            var assemblyType = this.peReaderHelper.GetAssemblyType("invalidFile.dll");
+            var assemblyType = this.assemblyProperties.GetAssemblyType("invalidFile.dll");
 
             Assert.AreEqual(AssemblyType.None, assemblyType);
         }