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
Next

Adding CancellationToken to Testhost launch async

  • Loading branch information...
mayankbansal018 committed Jul 6, 2017
commit 7d07627be381deb75d46343ef9bffbcb897e2633
@@ -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.
@@ -94,26 +93,28 @@ public virtual void SetupChannel(IEnumerable<string> sources)
// 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
{
// Launch the test host.
var hostLaunchedTask = this.testHostManager.LaunchTestHostAsync(testHostStartInfo, CancellationToken.None);
this.testHostProcessId = hostLaunchedTask.Result;
}
catch (OperationCanceledException ex)
{
throw new TestPlatformException(string.Format(CultureInfo.CurrentUICulture, ex.Message));
}
catch (AggregateException 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)),
string.Format("Process Id: {0}, Name: {1}", this.testHostProcessId, this.processHelper.GetProcessName(this.testHostProcessId)),
OutputLevel.Information);
// Increase connection timeout when debugging is enabled.
@@ -125,7 +126,7 @@ public virtual void SetupChannel(IEnumerable<string> sources)
{
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);
@@ -205,7 +206,9 @@ protected virtual TestProcessStartInfo UpdateTestProcessStartInfo(TestProcessSta
protected string GetTimestampedLogFile(string logFile)
{
if (string.IsNullOrWhiteSpace(logFile))
{
return null;
}
return Path.ChangeExtension(
logFile,
@@ -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.
@@ -65,8 +65,9 @@ public interface ITestRuntimeProvider
/// Launches the test host for discovery/execution.
/// </summary>
/// <param name="testHostStartInfo">Start parameters for the test host.</param>
/// <param name="cancellationToken"></param>
/// <returns>ProcessId of launched Process. 0 means not launched.</returns>

This comment has been minimized.

@Faizan2304

Faizan2304 Jul 11, 2017

Contributor

Need to update return string as the signature has been updated.

@Faizan2304

Faizan2304 Jul 11, 2017

Contributor

Need to update return string as the signature has been updated.

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

resolved

@mayankbansal018

mayankbansal018 Jul 11, 2017

Contributor

resolved

Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo);
Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken);

This comment has been minimized.

@codito

codito Jul 7, 2017

Contributor

I think the return code is used to terminate the host later.

Will the return type be always an int? Could there be a scenario where the TestRuntimeProvider requires more information (like the Emulator prodess id and the testhost process id)?

@codito

codito Jul 7, 2017

Contributor

I think the return code is used to terminate the host later.

Will the return type be always an int? Could there be a scenario where the TestRuntimeProvider requires more information (like the Emulator prodess id and the testhost process id)?

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 7, 2017

Contributor

If Runtime provider need to keep additional information, then it can store that info locally. As for lauchasync it's job it to return whether host was successfully launched or not.

@mayankbansal018

mayankbansal018 Jul 7, 2017

Contributor

If Runtime provider need to keep additional information, then it can store that info locally. As for lauchasync it's job it to return whether host was successfully launched or not.

This comment has been minimized.

@codito

codito Jul 10, 2017

Contributor

Make this a bool or just Task then?

@codito

codito Jul 10, 2017

Contributor

Make this a bool or just Task then?

/// <summary>
/// Gets the start parameters for the test host.
@@ -83,14 +84,22 @@ 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.
/// </summary>
/// <param name="processId">Process identifier for the test host.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <param name="processId">
/// Process identifier for the test host.
/// </param>
/// <param name="cancellationToken">
/// Cancellation token.
/// </param>
/// <returns>
/// The <see cref="Task"/>.
/// </returns>
Task TerminateAsync(int processId, CancellationToken cancellationToken);
}
@@ -114,9 +114,9 @@ public void SetCustomLauncher(ITestHostLauncher customLauncher)
}
/// <inheritdoc/>
public async Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo)
public async Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
return await Task.Run(() => this.LaunchHost(testHostStartInfo), CancellationToken.None);
return await Task.Run(() => this.LaunchHost(testHostStartInfo, cancellationToken), cancellationToken);
}
/// <inheritdoc/>
@@ -238,7 +238,7 @@ private void OnHostExited(HostProviderEventArgs e)
}
}
private int LaunchHost(TestProcessStartInfo testHostStartInfo)
private int LaunchHost(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
try
{
@@ -248,6 +248,8 @@ private int LaunchHost(TestProcessStartInfo testHostStartInfo)
if (this.customTestHostLauncher == null)
{
EqtTrace.Verbose("DefaultTestHostManager: Starting process '{0}' with command line '{1}'", testHostStartInfo.FileName, testHostStartInfo.Arguments);
cancellationToken.ThrowIfCancellationRequested();
this.testHostProcess = this.processHelper.LaunchProcess(testHostStartInfo.FileName, testHostStartInfo.Arguments, testHostStartInfo.WorkingDirectory, testHostStartInfo.EnvironmentVariables, this.ErrorReceivedCallback, this.ExitCallBack) as Process;
}
else
@@ -259,10 +261,10 @@ private int LaunchHost(TestProcessStartInfo testHostStartInfo)
catch (OperationCanceledException ex)
{
this.OnHostExited(new HostProviderEventArgs(ex.Message, -1, 0));
return -1;
throw;

This comment has been minimized.

@codito

codito Jul 7, 2017

Contributor

Is it an expectation that LaunchTestHost should throw? Why not let the caller get the failure from HostExited event?

@codito

codito Jul 7, 2017

Contributor

Is it an expectation that LaunchTestHost should throw? Why not let the caller get the failure from HostExited event?

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 7, 2017

Contributor

Yes, there is no reason that LaunchTest should throw, the caller can as much get the information from the exited event.

@mayankbansal018

mayankbansal018 Jul 7, 2017

Contributor

Yes, there is no reason that LaunchTest should throw, the caller can as much get the information from the exited event.

This comment has been minimized.

@Faizan2304

Faizan2304 Jul 7, 2017

Contributor

+1

@Faizan2304

Faizan2304 Jul 7, 2017

Contributor

+1

}
var pId = this.testHostProcess != null ? this.testHostProcess.Id : 0;
var pId = this.testHostProcess?.Id ?? 0;

This comment has been minimized.

@codito

codito Jul 7, 2017

Contributor

nit: earlier statement was more readable.

@codito

codito Jul 7, 2017

Contributor

nit: earlier statement was more readable.

This comment has been minimized.

@mayankbansal018

mayankbansal018 Jul 7, 2017

Contributor

resharper did this

@mayankbansal018

mayankbansal018 Jul 7, 2017

Contributor

resharper did this

this.OnHostLaunched(new HostProviderEventArgs("Test Runtime launched with Pid: " + pId));
return pId;
}
@@ -143,9 +143,9 @@ public void SetCustomLauncher(ITestHostLauncher customLauncher)
}
/// <inheritdoc/>
public virtual async Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo)
public virtual async Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
return await Task.Run(() => this.LaunchHost(testHostStartInfo), CancellationToken.None);
return await Task.Run(() => this.LaunchHost(testHostStartInfo, cancellationToken), cancellationToken);
}
/// <inheritdoc/>
@@ -304,18 +304,28 @@ private void OnHostExited(HostProviderEventArgs e)
}
}
private int LaunchHost(TestProcessStartInfo testHostStartInfo)
private int LaunchHost(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
this.testHostProcessStdError = new StringBuilder(this.ErrorLength, this.ErrorLength);
if (this.testHostLauncher == null)
try
{
EqtTrace.Verbose("DotnetTestHostManager: Starting process '{0}' with command line '{1}'", testHostStartInfo.FileName, testHostStartInfo.Arguments);
this.testHostProcess = this.processHelper.LaunchProcess(testHostStartInfo.FileName, testHostStartInfo.Arguments, testHostStartInfo.WorkingDirectory, testHostStartInfo.EnvironmentVariables, this.ErrorReceivedCallback, this.ExitCallBack) as Process;
this.testHostProcessStdError = new StringBuilder(this.ErrorLength, this.ErrorLength);
if (this.testHostLauncher == null)
{
EqtTrace.Verbose("DotnetTestHostManager: Starting process '{0}' with command line '{1}'", testHostStartInfo.FileName, testHostStartInfo.Arguments);
cancellationToken.ThrowIfCancellationRequested();
this.testHostProcess = this.processHelper.LaunchProcess(testHostStartInfo.FileName, testHostStartInfo.Arguments, testHostStartInfo.WorkingDirectory, testHostStartInfo.EnvironmentVariables, this.ErrorReceivedCallback, this.ExitCallBack) as Process;
}
else
{
var processId = this.testHostLauncher.LaunchTestHost(testHostStartInfo);
this.testHostProcess = Process.GetProcessById(processId);
}
}
else
catch (OperationCanceledException ex)
{
var processId = this.testHostLauncher.LaunchTestHost(testHostStartInfo);
this.testHostProcess = Process.GetProcessById(processId);
this.OnHostExited(new HostProviderEventArgs(ex.Message, -1, 0));
throw;
}
var pId = this.testHostProcess != null ? this.testHostProcess.Id : 0;
@@ -177,7 +177,7 @@ public void Initialize(IMessageLogger logger, string runsettingsXml)
this.Shared = !config.DisableAppDomain;
}
public Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo)
public Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
throw new NotImplementedException();
}
@@ -245,7 +245,7 @@ public void Initialize(IMessageLogger logger, string runsettingsXml)
this.Shared = !config.DisableAppDomain;
}
public Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo)
public Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
throw new NotImplementedException();
}
@@ -4,6 +4,7 @@
namespace TestPlatform.CrossPlatEngine.UnitTests.Client
{
using System.Collections.Generic;
using System.Threading;
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
@@ -148,7 +149,7 @@ public void DiscoverTestsShouldNotIntializeIfDoneSoAlready()
this.testDiscoveryManager.DiscoverTests(this.discoveryCriteria, null);
this.mockRequestSender.Verify(s => s.InitializeCommunication(), Times.AtMostOnce);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>()), Times.AtMostOnce);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>(), It.IsAny<CancellationToken>()), Times.AtMostOnce);
}
[TestMethod]
@@ -161,7 +162,7 @@ public void DiscoverTestsShouldIntializeIfNotInitializedAlready()
this.testDiscoveryManager.DiscoverTests(this.discoveryCriteria, null);
this.mockRequestSender.Verify(s => s.InitializeCommunication(), Times.Once);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>()), Times.Once);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>(), It.IsAny<CancellationToken>()), Times.Once);
}
[TestMethod]
@@ -151,7 +151,7 @@ public void StartTestRunShouldNotIntializeIfDoneSoAlready()
this.testExecutionManager.StartTestRun(this.mockTestRunCriteria.Object, null);
this.mockRequestSender.Verify(s => s.InitializeCommunication(), Times.AtMostOnce);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>()), Times.AtMostOnce);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>(), It.IsAny<CancellationToken>()), Times.AtMostOnce);
}
[TestMethod]
@@ -162,7 +162,7 @@ public void StartTestRunShouldInitializeIfNotInitializedAlready()
this.testExecutionManager.StartTestRun(this.mockTestRunCriteria.Object, null);
this.mockRequestSender.Verify(s => s.InitializeCommunication(), Times.Once);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>()), Times.Once);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>(), It.IsAny<CancellationToken>()), Times.Once);
}
[TestMethod]
@@ -57,7 +57,7 @@ public void SetupChannelShouldLaunchTestHost()
this.testOperationManager.SetupChannel(Enumerable.Empty<string>());
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.Is<TestProcessStartInfo>(si => si == expectedStartInfo)), Times.Once);
this.mockTestHostManager.Verify(thl => thl.LaunchTestHostAsync(It.Is<TestProcessStartInfo>(si => si == expectedStartInfo), It.IsAny<CancellationToken>()), Times.Once);
}
[TestMethod]
@@ -197,15 +197,15 @@ public void CloseShouldResetChannelInitialization()
this.testOperationManager.Close();
this.testOperationManager.SetupChannel(Enumerable.Empty<string>());
this.mockTestHostManager.Verify(th => th.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>()), Times.Exactly(2));
this.mockTestHostManager.Verify(th => th.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>(), It.IsAny<CancellationToken>()), Times.Exactly(2));
}
[TestMethod]
public void CloseShouldTerminateTesthostProcessIfWaitTimesout()
{
// Ensure testhost start returns a dummy process id
this.mockRequestSender.Setup(rs => rs.WaitForRequestHandlerConnection(this.connectionTimeout)).Returns(true);
this.mockTestHostManager.Setup(th => th.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>())).Returns(() => Task.FromResult<int>(123));
this.mockTestHostManager.Setup(th => th.LaunchTestHostAsync(It.IsAny<TestProcessStartInfo>(), It.IsAny<CancellationToken>())).Returns(() => Task.FromResult<int>(123));
this.testOperationManager.SetupChannel(Enumerable.Empty<string>());
this.testOperationManager.Close();
@@ -256,7 +256,7 @@ public TestableDotnetTestHostManager(bool checkRequired)
return new TestProcessStartInfo();
}
public override async Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo)
public override async Task<int> LaunchTestHostAsync(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken)
{
return await Task.Run(() => { return 0; });
}
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.