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

Added support for debugging external test processes #2325

Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
74f547c
[WIP] Initial support for debugging external test processes
cvpoienaru Feb 11, 2020
341b0ea
[WIP] Initial support for debugging external test processes
cvpoienaru Feb 11, 2020
b03b3f4
Merge branch 'copoiena/debugging-external-test-processes-2' of https:…
cvpoienaru Feb 17, 2020
11a0af2
Merge branch 'copoiena/debugging-external-test-processes-2' of https:…
cvpoienaru Feb 17, 2020
9627709
Merge branch 'copoiena/debugging-external-test-processes-2' of https:…
cvpoienaru Feb 18, 2020
02ea724
Merge branch 'copoiena/debugging-external-test-processes-2' of https:…
cvpoienaru Feb 18, 2020
5474a64
Merge branch 'copoiena/debugging-external-test-processes-2' of https:…
cvpoienaru Feb 18, 2020
72a6abc
Added support for the ITestExecutor2 interface
cvpoienaru Feb 19, 2020
d572add
Added support for ITestRunEventsHandler2
cvpoienaru Feb 24, 2020
3a12437
Final changes on test platform
cvpoienaru Feb 25, 2020
87ed797
Changed failing test to conform to new workflow
cvpoienaru Feb 26, 2020
57ba62f
Changed is-as to as when casting
cvpoienaru Feb 27, 2020
3e40728
Implemented IVsTestConsoleWrapper2 interface & changed the way the te…
cvpoienaru Mar 2, 2020
c3033e3
Changed as conversion to is-as
cvpoienaru Mar 2, 2020
52a9a7b
Disabled unnecessary parenthesis warning
cvpoienaru Mar 2, 2020
798c7bf
Unsubscribed from the correct event
cvpoienaru Mar 4, 2020
3ea3e3d
Changed AttachDebuggerToProcess messages between IDE and testhost
cvpoienaru Mar 4, 2020
edabd38
Ignored custom host launcher tests
cvpoienaru Mar 5, 2020
99b4591
Reimplemented VsTestConsoleWrapper calls for ITestHostLauncher2
cvpoienaru Mar 6, 2020
bbbd645
Removed IVsTestConsoleWrapper2 interface
cvpoienaru Mar 6, 2020
0af1e41
Fixed acceptance/performance tests
cvpoienaru Mar 6, 2020
ad2cb01
Fixed profiling workflow
cvpoienaru Mar 9, 2020
68e4c30
Fixed dotnet testhost launcher
cvpoienaru Mar 9, 2020
a0cad9b
Various code review fixes
cvpoienaru Mar 10, 2020
78b93aa
Fixed minor style issues
cvpoienaru Mar 10, 2020
4ec216c
Introduced ITestRunEventsHandler2 interface
cvpoienaru Mar 10, 2020
b81b1cd
Various code review changes
cvpoienaru Mar 11, 2020
50bd7d5
Fixed code review comments
cvpoienaru Mar 16, 2020
95b553a
Fixed compilation error
cvpoienaru Mar 16, 2020
bfe2386
Fixed test compilation issues
cvpoienaru Mar 16, 2020
9440e3f
Changed non-customer facing interfaces to internal from public
cvpoienaru Mar 16, 2020
5142161
Switched interface visibility back to public
cvpoienaru Mar 18, 2020
729b7f1
Resolved some code review comments
cvpoienaru Mar 19, 2020
b33c50f
Added localized resource string for failing to attach the debugger to…
cvpoienaru Mar 19, 2020
330083f
Merge branch 'master' into copoiena/debugging-external-test-processes-2
cvpoienaru Mar 20, 2020
c6f9c93
Fixed backward compatibility issues with older testhosts
cvpoienaru Apr 5, 2020
0d87d35
Fixed broken unit tests
cvpoienaru Apr 5, 2020
fa2bff1
Fixed VsTestConsoleRequestSender protocol version
cvpoienaru Apr 8, 2020
c409757
Fixed protocol version in unit tests
cvpoienaru Apr 8, 2020
fc9d044
Fixed compatibility issues between an older VS and the latest Test.SD…
cvpoienaru Apr 10, 2020
777657a
Removed Ignore attribute from unit test
cvpoienaru Apr 23, 2020
97a804c
Merge branch 'master' into copoiena/debugging-external-test-processes-2
cvpoienaru Apr 23, 2020
232918d
Removed last Ignore attribute from old unit tests
cvpoienaru Apr 23, 2020
aa2b38e
Implemented a selection algorithm for ITestExecutor and ITestExecutor…
cvpoienaru Apr 29, 2020
e0f4891
Fixed unit test failures
cvpoienaru May 4, 2020
0e20565
Merge branch 'master' into copoiena/debugging-external-test-processes-2
cvpoienaru May 4, 2020
581d97d
Changed test extension conflict from error to warning
cvpoienaru May 6, 2020
5dd0b38
Fixed some code review comments
cvpoienaru May 14, 2020
4dfbce3
Fixed compat issues with old versions of VS
cvpoienaru May 14, 2020
9f15160
Changed test execution protocol message names
cvpoienaru May 15, 2020
234a53c
Merge branch 'master' into copoiena/debugging-external-test-processes-2
cvpoienaru May 15, 2020
cf2261b
Final code review comments
cvpoienaru May 18, 2020
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 @@ -243,7 +243,7 @@ public void HandleRawMessage(string rawMessage)
}
}

public class RunEventHandler : ITestRunEventsHandler
public class RunEventHandler : ITestRunEventsHandler2
{
private AutoResetEvent waitHandle;

Expand Down Expand Up @@ -293,5 +293,11 @@ public int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testProcessSta
// No op
return -1;
}

public bool AttachDebuggerToProcess(int pid)
{
// No op
return false;
}
}
}
34 changes: 33 additions & 1 deletion src/Microsoft.TestPlatform.Client/DesignMode/DesignModeClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,16 @@ public class DesignModeClient : IDesignModeClient

private object ackLockObject = new object();
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

private object responseLockObject = new object();
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

private ProtocolConfig protocolConfig = Constants.DefaultProtocolConfig;

private IEnvironment platformEnvironment;

protected Action<Message> onAckMessageReceived;

protected Action<Message> onResponseMessageReceived;
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

private TestSessionMessageLogger testSessionMessageLogger;

/// <summary>
Expand Down Expand Up @@ -226,6 +230,10 @@ private void ProcessRequests(ITestRequestManager testRequestManager)
break;
}

case MessageType.VSAttachDebuggerToProcessCallback:
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
this.onResponseMessageReceived?.Invoke(message);
break;
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

case MessageType.SessionEnd:
{
EqtTrace.Info("DesignModeClient: Session End message received from server. Closing the connection.");
Expand Down Expand Up @@ -301,6 +309,31 @@ public int LaunchCustomHost(TestProcessStartInfo testProcessStartInfo, Cancellat
}
}

/// <inheritdoc/>
public bool AttachDebuggerToProcess(int pid, CancellationToken cancellationToken)
{
lock (this.responseLockObject)
{
var waitHandle = new AutoResetEvent(false);
Message responseMessage = null;
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
this.onResponseMessageReceived = (responseRawMessage) =>
{
responseMessage = responseRawMessage;
waitHandle.Set();
};

this.communicationManager.SendMessage(MessageType.VSAttachDebuggerToProcess, pid);

WaitHandle.WaitAny(new WaitHandle[] { waitHandle, cancellationToken.WaitHandle });

cancellationToken.ThrowTestPlatformExceptionIfCancellationRequested();
this.onResponseMessageReceived = null;

var response = this.dataSerializer.DeserializePayload<bool>(responseMessage);
return response;
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// <summary>
/// Send the raw messages to IDE
/// </summary>
Expand Down Expand Up @@ -416,7 +449,6 @@ public void Dispose()
// Do not change this code. Put cleanup code in Dispose(bool disposing) above.
Dispose(true);
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Client.DesignMode
/// <summary>
/// DesignMode TestHost Launcher for hosting of test process
/// </summary>
internal class DesignModeTestHostLauncher : ITestHostLauncher
internal class DesignModeTestHostLauncher : ITestHostLauncher2
{
private readonly IDesignModeClient designModeClient;

Expand All @@ -26,6 +26,18 @@ public DesignModeTestHostLauncher(IDesignModeClient designModeClient)
/// <inheritdoc/>
public virtual bool IsDebug => false;

/// <inheritdoc/>
public bool AttachDebuggerToProcess(int pid)
{
return this.designModeClient.AttachDebuggerToProcess(pid, CancellationToken.None);
}

/// <inheritdoc/>
public bool AttachDebuggerToProcess(int pid, CancellationToken cancellationToken)
{
return this.designModeClient.AttachDebuggerToProcess(pid, cancellationToken);
}

/// <inheritdoc/>
public int LaunchTestHost(TestProcessStartInfo defaultTestHostStartInfo)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ namespace Microsoft.VisualStudio.TestPlatform.Client.DesignMode
public static class DesignModeTestHostLauncherFactory
{
private static ITestHostLauncher defaultLauncher;

private static ITestHostLauncher debugLauncher;

public static ITestHostLauncher GetCustomHostLauncherForTestRun(IDesignModeClient designModeClient, TestRunRequestPayload testRunRequestPayload)
{
ITestHostLauncher testHostLauncher = null;

if (!testRunRequestPayload.DebuggingEnabled)
{
testHostLauncher = defaultLauncher = defaultLauncher ?? new DesignModeTestHostLauncher(designModeClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ public interface IDesignModeClient : IDisposable
/// <returns>Process id of the launched test host.</returns>
int LaunchCustomHost(TestProcessStartInfo defaultTestHostStartInfo, CancellationToken cancellationToken);

/// <summary>
/// Attach debugger to an already running process.
/// </summary>
/// <param name="pid">Process ID of the process to which the debugger should be attached.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns><see cref="true"/> if the debugger was successfully attached to the requested process, <see cref="false"/> otherwise.</returns>
bool AttachDebuggerToProcess(int pid, CancellationToken cancellationToken);
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Handles parent process exit
/// </summary>
Expand Down
12 changes: 11 additions & 1 deletion src/Microsoft.TestPlatform.Client/Execution/TestRunRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ namespace Microsoft.VisualStudio.TestPlatform.Client.Execution
using ClientResources = Microsoft.VisualStudio.TestPlatform.Client.Resources.Resources;
using CommunicationObjectModel = Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.Interfaces;
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

public class TestRunRequest : ITestRunRequest, ITestRunEventsHandler
public class TestRunRequest : ITestRunRequest, ITestRunEventsHandler2
{
/// <summary>
/// The criteria/config for this test run request.
Expand Down Expand Up @@ -659,6 +660,15 @@ public int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testProcessSta
return processId;
}

/// <inheritdoc />
public bool AttachDebuggerToProcess(int pid)
{
if (!(this.testRunCriteria.TestHostLauncher is ITestHostLauncher2 launcher))
return false;
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

return launcher.AttachDebuggerToProcess(pid);
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
/// Dispose the run
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.EventHandle
/// <summary>
/// The test run events handler.
/// </summary>
public class TestRunEventsHandler : ITestRunEventsHandler
public class TestRunEventsHandler : ITestRunEventsHandler2
{
private ITestRequestHandler requestHandler;

Expand Down Expand Up @@ -97,5 +97,12 @@ public int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testProcessSta
EqtTrace.Info("Sending LaunchProcessWithDebuggerAttached on additional test process: {0}", testProcessStartInfo?.FileName);
return this.requestHandler.LaunchProcessWithDebuggerAttached(testProcessStartInfo);
}

/// <inheritdoc/>
public bool AttachDebuggerToProcess(int pid)
{
EqtTrace.Info("Sending TestRunEventsHandler.AttachDebuggerToProcess on additional test process with pid: {0}", pid);
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
return this.requestHandler.AttachDebuggerToProcess(pid);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,12 @@ public interface ITestRequestHandler : IDisposable
/// <param name="testProcessStartInfo">Process start info</param>
/// <returns>ProcessId of the launched process</returns>
int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testProcessStartInfo);

cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// Attach debugger to an already running process.
/// </summary>
/// <param name="pid">Process ID of the process to which the debugger should be attached.</param>
/// <returns><see cref="true"/> if the debugger was successfully attached to the requested process, <see cref="false"/> otherwise.</returns>
bool AttachDebuggerToProcess(int pid);
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,26 @@ public static class MessageType
/// </summary>
public const string LaunchAdapterProcessWithDebuggerAttachedCallback = "TestExecution.LaunchAdapterProcessWithDebuggerAttachedCallback";

/// <summary>
/// Attach debugger to process.
/// </summary>
public const string AttachDebuggerToProcess = "TestExecution.AttachDebuggerToProcess";

/// <summary>
/// Attach debugger to process callback.
/// </summary>
public const string AttachDebuggerToProcessCallback = "TestExecution.AttachDebuggerToProcessCallback";

/// <summary>
/// Attach debugger to process.
/// </summary>
public const string VSAttachDebuggerToProcess = "TestExecution.VSAttachDebuggerToProcess";

/// <summary>
/// Attach debugger to process callback.
/// </summary>
public const string VSAttachDebuggerToProcessCallback = "TestExecution.VSAttachDebuggerToProcessCallback";
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Data Collection Message
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,28 @@ private void OnExecutionMessageReceived(object sender, MessageReceivedEventArgs

this.channel.Send(data);
break;

case MessageType.AttachDebuggerToProcess:
#pragma warning disable SA1119 // Statement must not use unnecessary parenthesis
if (!(testRunEventsHandler is ITestRunEventsHandler2 handler))
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
#pragma warning restore SA1119 // Statement must not use unnecessary parenthesis
{
throw new NotSupportedException(string.Format(
CultureInfo.CurrentUICulture,
"AttachDebuggerToProcess is only supported in ITestRunEventsHandler2, but it was called with {0}.",
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
testRunEventsHandler.GetType()));
}

var testProcessPid = this.dataSerializer.DeserializePayload<TestProcessAttachDebuggerPayload>(message);
bool result = handler.AttachDebuggerToProcess(testProcessPid.ProcessID);

var resultMessage = this.dataSerializer.SerializePayload(
MessageType.AttachDebuggerToProcessCallback,
result,
this.protocolVersion);

this.channel.Send(resultMessage);
break;
}
}
catch (Exception exception)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Adapter
{
using System;
using System.Collections.Generic;

using System.Globalization;
using Execution;

using Microsoft.VisualStudio.TestPlatform.ObjectModel;
Expand All @@ -19,7 +19,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Adapter
/// <summary>
/// Handle to the framework which is passed to the test executors.
/// </summary>
internal class FrameworkHandle : TestExecutionRecorder, IFrameworkHandle, IDisposable
internal class FrameworkHandle : TestExecutionRecorder, IFrameworkHandle2, IDisposable
{
/// <summary>
/// boolean that gives the value of EnableShutdownAfterTestRun.
Expand Down Expand Up @@ -110,6 +110,29 @@ public int LaunchProcessWithDebuggerAttached(string filePath, string workingDire
return this.testRunEventsHandler.LaunchProcessWithDebuggerAttached(processInfo);
}

/// <inheritdoc />
public bool AttachDebuggerToProcess(int pid)
{
if (pid < 0)
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
{
throw new ArgumentOutOfRangeException("FrameworkHandle.AttachDebuggerToProcess: PID cannot be negative.");
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
}
if (pid == 0)
{
EqtTrace.Warning("FrameworkHandle.AttachDebuggerToProcess: Attaching to a process with process id 0, is this intended ?");
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
}

var handler = this.testRunEventsHandler as ITestRunEventsHandler2;
if (handler == null)
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
{
throw new NotSupportedException(string.Format(
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
CultureInfo.CurrentUICulture,
"FrameworkHandle.AttachDebuggerToProcess is only supported in ITestRunEventsHandler2, but it was called with {0}.",
this.testRunEventsHandler.GetType()));
}

return handler.AttachDebuggerToProcess(pid);
}

public void Dispose()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel
{
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;

using System.Globalization;
using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
Expand All @@ -18,7 +19,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel
/// <summary>
/// ParallelRunEventsHandler for handling the run events in case of parallel execution
/// </summary>
internal class ParallelRunEventsHandler : ITestRunEventsHandler
internal class ParallelRunEventsHandler : ITestRunEventsHandler2
{
private IProxyExecutionManager proxyExecutionManager;

Expand Down Expand Up @@ -174,6 +175,20 @@ public int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testProcessSta
return this.actualRunEventsHandler.LaunchProcessWithDebuggerAttached(testProcessStartInfo);
}

/// <inheritdoc />
public bool AttachDebuggerToProcess(int pid)
{
if (!(this.actualRunEventsHandler is ITestRunEventsHandler2 handler))
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
{
throw new NotSupportedException(string.Format(
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
CultureInfo.CurrentUICulture,
"AttachDebuggerToProcess is only supported in ITestRunEventsHandler2, but it was called with {0}.",
this.actualRunEventsHandler.GetType()));
}

return handler.AttachDebuggerToProcess(pid);
}

private void ConvertToRawMessageAndSend(string messageType, object payload)
{
var rawMessage = this.dataSerializer.SerializePayload(messageType, payload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Globalization;
using System.Linq;

using Microsoft.VisualStudio.TestPlatform.Common;
Expand All @@ -27,7 +28,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
/// <summary>
/// Orchestrates test execution operations for the engine communicating with the client.
/// </summary>
internal class ProxyExecutionManager : ProxyOperationManager, IProxyExecutionManager, ITestRunEventsHandler
internal class ProxyExecutionManager : ProxyOperationManager, IProxyExecutionManager, ITestRunEventsHandler2
{
private readonly ITestRuntimeProvider testHostManager;
private IDataSerializer dataSerializer;
Expand Down Expand Up @@ -200,6 +201,20 @@ public virtual int LaunchProcessWithDebuggerAttached(TestProcessStartInfo testPr
return this.baseTestRunEventsHandler.LaunchProcessWithDebuggerAttached(testProcessStartInfo);
}

/// <inheritdoc />
public bool AttachDebuggerToProcess(int pid)
{
if (!(this.baseTestRunEventsHandler is ITestRunEventsHandler2 handler))
{
throw new NotSupportedException(string.Format(
cvpoienaru marked this conversation as resolved.
Show resolved Hide resolved
CultureInfo.CurrentUICulture,
"AttachDebuggerToProcess is only supported in ITestRunEventsHandler2, but it was called with {0}.",
this.baseTestRunEventsHandler.GetType()));
}

return handler.AttachDebuggerToProcess(pid);
}

/// <summary>
/// Aborts the test run.
/// </summary>
Expand Down
Loading