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

Perf improvements #1517

Merged
merged 6 commits into from
Mar 28, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 9 additions & 7 deletions src/Microsoft.TestPlatform.Client/TestPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Client
/// <summary>
/// Implementation for TestPlatform
/// </summary>
public class TestPlatform : ITestPlatform
internal class TestPlatform : ITestPlatform
Copy link
Contributor

@acesiddhu acesiddhu Mar 28, 2018

Choose a reason for hiding this comment

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

internal [](start = 4, length = 8)

I am assuming adapters to customer havent taken a dependency on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

{
private readonly TestRuntimeProviderManager testHostProviderManager;

Expand Down Expand Up @@ -79,11 +79,12 @@ protected TestPlatform(ITestEngine testEngine, IFileHelper filehelper, TestRunti
/// <summary>
/// The create discovery request.
/// </summary>
/// <param name="requestData"></param>
/// <param name="requestData">Request data.</param>
/// <param name="discoveryCriteria"> The discovery criteria. </param>
/// <param name="options">Test platform options.</param>
/// <returns> The <see cref="IDiscoveryRequest"/>. </returns>
/// <exception cref="ArgumentNullException"> Throws if parameter is null. </exception>
public IDiscoveryRequest CreateDiscoveryRequest(IRequestData requestData, DiscoveryCriteria discoveryCriteria)
public IDiscoveryRequest CreateDiscoveryRequest(IRequestData requestData, DiscoveryCriteria discoveryCriteria, TestPlatformOptions options)
{
if (discoveryCriteria == null)
{
Expand All @@ -110,19 +111,20 @@ public IDiscoveryRequest CreateDiscoveryRequest(IRequestData requestData, Discov
testHostManager.Initialize(TestSessionMessageLogger.Instance, discoveryCriteria.RunSettings);

var discoveryManager = this.TestEngine.GetDiscoveryManager(requestData, testHostManager, discoveryCriteria);
discoveryManager.Initialize();
discoveryManager.Initialize(options?.SkipDefaultAdapters ?? false);

return new DiscoveryRequest(requestData, discoveryCriteria, discoveryManager, loggerManager);
}

/// <summary>
/// The create test run request.
/// </summary>
/// <param name="requestData">Request data.</param>
/// <param name="testRunCriteria"> The test run criteria. </param>
/// <param name="protocolConfig"> Protocol related information. </param>
/// <param name="options">Test platform options.</param>
/// <returns> The <see cref="ITestRunRequest"/>. </returns>
/// <exception cref="ArgumentNullException"> Throws if parameter is null. </exception>
public ITestRunRequest CreateTestRunRequest(IRequestData requestData, TestRunCriteria testRunCriteria)
public ITestRunRequest CreateTestRunRequest(IRequestData requestData, TestRunCriteria testRunCriteria, TestPlatformOptions options)
{
if (testRunCriteria == null)
{
Expand Down Expand Up @@ -154,7 +156,7 @@ public ITestRunRequest CreateTestRunRequest(IRequestData requestData, TestRunCri
}

var executionManager = this.TestEngine.GetExecutionManager(requestData, testHostManager, testRunCriteria);
executionManager.Initialize();
executionManager.Initialize(options?.SkipDefaultAdapters ?? false);

return new TestRunRequest(requestData, testRunCriteria, executionManager, loggerManager);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,16 @@ internal set
/// Gets a list of all extension paths filtered by input string.
/// </summary>
/// <param name="endsWithPattern">Pattern to filter extension paths.</param>
public List<string> GetExtensionPaths(string endsWithPattern)
public List<string> GetExtensionPaths(string endsWithPattern, bool skipDefaultExtensions = false)
{
return this.GetFilteredExtensions(this.filterableExtensionPaths, endsWithPattern)
.Concat(this.defaultExtensionPaths)
.Concat(this.unfilterableExtensionPaths)
.ToList();
var extensions = this.GetFilteredExtensions(this.filterableExtensionPaths, endsWithPattern);

if (!skipDefaultExtensions)
{
extensions = extensions.Concat(this.defaultExtensionPaths);
}

return extensions.Concat(this.unfilterableExtensionPaths).ToList();
}

/// <summary>
Expand Down Expand Up @@ -390,14 +394,14 @@ internal IList<string> GetDefaultResolutionPaths()
/// <returns>
/// The list of files which match the regex pattern
/// </returns>
protected virtual List<string> GetFilteredExtensions(List<string> extensions, string endsWithPattern)
protected virtual IEnumerable<string> GetFilteredExtensions(List<string> extensions, string endsWithPattern)
{
if (string.IsNullOrEmpty(endsWithPattern))
{
return extensions;
}

return extensions.Where(ext => ext.EndsWith(endsWithPattern, StringComparison.OrdinalIgnoreCase)).ToList();
return extensions.Where(ext => ext.EndsWith(endsWithPattern, StringComparison.OrdinalIgnoreCase));
}

private static bool TryMergeExtensionPaths(List<string> extensionsList, List<string> additionalExtensions, out List<string> mergedExtensionsList)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ public interface IProxyDiscoveryManager
{
/// <summary>
/// Initializes test discovery. Create the test host, setup channel and initialize extensions.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
void Initialize();
void Initialize(bool skipDefaultAdapters);

/// <summary>
/// Discovers tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ public interface IProxyExecutionManager

/// <summary>
/// Initializes test execution. Create the test host, setup channel and initialize extensions.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
void Initialize();
void Initialize(bool skipDefaultAdapters);

/// <summary>
/// Starts the test run.
Expand Down
12 changes: 12 additions & 0 deletions src/Microsoft.TestPlatform.CoreUtilities/Tracing/EqtTrace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ public static string LogFile
}
}

public static bool DoNotInitailize
{
get
{
return traceImpl.DoNotInitialize;
}
set
{
traceImpl.DoNotInitialize = value;
}
}

public static string ErrorOnInitialization
{
get;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ public InProcessProxyDiscoveryManager(ITestRuntimeProvider testHostManager, ITes

/// <summary>
/// Initializes test discovery.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
public void Initialize()
public void Initialize(bool skipDefaultAdapters)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ public InProcessProxyExecutionManager(ITestRuntimeProvider testHostManager, ITes

/// <summary>
/// Initialize adapters.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
public void Initialize()
public void Initialize(bool skipDefaultAdapters)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ internal ParallelProxyDiscoveryManager(IRequestData requestData, Func<IProxyDisc
#region IProxyDiscoveryManager

/// <inheritdoc/>
public void Initialize()
public void Initialize(bool skipDefaultAdapters)
{
this.DoActionOnAllManagers((proxyManager) => proxyManager.Initialize(), doActionsInParallel: true);
this.DoActionOnAllManagers((proxyManager) => proxyManager.Initialize(skipDefaultAdapters), doActionsInParallel: true);
}

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ internal class ParallelProxyExecutionManager : ParallelOperationManager<IProxyEx
private ParallelRunDataAggregator currentRunDataAggregator;

private IRequestData requestData;
private bool skipDefaultAdapters;

/// <inheritdoc/>
public bool IsInitialized { get; private set; } = false;
Expand Down Expand Up @@ -84,9 +85,10 @@ internal ParallelProxyExecutionManager(IRequestData requestData, Func<IProxyExec

#region IProxyExecutionManager

public void Initialize()
public void Initialize(bool skipDefaultAdapters)
{
this.DoActionOnAllManagers((proxyManager) => proxyManager.Initialize(), doActionsInParallel: true);
this.skipDefaultAdapters = skipDefaultAdapters;
this.DoActionOnAllManagers((proxyManager) => proxyManager.Initialize(skipDefaultAdapters), doActionsInParallel: true);
this.IsInitialized = true;
}

Expand Down Expand Up @@ -318,7 +320,7 @@ private void StartTestRunOnConcurrentManager(IProxyExecutionManager proxyExecuti
{
if (!proxyExecutionManager.IsInitialized)
{
proxyExecutionManager.Initialize();
proxyExecutionManager.Initialize(this.skipDefaultAdapters);
}

Task.Run(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class ProxyDiscoveryManager : ProxyOperationManager, IProxyDiscoveryManag
private bool isCommunicationEstablished;
private IRequestData requestData;
private ITestDiscoveryEventsHandler2 baseTestDiscoveryEventsHandler;
private bool skipDefaultAdapters;

#region Constructors

Expand Down Expand Up @@ -81,9 +82,11 @@ internal ProxyDiscoveryManager(

/// <summary>
/// Ensure that the discovery component of engine is ready for discovery usually by loading extensions.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
public void Initialize()
public void Initialize(bool skipDefaultAdapters)
{
this.skipDefaultAdapters = skipDefaultAdapters;
}

/// <summary>
Expand Down Expand Up @@ -175,7 +178,7 @@ public void HandleLogMessage(TestMessageLevel level, string message)

private void InitializeExtensions(IEnumerable<string> sources)
{
var extensions = TestPluginCache.Instance.GetExtensionPaths(TestPlatformConstants.TestAdapterEndsWithPattern);
var extensions = TestPluginCache.Instance.GetExtensionPaths(TestPlatformConstants.TestAdapterEndsWithPattern, this.skipDefaultAdapters);
var sourceList = sources.ToList();
var platformExtensions = this.testHostManager.GetTestPlatformExtensions(sourceList, extensions).ToList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ internal class ProxyExecutionManager : ProxyOperationManager, IProxyExecutionMan
private bool isCommunicationEstablished;
private IRequestData requestData;
private ITestRunEventsHandler baseTestRunEventsHandler;
private bool skipDefaultAdapters;

/// <inheritdoc/>
public bool IsInitialized { get; private set; } = false;
Expand Down Expand Up @@ -77,9 +78,11 @@ internal ProxyExecutionManager(IRequestData requestData, ITestRequestSender requ

/// <summary>
/// Ensure that the Execution component of engine is ready for execution usually by loading extensions.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
public virtual void Initialize()
public virtual void Initialize(bool skipDefaultAdapters)
{
this.skipDefaultAdapters = skipDefaultAdapters;
this.IsInitialized = true;
}

Expand Down Expand Up @@ -257,7 +260,7 @@ private void LogMessage(TestMessageLevel testMessageLevel, string message)

private void InitializeExtensions(IEnumerable<string> sources)
{
var extensions = TestPluginCache.Instance.GetExtensionPaths(TestPlatformConstants.TestAdapterEndsWithPattern);
var extensions = TestPluginCache.Instance.GetExtensionPaths(TestPlatformConstants.TestAdapterEndsWithPattern, this.skipDefaultAdapters);
var sourceList = sources.ToList();
var platformExtensions = this.testHostManager.GetTestPlatformExtensions(sourceList, extensions).ToList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ internal IProxyDataCollectionManager ProxyDataCollectionManager

/// <summary>
/// Ensure that the Execution component of engine is ready for execution usually by loading extensions.
/// <param name="skipDefaultAdapters">Skip default adapters flag.</param>
/// </summary>
public override void Initialize()
public override void Initialize(bool skipDefaultAdapters)
{
this.ProxyDataCollectionManager.Initialize();

Expand All @@ -97,7 +98,7 @@ public override void Initialize()
throw;
}

base.Initialize();
base.Initialize(skipDefaultAdapters);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,17 @@ public interface ITestPlatform : IDisposable
/// </summary>
/// <param name="requestData">Providing common services and data for Discovery</param>
/// <param name="discoveryCriteria">Specifies the discovery parameters</param>
/// <param name="options">Test platform options.</param>
/// <returns>DiscoveryRequest object</returns>
IDiscoveryRequest CreateDiscoveryRequest(IRequestData requestData, DiscoveryCriteria discoveryCriteria);
IDiscoveryRequest CreateDiscoveryRequest(IRequestData requestData, DiscoveryCriteria discoveryCriteria, TestPlatformOptions options);
Copy link
Contributor

Choose a reason for hiding this comment

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

IDiscoveryRequest [](start = 8, length = 17)

this is a public interface. isnt this a breakingchange

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 this is not, since Tpv2 this Interface is not meant to be implemented by anyone outside vstest.console
If you see in ITestPlatform we have also made the class internal


/// <summary>
/// Creates a test run request.
/// </summary>
/// <param name="requestData">Providing common services and data for Execution</param>
/// <param name="testRunCriteria">Specifies the test run criteria</param>
/// <param name="options">Test platform options.</param>
/// <returns>RunRequest object</returns>
ITestRunRequest CreateTestRunRequest(IRequestData requestData, TestRunCriteria testRunCriteria);
ITestRunRequest CreateTestRunRequest(IRequestData requestData, TestRunCriteria testRunCriteria, TestPlatformOptions options);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,11 @@ public class TestPlatformOptions
/// </summary>
[DataMember]
public bool CollectMetrics { get; set; }

/// <summary>
/// Gets or sets whether default adapters should be skipped or not.
/// </summary>
[DataMember]
public bool SkipDefaultAdapters { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel
/// </summary>
public partial interface IPlatformEqtTrace
{
// This is added to ensure that tracing for testhost/datacollector should not be instantiated if not enabled via --diag switch.
bool DoNotInitialize
{
get;
set;
}

/// <summary>
/// Adds the message to the trace log.
/// The line becomes:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public static string ErrorOnInitialization
}

// This is added to ensure that traceSource should not be instantiated in when creating appdomains if EqtTrace is not enabled.
internal static bool DoNotInitialize
public bool DoNotInitialize
{
get;
set;
Expand Down Expand Up @@ -175,7 +175,7 @@ public bool InitializeVerboseTrace(string customLogFile)
/// <inheritdoc/>
public bool ShouldTrace(PlatformTraceLevel traceLevel)
{
if (DoNotInitialize)
if (this.DoNotInitialize)
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void SetupRemoteEqtTraceListeners(AppDomain childDomain)
}
else
{
remoteEqtTrace.DoNotInitialize = true;
this.DoNotInitialize = true;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,6 @@ public TraceLevel TraceLevel
}
}

/// <summary>
/// This is added to ensure that traceSource should not be instantiated in when creating appdomains if EqtTrace is not enabled.
/// </summary>
internal bool DoNotInitialize
{
set
{
PlatformEqtTrace.DoNotInitialize = value;
}
}

/// <summary>
/// Register listeners from parent domain in current domain.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ public class PlatformEqtTrace : IPlatformEqtTrace
{
public static string ErrorOnInitialization { get; set; }

public bool DoNotInitialize
{
get => throw new NotImplementedException();
set => throw new NotImplementedException();
}

public void WriteLine(PlatformTraceLevel level, string message)
{
throw new NotImplementedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ public class PlatformEqtTrace : IPlatformEqtTrace

public static string LogFile { get; set; }

public bool DoNotInitialize
{
get;
set;
}

private static PlatformTraceLevel TraceLevel { get; set; }

/// <inheritdoc/>
Expand Down
Loading