Remove arbitrary timeouts from tests

Our CI can be quite slow, and these were just arbitrary timeouts. If the
tests succeed, they should never be hit. Let's instead give them as much
time as possible. CI will eventually kill the run if it hangs, and
locally devs can do the same. This should help improve CI stability.

Also fixes the disposal bug in the tests.

Author: andyleejordan

src/PowerShellEditorServices/Services/PowerShell/Utility/CancellationContext.cs

@@ -103,7 +103,18 @@ internal CancellationScope(
 
         public CancellationToken CancellationToken => _cancellationSource.Token;
 
-        public void Cancel() => _cancellationSource.Cancel();
+        public void Cancel()
+        {
+            try
+            {
+                _cancellationSource.Cancel();
+            }
+            catch (ObjectDisposedException)
+            {
+                // We don't want this race condition to cause flaky tests.
+                // TODO: Find out the cause of the race!
+            }
+        }
 
         public bool IsIdleScope { get; }
 

test/PowerShellEditorServices.Test.E2E/DebugAdapterClientExtensions.cs

@@ -2,11 +2,11 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.PowerShell.EditorServices.Handlers;
 using OmniSharp.Extensions.DebugAdapter.Client;
 using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
-using System.Threading;
 
 namespace PowerShellEditorServices.Test.E2E
 {
@@ -29,9 +29,10 @@ public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient
             }
 
             // This will check to see if we received the Initialized event from the server.
+            // NOTE: This needs to be spawned on a different thread, hence Task.Run().
             await Task.Run(
                 async () => await started.Task.ConfigureAwait(true),
-                new CancellationTokenSource(2000).Token).ConfigureAwait(true);
+                CancellationToken.None).ConfigureAwait(true);
         }
     }
 }

test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs

@@ -39,7 +39,7 @@ public async Task InitializeAsync()
         {
             LoggerFactory factory = new();
             _psesProcess = new PsesStdioProcess(factory, true);
-            await _psesProcess.Start().ConfigureAwait(false);
+            await _psesProcess.Start().ConfigureAwait(true);
 
             TaskCompletionSource<bool> initialized = new();
 
@@ -90,26 +90,21 @@ public async Task InitializeAsync()
             // that gets completed when we receive the response to Initialize
             // This tells us that we are ready to send messages to PSES... but are not stuck waiting for
             // Initialized.
-            PsesDebugAdapterClient.Initialize(CancellationToken.None).ConfigureAwait(false);
-            await initialized.Task.ConfigureAwait(false);
+#pragma warning disable CS4014
+            PsesDebugAdapterClient.Initialize(CancellationToken.None).ConfigureAwait(true);
+#pragma warning restore CS4014
+            await initialized.Task.ConfigureAwait(true);
         }
 
         public async Task DisposeAsync()
         {
-            try
+            await PsesDebugAdapterClient.RequestDisconnect(new DisconnectArguments
             {
-                await PsesDebugAdapterClient.RequestDisconnect(new DisconnectArguments
-                {
-                    Restart = false,
-                    TerminateDebuggee = true
-                }).ConfigureAwait(false);
-                await _psesProcess.Stop().ConfigureAwait(false);
-                PsesDebugAdapterClient?.Dispose();
-            }
-            catch (ObjectDisposedException)
-            {
-                // Language client has a disposal bug in it
-            }
+                Restart = false,
+                TerminateDebuggee = true
+            }).ConfigureAwait(true);
+            await _psesProcess.Stop().ConfigureAwait(true);
+            PsesDebugAdapterClient?.Dispose();
         }
 
         private static string NewTestFile(string script, bool isPester = false)
@@ -147,7 +142,16 @@ private string GenerateScriptFromLoggingStatements(params string[] logStatements
             return builder.ToString();
         }
 
-        private static string[] GetLog() => File.ReadLines(s_testOutputPath).ToArray();
+        private static async Task<string[]> GetLog()
+        {
+            while (!File.Exists(s_testOutputPath))
+            {
+                await Task.Delay(1000).ConfigureAwait(true);
+            }
+            // Sleep one more time after the file exists so whatever is writing can finish.
+            await Task.Delay(1000).ConfigureAwait(true);
+            return File.ReadLines(s_testOutputPath).ToArray();
+        }
 
         [Fact]
         public void CanInitializeWithCorrectServerSettings()
@@ -165,14 +169,12 @@ public async Task CanLaunchScriptWithNoBreakpointsAsync()
         {
             string filePath = NewTestFile(GenerateScriptFromLoggingStatements("works"));
 
-            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
+            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
 
-            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
+            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
             Assert.NotNull(configDoneResponse);
-            await Task.Delay(2000).ConfigureAwait(false);
-
-            string[] log = GetLog();
-            Assert.Equal("works", log[0]);
+            Assert.Collection(await GetLog().ConfigureAwait(true),
+                (i) => Assert.Equal("works", i));
         }
 
         [SkippableFact]
@@ -188,37 +190,32 @@ public async Task CanSetBreakpointsAsync()
                 "after breakpoint"
             ));
 
-            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
+            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
 
             // {"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,
-            }).ConfigureAwait(false);
+            }).ConfigureAwait(true);
 
             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()).ConfigureAwait(false);
+            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
             Assert.NotNull(configDoneResponse);
-            await Task.Delay(2000).ConfigureAwait(false);
-
-            string[] log = GetLog();
-            Assert.Single(log, (i) => i == "before breakpoint");
+            Assert.Collection(await GetLog().ConfigureAwait(true),
+                (i) => Assert.Equal("before breakpoint", i));
+            File.Delete(s_testOutputPath);
 
             ContinueResponse continueResponse = await PsesDebugAdapterClient.RequestContinue(
                 new ContinueArguments { ThreadId = 1 }).ConfigureAwait(true);
 
             Assert.NotNull(continueResponse);
-            await Task.Delay(2000).ConfigureAwait(false);
-
-            log = GetLog();
-            Assert.Collection(log,
-                (i) => Assert.Equal("before breakpoint", i),
+            Assert.Collection(await GetLog().ConfigureAwait(true),
                 (i) => Assert.Equal("at breakpoint", i),
                 (i) => Assert.Equal("after breakpoint", i));
         }
@@ -247,24 +244,24 @@ public async Task CanStepPastSystemWindowsForms()
                     "Write-Host $form"
                 }));
 
-            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
+            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
 
             SetFunctionBreakpointsResponse setBreakpointsResponse = await PsesDebugAdapterClient.SetFunctionBreakpoints(
                 new SetFunctionBreakpointsArguments
                 {
                     Breakpoints = new FunctionBreakpoint[]
                         { new FunctionBreakpoint { Name = "Write-Host", } }
-                }).ConfigureAwait(false);
+                }).ConfigureAwait(true);
 
             Breakpoint breakpoint = setBreakpointsResponse.Breakpoints.First();
             Assert.True(breakpoint.Verified);
 
-            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
+            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
             Assert.NotNull(configDoneResponse);
-            await Task.Delay(2000).ConfigureAwait(false);
+            await Task.Delay(5000).ConfigureAwait(true);
 
             VariablesResponse variablesResponse = await PsesDebugAdapterClient.RequestVariables(
-                new VariablesArguments { VariablesReference = 1 }).ConfigureAwait(false);
+                new VariablesArguments { VariablesReference = 1 }).ConfigureAwait(true);
 
             Variable form = variablesResponse.Variables.FirstOrDefault(v => v.Name == "$form");
             Assert.NotNull(form);
@@ -286,13 +283,12 @@ public async Task CanLaunchScriptWithCommentedLastLineAsync()
             // PsesLaunchRequestArguments.Script, which is then assigned to
             // DebugStateService.ScriptToLaunch in that handler, and finally used by the
             // ConfigurationDoneHandler in LaunchScriptAsync.
-            await PsesDebugAdapterClient.LaunchScript(script, Started).ConfigureAwait(false);
+            await PsesDebugAdapterClient.LaunchScript(script, Started).ConfigureAwait(true);
 
-            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
+            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
             Assert.NotNull(configDoneResponse);
-            await Task.Delay(2000).ConfigureAwait(false);
-
-            Assert.Collection(GetLog(), (i) => Assert.Equal("a log statement", i));
+            Assert.Collection(await GetLog().ConfigureAwait(true),
+                (i) => Assert.Equal("a log statement", i));
         }
 
         [Fact]
@@ -311,13 +307,12 @@ public async Task CanRunPesterTestFile()
                 }
             ");
 
-            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
-
-            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
+            await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
+            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
             Assert.NotNull(configDoneResponse);
-            await Task.Delay(2000).ConfigureAwait(false);
 
-            Assert.Collection(GetLog(), (i) => Assert.Equal("pester", i));
+            Assert.Collection(await GetLog().ConfigureAwait(true),
+                (i) => Assert.Equal("pester", i));
         }
     }
 }

test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs

@@ -95,16 +95,9 @@ public async Task InitializeAsync()
 
         public async Task DisposeAsync()
         {
-            try
-            {
-                await PsesLanguageClient.Shutdown().ConfigureAwait(false);
-                await _psesProcess.Stop().ConfigureAwait(false);
-                PsesLanguageClient?.Dispose();
-            }
-            catch (ObjectDisposedException)
-            {
-                // Language client has a disposal bug in it
-            }
+            await PsesLanguageClient.Shutdown().ConfigureAwait(false);
+            await _psesProcess.Stop().ConfigureAwait(false);
+            PsesLanguageClient?.Dispose();
         }
     }
 }

test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs

@@ -12,6 +12,10 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.PowerShell.EditorServices.Handlers;
+using Microsoft.PowerShell.EditorServices.Logging;
+using Microsoft.PowerShell.EditorServices.Services.Configuration;
+using Microsoft.PowerShell.EditorServices.Services.PowerShell;
+using Microsoft.PowerShell.EditorServices.Services.Template;
 using Newtonsoft.Json.Linq;
 using OmniSharp.Extensions.LanguageServer.Protocol;
 using OmniSharp.Extensions.LanguageServer.Protocol.Client;
@@ -21,10 +25,6 @@
 using Xunit;
 using Xunit.Abstractions;
 using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;
-using Microsoft.PowerShell.EditorServices.Logging;
-using Microsoft.PowerShell.EditorServices.Services.Configuration;
-using Microsoft.PowerShell.EditorServices.Services.PowerShell;
-using Microsoft.PowerShell.EditorServices.Services.Template;
 
 namespace PowerShellEditorServices.Test.E2E
 {
@@ -1149,8 +1149,6 @@ await PsesLanguageClient
         [Fact]
         public async Task CanSendEvaluateRequestAsync()
         {
-            using CancellationTokenSource cancellationSource = new(millisecondsDelay: 5000);
-
             EvaluateResponseBody evaluateResponseBody =
                 await PsesLanguageClient
                     .SendRequest(
@@ -1159,7 +1157,7 @@ await PsesLanguageClient
                         {
                             Expression = "Get-ChildItem"
                         })
-                    .Returning<EvaluateResponseBody>(cancellationSource.Token).ConfigureAwait(true);
+                    .Returning<EvaluateResponseBody>(CancellationToken.None).ConfigureAwait(true);
 
             // These always gets returned so this test really just makes sure we get _any_ response.
             Assert.Equal("", evaluateResponseBody.Result);

test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs

@@ -103,7 +103,7 @@ private Task ExecutePowerShellCommand(string command, params string[] args)
 
         private void AssertDebuggerPaused()
         {
-            DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(new CancellationTokenSource(5000).Token);
+            DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(CancellationToken.None);
             Assert.Empty(eventArgs.OriginalEvent.Breakpoints);
         }
 
@@ -112,7 +112,7 @@ private void AssertDebuggerStopped(
             int lineNumber = -1,
             CommandBreakpointDetails commandBreakpointDetails = default)
         {
-            DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(new CancellationTokenSource(5000).Token);
+            DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(CancellationToken.None);
 
             Assert.True(psesHost.DebugContext.IsStopped);
 

test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs

@@ -86,11 +86,12 @@ public async Task CanQueueParallelPSCommands()
         [Fact]
         public async Task CanCancelExecutionWithToken()
         {
+            using CancellationTokenSource cancellationSource = new(millisecondsDelay: 1000);
             await Assert.ThrowsAsync<TaskCanceledException>(() =>
             {
                 return psesHost.ExecutePSCommandAsync(
                     new PSCommand().AddScript("Start-Sleep 10"),
-                    new CancellationTokenSource(1000).Token);
+                    cancellationSource.Token);
             }).ConfigureAwait(true);
         }