diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ScopesHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ScopesHandler.cs index e74bd1eb9..daa3d8e60 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ScopesHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ScopesHandler.cs @@ -18,12 +18,20 @@ internal class ScopesHandler : IScopesHandler public ScopesHandler(DebugService debugService) => _debugService = debugService; + /// + /// Retrieves the variable scopes (containers) for the currently selected stack frame. Variables details are fetched via a separate request. + /// public Task Handle(ScopesArguments request, CancellationToken cancellationToken) { - //We have an artificial breakpoint label, so just copy the stacktrace from the first stack entry for this. - int frameId = request.FrameId == 0 ? 0 : (int)request.FrameId - 1; + // HACK: The StackTraceHandler injects an artificial label frame as the first frame as a performance optimization, so when scopes are requested by the client, we need to adjust the frame index accordingly to match the underlying PowerShell frame, so when the client clicks on the label (or hit the default breakpoint), they get variables populated from the top of the PowerShell stackframe. If the client dives deeper, we need to reflect that as well (though 90% of debug users don't actually investigate this) + // VSCode Frame 0 (Label) -> PowerShell StackFrame 0 (for convenience) + // VSCode Frame 1 (First Real PS Frame) -> Also PowerShell StackFrame 0 + // VSCode Frame 2 -> PowerShell StackFrame 1 + // VSCode Frame 3 -> PowerShell StackFrame 2 + // etc. + int powershellFrameId = request.FrameId == 0 ? 0 : (int)request.FrameId - 1; - VariableScope[] variableScopes = _debugService.GetVariableScopes(frameId); + VariableScope[] variableScopes = _debugService.GetVariableScopes(powershellFrameId); return Task.FromResult(new ScopesResponse { diff --git a/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs index a3c68eadb..bb794516f 100644 --- a/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs @@ -2,22 +2,31 @@ // Licensed under the MIT License. using System; +using System.Diagnostics; using System.IO; using System.Linq; using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Debug; using OmniSharp.Extensions.DebugAdapter.Client; +using DapStackFrame = OmniSharp.Extensions.DebugAdapter.Protocol.Models.StackFrame; +using OmniSharp.Extensions.DebugAdapter.Protocol.Events; using OmniSharp.Extensions.DebugAdapter.Protocol.Models; using OmniSharp.Extensions.DebugAdapter.Protocol.Requests; +using OmniSharp.Extensions.JsonRpc.Server; using Xunit; using Xunit.Abstractions; +using Microsoft.Extensions.Logging.Abstractions; namespace PowerShellEditorServices.Test.E2E { + public class XunitOutputTraceListener(ITestOutputHelper output) : TraceListener + { + public override void Write(string message) => output.WriteLine(message); + public override void WriteLine(string message) => output.WriteLine(message); + } + [Trait("Category", "DAP")] public class DebugAdapterProtocolMessageTests : IAsyncLifetime, IDisposable { @@ -28,16 +37,26 @@ public class DebugAdapterProtocolMessageTests : IAsyncLifetime, IDisposable private DebugAdapterClient PsesDebugAdapterClient; private PsesStdioProcess _psesProcess; + /// + /// Completes when the debug adapter is started. + /// public TaskCompletionSource Started { get; } = new TaskCompletionSource(); - + /// + /// Completes when the first breakpoint is reached. + /// + public TaskCompletionSource Stopped { get; } = new TaskCompletionSource(); + + /// + /// Constructor. The ITestOutputHelper is injected by xUnit and used to write diagnostic logs. + /// + /// public DebugAdapterProtocolMessageTests(ITestOutputHelper output) => _output = output; public async Task InitializeAsync() { - LoggerFactory debugLoggerFactory = new(); - debugLoggerFactory.AddProvider(new DebugLoggerProvider()); + // NOTE: To see debug logger output, add this line to your test - _psesProcess = new PsesStdioProcess(debugLoggerFactory, true); + _psesProcess = new PsesStdioProcess(new NullLoggerFactory(), true); await _psesProcess.Start(); TaskCompletionSource initialized = new(); @@ -53,11 +72,6 @@ public async Task InitializeAsync() options .WithInput(_psesProcess.OutputStream) .WithOutput(_psesProcess.InputStream) - .ConfigureLogging(builder => - builder - .AddDebug() - .SetMinimumLevel(LogLevel.Trace) - ) // The OnStarted delegate gets run when we receive the _Initialized_ event from the server: // https://microsoft.github.io/debug-adapter-protocol/specification#Events_Initialized .OnStarted((_, _) => @@ -65,6 +79,13 @@ public async Task InitializeAsync() Started.SetResult(true); return Task.CompletedTask; }) + // We use this to create a task we can await to test debugging after a breakpoint has been received. + .OnNotification(null, (stoppedEvent, _) => + { + Console.WriteLine("StoppedEvent received"); + Stopped.SetResult(stoppedEvent); + return Task.CompletedTask; + }) // The OnInitialized delegate gets run when we first receive the _Initialize_ response: // https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Initialize .OnInitialized((_, _, _, _) => @@ -263,6 +284,83 @@ public async Task CanSetBreakpointsAsync() (i) => Assert.Equal("after breakpoint", i)); } + [SkippableFact] + public async Task FailsIfStacktraceRequestedWhenNotPaused() + { + Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode, + "Breakpoints can't be set in Constrained Language Mode."); + string filePath = NewTestFile(GenerateScriptFromLoggingStatements( + "labelTestBreakpoint" + )); + // Set a breakpoint + await PsesDebugAdapterClient.SetBreakpoints( + new SetBreakpointsArguments + { + Source = new Source { Name = Path.GetFileName(filePath), Path = filePath }, + Breakpoints = new SourceBreakpoint[] { new SourceBreakpoint { Line = 1 } }, + SourceModified = false, + } + ); + + // Signal to start the script + await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()); + await PsesDebugAdapterClient.LaunchScript(filePath, Started); + + + // Get the stacktrace for the breakpoint + await Assert.ThrowsAsync(() => PsesDebugAdapterClient.RequestStackTrace( + new StackTraceArguments { } + )); + } + + [SkippableFact] + public async Task SendsInitialLabelBreakpointForPerformanceReasons() + { + Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode, + "Breakpoints can't be set in Constrained Language Mode."); + string filePath = NewTestFile(GenerateScriptFromLoggingStatements( + "before breakpoint", + "at breakpoint", + "after breakpoint" + )); + + //TODO: This is technically wrong per the spec, configDone should be completed BEFORE launching, but this is how the vscode client does it today and we really need to fix that. + await PsesDebugAdapterClient.LaunchScript(filePath, Started); + + // {"command":"setBreakpoints","arguments":{"source":{"name":"dfsdfg.ps1","path":"/Users/tyleonha/Code/PowerShell/Misc/foo/dfsdfg.ps1"},"lines":[2],"breakpoints":[{"line":2}],"sourceModified":false},"type":"request","seq":3} + SetBreakpointsResponse setBreakpointsResponse = await PsesDebugAdapterClient.SetBreakpoints(new SetBreakpointsArguments + { + Source = new Source { Name = Path.GetFileName(filePath), Path = filePath }, + Breakpoints = new SourceBreakpoint[] { new SourceBreakpoint { Line = 2 } }, + SourceModified = false, + }); + + Breakpoint breakpoint = setBreakpointsResponse.Breakpoints.First(); + Assert.True(breakpoint.Verified); + Assert.Equal(filePath, breakpoint.Source.Path, ignoreCase: s_isWindows); + Assert.Equal(2, breakpoint.Line); + + ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()); + + // FIXME: I think there is a race condition here. If you remove this, the following line Stack Trace fails because the breakpoint hasn't been hit yet. I think the whole getLog process just works long enough for ConfigurationDone to complete and for the breakpoint to be hit. + + // I've tried to do this properly by waiting for a StoppedEvent, but that doesn't seem to work, I'm probably just not wiring it up right in the handler. + Assert.NotNull(configDoneResponse); + Assert.Collection(await GetLog(), + (i) => Assert.Equal("before breakpoint", i)); + File.Delete(s_testOutputPath); + + // Get the stacktrace for the breakpoint + StackTraceResponse stackTraceResponse = await PsesDebugAdapterClient.RequestStackTrace( + new StackTraceArguments { ThreadId = 1 } + ); + DapStackFrame firstFrame = stackTraceResponse.StackFrames.First(); + Assert.Equal( + firstFrame.PresentationHint, + StackFramePresentationHint.Label + ); + } + // This is a regression test for a bug where user code causes a new synchronization context // to be created, breaking the extension. It's most evident when debugging PowerShell // scripts that use System.Windows.Forms. It required fixing both Editor Services and