From 84d9cbb07cf04d3c8e5774fd494cb7be802ada38 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Mon, 16 Aug 2021 19:04:48 -0700 Subject: [PATCH 1/4] Clean up `DebugServiceTests.cs` --- .../Debugging/DebugServiceTests.cs | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index b3cedbd14..8dd322d0a 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -32,6 +32,16 @@ public class DebugServiceTests : IDisposable private AsyncQueue sessionStateQueue = new AsyncQueue(); + private ScriptFile GetDebugScript(string fileName) + { + return this.workspace.GetFile( + TestUtilities.NormalizePath(Path.Combine( + Path.GetDirectoryName(typeof(DebugServiceTests).Assembly.Location), + "../../../../PowerShellEditorServices.Test.Shared/Debugging", + fileName + ))); + } + public DebugServiceTests() { var logger = NullLogger.Instance; @@ -41,18 +51,9 @@ public DebugServiceTests() this.workspace = new WorkspaceService(NullLoggerFactory.Instance); - // Load the test debug file - this.debugScriptFile = - this.workspace.GetFile( - TestUtilities.NormalizePath(Path.Combine( - Path.GetDirectoryName(typeof(DebugServiceTests).Assembly.Location), - "../../../../PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1"))); - - this.variableScriptFile = - this.workspace.GetFile( - TestUtilities.NormalizePath(Path.Combine( - Path.GetDirectoryName(typeof(DebugServiceTests).Assembly.Location), - "../../../../PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1"))); + // Load the test debug files + this.debugScriptFile = GetDebugScript("DebugTest.ps1"); + this.variableScriptFile = GetDebugScript("VariableTest.ps1"); this.debugService = new DebugService( this.powerShellContext, @@ -65,13 +66,6 @@ public DebugServiceTests() this.debugService.DebuggerStopped += debugService_DebuggerStopped; this.debugService.BreakpointUpdated += debugService_BreakpointUpdated; - - // Load the test debug file - this.debugScriptFile = - this.workspace.GetFile( - TestUtilities.NormalizePath(Path.Combine( - Path.GetDirectoryName(typeof(DebugServiceTests).Assembly.Location), - "../../../../PowerShellEditorServices.Test.Shared/Debugging/DebugTest.ps1"))); } async void powerShellContext_SessionStateChanged(object sender, SessionStateChangedEventArgs e) @@ -123,11 +117,7 @@ public async Task DebuggerAcceptsScriptArgs(string[] args) // The path is intentionally odd (some escaped chars but not all) because we are testing // the internal path escaping mechanism - it should escape certains chars ([, ] and space) but // it should not escape already escaped chars. - ScriptFile debugWithParamsFile = - this.workspace.GetFile( - TestUtilities.NormalizePath(Path.Combine( - Path.GetDirectoryName(typeof(DebugServiceTests).Assembly.Location), - "../../../../PowerShellEditorServices.Test.Shared/Debugging/Debug W&ith Params [Test].ps1"))); + ScriptFile debugWithParamsFile = GetDebugScript("Debug W&ith Params [Test].ps1"); await this.debugService.SetLineBreakpointsAsync( debugWithParamsFile, @@ -889,7 +879,7 @@ await this.debugService.SetLineBreakpointsAsync( var nullStringVar = variables.FirstOrDefault(v => v.Name == "$nullString"); Assert.NotNull(nullStringVar); - Assert.True("[NullString]".Equals(nullStringVar.ValueString)); + Assert.Equal("[NullString]", nullStringVar.ValueString); Assert.True(nullStringVar.IsExpandable); // Abort script execution early and wait for completion @@ -973,6 +963,7 @@ await this.debugService.SetLineBreakpointsAsync( // Verifies fix for issue #86, $proc = Get-Process foo displays just the ETS property set // and not all process properties. + [Trait("Category", "DebugService")] [Fact] public async Task DebuggerVariableProcessObjDisplaysCorrectly() { From 6d7a6b39f119f405465f14d955049d3c57494231 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 12 Aug 2021 15:15:44 -0700 Subject: [PATCH 2/4] Add regression test for `System.Windows.Forms` bug --- .../DebugAdapterProtocolMessageTests.cs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs index 5c922e886..6e23abff2 100644 --- a/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs @@ -228,5 +228,64 @@ public async Task CanSetBreakpointsAsync() (i) => Assert.Equal("at breakpoint", i), (i) => Assert.Equal("after breakpoint", i)); } + + // 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 + // OmniSharp. + // + // This test depends on PowerShell being able to load System.Windows.Forms, which only works + // reliably with Windows PowerShell. It works with PowerShell Core in the real-world; + // however, our host executable is xUnit, not PowerShell. So by restricting to Windows + // PowerShell, we avoid all issues with our test project (and the xUnit executable) not + // having System.Windows.Forms deployed, and can instead rely on the Windows Global Assembly + // Cache (GAC) to find it. + [Trait("Category", "DAP")] + [SkippableFact] + public async Task CanStepPastSystemWindowsForms() + { + Skip.IfNot(PsesStdioProcess.IsWindowsPowerShell); + Skip.If(PsesStdioProcess.RunningInConstainedLanguageMode); + + string filePath = NewTestFile(string.Join(Environment.NewLine, new [] + { + "Add-Type -AssemblyName System.Windows.Forms", + "$form = New-Object System.Windows.Forms.Form", + "Write-Host $form" + })); + + await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false); + + var setBreakpointsResponse = await PsesDebugAdapterClient.SetFunctionBreakpoints( + new SetFunctionBreakpointsArguments + { + Breakpoints = new FunctionBreakpoint[] + { + new FunctionBreakpoint + { + Name = "Write-Host", + } + } + }).ConfigureAwait(false); + + var breakpoint = setBreakpointsResponse.Breakpoints.First(); + Assert.True(breakpoint.Verified); + + ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false); + Assert.NotNull(configDoneResponse); + + // At this point the script should be running so lets give it time + await Task.Delay(2000).ConfigureAwait(false); + + var variablesResponse = await PsesDebugAdapterClient.RequestVariables( + new VariablesArguments + { + VariablesReference = 1 + }).ConfigureAwait(false); + + var form = variablesResponse.Variables.FirstOrDefault(v => v.Name == "$form"); + Assert.NotNull(form); + Assert.Equal("System.Windows.Forms.Form, Text: ", form.Value); + } } } From e28292fa091c28079c2a8e33b30a698d91ee179d Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 17 Aug 2021 16:19:35 -0700 Subject: [PATCH 3/4] Temporarily include OmniSharp via project reference So that we can build from our fork while we wait for an upcoming release. --- .vsts-ci/azure-pipelines-ci.yml | 9 +++++++++ .vsts-ci/templates/ci-general.yml | 9 +++++++-- .../PowerShellEditorServices.csproj | 9 ++++++--- .../PowerShellEditorServices.Test.E2E.csproj | 7 +++++-- .../PowerShellEditorServices.Test.csproj | 5 +++-- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/.vsts-ci/azure-pipelines-ci.yml b/.vsts-ci/azure-pipelines-ci.yml index 25706f2c0..f0e84e1f2 100644 --- a/.vsts-ci/azure-pipelines-ci.yml +++ b/.vsts-ci/azure-pipelines-ci.yml @@ -16,6 +16,15 @@ trigger: pr: - master +# TODO: Remove this when OmniSharp updates +resources: + repositories: + - repository: OmniSharp + type: github + endpoint: GitHub + name: andschwa/csharp-language-server-protocol + ref: preview + jobs: - job: PS51_Win2016 displayName: PowerShell 5.1 - Windows Server 2016 diff --git a/.vsts-ci/templates/ci-general.yml b/.vsts-ci/templates/ci-general.yml index 016b340dc..d853e3802 100644 --- a/.vsts-ci/templates/ci-general.yml +++ b/.vsts-ci/templates/ci-general.yml @@ -7,10 +7,15 @@ steps: - pwsh: $PSVersionTable displayName: PowerShell version +# TODO: Remove this when OmniSharp updates +- checkout: self +- checkout: OmniSharp + - task: PowerShell@2 displayName: Build and test inputs: - filePath: tools/azurePipelinesBuild.ps1 + filePath: $(Build.SourcesDirectory)/PowerShellEditorServices/tools/azurePipelinesBuild.ps1 + workingDirectory: $(Build.SourcesDirectory)/PowerShellEditorServices pwsh: ${{ parameters.pwsh }} # NOTE: We zip the artifacts because they're ~20 MB compressed, but ~300 MB raw, @@ -18,7 +23,7 @@ steps: - task: ArchiveFiles@2 displayName: Zip pipeline artifacts inputs: - rootFolderOrFile: module + rootFolderOrFile: $(Build.SourcesDirectory)/PowerShellEditorServices/module includeRootFolder: false archiveType: zip archiveFile: PowerShellEditorServices-Build.zip diff --git a/src/PowerShellEditorServices/PowerShellEditorServices.csproj b/src/PowerShellEditorServices/PowerShellEditorServices.csproj index 562239ded..fe2424380 100644 --- a/src/PowerShellEditorServices/PowerShellEditorServices.csproj +++ b/src/PowerShellEditorServices/PowerShellEditorServices.csproj @@ -28,8 +28,11 @@ - - + + + + + @@ -42,6 +45,6 @@ - + diff --git a/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj b/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj index 756ac8bd0..224c8f3fb 100644 --- a/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj +++ b/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj @@ -10,8 +10,11 @@ - - + + + + + diff --git a/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj b/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj index 9c146f426..1434afa0e 100644 --- a/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj +++ b/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj @@ -34,8 +34,9 @@ - - + + + From cd29c499ca77c0686d26d0657def25155743fca3 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 18 Aug 2021 12:51:05 -0700 Subject: [PATCH 4/4] Bump OmniSharp to `v0.19.3` So `v0.19.4` is out on GitHub, but has not yet released to the NuGet gallery, and I don't wish to delay this any longer. We'll let Dependabot bring that in as soon as it can be. The only thing missing is the DryIoc update, which we're simply hoping solves an issue. --- .vsts-ci/azure-pipelines-ci.yml | 9 --------- .vsts-ci/templates/ci-general.yml | 9 ++------- .../PowerShellEditorServices.csproj | 9 +++------ .../PowerShellEditorServices.Test.E2E.csproj | 7 ++----- .../PowerShellEditorServices.Test.csproj | 4 +--- 5 files changed, 8 insertions(+), 30 deletions(-) diff --git a/.vsts-ci/azure-pipelines-ci.yml b/.vsts-ci/azure-pipelines-ci.yml index f0e84e1f2..25706f2c0 100644 --- a/.vsts-ci/azure-pipelines-ci.yml +++ b/.vsts-ci/azure-pipelines-ci.yml @@ -16,15 +16,6 @@ trigger: pr: - master -# TODO: Remove this when OmniSharp updates -resources: - repositories: - - repository: OmniSharp - type: github - endpoint: GitHub - name: andschwa/csharp-language-server-protocol - ref: preview - jobs: - job: PS51_Win2016 displayName: PowerShell 5.1 - Windows Server 2016 diff --git a/.vsts-ci/templates/ci-general.yml b/.vsts-ci/templates/ci-general.yml index d853e3802..016b340dc 100644 --- a/.vsts-ci/templates/ci-general.yml +++ b/.vsts-ci/templates/ci-general.yml @@ -7,15 +7,10 @@ steps: - pwsh: $PSVersionTable displayName: PowerShell version -# TODO: Remove this when OmniSharp updates -- checkout: self -- checkout: OmniSharp - - task: PowerShell@2 displayName: Build and test inputs: - filePath: $(Build.SourcesDirectory)/PowerShellEditorServices/tools/azurePipelinesBuild.ps1 - workingDirectory: $(Build.SourcesDirectory)/PowerShellEditorServices + filePath: tools/azurePipelinesBuild.ps1 pwsh: ${{ parameters.pwsh }} # NOTE: We zip the artifacts because they're ~20 MB compressed, but ~300 MB raw, @@ -23,7 +18,7 @@ steps: - task: ArchiveFiles@2 displayName: Zip pipeline artifacts inputs: - rootFolderOrFile: $(Build.SourcesDirectory)/PowerShellEditorServices/module + rootFolderOrFile: module includeRootFolder: false archiveType: zip archiveFile: PowerShellEditorServices-Build.zip diff --git a/src/PowerShellEditorServices/PowerShellEditorServices.csproj b/src/PowerShellEditorServices/PowerShellEditorServices.csproj index fe2424380..a18cc4be0 100644 --- a/src/PowerShellEditorServices/PowerShellEditorServices.csproj +++ b/src/PowerShellEditorServices/PowerShellEditorServices.csproj @@ -28,11 +28,8 @@ - - - - - + + @@ -45,6 +42,6 @@ - + diff --git a/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj b/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj index 224c8f3fb..e7c907a00 100644 --- a/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj +++ b/test/PowerShellEditorServices.Test.E2E/PowerShellEditorServices.Test.E2E.csproj @@ -10,11 +10,8 @@ - - - - - + + diff --git a/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj b/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj index 1434afa0e..c801384ab 100644 --- a/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj +++ b/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj @@ -34,9 +34,7 @@ - - - +