Skip to content

Commit

Permalink
Update framework detection logic to not rely on throwing/catching NRE
Browse files Browse the repository at this point in the history
  • Loading branch information
Evangelink committed Jun 2, 2022
1 parent 9e19aca commit 43c490b
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 64 deletions.
6 changes: 3 additions & 3 deletions src/vstest.console/CommandLine/AssemblyMetadataProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down
33 changes: 12 additions & 21 deletions src/vstest.console/CommandLine/InferHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>();

Expand All @@ -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
{
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions test/vstest.console.UnitTests/CommandLine/InferHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
Loading

0 comments on commit 43c490b

Please sign in to comment.