Skip to content

Re-enable DebugServiceTests suite #1635

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

Merged
merged 12 commits into from
Dec 21, 2021
Merged

Re-enable DebugServiceTests suite #1635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bd7b7a9
Add error check to `PsesHostFactory.Create()` and clean up
andyleejordan Dec 17, 2021
a5d3a97
Make `BuildCommandFromArguments()` reusable
andyleejordan Nov 30, 2021
a0ab8e9
Fix null dereference bug in `PopPowerShell()`
andyleejordan Dec 17, 2021
31b4b82
Remove unused cancellation token from `EnterDebugLoop()`
andyleejordan Dec 17, 2021
dd96c86
Fix `SetDebugResuming()` for unit test compatibility
andyleejordan Dec 17, 2021
8848348
Simplify logic in `ConfigurationDoneHandler.cs`
andyleejordan Dec 17, 2021
d808ca9
Fix null dereference bug in `RaiseDebuggerStoppedEvent()`
andyleejordan Dec 17, 2021
418e3d4
Clean up `DebugService.cs`
andyleejordan Dec 17, 2021
3ca2ae6
Reimplement and reenable `DebugServiceTests.cs`
andyleejordan Dec 17, 2021
2aacfb2
Add notes learned from debugging
andyleejordan Dec 17, 2021
593cd46
Miscellaneous cleanups
andyleejordan Dec 17, 2021
f14163d
Remove `RunNoPromptExecutionLoop()`
andyleejordan Dec 17, 2021
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: 4 additions & 0 deletions PowerShellEditorServices.build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ task TestServer TestServerWinPS,TestServerPS7,TestServerPS72

task TestServerWinPS -If (-not $script:IsNix) {
Set-Location .\test\PowerShellEditorServices.Test\
# TODO: See https://github.com/dotnet/sdk/issues/18353 for x64 test host
# that is debuggable! If architecture is added, the assembly path gets an
# additional folder, necesstiating fixes to find the commands definition
# file and test files.
exec { & $script:dotnetExe $script:dotnetTestArgs $script:NetRuntime.Desktop }
}

Expand Down
6 changes: 3 additions & 3 deletions src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Management.Automation;
Expand Down Expand Up @@ -131,8 +131,8 @@ private void OnDebuggerResuming(object sender, DebuggerResumingEventArgs e)
_debugAdapterServer.SendNotification(EventNames.Continued,
new ContinuedEvent
{
AllThreadsContinued = true,
ThreadId = ThreadsHandler.PipelineThread.Id,
AllThreadsContinued = true,
});
}

Expand All @@ -151,7 +151,7 @@ private void OnBreakpointUpdated(object sender, BreakpointUpdatedEventArgs e)
BreakpointDetails.Create(e.Breakpoint, e.UpdateType)
);

string reason = (e.UpdateType) switch {
string reason = e.UpdateType switch {
BreakpointUpdateType.Set => BreakpointEventReason.New,
BreakpointUpdateType.Removed => BreakpointEventReason.Removed,
BreakpointUpdateType.Enabled => BreakpointEventReason.Changed,
Expand Down
103 changes: 32 additions & 71 deletions src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal class DebugService
private readonly ILogger _logger;
private readonly IInternalPowerShellExecutionService _executionService;
private readonly BreakpointService _breakpointService;
private readonly RemoteFileManagerService remoteFileManager;
private readonly RemoteFileManagerService _remoteFileManager;

private readonly PsesInternalHost _psesHost;

Expand All @@ -45,7 +45,7 @@ internal class DebugService
private int nextVariableId;
private string temporaryScriptListingPath;
private List<VariableDetailsBase> variables;
private VariableContainerDetails globalScopeVariables;
internal VariableContainerDetails globalScopeVariables; // Internal for unit testing.
private VariableContainerDetails scriptScopeVariables;
private VariableContainerDetails localScopeVariables;
private StackFrameDetails[] stackFrameDetails;
Expand Down Expand Up @@ -80,28 +80,8 @@ internal class DebugService

/// <summary>
/// Initializes a new instance of the DebugService class and uses
/// the given PowerShellContext for all future operations.
/// the given execution service for all future operations.
/// </summary>
/// <param name="powerShellContext">
/// The PowerShellContext to use for all debugging operations.
/// </param>
/// <param name="logger">An ILogger implementation used for writing log messages.</param>
//public DebugService(PowerShellContextService powerShellContext, ILogger logger)
// : this(powerShellContext, null, logger)
//{
//}

/// <summary>
/// Initializes a new instance of the DebugService class and uses
/// the given PowerShellContext for all future operations.
/// </summary>
/// <param name="powerShellContext">
/// The PowerShellContext to use for all debugging operations.
/// </param>
//// <param name = "remoteFileManager" >
//// A RemoteFileManagerService instance to use for accessing files in remote sessions.
//// </param>
/// <param name="logger">An ILogger implementation used for writing log messages.</param>
public DebugService(
IInternalPowerShellExecutionService executionService,
IPowerShellDebugContext debugContext,
Expand All @@ -120,8 +100,7 @@ public DebugService(
_debugContext.DebuggerStopped += OnDebuggerStopAsync;
_debugContext.DebuggerResuming += OnDebuggerResuming;
_debugContext.BreakpointUpdated += OnBreakpointUpdated;

this.remoteFileManager = remoteFileManager;
_remoteFileManager = remoteFileManager;

invocationTypeScriptPositionProperty =
typeof(InvocationInfo)
Expand Down Expand Up @@ -150,32 +129,27 @@ public async Task<BreakpointDetails[]> SetLineBreakpointsAsync(

string scriptPath = scriptFile.FilePath;
// Make sure we're using the remote script path
if (_psesHost.CurrentRunspace.IsOnRemoteMachine && remoteFileManager is not null)
if (_psesHost.CurrentRunspace.IsOnRemoteMachine && _remoteFileManager is not null)
{
if (!remoteFileManager.IsUnderRemoteTempPath(scriptPath))
if (!_remoteFileManager.IsUnderRemoteTempPath(scriptPath))
{
_logger.LogTrace($"Could not set breakpoints for local path '{scriptPath}' in a remote session.");

return Array.Empty<BreakpointDetails>();
}

string mappedPath = remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace);

scriptPath = mappedPath;
scriptPath = _remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace);
}
else if (temporaryScriptListingPath is not null
&& temporaryScriptListingPath.Equals(scriptPath, StringComparison.CurrentCultureIgnoreCase))
else if (temporaryScriptListingPath?.Equals(scriptPath, StringComparison.CurrentCultureIgnoreCase) == true)
{
_logger.LogTrace($"Could not set breakpoint on temporary script listing path '{scriptPath}'.");

return Array.Empty<BreakpointDetails>();
}

// Fix for issue #123 - file paths that contain wildcard chars [ and ] need to
// quoted and have those wildcard chars escaped.
string escapedScriptPath = PathUtils.WildcardEscapePath(scriptPath);

if (dscBreakpoints is null || !dscBreakpoints.IsDscResourcePath(escapedScriptPath))
if (dscBreakpoints?.IsDscResourcePath(escapedScriptPath) != true)
{
if (clearExisting)
{
Expand All @@ -185,10 +159,8 @@ public async Task<BreakpointDetails[]> SetLineBreakpointsAsync(
return (await _breakpointService.SetBreakpointsAsync(escapedScriptPath, breakpoints).ConfigureAwait(false)).ToArray();
}

return await dscBreakpoints.SetLineBreakpointsAsync(
_executionService,
escapedScriptPath,
breakpoints)
return await dscBreakpoints
.SetLineBreakpointsAsync(_executionService, escapedScriptPath, breakpoints)
.ConfigureAwait(false);
}

Expand Down Expand Up @@ -362,7 +334,7 @@ public VariableDetailsBase GetVariableFromExpression(string variableExpression)
variableName,
StringComparison.CurrentCultureIgnoreCase));

if (resolvedVariable is not null && resolvedVariable.IsExpandable)
if (resolvedVariable?.IsExpandable == true)
{
// Continue by searching in this variable's children.
variableList = GetVariables(resolvedVariable.Id);
Expand All @@ -383,6 +355,7 @@ public VariableDetailsBase GetVariableFromExpression(string variableExpression)
/// <param name="value">The new string value. This value must not be null. If you want to set the variable to $null
/// pass in the string "$null".</param>
/// <returns>The string representation of the value the variable was set to.</returns>
/// <exception cref="InvalidPowerShellExpressionException"></exception>
public async Task<string> SetVariableAsync(int variableContainerReferenceId, string name, string value)
{
Validate.IsNotNull(nameof(name), name);
Expand Down Expand Up @@ -476,20 +449,18 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
{
_logger.LogTrace($"Setting variable '{name}' using conversion to value: {expressionResult ?? "<null>"}");

psVariable.Value = await _executionService.ExecuteDelegateAsync<object>(
psVariable.Value = await _executionService.ExecuteDelegateAsync(
"PS debugger argument converter",
ExecutionOptions.Default,
(pwsh, cancellationToken) =>
(pwsh, _) =>
{
var engineIntrinsics = (EngineIntrinsics)pwsh.Runspace.SessionStateProxy.GetVariable("ExecutionContext");

// TODO: This is almost (but not quite) the same as LanguagePrimitives.Convert(), which does not require the pipeline thread.
// We should investigate changing it.
return argTypeConverterAttr.Transform(engineIntrinsics, expressionResult);

},
CancellationToken.None).ConfigureAwait(false);

}
else
{
Expand Down Expand Up @@ -634,17 +605,14 @@ private async Task FetchStackFramesAndVariablesAsync(string scriptNameOverride)
nextVariableId = VariableDetailsBase.FirstVariableId;
variables = new List<VariableDetailsBase>
{

// Create a dummy variable for index 0, should never see this.
new VariableDetails("Dummy", null)
};

// Must retrieve in order of broadest to narrowest scope for efficient
// deduplication: global, script, local.
globalScopeVariables = await FetchVariableContainerAsync(VariableContainerDetails.GlobalScopeName).ConfigureAwait(false);

scriptScopeVariables = await FetchVariableContainerAsync(VariableContainerDetails.ScriptScopeName).ConfigureAwait(false);

localScopeVariables = await FetchVariableContainerAsync(VariableContainerDetails.LocalScopeName).ConfigureAwait(false);

await FetchStackFramesAsync(scriptNameOverride).ConfigureAwait(false);
Expand All @@ -662,9 +630,7 @@ private Task<VariableContainerDetails> FetchVariableContainerAsync(string scope)

private async Task<VariableContainerDetails> FetchVariableContainerAsync(string scope, bool autoVarsOnly)
{
PSCommand psCommand = new PSCommand()
.AddCommand("Get-Variable")
.AddParameter("Scope", scope);
PSCommand psCommand = new PSCommand().AddCommand("Get-Variable").AddParameter("Scope", scope);

var scopeVariableContainer = new VariableContainerDetails(nextVariableId++, "Scope: " + scope);
variables.Add(scopeVariableContainer);
Expand All @@ -674,17 +640,13 @@ private async Task<VariableContainerDetails> FetchVariableContainerAsync(string
{
results = await _executionService.ExecutePSCommandAsync<PSObject>(psCommand, CancellationToken.None).ConfigureAwait(false);
}
catch (CmdletInvocationException ex)
// It's possible to be asked to run `Get-Variable -Scope N` where N is a number that
// exceeds the available scopes. In this case, the command throws this exception, but
// there's nothing we can do about it, nor can we know the number of scopes that exist,
// and we shouldn't crash the debugger, so we just return no results instead. All other
// exceptions should be thrown again.
catch (CmdletInvocationException ex) when (ex.ErrorRecord.CategoryInfo.Reason.Equals("PSArgumentOutOfRangeException"))
{
// It's possible to be asked to run `Get-Variable -Scope N` where N is a number that
// exceeds the available scopes. In this case, the command throws this exception,
// but there's nothing we can do about it, nor can we know the number of scopes that
// exist, and we shouldn't crash the debugger, so we just return no results instead.
// All other exceptions should be thrown again.
if (!ex.ErrorRecord.CategoryInfo.Reason.Equals("PSArgumentOutOfRangeException"))
{
throw;
}
results = null;
}

Expand Down Expand Up @@ -749,8 +711,8 @@ private static bool ShouldAddAsVariable(VariableInfo variableInfo)
{
// Filter built-in constant or readonly variables like $true, $false, $null, etc.
ScopedItemOptions variableScope = variableInfo.Variable.Options;
var constantAllScope = ScopedItemOptions.AllScope | ScopedItemOptions.Constant;
var readonlyAllScope = ScopedItemOptions.AllScope | ScopedItemOptions.ReadOnly;
const ScopedItemOptions constantAllScope = ScopedItemOptions.AllScope | ScopedItemOptions.Constant;
const ScopedItemOptions readonlyAllScope = ScopedItemOptions.AllScope | ScopedItemOptions.ReadOnly;
if (((variableScope & constantAllScope) == constantAllScope)
|| ((variableScope & readonlyAllScope) == readonlyAllScope))
{
Expand Down Expand Up @@ -909,11 +871,11 @@ private async Task FetchStackFramesAsync(string scriptNameOverride)
stackFrameDetailsEntry.ScriptPath = scriptNameOverride;
}
else if (isOnRemoteMachine
&& remoteFileManager is not null
&& _remoteFileManager is not null
&& !string.Equals(stackFrameScriptPath, StackFrameDetails.NoFileScriptPath))
{
stackFrameDetailsEntry.ScriptPath =
remoteFileManager.GetMappedPath(stackFrameScriptPath, _psesHost.CurrentRunspace);
_remoteFileManager.GetMappedPath(stackFrameScriptPath, _psesHost.CurrentRunspace);
}

stackFrameDetailList.Add(stackFrameDetailsEntry);
Expand Down Expand Up @@ -956,7 +918,7 @@ internal async void OnDebuggerStopAsync(object sender, DebuggerStopEventArgs e)
string localScriptPath = e.InvocationInfo.ScriptName;

// If there's no ScriptName, get the "list" of the current source
if (remoteFileManager is not null && string.IsNullOrEmpty(localScriptPath))
if (_remoteFileManager is not null && string.IsNullOrEmpty(localScriptPath))
{
// Get the current script listing and create the buffer
PSCommand command = new PSCommand().AddScript($"list 1 {int.MaxValue}");
Expand All @@ -977,7 +939,7 @@ await _executionService.ExecutePSCommandAsync<PSObject>(
.Where(s => s is not null));

temporaryScriptListingPath =
remoteFileManager.CreateTemporaryFile(
_remoteFileManager.CreateTemporaryFile(
$"[{_psesHost.CurrentRunspace.SessionDetails.ComputerName}] {TemporaryScriptFileName}",
scriptListing,
_psesHost.CurrentRunspace);
Expand All @@ -1000,19 +962,18 @@ await _executionService.ExecutePSCommandAsync<PSObject>(
// If this is a remote connection and the debugger stopped at a line
// in a script file, get the file contents
if (_psesHost.CurrentRunspace.IsOnRemoteMachine
&& remoteFileManager is not null
&& _remoteFileManager is not null
&& !noScriptName)
{
localScriptPath =
await remoteFileManager.FetchRemoteFileAsync(
await _remoteFileManager.FetchRemoteFileAsync(
e.InvocationInfo.ScriptName,
_psesHost.CurrentRunspace).ConfigureAwait(false);
}

if (stackFrameDetails.Length > 0)
{
// Augment the top stack frame with details from the stop event

if (invocationTypeScriptPositionProperty.GetValue(e.InvocationInfo) is IScriptExtent scriptExtent)
{
stackFrameDetails[0].StartLineNumber = scriptExtent.StartLineNumber;
Expand Down Expand Up @@ -1054,9 +1015,9 @@ private void OnBreakpointUpdated(object sender, BreakpointUpdatedEventArgs e)
// TODO: This could be either a path or a script block!
string scriptPath = lineBreakpoint.Script;
if (_psesHost.CurrentRunspace.IsOnRemoteMachine
&& remoteFileManager is not null)
&& _remoteFileManager is not null)
{
string mappedPath = remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace);
string mappedPath = _remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace);

if (mappedPath is null)
{
Expand Down
3 changes: 1 addition & 2 deletions src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ internal void ReleaseSetBreakpointHandle()

internal async Task WaitForSetBreakpointHandleAsync()
{
await _setBreakpointInProgressHandle.WaitAsync()
.ConfigureAwait(continueOnCapturedContext: false);
await _setBreakpointInProgressHandle.WaitAsync().ConfigureAwait(false);
}
}
}
5 changes: 3 additions & 2 deletions src/PowerShellEditorServices/Services/DebugAdapter/Debugging/DebuggerStoppedEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Management.Automation;
using System.Security.Cryptography;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
using Microsoft.PowerShell.EditorServices.Utility;
using System.Management.Automation;

namespace Microsoft.PowerShell.EditorServices.Services.DebugAdapter
{
Expand Down
Loading