Update framework detection logic to not rely on throwing/catching NRE

microsoft · Jun 2, 2022 · 6128a31 · 6128a31
1 parent f661007
commit 6128a31
Show file tree

Hide file tree

Showing 11 changed files with 55 additions and 64 deletions.
diff --git a/src/vstest.console/CommandLine/AssemblyMetadataProvider.cs b/src/vstest.console/CommandLine/AssemblyMetadataProvider.cs
@@ -32,7 +32,7 @@ internal AssemblyMetadataProvider(IFileHelper fileHelper)
     }
 
     /// <inheritdoc />
-    public FrameworkName GetFrameWork(string filePath)
+    public FrameworkName GetFrameworkName(string filePath)
     {
         FrameworkName frameworkName = new(Framework.DefaultFramework.Name);
         try
@@ -42,10 +42,10 @@ public FrameworkName GetFrameWork(string filePath)
         }
         catch (Exception ex)
         {
-            EqtTrace.Warning("AssemblyMetadataProvider.GetFrameWork: failed to determine TargetFrameworkVersion exception: {0} for assembly: {1}", ex, filePath);
+            EqtTrace.Warning("AssemblyMetadataProvider.GetFrameworkName: failed to determine TargetFrameworkVersion exception: {0} for assembly: {1}", ex, filePath);
         }
 
-        EqtTrace.Info("AssemblyMetadataProvider.GetFrameWork: Determined framework:'{0}' for source: '{1}'", frameworkName, filePath);
+        EqtTrace.Info("AssemblyMetadataProvider.GetFrameworkName: Determined framework:'{0}' for source: '{1}'", frameworkName, filePath);
 
         return frameworkName;
     }

diff --git a/src/vstest.console/CommandLine/InferHelper.cs b/src/vstest.console/CommandLine/InferHelper.cs
@@ -133,35 +133,24 @@ public Architecture AutoDetectArchitecture(IList<string?>? sources, Architecture
     public Framework AutoDetectFramework(IList<string?>? sources, out IDictionary<string, Framework> sourceToFrameworkMap)
     {
         sourceToFrameworkMap = new Dictionary<string, Framework>();
-        Framework framework = Framework.DefaultFramework;
 
         if (sources == null || sources.Count == 0)
-            return framework;
+            return Framework.DefaultFramework;
 
-        try
+        var framework = DetermineFramework(sources, out sourceToFrameworkMap, out var conflictInFxIdentifier);
+        if (conflictInFxIdentifier)
         {
-            var finalFx = DetermineFrameworkName(sources, out sourceToFrameworkMap, out var conflictInFxIdentifier);
-            TPDebug.Assert(finalFx != null, "finalFx should not be null");
-            framework = Framework.FromString(finalFx.FullName);
-            if (conflictInFxIdentifier)
-            {
-                // TODO Log to console and client.
-                EqtTrace.Info(
-                    "conflicts in Framework identifier of provided sources(test assemblies), using default framework:{0}",
-                    framework);
-            }
-        }
-        catch (Exception ex)
-        {
-            EqtTrace.Error("Failed to determine framework:{0}, using default: {1}", ex, framework);
+            // TODO Log to console and client.
+            EqtTrace.Info(
+                "conflicts in Framework identifier of provided sources(test assemblies), using default framework: {0}",
+                framework);
         }
 
         EqtTrace.Info("Determined framework for all sources: {0}", framework);
-
         return framework;
     }
 
-    private FrameworkName? DetermineFrameworkName(IEnumerable<string?> sources, out IDictionary<string, Framework> sourceToFrameworkMap, out bool conflictInFxIdentifier)
+    private Framework DetermineFramework(IEnumerable<string?> sources, out IDictionary<string, Framework> sourceToFrameworkMap, out bool conflictInFxIdentifier)
     {
         sourceToFrameworkMap = new Dictionary<string, Framework>();
 
@@ -180,7 +169,7 @@ public Framework AutoDetectFramework(IList<string?>? sources, out IDictionary<st
                 FrameworkName fx;
                 if (IsDllOrExe(source))
                 {
-                    fx = _assemblyMetadataProvider.GetFrameWork(source);
+                    fx = _assemblyMetadataProvider.GetFrameworkName(source);
                 }
                 else
                 {
@@ -231,7 +220,9 @@ public Framework AutoDetectFramework(IList<string?>? sources, out IDictionary<st
             }
         }
 
-        return finalFx;
+        return finalFx != null
+            ? Framework.FromString(finalFx.FullName)
+            : defaultFramework;
     }
 
     private static bool IsDllOrExe(string? filePath)

diff --git a/src/vstest.console/CommandLine/Interfaces/IAssemblyMetadataProvider.cs b/src/vstest.console/CommandLine/Interfaces/IAssemblyMetadataProvider.cs
@@ -15,7 +15,7 @@ internal interface IAssemblyMetadataProvider
     /// <summary>
     /// Determines FrameworkName from filePath.
     /// </summary>
-    FrameworkName GetFrameWork(string filePath);
+    FrameworkName GetFrameworkName(string filePath);
 
     /// <summary>
     /// Determines Architecture from filePath.

diff --git a/test/vstest.ProgrammerTests/Fakes/FakeAssemblyMetadataProvider.cs b/test/vstest.ProgrammerTests/Fakes/FakeAssemblyMetadataProvider.cs
@@ -26,7 +26,7 @@ public Architecture GetArchitecture(string filePath)
         return file.Architecture;
     }
 
-    public FrameworkName GetFrameWork(string filePath)
+    public FrameworkName GetFrameworkName(string filePath)
     {
         var file = FakeFileHelper.GetFakeFile<FakeTestDllFile>(filePath);
         return file.FrameworkName;

diff --git a/test/vstest.console.PlatformTests/AssemblyMetadataProviderTests.cs b/test/vstest.console.PlatformTests/AssemblyMetadataProviderTests.cs
@@ -115,7 +115,7 @@ public void GetFrameWorkForDotNetAssembly(string framework)
         var assemblyPath = _testEnvironment.GetTestAsset("SimpleTestProject3.dll", framework);
         LoadAssemblyIntoMemory(assemblyPath);
         var stopWatch = Stopwatch.StartNew();
-        var actualFx = _assemblyMetadataProvider.GetFrameWork(assemblyPath);
+        var actualFx = _assemblyMetadataProvider.GetFrameworkName(assemblyPath);
         stopWatch.Stop();
 
         if (framework.Equals("net451"))
@@ -142,7 +142,7 @@ public void GetFrameWorkForNativeDll()
         var assemblyPath = $@"{_testEnvironment.PackageDirectory}\microsoft.testplatform.testasset.nativecpp\2.0.0\contentFiles\any\any\Microsoft.TestPlatform.TestAsset.NativeCPP.dll";
         LoadAssemblyIntoMemory(assemblyPath);
         var stopWatch = Stopwatch.StartNew();
-        var fx = _assemblyMetadataProvider.GetFrameWork(assemblyPath);
+        var fx = _assemblyMetadataProvider.GetFrameworkName(assemblyPath);
         stopWatch.Stop();
 
         Console.WriteLine(PerfAssertMessageFormat, expectedElapsedTime, stopWatch.ElapsedMilliseconds);

diff --git a/test/vstest.console.UnitTests/CommandLine/InferHelperTests.cs b/test/vstest.console.UnitTests/CommandLine/InferHelperTests.cs
@@ -222,18 +222,18 @@ public void AutoDetectFrameworkShouldReturnDefaultFullFrameworkForJsFiles()
     [TestMethod]
     public void AutoDetectFrameworkShouldReturnHighestVersionFxOnSameFxName()
     {
-        _mockAssemblyHelper.SetupSequence(sh => sh.GetFrameWork(It.IsAny<string>()))
+        _mockAssemblyHelper.SetupSequence(sh => sh.GetFrameworkName(It.IsAny<string>()))
             .Returns(new FrameworkName(_frameworkNet46.Name))
             .Returns(new FrameworkName(_frameworkNet47.Name))
             .Returns(new FrameworkName(_frameworkNet45.Name));
         Assert.AreEqual(_frameworkNet47.Name, _inferHelper.AutoDetectFramework(new List<string?>() { "net46.dll", "net47.exe", "net45.dll" }, out _).Name);
-        _mockAssemblyHelper.Verify(ah => ah.GetFrameWork(It.IsAny<string>()), Times.Exactly(3));
+        _mockAssemblyHelper.Verify(ah => ah.GetFrameworkName(It.IsAny<string>()), Times.Exactly(3));
     }
 
     [TestMethod]
     public void AutoDetectFrameworkShouldPopulatetheDictionaryForAllTheSources()
     {
-        _mockAssemblyHelper.SetupSequence(sh => sh.GetFrameWork(It.IsAny<string>()))
+        _mockAssemblyHelper.SetupSequence(sh => sh.GetFrameworkName(It.IsAny<string>()))
             .Returns(new FrameworkName(_frameworkNet46.Name))
             .Returns(new FrameworkName(_frameworkNet47.Name))
             .Returns(new FrameworkName(_frameworkNet45.Name));
@@ -244,28 +244,28 @@ public void AutoDetectFrameworkShouldPopulatetheDictionaryForAllTheSources()
         Assert.AreEqual(_frameworkNet46.Name, sourceFrameworks["net46.dll"].Name);
         Assert.AreEqual(_frameworkNet47.Name, sourceFrameworks["net47.exe"].Name);
         Assert.AreEqual(_frameworkNet45.Name, sourceFrameworks["net45.dll"].Name);
-        _mockAssemblyHelper.Verify(ah => ah.GetFrameWork(It.IsAny<string>()), Times.Exactly(3));
+        _mockAssemblyHelper.Verify(ah => ah.GetFrameworkName(It.IsAny<string>()), Times.Exactly(3));
     }
 
     [TestMethod]
     public void AutoDetectFrameworkShouldReturnHighestVersionFxOnEvenManyLowerVersionFxNameExists()
     {
-        _mockAssemblyHelper.SetupSequence(sh => sh.GetFrameWork(It.IsAny<string>()))
+        _mockAssemblyHelper.SetupSequence(sh => sh.GetFrameworkName(It.IsAny<string>()))
             .Returns(new FrameworkName(_frameworkCore10.Name))
             .Returns(new FrameworkName(_frameworkCore11.Name))
             .Returns(new FrameworkName(_frameworkCore10.Name));
         Assert.AreEqual(_frameworkCore11.Name, _inferHelper.AutoDetectFramework(new List<string?>() { "netcore10_1.dll", "netcore11.dll", "netcore10_2.dll" }, out _).Name);
-        _mockAssemblyHelper.Verify(ah => ah.GetFrameWork(It.IsAny<string>()), Times.Exactly(3));
+        _mockAssemblyHelper.Verify(ah => ah.GetFrameworkName(It.IsAny<string>()), Times.Exactly(3));
     }
 
     private void SetupAndValidateForSingleAssembly(string assemblyName, Framework fx, bool verify)
     {
-        _mockAssemblyHelper.Setup(sh => sh.GetFrameWork(assemblyName))
+        _mockAssemblyHelper.Setup(sh => sh.GetFrameworkName(assemblyName))
             .Returns(new FrameworkName(fx.Name));
         Assert.AreEqual(fx.Name, _inferHelper.AutoDetectFramework(new List<string?>() { assemblyName }, out _).Name);
         if (verify)
         {
-            _mockAssemblyHelper.Verify(ah => ah.GetFrameWork(assemblyName));
+            _mockAssemblyHelper.Verify(ah => ah.GetFrameworkName(assemblyName));
         }
     }
 }
diff --git a/test/vstest.console.UnitTests/Processors/ListFullyQualifiedTestsArgumentProcessorTests.cs b/test/vstest.console.UnitTests/Processors/ListFullyQualifiedTestsArgumentProcessorTests.cs
@@ -80,7 +80,7 @@ public ListFullyQualifiedTestsArgumentProcessorTests()
         _mockTestPlatformEventSource = new Mock<ITestPlatformEventSource>();
         _mockAssemblyMetadataProvider = new Mock<IAssemblyMetadataProvider>();
         _mockAssemblyMetadataProvider.Setup(x => x.GetArchitecture(It.IsAny<string>())).Returns(Architecture.X64);
-        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameWork(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
+        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameworkName(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
         _inferHelper = new InferHelper(_mockAssemblyMetadataProvider.Object);
         _mockProcessHelper = new Mock<IProcessHelper>();
         _mockAttachmentsProcessingManager = new Mock<ITestRunAttachmentsProcessingManager>();

diff --git a/test/vstest.console.UnitTests/Processors/ListTestsArgumentProcessorTests.cs b/test/vstest.console.UnitTests/Processors/ListTestsArgumentProcessorTests.cs
@@ -78,7 +78,7 @@ public ListTestsArgumentProcessorTests()
         _mockTestPlatformEventSource = new Mock<ITestPlatformEventSource>();
         _mockAssemblyMetadataProvider = new Mock<IAssemblyMetadataProvider>();
         _mockAssemblyMetadataProvider.Setup(x => x.GetArchitecture(It.IsAny<string>())).Returns(Architecture.X64);
-        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameWork(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
+        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameworkName(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
         _inferHelper = new InferHelper(_mockAssemblyMetadataProvider.Object);
         _mockProcessHelper = new Mock<IProcessHelper>();
         _mockAttachmentsProcessingManager = new Mock<ITestRunAttachmentsProcessingManager>();

diff --git a/test/vstest.console.UnitTests/Processors/RunSpecificTestsArgumentProcessorTests.cs b/test/vstest.console.UnitTests/Processors/RunSpecificTestsArgumentProcessorTests.cs
@@ -62,7 +62,7 @@ public RunSpecificTestsArgumentProcessorTests()
         _mockAssemblyMetadataProvider = new Mock<IAssemblyMetadataProvider>();
         _inferHelper = new InferHelper(_mockAssemblyMetadataProvider.Object);
         _mockAssemblyMetadataProvider.Setup(x => x.GetArchitecture(It.IsAny<string>())).Returns(Architecture.X64);
-        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameWork(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
+        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameworkName(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
         _mockFileHelper.Setup(fh => fh.Exists(_dummyTestFilePath)).Returns(true);
         _mockFileHelper.Setup(x => x.GetCurrentDirectory()).Returns("");
         _mockMetricsPublisher = new Mock<IMetricsPublisher>();

diff --git a/test/vstest.console.UnitTests/Processors/RunTestsArgumentProcessorTests.cs b/test/vstest.console.UnitTests/Processors/RunTestsArgumentProcessorTests.cs
@@ -69,7 +69,7 @@ public RunTestsArgumentProcessorTests()
         SetupMockExtensions();
         _mockAssemblyMetadataProvider.Setup(a => a.GetArchitecture(It.IsAny<string>()))
             .Returns(Architecture.X86);
-        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameWork(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
+        _mockAssemblyMetadataProvider.Setup(x => x.GetFrameworkName(It.IsAny<string>())).Returns(new FrameworkName(Constants.DotNetFramework40));
         _mockProcessHelper = new Mock<IProcessHelper>();
         _mockAttachmentsProcessingManager = new Mock<ITestRunAttachmentsProcessingManager>();
         _environment = new Mock<IEnvironment>();