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

Create new RuntimeProvider to be associated with each ProxyOperationManager #653

Merged
merged 2 commits into from
Mar 23, 2017
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
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ internal DefaultTestHostManager(Architecture architecture, Framework framework,
var exitCode = 0;
this.processHelper.TryGetExitCode(process, out exitCode);

this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode));
this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode, process.Id));
});

/// <summary>
Expand Down Expand Up @@ -129,7 +129,7 @@ internal DefaultTestHostManager(Architecture architecture, Framework framework,
if (this.processHelper.TryGetExitCode(process, out exitCode))
{
EqtTrace.Error("Test host exited with error: {0}", this.testHostProcessStdError);
this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode));
this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode, process.Id));
}
});

Expand Down Expand Up @@ -258,7 +258,7 @@ private int LaunchHost(TestProcessStartInfo testHostStartInfo)
}
catch (OperationCanceledException ex)
{
this.OnHostExited(new HostProviderEventArgs(ex.Message, -1));
this.OnHostExited(new HostProviderEventArgs(ex.Message, -1, 0));
return -1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ internal DotnetTestHostManager(
var exitCode = 0;
this.processHelper.TryGetExitCode(process, out exitCode);

this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode));
this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode, process.Id));
});

/// <summary>
Expand Down Expand Up @@ -140,7 +140,7 @@ internal DotnetTestHostManager(
if (this.processHelper.TryGetExitCode(process, out exitCode))
{
EqtTrace.Error("Test host exited with error: {0}", this.testHostProcessStdError);
this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode));
this.OnHostExited(new HostProviderEventArgs(this.testHostProcessStdError.ToString(), exitCode, process.Id));
}
};

Expand Down
75 changes: 50 additions & 25 deletions src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine
using System;
using System.Linq;

using Microsoft.VisualStudio.TestPlatform.Common.Logging;
using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.Interfaces;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Hosting;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Host;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;

/// <summary>
/// Cross Platform test engine entry point for the client.
Expand All @@ -34,21 +34,28 @@ public class TestEngine : ITestEngine
/// <summary>
/// Fetches the DiscoveryManager for this engine. This manager would provide all functionality required for discovery.
/// </summary>
/// <param name="testHostManager"></param>
/// <returns>ITestDiscoveryManager object that can do discovery</returns>
/// <param name="testHostManager">
/// Test host manager
/// </param>
/// <param name="discoveryCriteria">
/// The discovery Criteria.
/// </param>
/// <returns>
/// ITestDiscoveryManager object that can do discovery
/// </returns>
public IProxyDiscoveryManager GetDiscoveryManager(ITestRuntimeProvider testHostManager, DiscoveryCriteria discoveryCriteria)
{
int parallelLevel = this.VerifyParallelSettingAndCalculateParallelLevel(discoveryCriteria.Sources.Count(), discoveryCriteria.RunSettings);
var parallelLevel = this.VerifyParallelSettingAndCalculateParallelLevel(discoveryCriteria.Sources.Count(), discoveryCriteria.RunSettings);

Func<IProxyDiscoveryManager> proxyDiscoveryManagerCreator = () => new ProxyDiscoveryManager(testHostManager);
if (!testHostManager.Shared)
Func<IProxyDiscoveryManager> proxyDiscoveryManagerCreator = delegate
{
return new ParallelProxyDiscoveryManager(proxyDiscoveryManagerCreator, parallelLevel, sharedHosts: testHostManager.Shared);
}
else
{
return proxyDiscoveryManagerCreator();
}
// Create a new HostProvider, to be associated with individual ProxyDiscoveryManager(&POM)
var hostManager = this.GetDefaultTestHostManager(XmlRunSettingsUtilities.GetRunConfigurationNode(discoveryCriteria.RunSettings));
hostManager.Initialize(TestSessionMessageLogger.Instance);

return new ProxyDiscoveryManager(hostManager);
};
return !testHostManager.Shared ? new ParallelProxyDiscoveryManager(proxyDiscoveryManagerCreator, parallelLevel, sharedHosts: testHostManager.Shared) : proxyDiscoveryManagerCreator();
}

/// <summary>
Expand All @@ -62,18 +69,26 @@ public IProxyDiscoveryManager GetDiscoveryManager(ITestRuntimeProvider testHostM
public IProxyExecutionManager GetExecutionManager(ITestRuntimeProvider testHostManager, TestRunCriteria testRunCriteria)
{
var distinctSources = GetDistinctNumberOfSources(testRunCriteria);
int parallelLevel = this.VerifyParallelSettingAndCalculateParallelLevel(distinctSources, testRunCriteria.TestRunSettings);
var parallelLevel = this.VerifyParallelSettingAndCalculateParallelLevel(distinctSources, testRunCriteria.TestRunSettings);

var runconfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(testRunCriteria.TestRunSettings);
var isDataCollectorEnabled = XmlRunSettingsUtilities.IsDataCollectionEnabled(testRunCriteria.TestRunSettings);

// SetupChannel ProxyExecutionManager with data collection if data collectors are specififed in run settings.
Func<IProxyExecutionManager> proxyExecutionManagerCreator =
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need tests for these changes ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we do, I was thinking to put them in next PR, as to unblock the CI flow?

() =>
isDataCollectorEnabled
? new ProxyExecutionManagerWithDataCollection(testHostManager, new ProxyDataCollectionManager(testRunCriteria.TestRunSettings))
: new ProxyExecutionManager(testHostManager);
Func<IProxyExecutionManager> proxyExecutionManagerCreator = delegate
{
// Create a new HostManager, to be associated with individual ProxyExecutionManager(&POM)
var hostManager = this.GetDefaultTestHostManager(XmlRunSettingsUtilities.GetRunConfigurationNode(testRunCriteria.TestRunSettings));
hostManager.Initialize(TestSessionMessageLogger.Instance);

if (testRunCriteria.TestHostLauncher != null)
{
hostManager.SetCustomLauncher(testRunCriteria.TestHostLauncher);
}

return isDataCollectorEnabled ? new ProxyExecutionManagerWithDataCollection(hostManager, new ProxyDataCollectionManager(testRunCriteria.TestRunSettings))
: new ProxyExecutionManager(hostManager);
};

// parallelLevel = 1 for desktop should go via else route.
if (parallelLevel > 1 || !testHostManager.Shared)
{
Expand All @@ -97,9 +112,12 @@ public ITestExtensionManager GetExtensionManager()
/// <summary>
/// Retrieves the default test host manager for this engine.
/// </summary>
/// <param name="architecture">The architecture we want the test host manager for.</param>
/// <param name="framework">Framework for the test session.</param>
/// <returns>An instance of the test host manager.</returns>
/// <param name="runConfiguration">
/// The run Configuration.
/// </param>
/// <returns>
/// An instance of the test host manager.
/// </returns>
public ITestRuntimeProvider GetDefaultTestHostManager(RunConfiguration runConfiguration)
{
var framework = runConfiguration.TargetFrameworkVersion;
Expand Down Expand Up @@ -138,8 +156,15 @@ private static int GetDistinctNumberOfSources(TestRunCriteria testRunCriteria)
/// <summary>
/// Verifies Parallel Setting and returns parallel level to use based on the run criteria
/// </summary>
/// <param name="testRunCriteria">Test Run Criteria</param>
/// <returns>Parallel Level to use</returns>
/// <param name="sourceCount">
/// The source Count.
/// </param>
/// <param name="runSettings">
/// The run Settings.
/// </param>
/// <returns>
/// Parallel Level to use
/// </returns>
private int VerifyParallelSettingAndCalculateParallelLevel(int sourceCount, string runSettings)
{
// Default is 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,17 @@ public HostProviderEventArgs(string message)
this.ErrroCode = 0;
}

public HostProviderEventArgs(string message, int errorCode)
public HostProviderEventArgs(string message, int errorCode, int processId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:
Why don't we have separate cs file for these ?
Please add xml comments for anything that is public.

{
this.Data = message;
this.ErrroCode = errorCode;
this.ProcessId = processId;
}

public string Data { get; set; }

public int ErrroCode { get; set; }

public int ProcessId { get; set; }
}
}