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.

Sign up for GitHub

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

Port : Cancellation timing issue fix. (#1398) #1436

Merged
merged 2 commits into from
Feb 19, 2018
Merged

Port : Cancellation timing issue fix. (#1398) #1436

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c534bcb
Cancellation timing issue fix. (#1398)
AbhitejJohn Jan 31, 2018
cd0b36c
Merge branch 'master' into portCancelFix
singhsarab Feb 19, 2018
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
4 changes: 2 additions & 2 deletions src/Microsoft.TestPlatform.Client/Execution/TestRunRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ public void CancelAsync()
else
{
// Inform the service about run cancellation
this.ExecutionManager.Cancel();
this.ExecutionManager.Cancel(this);
}
}

Expand Down Expand Up @@ -279,7 +279,7 @@ public void Abort()
}
else
{
this.ExecutionManager.Abort();
this.ExecutionManager.Abort(this);
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/Microsoft.TestPlatform.Common/Interfaces/Engine/ClientProtocol/IProxyExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ public interface IProxyExecutionManager
/// <summary>
/// Cancels the test run. On the test host, this will send a message to adapters.
/// </summary>
void Cancel();
// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
void Cancel(ITestRunEventsHandler eventHandler);

/// <summary>
/// Aborts the test operation. This will forcefully terminate the test host.
/// </summary>
void Abort();
// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
void Abort(ITestRunEventsHandler eventHandler);

/// <summary>
/// Closes the current test operation by sending a end session message.
Expand Down
6 changes: 4 additions & 2 deletions src/Microsoft.TestPlatform.Common/Interfaces/Engine/TesthostProtocol/IExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ public interface IExecutionManager
/// <summary>
/// Cancel the test execution.
/// </summary>
void Cancel();
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
void Cancel(ITestRunEventsHandler testRunEventsHandler);

/// <summary>
/// Aborts the test execution.
/// </summary>
void Abort();
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
void Abort(ITestRunEventsHandler testRunEventsHandler);
}
}
10 changes: 6 additions & 4 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyexecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,19 @@ public int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsHandler e
/// <summary>
/// Aborts the test operation.
/// </summary>
public void Abort()
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
public void Abort(ITestRunEventsHandler eventHandler)
{
Task.Run(() => this.testHostManagerFactory.GetExecutionManager().Abort());
Task.Run(() => this.testHostManagerFactory.GetExecutionManager().Abort(eventHandler));
}

/// <summary>
/// Cancels the test run.
/// </summary>
public void Cancel()
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
public void Cancel(ITestRunEventsHandler eventHandler)
{
Task.Run(() => this.testHostManagerFactory.GetExecutionManager().Cancel());
Task.Run(() => this.testHostManagerFactory.GetExecutionManager().Cancel(eventHandler));
}

/// <summary>
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,16 @@ public int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsHandler e
return this.StartTestRunPrivate(eventHandler);
}

public void Abort()
public void Abort(ITestRunEventsHandler runEventsHandler)
{
// Test platform initiated abort.
abortRequested = true;
this.DoActionOnAllManagers((proxyManager) => proxyManager.Abort(), doActionsInParallel: true);
this.DoActionOnAllManagers((proxyManager) => proxyManager.Abort(runEventsHandler), doActionsInParallel: true);
}

public void Cancel()
public void Cancel(ITestRunEventsHandler runEventsHandler)
{
this.DoActionOnAllManagers((proxyManager) => proxyManager.Cancel(), doActionsInParallel: true);
this.DoActionOnAllManagers((proxyManager) => proxyManager.Cancel(runEventsHandler), doActionsInParallel: true);
}

public void Close()
Expand Down
18 changes: 16 additions & 2 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,15 @@ public virtual int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsH
/// <summary>
/// Cancels the test run.
/// </summary>
public virtual void Cancel()
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
public virtual void Cancel(ITestRunEventsHandler eventHandler)
{
// Just in case ExecuteAsync isn't called yet, set the eventhandler
if(this.baseTestRunEventsHandler == null)
{
this.baseTestRunEventsHandler = eventHandler;
}

// Cancel fast, try to stop testhost deployment/launch
this.cancellationTokenSource.Cancel();
if (this.isCommunicationEstablished)
Expand All @@ -187,8 +194,15 @@ public virtual int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testPr
/// <summary>
/// Aborts the test run.
/// </summary>
public void Abort()
/// <param name="eventHandler"> EventHandler for handling execution events from Engine. </param>
public void Abort(ITestRunEventsHandler eventHandler)
{
// Just in case ExecuteAsync isn't called yet, set the eventhandler
if(this.baseTestRunEventsHandler == null)
{
this.baseTestRunEventsHandler = eventHandler;
}

this.RequestSender.SendTestRunAbort();
}

Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManagerWithDataCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,15 @@ public override int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEvents
}

/// <inheritdoc/>
public override void Cancel()
public override void Cancel(ITestRunEventsHandler eventHandler)
{
try
{
this.ProxyDataCollectionManager.AfterTestRunEnd(isCanceled: true, runEventsHandler: this.DataCollectionRunEventsHandler);
}
finally
{
base.Cancel();
base.Cancel(eventHandler);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.TestPlatform.CrossPlatEngine/EventHandlers/TestRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ public void OnMessageReceived(object sender, MessageReceivedEventArgs messageRec
case MessageType.CancelTestRun:
jobQueue.Pause();
this.testHostManagerFactoryReady.Wait();
testHostManagerFactory.GetExecutionManager().Cancel();
testHostManagerFactory.GetExecutionManager().Cancel(new TestRunEventsHandler(this));
break;

case MessageType.LaunchAdapterProcessWithDebuggerAttachedCallback:
Expand All @@ -331,7 +331,7 @@ public void OnMessageReceived(object sender, MessageReceivedEventArgs messageRec
case MessageType.AbortTestRun:
jobQueue.Pause();
this.testHostManagerFactoryReady.Wait();
testHostManagerFactory.GetExecutionManager().Abort();
testHostManagerFactory.GetExecutionManager().Abort(new TestRunEventsHandler(this));
break;

case MessageType.SessionEnd:
Expand Down
21 changes: 8 additions & 13 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Execution/ExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution
/// </summary>
public class ExecutionManager : IExecutionManager
{
private ITestRunEventsHandler testRunEventsHandler;

private readonly ITestPlatformEventSource testPlatformEventSource;

private BaseRunTests activeTestRun;
Expand Down Expand Up @@ -85,7 +83,6 @@ public void StartTestRun(
ITestCaseEventsHandler testCaseEventsHandler,
ITestRunEventsHandler runEventsHandler)
{
this.testRunEventsHandler = runEventsHandler;
try
{
this.activeTestRun = new RunTestsWithSources(this.requestData, adapterSourceMap, package, runSettings, testExecutionContext, testCaseEventsHandler, runEventsHandler);
Expand All @@ -94,8 +91,8 @@ public void StartTestRun(
}
catch(Exception e)
{
this.testRunEventsHandler.HandleLogMessage(ObjectModel.Logging.TestMessageLevel.Error, e.ToString());
this.Abort();
runEventsHandler.HandleLogMessage(ObjectModel.Logging.TestMessageLevel.Error, e.ToString());
this.Abort(runEventsHandler);
}
finally
{
Expand All @@ -120,8 +117,6 @@ public void StartTestRun(
ITestCaseEventsHandler testCaseEventsHandler,
ITestRunEventsHandler runEventsHandler)
{
this.testRunEventsHandler = runEventsHandler;

try
{
this.activeTestRun = new RunTestsWithTests(this.requestData, tests, package, runSettings, testExecutionContext, testCaseEventsHandler, runEventsHandler);
Expand All @@ -130,8 +125,8 @@ public void StartTestRun(
}
catch(Exception e)
{
this.testRunEventsHandler.HandleLogMessage(ObjectModel.Logging.TestMessageLevel.Error, e.ToString());
this.Abort();
runEventsHandler.HandleLogMessage(ObjectModel.Logging.TestMessageLevel.Error, e.ToString());
this.Abort(runEventsHandler);
}
finally
{
Expand All @@ -142,12 +137,12 @@ public void StartTestRun(
/// <summary>
/// Cancel the test execution.
/// </summary>
public void Cancel()
public void Cancel(ITestRunEventsHandler testRunEventsHandler)
{
if (this.activeTestRun == null)
{
var testRunCompleteEventArgs = new TestRunCompleteEventArgs(null, true, false, null, null, TimeSpan.Zero);
this.testRunEventsHandler.HandleTestRunComplete(testRunCompleteEventArgs, null, null, null);
testRunEventsHandler.HandleTestRunComplete(testRunCompleteEventArgs, null, null, null);
}
else
{
Expand All @@ -158,12 +153,12 @@ public void Cancel()
/// <summary>
/// Aborts the test execution.
/// </summary>
public void Abort()
public void Abort(ITestRunEventsHandler testRunEventsHandler)
{
if (this.activeTestRun == null)
{
var testRunCompleteEventArgs = new TestRunCompleteEventArgs(null, false, true, null, null, TimeSpan.Zero);
this.testRunEventsHandler.HandleTestRunComplete(testRunCompleteEventArgs, null, null, null);
testRunEventsHandler.HandleTestRunComplete(testRunCompleteEventArgs, null, null, null);
}
else
{
Expand Down
9 changes: 5 additions & 4 deletions src/vstest.console/TestPlatformHelpers/TestRequestManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ internal class TestRequestManager : ITestRequestManager
/// </summary>
private ITestRunRequest currentTestRunRequest;

private readonly EventWaitHandle runRequestCreatedEventHandle = new AutoResetEvent(false);
private readonly EventWaitHandle runRequestStartedEventHandle = new AutoResetEvent(false);

private object syncobject = new object();

Expand Down Expand Up @@ -308,7 +308,7 @@ public void CancelTestRun()
{
EqtTrace.Info("TestRequestManager.CancelTestRun: Sending cancel request.");

this.runRequestCreatedEventHandle.WaitOne(runRequestTimeout);
this.runRequestStartedEventHandle.WaitOne(runRequestTimeout);
this.currentTestRunRequest?.CancelAsync();
}

Expand All @@ -319,7 +319,7 @@ public void AbortTestRun()
{
EqtTrace.Info("TestRequestManager.AbortTestRun: Sending abort request.");

this.runRequestCreatedEventHandle.WaitOne(runRequestTimeout);
this.runRequestStartedEventHandle.WaitOne(runRequestTimeout);
this.currentTestRunRequest?.Abort();
}

Expand Down Expand Up @@ -524,14 +524,15 @@ private bool RunTests(IRequestData requestData, TestRunCriteria testRunCriteria,
try
{
this.currentTestRunRequest = this.testPlatform.CreateTestRunRequest(requestData, testRunCriteria);
this.runRequestCreatedEventHandle.Set();

this.testRunResultAggregator.RegisterTestRunEvents(this.currentTestRunRequest);
testRunEventsRegistrar?.RegisterTestRunEvents(this.currentTestRunRequest);

this.testPlatformEventSource.ExecutionRequestStart();

this.currentTestRunRequest.ExecuteAsync();

this.runRequestStartedEventHandle.Set();

// Wait for the run completion event
this.currentTestRunRequest.WaitForCompletion();
Expand Down
16 changes: 8 additions & 8 deletions test/Microsoft.TestPlatform.Client.UnitTests/Execution/TestRunRequestTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void AbortIfTestRunStateIsNotInProgressShouldNotCallExecutionManagerAbort
{
//ExecuteAsync has not been called, so State is not InProgress
testRunRequest.Abort();
executionManager.Verify(dm => dm.Abort(), Times.Never);
executionManager.Verify(dm => dm.Abort(It.IsAny<ITestRunEventsHandler>()), Times.Never);
}

[TestMethod]
Expand All @@ -118,7 +118,7 @@ public void AbortIfDiscoveryIsinProgressShouldCallDiscoveryManagerAbort()
testRunRequest.ExecuteAsync();

testRunRequest.Abort();
executionManager.Verify(dm => dm.Abort(), Times.Once);
executionManager.Verify(dm => dm.Abort(It.IsAny<ITestRunEventsHandler>()), Times.Once);
}

[TestMethod]
Expand All @@ -145,15 +145,15 @@ public void CancelAsyncIfTestRunRequestDisposedShouldNotThrowException()
public void CancelAsyncIfTestRunStateNotInProgressWillNotCallExecutionManagerCancel()
{
testRunRequest.CancelAsync();
executionManager.Verify(dm => dm.Cancel(), Times.Never);
executionManager.Verify(dm => dm.Cancel(It.IsAny<ITestRunEventsHandler>()), Times.Never);
}

[TestMethod]
public void CancelAsyncIfTestRunStateInProgressCallsExecutionManagerCancel()
{
testRunRequest.ExecuteAsync();
testRunRequest.CancelAsync();
executionManager.Verify(dm => dm.Cancel(), Times.Once);
executionManager.Verify(dm => dm.Cancel(It.IsAny<ITestRunEventsHandler>()), Times.Once);
}


Expand All @@ -162,7 +162,7 @@ public void OnTestSessionTimeoutShouldCallAbort()
{
this.testRunRequest.ExecuteAsync();
this.testRunRequest.OnTestSessionTimeout(null);
this.executionManager.Verify(o => o.Abort(), Times.Once);
this.executionManager.Verify(o => o.Abort(It.IsAny<ITestRunEventsHandler>()), Times.Once);
}

[TestMethod]
Expand Down Expand Up @@ -204,12 +204,12 @@ public void OnTestSessionTimeoutShouldGetCalledWhenExecutionCrossedTestSessionTi

ManualResetEvent onTestSessionTimeoutCalled = new ManualResetEvent(true);
onTestSessionTimeoutCalled.Reset();
executionManager.Setup(o => o.Abort()).Callback(() => onTestSessionTimeoutCalled.Set());
executionManager.Setup(o => o.Abort(It.IsAny<ITestRunEventsHandler>())).Callback(() => onTestSessionTimeoutCalled.Set());

testRunRequest.ExecuteAsync();
onTestSessionTimeoutCalled.WaitOne(20 * 1000);

executionManager.Verify(o => o.Abort(), Times.Once);
executionManager.Verify(o => o.Abort(It.IsAny<ITestRunEventsHandler>()), Times.Once);
}

/// <summary>
Expand All @@ -234,7 +234,7 @@ public void OnTestSessionTimeoutShouldNotGetCalledWhenTestSessionTimeoutIsZero()

testRunRequest.ExecuteAsync();

executionManager.Verify(o => o.Abort(), Times.Never);
executionManager.Verify(o => o.Abort(It.IsAny<ITestRunEventsHandler>()), Times.Never);
}

[TestMethod]
Expand Down
8 changes: 4 additions & 4 deletions ...soft.TestPlatform.CrossPlatEngine.UnitTests/Client/InProcessProxyexecutionManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@ public void AbortShouldCallExecutionManagerAbort()
{
var manualResetEvent = new ManualResetEvent(true);

this.mockExecutionManager.Setup(o => o.Abort()).Callback(
this.mockExecutionManager.Setup(o => o.Abort(It.IsAny<ITestRunEventsHandler>())).Callback(
() => manualResetEvent.Set());

this.inProcessProxyExecutionManager.Abort();
this.inProcessProxyExecutionManager.Abort(It.IsAny<ITestRunEventsHandler>());

Assert.IsTrue(manualResetEvent.WaitOne(5000), "IExecutionManager.Abort should get called");
}
Expand All @@ -192,10 +192,10 @@ public void CancelShouldCallExecutionManagerCancel()
{
var manualResetEvent = new ManualResetEvent(true);

this.mockExecutionManager.Setup(o => o.Cancel()).Callback(
this.mockExecutionManager.Setup(o => o.Cancel(It.IsAny<ITestRunEventsHandler>())).Callback(
() => manualResetEvent.Set());

this.inProcessProxyExecutionManager.Cancel();
this.inProcessProxyExecutionManager.Cancel(It.IsAny<ITestRunEventsHandler>());

Assert.IsTrue(manualResetEvent.WaitOne(5000), "IExecutionManager.Abort should get called");
}
Expand Down
Loading