Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filtering sources based on assembly type #1537

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: needed a AssemblyProperties or remove comment.

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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -237,14 +254,24 @@ internal static Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabil
return null;
}

IDictionary<AssemblyType, IEnumerable<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 ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance discoverer can support both managed and native?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. adapter can't mention both

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(
Expand Down Expand Up @@ -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, IEnumerable<string>> GetAssemblyTypeToSoucesMap(IEnumerable<string> sources, IAssemblyProperties assemblyProperties)
{
var assemblyTypeToSoucesMap = new Dictionary<AssemblyType, IEnumerable<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];

((List<string>)sourcesList).Add(source);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use IList here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
}

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")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
Loading