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

Adding CancellationToken to Testhost launch async #917

Merged
merged 9 commits into from Jul 11, 2017
@@ -7,6 +7,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
using Microsoft.VisualStudio.TestPlatform.Common;
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
@@ -26,6 +27,7 @@ public class ProxyDiscoveryManager : ProxyOperationManager, IProxyDiscoveryManag
{
private readonly ITestRuntimeProvider testHostManager;
private IDataSerializer dataSerializer;
private CancellationTokenSource cancellationTokenSource;
#region Constructors
@@ -50,6 +52,7 @@ public ProxyDiscoveryManager(ITestRequestSender testRequestSender, ITestRuntimeP
/// <param name="testHostManager">
/// Test host Manager instance
/// </param>
/// <param name="dataSerializer"></param>
/// <param name="clientConnectionTimeout">
/// The client Connection Timeout
/// </param>
@@ -62,6 +65,7 @@ public ProxyDiscoveryManager(ITestRequestSender testRequestSender, ITestRuntimeP
{
this.dataSerializer = dataSerializer;
this.testHostManager = testHostManager;
this.cancellationTokenSource = new CancellationTokenSource();
}
#endregion
@@ -97,7 +101,7 @@ public void DiscoverTests(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEve
this.InitializeExtensions(discoveryCriteria.Sources);
}
this.SetupChannel(discoveryCriteria.Sources);
this.SetupChannel(discoveryCriteria.Sources, this.cancellationTokenSource.Token);
this.RequestSender.DiscoverTests(discoveryCriteria, eventHandler);
}
catch (Exception exception)
@@ -146,7 +150,7 @@ private void InitializeExtensions(IEnumerable<string> sources)
// Only send this if needed.
if (platformExtensions.Any())
{
this.SetupChannel(sourceList);
this.SetupChannel(sourceList, this.cancellationTokenSource.Token);
this.RequestSender.InitializeDiscovery(platformExtensions, TestPluginCache.Instance.LoadOnlyWellKnownExtensions);
}
@@ -7,7 +7,10 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
using Microsoft.VisualStudio.TestPlatform.Common;
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
@@ -20,8 +23,6 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
using Constants = Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Constants;
using System.Text.RegularExpressions;
using Microsoft.VisualStudio.TestPlatform.Common;
/// <summary>
/// Orchestrates test execution operations for the engine communicating with the client.
@@ -30,17 +31,17 @@ internal class ProxyExecutionManager : ProxyOperationManager, IProxyExecutionMan
{
private readonly ITestRuntimeProvider testHostManager;
private IDataSerializer dataSerializer;
private CancellationTokenSource cancellationTokenSource;
/// <inheritdoc/>
public bool IsInitialized { get; private set; } = false;
#region Constructors
/// <summary>
/// Initializes a new instance of the <see cref="ProxyExecutionManager"/> class.
/// </summary>
/// <param name="testRequestSender">Test request sender instance.</param>
/// <param name="requestSender">Test request sender instance.</param>
/// <param name="testHostManager">Test host manager for this proxy.</param>
public ProxyExecutionManager(ITestRequestSender requestSender, ITestRuntimeProvider testHostManager) : this(requestSender, testHostManager, JsonDataSerializer.Instance, Constants.ClientConnectionTimeout)
{
@@ -52,12 +53,14 @@ public ProxyExecutionManager(ITestRequestSender requestSender, ITestRuntimeProvi
/// </summary>
/// <param name="requestSender">Request Sender instance</param>
/// <param name="testHostManager">Test host manager instance</param>
/// <param name="dataSerializer"></param>
/// <param name="clientConnectionTimeout">The client Connection Timeout</param>
internal ProxyExecutionManager(ITestRequestSender requestSender, ITestRuntimeProvider testHostManager, IDataSerializer dataSerializer, int clientConnectionTimeout)
: base(requestSender, testHostManager, clientConnectionTimeout)
{
this.testHostManager = testHostManager;
this.dataSerializer = dataSerializer;
this.cancellationTokenSource = new CancellationTokenSource();
}
#endregion
@@ -76,6 +79,7 @@ public virtual void Initialize()
EqtTrace.Verbose("ProxyExecutionManager: Test host is shared. SetupChannel it early.");
this.InitializeExtensions(Enumerable.Empty<string>());
}
this.IsInitialized = true;
}
@@ -105,7 +109,7 @@ public virtual int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsH
this.InitializeExtensions(testSources);
}
this.SetupChannel(testRunCriteria.Sources);
this.SetupChannel(testRunCriteria.Sources, this.cancellationTokenSource.Token);

This comment has been minimized.

@codito

codito Jul 10, 2017

Contributor

Should we consider defining this CancellationTokenSource in ProxyOperationManager?

@codito

codito Jul 10, 2017

Contributor

Should we consider defining this CancellationTokenSource in ProxyOperationManager?

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

next PR

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

next PR

var executionContext = new TestExecutionContext(
testRunCriteria.FrequencyOfRunStatsChangeEvent,
@@ -191,7 +195,7 @@ private void InitializeExtensions(IEnumerable<string> sources)
// Only send this if needed.
if (platformExtensions.Any())
{
this.SetupChannel(sourceList);
this.SetupChannel(sourceList, this.cancellationTokenSource.Token);
this.RequestSender.InitializeExecution(platformExtensions, TestPluginCache.Instance.LoadOnlyWellKnownExtensions);
}
@@ -7,20 +7,19 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Host;
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces;
using Microsoft.VisualStudio.TestPlatform.Utilities;
using CrossPlatEngineResources = Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Resources.Resources;
using System.Reflection;
using System.Linq;
/// <summary>
/// Base class for any operations that the client needs to drive through the engine.
@@ -33,9 +32,10 @@ public abstract class ProxyOperationManager
private readonly string versionCheckPropertyName = "IsVersionCheckRequired";
private readonly ManualResetEventSlim testHostExited = new ManualResetEventSlim(false);
private bool channelConnected;
private bool initialized;
private string testHostProcessStdError;
private int testHostProcessId;
private bool testHostLaunched;
#region Constructors
@@ -52,7 +52,8 @@ protected ProxyOperationManager(ITestRequestSender requestSender, ITestRuntimePr
this.testHostManager = testHostManager;
this.processHelper = new ProcessHelper();
this.initialized = false;
this.testHostProcessId = -1;
this.testHostLaunched = false;
this.channelConnected = false;
}
#endregion
@@ -75,10 +76,10 @@ protected ProxyOperationManager(ITestRequestSender requestSender, ITestRuntimePr
/// Usually includes starting up the test host process.
/// </summary>
/// <param name="sources">List of test sources.</param>
public virtual void SetupChannel(IEnumerable<string> sources)
/// <param name="cancellationToken"></param>
public virtual void SetupChannel(IEnumerable<string> sources, CancellationToken cancellationToken)
{
var connTimeout = this.connectionTimeout;
// ToDo: change SetupChannel to return bool, which will specify that a successfull connection with testhost was established or not
if (!this.initialized)
{
this.testHostProcessStdError = string.Empty;
@@ -91,41 +92,27 @@ public virtual void SetupChannel(IEnumerable<string> sources)
// Subscribe to TestHost Event
this.testHostManager.HostLaunched += this.TestHostManagerHostLaunched;
this.testHostManager.HostExited += this.TestHostManagerHostExited;

This comment has been minimized.

@codito

codito Jul 11, 2017

Contributor

nit: remove extra space

@codito

codito Jul 11, 2017

Contributor

nit: remove extra space

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

resolved

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

resolved

// Get the test process start info
var testHostStartInfo = this.UpdateTestProcessStartInfo(this.testHostManager.GetTestHostProcessStartInfo(sources, null, connectionInfo));
// Launch the test host.
var hostLaunchedTask = this.testHostManager.LaunchTestHostAsync(testHostStartInfo);
try
{
this.testHostProcessId = hostLaunchedTask.Result;
// Launch the test host.
var hostLaunchedTask = this.testHostManager.LaunchTestHostAsync(testHostStartInfo, cancellationToken);
this.testHostLaunched = hostLaunchedTask.Result;
}
catch (OperationCanceledException ex)
catch (Exception ex)
{
throw new TestPlatformException(string.Format(CultureInfo.CurrentUICulture, ex.Message));
}
// Warn the user that execution will wait for debugger attach.
var hostDebugEnabled = Environment.GetEnvironmentVariable("VSTEST_HOST_DEBUG");
if (!string.IsNullOrEmpty(hostDebugEnabled) && hostDebugEnabled.Equals("1", StringComparison.Ordinal))
{
ConsoleOutput.Instance.WriteLine(CrossPlatEngineResources.HostDebuggerWarning, OutputLevel.Warning);
ConsoleOutput.Instance.WriteLine(
string.Format("Process Id: {0}, Name: {1}", hostLaunchedTask.Result, this.processHelper.GetProcessName(hostLaunchedTask.Result)),
OutputLevel.Information);
// Increase connection timeout when debugging is enabled.
connTimeout = 5 * this.connectionTimeout;
}
// Wait for a timeout for the client to connect.
if (!this.RequestSender.WaitForRequestHandlerConnection(connTimeout))
// Test host launched failed because of user cancellation or any other runtime exception
// or connection with test host did not happen
if (!this.testHostLaunched || !this.channelConnected)
{
var errorMsg = CrossPlatEngineResources.InitializationFailed;
if (!string.IsNullOrWhiteSpace(this.testHostProcessStdError.ToString()))
if (!string.IsNullOrWhiteSpace(this.testHostProcessStdError))
{
// Testhost failed with error
errorMsg = string.Format(CrossPlatEngineResources.TestHostExitedWithError, this.testHostProcessStdError);
@@ -160,7 +147,15 @@ public virtual void Close()
{
try
{
this.RequestSender.EndSession();
// do not send message if host did not launch
if (this.testHostLaunched)
{
this.RequestSender.EndSession();
// We want to give test host a chance to safely close.
// The upper bound for wait should be 100ms.
this.testHostExited.Wait(100);
}
}
catch (Exception ex)
{
@@ -172,13 +167,10 @@ public virtual void Close()
{
this.initialized = false;
// We don't need to terminate if the test host has already terminated. The upper bound
// for wait should be 100ms.
if (this.testHostProcessId != -1 && !this.testHostExited.Wait(100))
{
EqtTrace.Warning("ProxyOperationManager: Timed out waiting for test host to exit. Will terminate process.");
this.testHostManager.TerminateAsync(this.testHostProcessId, CancellationToken.None).Wait();
}
EqtTrace.Warning("ProxyOperationManager: Timed out waiting for test host to exit. Will terminate process.");
// please clean up test host.
this.testHostManager.CleanTestHostAsync(CancellationToken.None).Wait();
this.testHostManager.HostExited -= this.TestHostManagerHostExited;
this.testHostManager.HostLaunched -= this.TestHostManagerHostLaunched;
@@ -205,7 +197,9 @@ protected virtual TestProcessStartInfo UpdateTestProcessStartInfo(TestProcessSta
protected string GetTimestampedLogFile(string logFile)
{
if (string.IsNullOrWhiteSpace(logFile))
{
return null;
}
return Path.ChangeExtension(
logFile,
@@ -218,7 +212,29 @@ protected string GetTimestampedLogFile(string logFile)
private void TestHostManagerHostLaunched(object sender, HostProviderEventArgs e)
{
EqtTrace.Verbose(e.Data);
var connTimeout = this.connectionTimeout;
// Warn the user that execution will wait for debugger attach.
var hostDebugEnabled = Environment.GetEnvironmentVariable("VSTEST_HOST_DEBUG");
if (!string.IsNullOrEmpty(hostDebugEnabled) && hostDebugEnabled.Equals("1", StringComparison.Ordinal))
{
ConsoleOutput.Instance.WriteLine(CrossPlatEngineResources.HostDebuggerWarning, OutputLevel.Warning);
ConsoleOutput.Instance.WriteLine(
string.Format("Process Id: {0}, Name: {1}", e.ProcessId, this.processHelper.GetProcessName(e.ProcessId)),
OutputLevel.Information);
// Increase connection timeout when debugging is enabled.
connTimeout = 5 * this.connectionTimeout;
}
// Wait for a timeout for the client to connect.
if (!this.RequestSender.WaitForRequestHandlerConnection(connTimeout))
{
this.channelConnected = false;
return;
}
this.channelConnected = true;
}
private void TestHostManagerHostExited(object sender, HostProviderEventArgs e)
@@ -9,6 +9,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine
using Microsoft.VisualStudio.TestPlatform.Common.Hosting;
using Microsoft.VisualStudio.TestPlatform.Common.Logging;
using Microsoft.VisualStudio.TestPlatform.Common.Utilities;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel;
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection;
@@ -17,7 +18,6 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Host;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
/// <summary>
/// Cross Platform test engine entry point for the client.
@@ -5,12 +5,12 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Host
{
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.Interfaces;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
using System.Threading;
/// <summary>
/// Interface for TestRuntimeProvider which manages test host processes for test engine.
@@ -20,11 +20,12 @@ public interface ITestRuntimeProvider
#region events
/// <summary>
/// Raised when host is launched successfully
/// Consumed by TestPlatform to initialize connection b/w test host and testplatform
/// </summary>
event EventHandler<HostProviderEventArgs> HostLaunched;
/// <summary>
/// Raised when host is reports Error
/// Raised when host is cleaned up and removes all it's dependecies

This comment has been minimized.

@codito

codito Jul 11, 2017

Contributor

nit: spell check

@codito

codito Jul 11, 2017

Contributor

nit: spell check

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

resolved

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

resolved

/// </summary>
event EventHandler<HostProviderEventArgs> HostExited;
@@ -65,8 +66,9 @@ public interface ITestRuntimeProvider
/// Launches the test host for discovery/execution.
/// </summary>
/// <param name="testHostStartInfo">Start parameters for the test host.</param>
/// <returns>ProcessId of launched Process. 0 means not launched.</returns>
Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo);
/// <param name="cancellationToken"></param>
/// <returns>Returns whether the test host lauched successfully or not.</returns>
Task<bool> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken);

This comment has been minimized.

@codito

codito Jul 11, 2017

Contributor

Update documentation

@codito

codito Jul 11, 2017

Contributor

Update documentation

/// <summary>
/// Gets the start parameters for the test host.
@@ -83,15 +85,20 @@ public interface ITestRuntimeProvider
/// mechanism. E.g. for .net core, extensions are discovered from the <c>testproject.deps.json</c> file.
/// </summary>
/// <param name="sources">List of test sources.</param>
/// <param name="extensions"></param>
/// <returns>List of paths to extension assemblies.</returns>
IEnumerable<string> GetTestPlatformExtensions(IEnumerable<string> sources, IEnumerable<string> extensions);
/// <summary>
/// Terminate the test host process.
/// Cleanup the test host process and it's dependencies.
/// </summary>
/// <param name="processId">Process identifier for the test host.</param>
/// <param name="cancellationToken">Cancellation token.</param>
Task TerminateAsync(int processId, CancellationToken cancellationToken);
/// <param name="cancellationToken">
/// Cancellation token.
/// </param>
/// <returns>
/// The <see cref="Task"/>.
/// </returns>
Task CleanTestHostAsync(CancellationToken cancellationToken);
}
/// <summary>
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.