From 243f2ea4de5cf86d4025b08216cb13e98d7b52da Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Fri, 16 Dec 2022 16:51:36 -0800 Subject: [PATCH 1/5] Small suggested cleanups while browsing --- .../Commands/StartEditorServicesCommand.cs | 5 +++-- .../DebugAdapter/Debugging/VariableDetails.cs | 15 ++++----------- .../Debugging/VariableDetailsBase.cs | 1 - .../Handlers/DebugEvaluateHandler.cs | 16 +++++----------- .../Debugging/PowerShellDebugContext.cs | 2 +- .../Debugging/DebugServiceTests.cs | 8 ++++---- .../Session/PsesInternalHostTests.cs | 6 +++--- 7 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs index 51e2d1b26..743b6aac9 100644 --- a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs +++ b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs @@ -15,7 +15,6 @@ #if DEBUG using System.Diagnostics; using System.Threading; - using Debugger = System.Diagnostics.Debugger; #endif @@ -197,9 +196,11 @@ protected override void BeginProcessing() #if DEBUG if (WaitForDebugger) { + // NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for + // .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI. + Console.WriteLine($"Waiting for debugger to attach, PID: {Process.GetCurrentProcess().Id}"); while (!Debugger.IsAttached) { - Console.WriteLine($"PID: {Process.GetCurrentProcess().Id}"); Thread.Sleep(1000); } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs index 96a6b2b99..df8d93e83 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -98,16 +98,11 @@ public VariableDetails(string name, object value) /// If this variable instance is expandable, this method returns the /// details of its children. Otherwise it returns an empty array. /// - /// public override VariableDetailsBase[] GetChildren(ILogger logger) { if (IsExpandable) { - if (cachedChildren == null) - { - cachedChildren = GetChildren(ValueObject, logger); - } - + cachedChildren ??= GetChildren(ValueObject, logger); return cachedChildren; } @@ -131,9 +126,7 @@ private static bool GetIsExpandable(object valueObject) valueObject = psobject.BaseObject; } - Type valueType = - valueObject?.GetType(); - + Type valueType = valueObject?.GetType(); TypeInfo valueTypeInfo = valueType.GetTypeInfo(); return @@ -379,7 +372,7 @@ protected static void AddDotNetProperties(object obj, List chil #endregion - private struct UnableToRetrievePropertyMessage + private readonly struct UnableToRetrievePropertyMessage { public UnableToRetrievePropertyMessage(string message) => Message = message; diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetailsBase.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetailsBase.cs index 51e613f2c..ea4f14a56 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetailsBase.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetailsBase.cs @@ -51,7 +51,6 @@ internal abstract class VariableDetailsBase /// If this variable instance is expandable, this method returns the /// details of its children. Otherwise it returns an empty array. /// - /// public abstract VariableDetailsBase[] GetChildren(ILogger logger); } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs index 056f2f778..23c6ee0b4 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -64,21 +64,15 @@ await _executionService.ExecutePSCommandAsync( result = _debugService.GetVariableFromExpression(request.Expression); // If the expression is not a naked variable reference, then evaluate the expression. - if (result == null) - { - result = - await _debugService.EvaluateExpressionAsync( - request.Expression, - isFromRepl).ConfigureAwait(false); - } + result ??= await _debugService.EvaluateExpressionAsync( + request.Expression, + isFromRepl).ConfigureAwait(false); } if (result != null) { valueString = result.ValueString; - variableId = - result.IsExpandable ? - result.Id : 0; + variableId = result.IsExpandable ? result.Id : 0; } } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs index 236403e5a..1d45c435d 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs @@ -146,7 +146,7 @@ public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction) // TODO: We need to assign cancellation tokens to each frame, because the current // logic results in a deadlock here when we try to cancel the scopes...which // includes ourself (hence running it in a separate thread). - _ = Task.Run(() => _psesHost.UnwindCallStack()); + _ = Task.Run(_psesHost.UnwindCallStack); return; } diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index 9362772d2..f80b21eb9 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -496,7 +496,7 @@ public async Task DebuggerBreaksWhenRequested() Assert.Equal(0, confirmedBreakpoints.Count); Task _ = ExecuteDebugFileAsync(); // NOTE: This must be run on a separate thread so the async event handlers can fire. - await Task.Run(() => debugService.Break()).ConfigureAwait(true); + await Task.Run(debugService.Break).ConfigureAwait(true); AssertDebuggerPaused(); } @@ -505,7 +505,7 @@ public async Task DebuggerRunsCommandsWhileStopped() { Task _ = ExecuteDebugFileAsync(); // NOTE: This must be run on a separate thread so the async event handlers can fire. - await Task.Run(() => debugService.Break()).ConfigureAwait(true); + await Task.Run(debugService.Break).ConfigureAwait(true); AssertDebuggerPaused(); // Try running a command from outside the pipeline thread @@ -723,7 +723,7 @@ await debugService.SetLineBreakpointsAsync( // The above just tests that the debug service returns the correct new value string. // Let's step the debugger and make sure the values got set to the new values. - await Task.Run(() => debugService.StepOver()).ConfigureAwait(true); + await Task.Run(debugService.StepOver).ConfigureAwait(true); AssertDebuggerStopped(variableScriptFile.FilePath); // Test set of a local string variable (not strongly typed) @@ -779,7 +779,7 @@ await debugService.SetLineBreakpointsAsync( // The above just tests that the debug service returns the correct new value string. // Let's step the debugger and make sure the values got set to the new values. - await Task.Run(() => debugService.StepOver()).ConfigureAwait(true); + await Task.Run(debugService.StepOver).ConfigureAwait(true); AssertDebuggerStopped(variableScriptFile.FilePath); // Test set of a local string variable (not strongly typed but force conversion) diff --git a/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs b/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs index d1fefad79..76f303681 100644 --- a/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs +++ b/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs @@ -180,7 +180,7 @@ public async Task CanRunOnIdleTask() new PSCommand().AddScript("$handled"), CancellationToken.None).ConfigureAwait(true); - Assert.Collection(handled, (p) => Assert.False(p)); + Assert.Collection(handled, Assert.False); await psesHost.ExecuteDelegateAsync( nameof(psesHost.OnPowerShellIdle), @@ -195,7 +195,7 @@ await psesHost.ExecuteDelegateAsync( new PSCommand().AddScript("$handled"), CancellationToken.None).ConfigureAwait(true); - Assert.Collection(handled, (p) => Assert.True(p)); + Assert.Collection(handled, Assert.True); } [Fact] @@ -301,7 +301,7 @@ await psesHost.ExecuteDelegateAsync( new PSCommand().AddScript("$handledInProfile"), CancellationToken.None).ConfigureAwait(true); - Assert.Collection(handled, (p) => Assert.True(p)); + Assert.Collection(handled, Assert.True); } } } From e438e766977517437846f26f72a6d231eafc5d6e Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Fri, 16 Dec 2022 17:34:36 -0800 Subject: [PATCH 2/5] Expand variables on pipeline thread So that expansions of embedded PowerShell code succeed. --- .../Services/DebugAdapter/DebugService.cs | 25 ++-- .../DebugAdapter/Debugging/VariableDetails.cs | 132 +++++++++--------- .../Handlers/DebugEvaluateHandler.cs | 9 +- .../DebugAdapter/Handlers/VariablesHandler.cs | 8 +- .../Debugging/DebugServiceTests.cs | 95 ++++++------- 5 files changed, 137 insertions(+), 132 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs index 0002245cb..4c9ef8735 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs @@ -241,11 +241,12 @@ public async Task SetCommandBreakpointsAsync( /// that is identified by the given referenced ID. /// /// + /// /// An array of VariableDetails instances which describe the requested variables. - public VariableDetailsBase[] GetVariables(int variableReferenceId) + public async Task GetVariables(int variableReferenceId, CancellationToken cancellationToken) { VariableDetailsBase[] childVariables; - debugInfoHandle.Wait(); + await debugInfoHandle.WaitAsync(cancellationToken).ConfigureAwait(false); try { if ((variableReferenceId < 0) || (variableReferenceId >= variables.Count)) @@ -257,7 +258,12 @@ public VariableDetailsBase[] GetVariables(int variableReferenceId) VariableDetailsBase parentVariable = variables[variableReferenceId]; if (parentVariable.IsExpandable) { - childVariables = parentVariable.GetChildren(_logger); + // We execute this on the pipeline thread so the expansion of child variables works. + childVariables = await _executionService.ExecuteDelegateAsync( + $"Getting children of variable ${parentVariable.Name}", + new ExecutionOptions { Priority = ExecutionPriority.Next }, + (_, _) => parentVariable.GetChildren(_logger), cancellationToken).ConfigureAwait(false); + foreach (VariableDetailsBase child in childVariables) { // Only add child if it hasn't already been added. @@ -287,8 +293,9 @@ public VariableDetailsBase[] GetVariables(int variableReferenceId) /// walk the cached variable data for the specified stack frame. /// /// The variable expression string to evaluate. + /// /// A VariableDetailsBase object containing the result. - public VariableDetailsBase GetVariableFromExpression(string variableExpression) + public async Task GetVariableFromExpression(string variableExpression, CancellationToken cancellationToken) { // NOTE: From a watch we will get passed expressions that are not naked variables references. // Probably the right way to do this would be to examine the AST of the expr before calling @@ -302,7 +309,7 @@ public VariableDetailsBase GetVariableFromExpression(string variableExpression) IEnumerable variableList; // Ensure debug info isn't currently being built. - debugInfoHandle.Wait(); + await debugInfoHandle.WaitAsync(cancellationToken).ConfigureAwait(false); try { variableList = variables; @@ -331,7 +338,7 @@ public VariableDetailsBase GetVariableFromExpression(string variableExpression) if (resolvedVariable?.IsExpandable == true) { // Continue by searching in this variable's children. - variableList = GetVariables(resolvedVariable.Id); + variableList = await GetVariables(resolvedVariable.Id, cancellationToken).ConfigureAwait(false); } } @@ -491,10 +498,12 @@ public async Task SetVariableAsync(int variableContainerReferenceId, str /// /// If true, writes the expression result as host output rather than returning the results. /// In this case, the return value of this function will be null. + /// /// A VariableDetails object containing the result. public async Task EvaluateExpressionAsync( string expressionString, - bool writeResultAsOutput) + bool writeResultAsOutput, + CancellationToken cancellationToken) { PSCommand command = new PSCommand().AddScript(expressionString); IReadOnlyList results; @@ -502,7 +511,7 @@ public async Task EvaluateExpressionAsync( { results = await _executionService.ExecutePSCommandAsync( command, - CancellationToken.None, + cancellationToken, new PowerShellExecutionOptions { WriteOutputToHost = writeResultAsOutput, ThrowOnError = !writeResultAsOutput }).ConfigureAwait(false); } catch (Exception e) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs index df8d93e83..457c43c7c 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs @@ -229,7 +229,7 @@ private static string InsertDimensionSize(string value, int dimensionSize) return value + ": " + dimensionSize; } - private VariableDetails[] GetChildren(object obj, ILogger logger) + private static VariableDetails[] GetChildren(object obj, ILogger logger) { List childVariables = new(); @@ -238,86 +238,82 @@ private VariableDetails[] GetChildren(object obj, ILogger logger) return childVariables.ToArray(); } - try - { - PSObject psObject = obj as PSObject; + // NOTE: Variable expansion now takes place on the pipeline thread as an async delegate, + // so expansion of children that cause PowerShell script code to execute should + // generally work. However, we might need more error handling. + PSObject psObject = obj as PSObject; - if ((psObject != null) && - (psObject.TypeNames[0] == typeof(PSCustomObject).ToString())) + if ((psObject != null) && + (psObject.TypeNames[0] == typeof(PSCustomObject).ToString())) + { + // PowerShell PSCustomObject's properties are completely defined by the ETS type system. + logger.LogDebug("PSObject was a PSCustomObject"); + childVariables.AddRange( + psObject + .Properties + .Select(p => new VariableDetails(p))); + } + else + { + // If a PSObject other than a PSCustomObject, unwrap it. + if (psObject != null) { - // PowerShell PSCustomObject's properties are completely defined by the ETS type system. + // First add the PSObject's ETS properties + logger.LogDebug("PSObject was something else, first getting ETS properties"); childVariables.AddRange( psObject .Properties + // Here we check the object's MemberType against the `Properties` + // bit-mask to determine if this is a property. Hence the selection + // will only include properties. + .Where(p => (PSMemberTypes.Properties & p.MemberType) is not 0) .Select(p => new VariableDetails(p))); + + obj = psObject.BaseObject; } - else + + // We're in the realm of regular, unwrapped .NET objects + if (obj is IDictionary dictionary) { - // If a PSObject other than a PSCustomObject, unwrap it. - if (psObject != null) + logger.LogDebug("PSObject was an IDictionary"); + // Buckle up kids, this is a bit weird. We could not use the LINQ + // operator OfType. Even though R# will squiggle the + // "foreach" keyword below and offer to convert to a LINQ-expression - DON'T DO IT! + // The reason is that LINQ extension methods work with objects of type + // IEnumerable. Objects of type Dictionary<,>, respond to iteration via + // IEnumerable by returning KeyValuePair<,> objects. Unfortunately non-generic + // dictionaries like HashTable return DictionaryEntry objects. + // It turns out that iteration via C#'s foreach loop, operates on the variable's + // type which in this case is IDictionary. IDictionary was designed to always + // return DictionaryEntry objects upon iteration and the Dictionary<,> implementation + // honors that when the object is reinterpreted as an IDictionary object. + // FYI, a test case for this is to open $PSBoundParameters when debugging a + // function that defines parameters and has been passed parameters. + // If you open the $PSBoundParameters variable node in this scenario and see nothing, + // this code is broken. + foreach (DictionaryEntry entry in dictionary) { - // First add the PSObject's ETS properties - childVariables.AddRange( - psObject - .Properties - // Here we check the object's MemberType against the `Properties` - // bit-mask to determine if this is a property. Hence the selection - // will only include properties. - .Where(p => (PSMemberTypes.Properties & p.MemberType) is not 0) - .Select(p => new VariableDetails(p))); - - obj = psObject.BaseObject; + childVariables.Add( + new VariableDetails( + "[" + entry.Key + "]", + entry)); } - - // We're in the realm of regular, unwrapped .NET objects - if (obj is IDictionary dictionary) - { - // Buckle up kids, this is a bit weird. We could not use the LINQ - // operator OfType. Even though R# will squiggle the - // "foreach" keyword below and offer to convert to a LINQ-expression - DON'T DO IT! - // The reason is that LINQ extension methods work with objects of type - // IEnumerable. Objects of type Dictionary<,>, respond to iteration via - // IEnumerable by returning KeyValuePair<,> objects. Unfortunately non-generic - // dictionaries like HashTable return DictionaryEntry objects. - // It turns out that iteration via C#'s foreach loop, operates on the variable's - // type which in this case is IDictionary. IDictionary was designed to always - // return DictionaryEntry objects upon iteration and the Dictionary<,> implementation - // honors that when the object is reinterpreted as an IDictionary object. - // FYI, a test case for this is to open $PSBoundParameters when debugging a - // function that defines parameters and has been passed parameters. - // If you open the $PSBoundParameters variable node in this scenario and see nothing, - // this code is broken. - foreach (DictionaryEntry entry in dictionary) - { - childVariables.Add( - new VariableDetails( - "[" + entry.Key + "]", - entry)); - } - } - else if (obj is IEnumerable enumerable and not string) + } + else if (obj is IEnumerable enumerable and not string) + { + logger.LogDebug("PSObject was an IEnumerable"); + int i = 0; + foreach (object item in enumerable) { - int i = 0; - foreach (object item in enumerable) - { - childVariables.Add( - new VariableDetails( - "[" + i++ + "]", - item)); - } + childVariables.Add( + new VariableDetails( + "[" + i++ + "]", + item)); } - - AddDotNetProperties(obj, childVariables); } - } - catch (GetValueInvocationException ex) - { - // This exception occurs when accessing the value of a - // variable causes a script to be executed. Right now - // we aren't loading children on the pipeline thread so - // this causes an exception to be raised. In this case, - // just return an empty list of children. - logger.LogWarning($"Failed to get properties of variable {Name}, value invocation was attempted: {ex.Message}"); + + logger.LogDebug("Adding .NET properties to PSObject"); + AddDotNetProperties(obj, childVariables); } return childVariables.ToArray(); diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs index 23c6ee0b4..2c6c47364 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -49,7 +49,7 @@ public async Task Handle(EvaluateRequestArguments request, { await _executionService.ExecutePSCommandAsync( new PSCommand().AddScript(request.Expression), - CancellationToken.None, + cancellationToken, new PowerShellExecutionOptions { WriteOutputToHost = true, ThrowOnError = false, AddToHistory = true }).HandleErrorsAsync(_logger).ConfigureAwait(false); } else @@ -61,12 +61,13 @@ await _executionService.ExecutePSCommandAsync( if (_debugContext.IsStopped) { // First check to see if the watch expression refers to a naked variable reference. - result = _debugService.GetVariableFromExpression(request.Expression); + result = await _debugService.GetVariableFromExpression(request.Expression, cancellationToken).ConfigureAwait(false); // If the expression is not a naked variable reference, then evaluate the expression. result ??= await _debugService.EvaluateExpressionAsync( request.Expression, - isFromRepl).ConfigureAwait(false); + isFromRepl, + cancellationToken).ConfigureAwait(false); } if (result != null) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/VariablesHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/VariablesHandler.cs index a4f5259ed..58fb4d5ef 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/VariablesHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/VariablesHandler.cs @@ -18,11 +18,9 @@ internal class VariablesHandler : IVariablesHandler public VariablesHandler(DebugService debugService) => _debugService = debugService; - public Task Handle(VariablesArguments request, CancellationToken cancellationToken) + public async Task Handle(VariablesArguments request, CancellationToken cancellationToken) { - VariableDetailsBase[] variables = - _debugService.GetVariables( - (int)request.VariablesReference); + VariableDetailsBase[] variables = await _debugService.GetVariables((int)request.VariablesReference, cancellationToken).ConfigureAwait(false); VariablesResponse variablesResponse = null; @@ -41,7 +39,7 @@ public Task Handle(VariablesArguments request, CancellationTo // TODO: This shouldn't be so broad } - return Task.FromResult(variablesResponse); + return variablesResponse; } } } diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index f80b21eb9..1db6fe890 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -99,12 +99,12 @@ public void Dispose() private ScriptFile GetDebugScript(string fileName) => workspace.GetFile(TestUtilities.GetSharedPath(Path.Combine("Debugging", fileName))); - private VariableDetailsBase[] GetVariables(string scopeName) + private Task GetVariables(string scopeName) { VariableScope scope = Array.Find( debugService.GetVariableScopes(0), s => s.Name == scopeName); - return debugService.GetVariables(scope.Id); + return debugService.GetVariables(scope.Id, CancellationToken.None); } private Task ExecuteScriptFileAsync(string scriptFilePath, params string[] args) @@ -185,7 +185,7 @@ await debugService.SetCommandBreakpointsAsync( // NOTE: This assertion will fail if any error occurs. Notably this happens in testing // when the assembly path changes and the commands definition file can't be found. - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.GlobalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.GlobalScopeName).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$Error"); Assert.NotNull(var); Assert.True(var.IsExpandable); @@ -220,7 +220,7 @@ public async Task DebuggerAcceptsScriptArgs(string[] args) AssertDebuggerStopped(oddPathScriptFile.FilePath, 3); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$Param1"); Assert.NotNull(var); @@ -231,7 +231,7 @@ public async Task DebuggerAcceptsScriptArgs(string[] args) Assert.NotNull(var); Assert.True(var.IsExpandable); - VariableDetailsBase[] childVars = debugService.GetVariables(var.Id); + VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); // 2 variables plus "Raw View" Assert.Equal(3, childVars.Length); Assert.Equal("\"Bar\"", childVars[0].ValueString); @@ -244,12 +244,12 @@ public async Task DebuggerAcceptsScriptArgs(string[] args) // NOTE: $args are no longer found in AutoVariables but CommandVariables instead. StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - variables = debugService.GetVariables(stackFrames[0].CommandVariables.Id); + variables = await debugService.GetVariables(stackFrames[0].CommandVariables.Id, CancellationToken.None).ConfigureAwait(true); var = Array.Find(variables, v => v.Name == "$args"); Assert.NotNull(var); Assert.True(var.IsExpandable); - childVars = debugService.GetVariables(var.Id); + childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); Assert.Equal(2, childVars.Length); Assert.Equal("\"Extra1\"", childVars[0].ValueString); } @@ -287,7 +287,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints() Task _ = ExecuteDebugFileAsync(); AssertDebuggerStopped(debugScriptFile.FilePath, 6); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Verify the function breakpoint broke at Write-Host and $i is 1 VariableDetailsBase i = Array.Find(variables, v => v.Name == "$i"); @@ -299,7 +299,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints() debugService.Continue(); AssertDebuggerStopped(debugScriptFile.FilePath, 6); - variables = GetVariables(VariableContainerDetails.LocalScopeName); + variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Verify the function breakpoint broke at Write-Host and $i is 1 i = Array.Find(variables, v => v.Name == "$i"); @@ -372,7 +372,7 @@ await debugService.SetLineBreakpointsAsync( Task _ = ExecuteDebugFileAsync(); AssertDebuggerStopped(debugScriptFile.FilePath, 7); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1 VariableDetailsBase i = Array.Find(variables, v => v.Name == "$i"); @@ -385,7 +385,7 @@ await debugService.SetLineBreakpointsAsync( debugService.Continue(); AssertDebuggerStopped(debugScriptFile.FilePath, 7); - variables = GetVariables(VariableContainerDetails.LocalScopeName); + variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1 i = Array.Find(variables, v => v.Name == "$i"); @@ -408,7 +408,7 @@ await debugService.SetLineBreakpointsAsync( Task _ = ExecuteDebugFileAsync(); AssertDebuggerStopped(debugScriptFile.FilePath, 6); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1 VariableDetailsBase i = Array.Find(variables, v => v.Name == "$i"); @@ -429,7 +429,7 @@ await debugService.SetLineBreakpointsAsync( Task _ = ExecuteDebugFileAsync(); AssertDebuggerStopped(debugScriptFile.FilePath, 6); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1 VariableDetailsBase i = Array.Find(variables, v => v.Name == "$i"); @@ -528,7 +528,7 @@ await debugService.SetCommandBreakpointsAsync( Task _ = ExecuteScriptFileAsync(testScript.FilePath); AssertDebuggerStopped(testScript.FilePath, 11); - VariableDetails prompt = await debugService.EvaluateExpressionAsync("prompt", false).ConfigureAwait(true); + VariableDetails prompt = await debugService.EvaluateExpressionAsync("prompt", false, CancellationToken.None).ConfigureAwait(true); Assert.Equal("\"True > \"", prompt.ValueString); } @@ -553,7 +553,7 @@ await debugService.SetCommandBreakpointsAsync( Task _ = configurationDoneHandler.LaunchScriptAsync(scriptPath); AssertDebuggerStopped(scriptPath, 1); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.CommandVariablesName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.CommandVariablesName).ConfigureAwait(true); VariableDetailsBase myInvocation = Array.Find(variables, v => v.Name == "$MyInvocation"); Assert.NotNull(myInvocation); Assert.True(myInvocation.IsExpandable); @@ -561,7 +561,7 @@ await debugService.SetCommandBreakpointsAsync( // Here we're asserting that our hacky workaround to support breakpoints in untitled // scripts is working, namely that we're actually dot-sourcing our first argument, which // should be a cached script block. See the `LaunchScriptAsync` for more info. - VariableDetailsBase[] myInvocationChildren = debugService.GetVariables(myInvocation.Id); + VariableDetailsBase[] myInvocationChildren = await debugService.GetVariables(myInvocation.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase myInvocationLine = Array.Find(myInvocationChildren, v => v.Name == "Line"); Assert.Equal("\". $args[0]\"", myInvocationLine.ValueString); } @@ -631,7 +631,7 @@ await debugService.SetLineBreakpointsAsync( Task _ = ExecuteVariableScriptFileAsync(); AssertDebuggerStopped(variableScriptFile.FilePath); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$strVar"); Assert.NotNull(var); @@ -649,7 +649,7 @@ await debugService.SetLineBreakpointsAsync( Task _ = ExecuteVariableScriptFileAsync(); AssertDebuggerStopped(variableScriptFile.FilePath); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // TODO: Add checks for correct value strings as well VariableDetailsBase strVar = Array.Find(variables, v => v.Name == "$strVar"); @@ -660,7 +660,7 @@ await debugService.SetLineBreakpointsAsync( Assert.NotNull(objVar); Assert.True(objVar.IsExpandable); - VariableDetailsBase[] objChildren = debugService.GetVariables(objVar.Id); + VariableDetailsBase[] objChildren = await debugService.GetVariables(objVar.Id, CancellationToken.None).ConfigureAwait(true); // Two variables plus "Raw View" Assert.Equal(3, objChildren.Length); @@ -668,14 +668,14 @@ await debugService.SetLineBreakpointsAsync( Assert.NotNull(arrVar); Assert.True(arrVar.IsExpandable); - VariableDetailsBase[] arrChildren = debugService.GetVariables(arrVar.Id); + VariableDetailsBase[] arrChildren = await debugService.GetVariables(arrVar.Id, CancellationToken.None).ConfigureAwait(true); Assert.Equal(5, arrChildren.Length); VariableDetailsBase classVar = Array.Find(variables, v => v.Name == "$classVar"); Assert.NotNull(classVar); Assert.True(classVar.IsExpandable); - VariableDetailsBase[] classChildren = debugService.GetVariables(classVar.Id); + VariableDetailsBase[] classChildren = await debugService.GetVariables(classVar.Id, CancellationToken.None).ConfigureAwait(true); Assert.Equal(2, classChildren.Length); VariableDetailsBase trueVar = Array.Find(variables, v => v.Name == "$trueVar"); @@ -700,7 +700,7 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); VariableScope[] scopes = debugService.GetVariableScopes(0); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Test set of a local string variable (not strongly typed) const string newStrValue = "\"Goodbye\""; @@ -727,17 +727,17 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); // Test set of a local string variable (not strongly typed) - variables = GetVariables(VariableContainerDetails.LocalScopeName); + variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); VariableDetailsBase strVar = Array.Find(variables, v => v.Name == "$strVar"); Assert.Equal(newStrValue, strVar.ValueString); // Test set of script scope int variable (not strongly typed) - variables = GetVariables(VariableContainerDetails.ScriptScopeName); + variables = await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true); VariableDetailsBase intVar = Array.Find(variables, v => v.Name == "$scriptInt"); Assert.Equal(newIntValue, intVar.ValueString); // Test set of global scope int variable (not strongly typed) - variables = GetVariables(VariableContainerDetails.GlobalScopeName); + variables = await GetVariables(VariableContainerDetails.GlobalScopeName).ConfigureAwait(true); VariableDetailsBase intGlobalVar = Array.Find(variables, v => v.Name == "$MaximumHistoryCount"); Assert.Equal(newGlobalIntValue, intGlobalVar.ValueString); } @@ -754,7 +754,7 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); VariableScope[] scopes = debugService.GetVariableScopes(0); - VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName); + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); // Test set of a local string variable (not strongly typed but force conversion) const string newStrValue = "\"False\""; @@ -783,17 +783,17 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); // Test set of a local string variable (not strongly typed but force conversion) - variables = GetVariables(VariableContainerDetails.LocalScopeName); + variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); VariableDetailsBase strVar = Array.Find(variables, v => v.Name == "$strVar2"); Assert.Equal(newStrValue, strVar.ValueString); // Test set of script scope bool variable (strongly typed) - variables = GetVariables(VariableContainerDetails.ScriptScopeName); + variables = await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true); VariableDetailsBase boolVar = Array.Find(variables, v => v.Name == "$scriptBool"); Assert.Equal(newBoolValue, boolVar.ValueString); // Test set of global scope ActionPreference variable (strongly typed) - variables = GetVariables(VariableContainerDetails.GlobalScopeName); + variables = await GetVariables(VariableContainerDetails.GlobalScopeName).ConfigureAwait(true); VariableDetailsBase globalVar = Array.Find(variables, v => v.Name == "$VerbosePreference"); Assert.Equal(newGlobalValue, globalVar.ValueString); } @@ -810,7 +810,7 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id); + VariableDetailsBase[] variables = await debugService.GetVariables(stackFrames[0].AutoVariables.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$enumVar"); Assert.NotNull(var); @@ -830,14 +830,14 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id); + VariableDetailsBase[] variables = await debugService.GetVariables(stackFrames[0].AutoVariables.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$assocArrVar"); Assert.NotNull(var); Assert.Equal("[Hashtable: 2]", var.ValueString); Assert.True(var.IsExpandable); - VariableDetailsBase[] childVars = debugService.GetVariables(var.Id); + VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); // 2 variables plus "Raw View" Assert.Equal(3, childVars.Length); @@ -863,7 +863,7 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id); + VariableDetailsBase[] variables = await debugService.GetVariables(stackFrames[0].AutoVariables.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase nullStringVar = Array.Find(variables, v => v.Name == "$nullString"); Assert.NotNull(nullStringVar); @@ -883,19 +883,20 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id); + VariableDetailsBase[] variables = await debugService.GetVariables(stackFrames[0].AutoVariables.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase psObjVar = Array.Find(variables, v => v.Name == "$psObjVar"); Assert.NotNull(psObjVar); Assert.True("@{Age=75; Name=John}".Equals(psObjVar.ValueString) || "@{Name=John; Age=75}".Equals(psObjVar.ValueString)); Assert.True(psObjVar.IsExpandable); - IDictionary childVars = debugService.GetVariables(psObjVar.Id).ToDictionary(v => v.Name, v => v.ValueString); - Assert.Equal(2, childVars.Count); - Assert.Contains("Age", childVars.Keys); - Assert.Contains("Name", childVars.Keys); - Assert.Equal("75", childVars["Age"]); - Assert.Equal("\"John\"", childVars["Name"]); + VariableDetailsBase[] childVars = await debugService.GetVariables(psObjVar.Id, CancellationToken.None).ConfigureAwait(true); + IDictionary dictionary = childVars.ToDictionary(v => v.Name, v => v.ValueString); + Assert.Equal(2, dictionary.Count); + Assert.Contains("Age", dictionary.Keys); + Assert.Contains("Name", dictionary.Keys); + Assert.Equal("75", dictionary["Age"]); + Assert.Equal("\"John\"", dictionary["Name"]); } [Fact] @@ -909,7 +910,7 @@ public async Task DebuggerEnumerableShowsRawView() AssertDebuggerStopped(commandBreakpointDetails: breakpoint); VariableDetailsBase simpleArrayVar = Array.Find( - GetVariables(VariableContainerDetails.ScriptScopeName), + await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true), v => v.Name == "$simpleArray"); Assert.NotNull(simpleArrayVar); VariableDetailsBase rawDetailsView = Array.Find( @@ -946,7 +947,7 @@ public async Task DebuggerDictionaryShowsRawView() AssertDebuggerStopped(commandBreakpointDetails: breakpoint); VariableDetailsBase simpleDictionaryVar = Array.Find( - GetVariables(VariableContainerDetails.ScriptScopeName), + await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true), v => v.Name == "$simpleDictionary"); Assert.NotNull(simpleDictionaryVar); VariableDetailsBase rawDetailsView = Array.Find( @@ -982,7 +983,7 @@ public async Task DebuggerDerivedDictionaryPropertyInRawView() AssertDebuggerStopped(commandBreakpointDetails: breakpoint); VariableDetailsBase sortedDictionaryVar = Array.Find( - GetVariables(VariableContainerDetails.ScriptScopeName), + await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true), v => v.Name == "$sortedDictionary"); Assert.NotNull(sortedDictionaryVar); VariableDetailsBase[] simpleDictionaryChildren = sortedDictionaryVar.GetChildren(NullLogger.Instance); @@ -1011,14 +1012,14 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id); + VariableDetailsBase[] variables = await debugService.GetVariables(stackFrames[0].AutoVariables.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$psCustomObjVar"); Assert.NotNull(var); Assert.Equal("@{Name=Paul; Age=73}", var.ValueString); Assert.True(var.IsExpandable); - VariableDetailsBase[] childVars = debugService.GetVariables(var.Id); + VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); Assert.Equal(2, childVars.Length); Assert.Equal("Name", childVars[0].Name); Assert.Equal("\"Paul\"", childVars[0].ValueString); @@ -1040,14 +1041,14 @@ await debugService.SetLineBreakpointsAsync( AssertDebuggerStopped(variableScriptFile.FilePath); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); - VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id); + VariableDetailsBase[] variables = await debugService.GetVariables(stackFrames[0].AutoVariables.Id, CancellationToken.None).ConfigureAwait(true); VariableDetailsBase var = Array.Find(variables, v => v.Name == "$procVar"); Assert.NotNull(var); Assert.StartsWith("System.Diagnostics.Process", var.ValueString); Assert.True(var.IsExpandable); - VariableDetailsBase[] childVars = debugService.GetVariables(var.Id); + VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); Assert.Equal(53, childVars.Length); } } From 89fb98f267b9b6293d0e30b75380cbf2043b9498 Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Mon, 19 Dec 2022 11:40:34 -0800 Subject: [PATCH 3/5] Guard expansion of `PSPropertyInfo.Value` Which fixes our test that gets the properties of a process object, since the value for `ExitCode` fails (as the process hasn't ended), we now handle that a bit more gracefully and can get all 130 properties. This also seemed to fix the overall bug where lots of expected properties failed to show up. While investigating, I also found many properties duplicated, seemingly due to being retrieved both off the PSObject and off the .NET object, so now when we do the latter we check (by name) that it hasn't already been added. --- .../DebugAdapter/Debugging/VariableDetails.cs | 42 +++-- .../Debugging/GetChildItemTest.ps1 | 2 + .../Debugging/DebugServiceTests.cs | 159 ++++++++++++++---- 3 files changed, 155 insertions(+), 48 deletions(-) create mode 100644 test/PowerShellEditorServices.Test.Shared/Debugging/GetChildItemTest.ps1 diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs index 457c43c7c..f67230af5 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs @@ -68,7 +68,7 @@ public VariableDetails(PSObject psVariableObject) /// The PSPropertyInfo instance from which variable details will be obtained. /// public VariableDetails(PSPropertyInfo psProperty) - : this(psProperty.Name, psProperty.Value) + : this(psProperty.Name, SafeGetValue(psProperty)) { } @@ -113,6 +113,20 @@ public override VariableDetailsBase[] GetChildren(ILogger logger) #region Private Methods + private static object SafeGetValue(PSPropertyInfo psProperty) + { + try + { + return psProperty.Value; + } + catch (GetValueInvocationException ex) + { + // Sometimes we can't get the value, like ExitCode, for reasons beyond our control, + // so just return the message from the exception that arises. + return new UnableToRetrievePropertyMessage { Name = psProperty.Name, Message = ex.Message }; + } + } + private static bool GetIsExpandable(object valueObject) { if (valueObject == null) @@ -331,9 +345,8 @@ protected static void AddDotNetProperties(object obj, List chil return; } - PropertyInfo[] properties = objectType.GetProperties(BindingFlags.Public | BindingFlags.Instance); - - foreach (PropertyInfo property in properties) + // Search all the public instance properties and add those missing. + foreach (PropertyInfo property in objectType.GetProperties(BindingFlags.Public | BindingFlags.Instance)) { // Don't display indexer properties, it causes an exception anyway. if (property.GetIndexParameters().Length > 0) @@ -343,10 +356,11 @@ protected static void AddDotNetProperties(object obj, List chil try { - childVariables.Add( - new VariableDetails( - property.Name, - property.GetValue(obj))); + // Only add unique properties because we may have already added some. + if (!childVariables.Exists(p => p.Name == property.Name)) + { + childVariables.Add(new VariableDetails(property.Name, property.GetValue(obj))); + } } catch (Exception ex) { @@ -360,21 +374,19 @@ protected static void AddDotNetProperties(object obj, List chil childVariables.Add( new VariableDetails( property.Name, - new UnableToRetrievePropertyMessage( - "Error retrieving property - " + ex.GetType().Name))); + new UnableToRetrievePropertyMessage { Name = property.Name, Message = ex.Message })); } } } #endregion - private readonly struct UnableToRetrievePropertyMessage + private record UnableToRetrievePropertyMessage { - public UnableToRetrievePropertyMessage(string message) => Message = message; - - public string Message { get; } + public string Name { get; init; } + public string Message { get; init; } - public override string ToString() => "<" + Message + ">"; + public override string ToString() => $"Error retrieving property '${Name}': ${Message}"; } } diff --git a/test/PowerShellEditorServices.Test.Shared/Debugging/GetChildItemTest.ps1 b/test/PowerShellEditorServices.Test.Shared/Debugging/GetChildItemTest.ps1 new file mode 100644 index 000000000..bab9ef52c --- /dev/null +++ b/test/PowerShellEditorServices.Test.Shared/Debugging/GetChildItemTest.ps1 @@ -0,0 +1,2 @@ +$file = Get-ChildItem -Path "." | Select-Object -First 1 +Write-Host "Debug over" diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index 1db6fe890..c65b6a040 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -920,20 +920,40 @@ await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true Assert.Empty(rawDetailsView.Type); Assert.Empty(rawDetailsView.ValueString); VariableDetailsBase[] rawViewChildren = rawDetailsView.GetChildren(NullLogger.Instance); - Assert.Equal(7, rawViewChildren.Length); - Assert.Equal("Length", rawViewChildren[0].Name); - Assert.Equal("4", rawViewChildren[0].ValueString); - Assert.Equal("LongLength", rawViewChildren[1].Name); - Assert.Equal("4", rawViewChildren[1].ValueString); - Assert.Equal("Rank", rawViewChildren[2].Name); - Assert.Equal("1", rawViewChildren[2].ValueString); - Assert.Equal("SyncRoot", rawViewChildren[3].Name); - Assert.Equal("IsReadOnly", rawViewChildren[4].Name); - Assert.Equal("$false", rawViewChildren[4].ValueString); - Assert.Equal("IsFixedSize", rawViewChildren[5].Name); - Assert.Equal("$true", rawViewChildren[5].ValueString); - Assert.Equal("IsSynchronized", rawViewChildren[6].Name); - Assert.Equal("$false", rawViewChildren[6].ValueString); + Assert.Collection(rawViewChildren, + (i) => + { + Assert.Equal("Length", i.Name); + Assert.Equal("4", i.ValueString); + }, + (i) => + { + Assert.Equal("LongLength", i.Name); + Assert.Equal("4", i.ValueString); + }, + (i) => + { + Assert.Equal("Rank", i.Name); + Assert.Equal("1", i.ValueString); + }, + (i) => + { + Assert.Equal("SyncRoot", i.Name); + Assert.True(i.IsExpandable); + }, + (i) => + { + Assert.Equal("IsReadOnly", i.Name); + Assert.Equal("$false", i.ValueString); + }, (i) => + { + Assert.Equal("IsFixedSize", i.Name); + Assert.Equal("$true", i.ValueString); + }, (i) => + { + Assert.Equal("IsSynchronized", i.Name); + Assert.Equal("$false", i.ValueString); + }); } [Fact] @@ -956,20 +976,47 @@ await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true Assert.NotNull(rawDetailsView); Assert.Empty(rawDetailsView.Type); Assert.Empty(rawDetailsView.ValueString); - VariableDetailsBase[] rawViewChildren = rawDetailsView.GetChildren(NullLogger.Instance); - Assert.Equal(7, rawViewChildren.Length); - Assert.Equal("IsReadOnly", rawViewChildren[0].Name); - Assert.Equal("$false", rawViewChildren[0].ValueString); - Assert.Equal("IsFixedSize", rawViewChildren[1].Name); - Assert.Equal("$false", rawViewChildren[1].ValueString); - Assert.Equal("IsSynchronized", rawViewChildren[2].Name); - Assert.Equal("$false", rawViewChildren[2].ValueString); - Assert.Equal("Keys", rawViewChildren[3].Name); - Assert.Equal("Values", rawViewChildren[4].Name); - Assert.Equal("[ValueCollection: 4]", rawViewChildren[4].ValueString); - Assert.Equal("SyncRoot", rawViewChildren[5].Name); - Assert.Equal("Count", rawViewChildren[6].Name); - Assert.Equal("4", rawViewChildren[6].ValueString); + VariableDetailsBase[] rawDetailsChildren = rawDetailsView.GetChildren(NullLogger.Instance); + Assert.Collection(rawDetailsChildren, + (i) => + { + Assert.Equal("IsReadOnly", i.Name); + Assert.Equal("$false", i.ValueString); + }, + (i) => + { + Assert.Equal("IsFixedSize", i.Name); + Assert.Equal("$false", i.ValueString); + }, + (i) => + { + Assert.Equal("IsSynchronized", i.Name); + Assert.Equal("$false", i.ValueString); + }, + (i) => + { + Assert.Equal("Keys", i.Name); + Assert.Equal("[KeyCollection: 4]", i.ValueString); + }, + (i) => + { + Assert.Equal("Values", i.Name); + Assert.Equal("[ValueCollection: 4]", i.ValueString); + }, + (i) => + { + Assert.Equal("SyncRoot", i.Name); +#if CoreCLR + Assert.Equal("[Hashtable: 4]", i.ValueString); +#else + Assert.Equal("[Object]", i.ValueString); +#endif + }, + (i) => + { + Assert.Equal("Count", i.Name); + Assert.Equal("4", i.ValueString); + }); } [Fact] @@ -996,8 +1043,28 @@ await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true Assert.Empty(rawDetailsView.Type); Assert.Empty(rawDetailsView.ValueString); VariableDetailsBase[] rawViewChildren = rawDetailsView.GetChildren(NullLogger.Instance); - Assert.Equal(4, rawViewChildren.Length); - Assert.NotNull(Array.Find(rawViewChildren, v => v.Name == "Comparer")); + Assert.Collection(rawViewChildren, + (i) => + { + Assert.Equal("Count", i.Name); + Assert.Equal("4", i.ValueString); + }, + (i) => + { + Assert.Equal("Comparer", i.Name); + Assert.Equal("[GenericComparer`1]", i.ValueString); + }, + (i) => + { + Assert.Equal("Keys", i.Name); + Assert.Equal("[KeyCollection: 4]", i.ValueString); + }, + (i) => + { + Assert.Equal("Values", i.Name); + Assert.Equal("[ValueCollection: 4]", i.ValueString); + } + ); } [Fact] @@ -1029,8 +1096,8 @@ await debugService.SetLineBreakpointsAsync( // Verifies fix for issue #86, $proc = Get-Process foo displays just the ETS property set // and not all process properties. - [Fact(Skip = "Length of child vars is wrong now")] - public async Task DebuggerVariableProcessObjDisplaysCorrectly() + [Fact] + public async Task DebuggerVariableProcessObjectDisplaysCorrectly() { await debugService.SetLineBreakpointsAsync( variableScriptFile, @@ -1049,7 +1116,33 @@ await debugService.SetLineBreakpointsAsync( Assert.True(var.IsExpandable); VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); - Assert.Equal(53, childVars.Length); + Assert.Contains(childVars, i => i.Name is "Name"); + Assert.Contains(childVars, i => i.Name is "Handles"); +#if CoreCLR + Assert.Contains(childVars, i => i.Name is "CommandLine"); + Assert.Contains(childVars, i => i.Name is "ExitCode"); + Assert.Contains(childVars, i => i.Name is "HasExited" && i.ValueString is "$false"); +#endif + Assert.Contains(childVars, i => i.Name is "Id"); + } + + [Fact] + public async Task DebuggerVariableFileObjectDisplaysCorrectly() + { + await debugService.SetCommandBreakpointsAsync( + new[] { CommandBreakpointDetails.Create("Write-Host") }).ConfigureAwait(true); + + ScriptFile testScript = GetDebugScript("GetChildItemTest.ps1"); + Task _ = ExecuteScriptFileAsync(testScript.FilePath); + AssertDebuggerStopped(testScript.FilePath, 2); + + VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); + VariableDetailsBase var = Array.Find(variables, v => v.Name == "$file"); + VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true); + Assert.Contains(childVars, i => i.Name is "PSPath"); + Assert.Contains(childVars, i => i.Name is "PSProvider" && i.ValueString is "Microsoft.PowerShell.Core\\FileSystem"); + Assert.Contains(childVars, i => i.Name is "Exists" && i.ValueString is "$true"); + Assert.Contains(childVars, i => i.Name is "LastAccessTime"); } } } From 874e1b82d805c4365eef476210aef000a2871ab2 Mon Sep 17 00:00:00 2001 From: Justin Grote Date: Mon, 19 Dec 2022 14:26:19 -0800 Subject: [PATCH 4/5] Add test for custom `ToString` implementations This now works fine since the expansion takes place on the pipeline thread. --- .../Debugging/VariableTest.ps1 | 13 ++++++++++ .../Debugging/DebugServiceTests.cs | 25 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1 b/test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1 index e8c23d9a0..2dc513746 100644 --- a/test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1 +++ b/test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1 @@ -51,3 +51,16 @@ $sortedDictionary['blue'] = 'red' # This is a dummy function that the test will use to stop and evaluate the debug environment function __BreakDebuggerDerivedDictionaryPropertyInRawView{}; __BreakDebuggerDerivedDictionaryPropertyInRawView + +class CustomToString { + [String]$String = 'Hello' + [String]ToString() { + return $this.String.ToUpper() + } +} +$SCRIPT:CustomToStrings = 1..1000 | ForEach-Object { + [CustomToString]::new() +} + +# This is a dummy function that the test will use to stop and evaluate the debug environment +function __BreakDebuggerToStringShouldMarshallToPipeline{}; __BreakDebuggerToStringShouldMarshallToPipeline diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index c65b6a040..7a5025b85 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -1144,5 +1144,30 @@ await debugService.SetCommandBreakpointsAsync( Assert.Contains(childVars, i => i.Name is "Exists" && i.ValueString is "$true"); Assert.Contains(childVars, i => i.Name is "LastAccessTime"); } + + // Verifies Issue #1686 + [Fact] + public async Task DebuggerToStringShouldMarshallToPipeline() + { + CommandBreakpointDetails breakpoint = CommandBreakpointDetails.Create("__BreakDebuggerToStringShouldMarshallToPipeline"); + await debugService.SetCommandBreakpointsAsync(new[] { breakpoint }).ConfigureAwait(true); + + // Execute the script and wait for the breakpoint to be hit + Task _ = ExecuteVariableScriptFileAsync(); + AssertDebuggerStopped(commandBreakpointDetails: breakpoint); + + VariableDetailsBase[] vars = await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true); + VariableDetailsBase customToStrings = Array.Find(vars, i => i.Name is "$CustomToStrings"); + Assert.True(customToStrings.IsExpandable); + Assert.Equal("[System.Object[]]", customToStrings.Type); + VariableDetailsBase[] childVars = await debugService.GetVariables(customToStrings.Id, CancellationToken.None).ConfigureAwait(true); + // Check everything but the last variable (which is "Raw View") + Assert.Equal(1001, childVars.Length); // 1000 custom variables plus "Raw View" + Assert.All(childVars.Take(childVars.Length - 1), i => + { + Assert.Equal("HELLO", i.ValueString); + Assert.Equal("[CustomToString]", i.Type); + }); + } } } From f6b1ffa103766c92d43c9c2eef8fffaa8558da75 Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Tue, 20 Dec 2022 11:05:57 -0800 Subject: [PATCH 5/5] Run `debugService.Continue()` in the thread pool So that it doesn't deadlock with the pipeline thread. --- .../Debugging/DebugServiceTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index 7a5025b85..fa1ed7be8 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -177,7 +177,7 @@ await debugService.SetCommandBreakpointsAsync( new PSCommand().AddScript("Get-Random -SetSeed 42 -Maximum 100"), CancellationToken.None); AssertDebuggerStopped("", 1); - debugService.Continue(); + await Task.Run(debugService.Continue).ConfigureAwait(true); Assert.Equal(17, (await executeTask.ConfigureAwait(true))[0]); StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true); @@ -296,7 +296,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints() Assert.Equal("1", i.ValueString); // The function breakpoint should fire the next time through the loop. - debugService.Continue(); + await Task.Run(debugService.Continue).ConfigureAwait(true); AssertDebuggerStopped(debugScriptFile.FilePath, 6); variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true); @@ -353,7 +353,7 @@ await debugService.SetLineBreakpointsAsync( Task _ = ExecuteDebugFileAsync(); AssertDebuggerStopped(debugScriptFile.FilePath, 5); - debugService.Continue(); + await Task.Run(debugService.Continue).ConfigureAwait(true); AssertDebuggerStopped(debugScriptFile.FilePath, 7); } @@ -382,7 +382,7 @@ await debugService.SetLineBreakpointsAsync( // The conditional breakpoint should not fire again, until the value of // i reaches breakpointValue2. - debugService.Continue(); + await Task.Run(debugService.Continue).ConfigureAwait(true); AssertDebuggerStopped(debugScriptFile.FilePath, 7); variables = await GetVariables(VariableContainerDetails.LocalScopeName).ConfigureAwait(true);