Skip to content

Commit 65456bd

Browse files
andyleejordanandyleejordan
committed
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.
1 parent e4f9130 commit 65456bd

File tree

8 files changed

+68
-77
lines changed

8 files changed

+68
-77
lines changed

‎src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Microsoft Corporation.
1+
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

44
using System;
@@ -291,11 +291,6 @@ public void TriggerShutdown()
291291
if (Interlocked.Exchange(ref _shuttingDown, 1) == 0)
292292
{
293293
_cancellationContext.CancelCurrentTaskStack();
294-
// NOTE: This is mostly for sanity's sake, as during debugging of tests I became
295-
// concerned that the repeated creation and disposal of the host was not also
296-
// joining and disposing this thread, leaving the tests in a weird state. Because
297-
// the tasks have been canceled, we should be able to join this thread.
298-
_pipelineThread.Join();
299294
}
300295
}
301296

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,18 @@ internal CancellationScope(
103103

104104
public CancellationToken CancellationToken => _cancellationSource.Token;
105105

106-
public void Cancel() => _cancellationSource.Cancel();
106+
public void Cancel()
107+
{
108+
try
109+
{
110+
_cancellationSource.Cancel();
111+
}
112+
catch (ObjectDisposedException)
113+
{
114+
// We don't want this race condition to cause flaky tests.
115+
// TODO: Find out the cause of the race!
116+
}
117+
}
107118

108119
public bool IsIdleScope { get; }
109120

‎test/PowerShellEditorServices.Test.E2E/DebugAdapterClientExtensions.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using Microsoft.PowerShell.EditorServices.Handlers;
77
using OmniSharp.Extensions.DebugAdapter.Client;
88
using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
9-
using System.Threading;
109

1110
namespace PowerShellEditorServices.Test.E2E
1211
{
@@ -29,9 +28,7 @@ public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient
2928
}
3029

3130
// This will check to see if we received the Initialized event from the server.
32-
await Task.Run(
33-
async () => await started.Task.ConfigureAwait(true),
34-
new CancellationTokenSource(2000).Token).ConfigureAwait(true);
31+
await started.Task.ConfigureAwait(true);
3532
}
3633
}
3734
}

‎test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs‎

Lines changed: 42 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Microsoft Corporation.
1+
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

44
using System;
@@ -39,7 +39,7 @@ public async Task InitializeAsync()
3939
{
4040
LoggerFactory factory = new();
4141
_psesProcess = new PsesStdioProcess(factory, true);
42-
await _psesProcess.Start().ConfigureAwait(false);
42+
await _psesProcess.Start().ConfigureAwait(true);
4343

4444
TaskCompletionSource<bool> initialized = new();
4545

@@ -90,26 +90,21 @@ public async Task InitializeAsync()
9090
// that gets completed when we receive the response to Initialize
9191
// This tells us that we are ready to send messages to PSES... but are not stuck waiting for
9292
// Initialized.
93-
PsesDebugAdapterClient.Initialize(CancellationToken.None).ConfigureAwait(false);
94-
await initialized.Task.ConfigureAwait(false);
93+
#pragma warning disable CS4014
94+
PsesDebugAdapterClient.Initialize(CancellationToken.None).ConfigureAwait(true);
95+
#pragma warning restore CS4014
96+
await initialized.Task.ConfigureAwait(true);
9597
}
9698

9799
public async Task DisposeAsync()
98100
{
99-
try
101+
await PsesDebugAdapterClient.RequestDisconnect(new DisconnectArguments
100102
{
101-
await PsesDebugAdapterClient.RequestDisconnect(new DisconnectArguments
102-
{
103-
Restart = false,
104-
TerminateDebuggee = true
105-
}).ConfigureAwait(false);
106-
await _psesProcess.Stop().ConfigureAwait(false);
107-
PsesDebugAdapterClient?.Dispose();
108-
}
109-
catch (ObjectDisposedException)
110-
{
111-
// Language client has a disposal bug in it
112-
}
103+
Restart = false,
104+
TerminateDebuggee = true
105+
}).ConfigureAwait(true);
106+
await _psesProcess.Stop().ConfigureAwait(true);
107+
PsesDebugAdapterClient?.Dispose();
113108
}
114109

115110
private static string NewTestFile(string script, bool isPester = false)
@@ -147,7 +142,16 @@ private string GenerateScriptFromLoggingStatements(params string[] logStatements
147142
return builder.ToString();
148143
}
149144

150-
private static string[] GetLog() => File.ReadLines(s_testOutputPath).ToArray();
145+
private static async Task<string[]> GetLog()
146+
{
147+
while (!File.Exists(s_testOutputPath))
148+
{
149+
await Task.Delay(1000).ConfigureAwait(true);
150+
}
151+
// Sleep one more time after the file exists so whatever is writing can finish.
152+
await Task.Delay(1000).ConfigureAwait(true);
153+
return File.ReadLines(s_testOutputPath).ToArray();
154+
}
151155

152156
[Fact]
153157
public void CanInitializeWithCorrectServerSettings()
@@ -165,14 +169,12 @@ public async Task CanLaunchScriptWithNoBreakpointsAsync()
165169
{
166170
string filePath = NewTestFile(GenerateScriptFromLoggingStatements("works"));
167171

168-
await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
172+
await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
169173

170-
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
174+
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
171175
Assert.NotNull(configDoneResponse);
172-
await Task.Delay(2000).ConfigureAwait(false);
173-
174-
string[] log = GetLog();
175-
Assert.Equal("works", log[0]);
176+
Assert.Collection(await GetLog().ConfigureAwait(true),
177+
(i) => Assert.Equal("works", i));
176178
}
177179

178180
[SkippableFact]
@@ -188,37 +190,32 @@ public async Task CanSetBreakpointsAsync()
188190
"after breakpoint"
189191
));
190192

191-
await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
193+
await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
192194

193195
// {"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}
194196
SetBreakpointsResponse setBreakpointsResponse = await PsesDebugAdapterClient.SetBreakpoints(new SetBreakpointsArguments
195197
{
196198
Source = new Source { Name = Path.GetFileName(filePath), Path = filePath },
197199
Breakpoints = new SourceBreakpoint[] { new SourceBreakpoint { Line = 2 } },
198200
SourceModified = false,
199-
}).ConfigureAwait(false);
201+
}).ConfigureAwait(true);
200202

201203
Breakpoint breakpoint = setBreakpointsResponse.Breakpoints.First();
202204
Assert.True(breakpoint.Verified);
203205
Assert.Equal(filePath, breakpoint.Source.Path, ignoreCase: s_isWindows);
204206
Assert.Equal(2, breakpoint.Line);
205207

206-
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
208+
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
207209
Assert.NotNull(configDoneResponse);
208-
await Task.Delay(2000).ConfigureAwait(false);
209-
210-
string[] log = GetLog();
211-
Assert.Single(log, (i) => i == "before breakpoint");
210+
Assert.Collection(await GetLog().ConfigureAwait(true),
211+
(i) => Assert.Equal("before breakpoint", i));
212+
File.Delete(s_testOutputPath);
212213

213214
ContinueResponse continueResponse = await PsesDebugAdapterClient.RequestContinue(
214215
new ContinueArguments { ThreadId = 1 }).ConfigureAwait(true);
215216

216217
Assert.NotNull(continueResponse);
217-
await Task.Delay(2000).ConfigureAwait(false);
218-
219-
log = GetLog();
220-
Assert.Collection(log,
221-
(i) => Assert.Equal("before breakpoint", i),
218+
Assert.Collection(await GetLog().ConfigureAwait(true),
222219
(i) => Assert.Equal("at breakpoint", i),
223220
(i) => Assert.Equal("after breakpoint", i));
224221
}
@@ -247,24 +244,24 @@ public async Task CanStepPastSystemWindowsForms()
247244
"Write-Host $form"
248245
}));
249246

250-
await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(false);
247+
await PsesDebugAdapterClient.LaunchScript(filePath, Started).ConfigureAwait(true);
251248

252249
SetFunctionBreakpointsResponse setBreakpointsResponse = await PsesDebugAdapterClient.SetFunctionBreakpoints(
253250
new SetFunctionBreakpointsArguments
254251
{
255252
Breakpoints = new FunctionBreakpoint[]
256253
{ new FunctionBreakpoint { Name = "Write-Host", } }
257-
}).ConfigureAwait(false);
254+
}).ConfigureAwait(true);
258255

259256
Breakpoint breakpoint = setBreakpointsResponse.Breakpoints.First();
260257
Assert.True(breakpoint.Verified);
261258

262-
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
259+
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
263260
Assert.NotNull(configDoneResponse);
264-
await Task.Delay(2000).ConfigureAwait(false);
261+
await Task.Delay(5000).ConfigureAwait(true);
265262

266263
VariablesResponse variablesResponse = await PsesDebugAdapterClient.RequestVariables(
267-
new VariablesArguments { VariablesReference = 1 }).ConfigureAwait(false);
264+
new VariablesArguments { VariablesReference = 1 }).ConfigureAwait(true);
268265

269266
Variable form = variablesResponse.Variables.FirstOrDefault(v => v.Name == "$form");
270267
Assert.NotNull(form);
@@ -286,13 +283,12 @@ public async Task CanLaunchScriptWithCommentedLastLineAsync()
286283
// PsesLaunchRequestArguments.Script, which is then assigned to
287284
// DebugStateService.ScriptToLaunch in that handler, and finally used by the
288285
// ConfigurationDoneHandler in LaunchScriptAsync.
289-
await PsesDebugAdapterClient.LaunchScript(script, Started).ConfigureAwait(false);
286+
await PsesDebugAdapterClient.LaunchScript(script, Started).ConfigureAwait(true);
290287

291-
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
288+
ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(true);
292289
Assert.NotNull(configDoneResponse);
293-
await Task.Delay(2000).ConfigureAwait(false);
294-
295-
Assert.Collection(GetLog(), (i) => Assert.Equal("a log statement", i));
290+
Assert.Collection(await GetLog().ConfigureAwait(true),
291+
(i) => Assert.Equal("a log statement", i));
296292
}
297293
}
298294
}

‎test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,16 +95,9 @@ public async Task InitializeAsync()
9595

9696
public async Task DisposeAsync()
9797
{
98-
try
99-
{
100-
await PsesLanguageClient.Shutdown().ConfigureAwait(false);
101-
await _psesProcess.Stop().ConfigureAwait(false);
102-
PsesLanguageClient?.Dispose();
103-
}
104-
catch (ObjectDisposedException)
105-
{
106-
// Language client has a disposal bug in it
107-
}
98+
await PsesLanguageClient.Shutdown().ConfigureAwait(false);
99+
await _psesProcess.Stop().ConfigureAwait(false);
100+
PsesLanguageClient?.Dispose();
108101
}
109102
}
110103
}

‎test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
using System.Threading;
1313
using System.Threading.Tasks;
1414
using Microsoft.PowerShell.EditorServices.Handlers;
15+
using Microsoft.PowerShell.EditorServices.Logging;
16+
using Microsoft.PowerShell.EditorServices.Services.Configuration;
17+
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
18+
using Microsoft.PowerShell.EditorServices.Services.Template;
1519
using Newtonsoft.Json.Linq;
1620
using OmniSharp.Extensions.LanguageServer.Protocol;
1721
using OmniSharp.Extensions.LanguageServer.Protocol.Client;
@@ -21,10 +25,6 @@
2125
using Xunit;
2226
using Xunit.Abstractions;
2327
using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;
24-
using Microsoft.PowerShell.EditorServices.Logging;
25-
using Microsoft.PowerShell.EditorServices.Services.Configuration;
26-
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
27-
using Microsoft.PowerShell.EditorServices.Services.Template;
2828

2929
namespace PowerShellEditorServices.Test.E2E
3030
{
@@ -1149,8 +1149,6 @@ await PsesLanguageClient
11491149
[Fact]
11501150
public async Task CanSendEvaluateRequestAsync()
11511151
{
1152-
using CancellationTokenSource cancellationSource = new(millisecondsDelay: 5000);
1153-
11541152
EvaluateResponseBody evaluateResponseBody =
11551153
await PsesLanguageClient
11561154
.SendRequest(
@@ -1159,7 +1157,7 @@ await PsesLanguageClient
11591157
{
11601158
Expression = "Get-ChildItem"
11611159
})
1162-
.Returning<EvaluateResponseBody>(cancellationSource.Token).ConfigureAwait(true);
1160+
.Returning<EvaluateResponseBody>(CancellationToken.None).ConfigureAwait(true);
11631161

11641162
// These always gets returned so this test really just makes sure we get _any_ response.
11651163
Assert.Equal("", evaluateResponseBody.Result);

‎test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ private Task ExecutePowerShellCommand(string command, params string[] args)
103103

104104
private void AssertDebuggerPaused()
105105
{
106-
DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(new CancellationTokenSource(5000).Token);
106+
DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(CancellationToken.None);
107107
Assert.Empty(eventArgs.OriginalEvent.Breakpoints);
108108
}
109109

@@ -112,7 +112,7 @@ private void AssertDebuggerStopped(
112112
int lineNumber = -1,
113113
CommandBreakpointDetails commandBreakpointDetails = default)
114114
{
115-
DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(new CancellationTokenSource(5000).Token);
115+
DebuggerStoppedEventArgs eventArgs = debuggerStoppedQueue.Take(CancellationToken.None);
116116

117117
Assert.True(psesHost.DebugContext.IsStopped);
118118

‎test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,12 @@ public async Task CanQueueParallelPSCommands()
8686
[Fact]
8787
public async Task CanCancelExecutionWithToken()
8888
{
89+
using CancellationTokenSource cancellationSource = new(millisecondsDelay: 1000);
8990
await Assert.ThrowsAsync<TaskCanceledException>(() =>
9091
{
9192
return psesHost.ExecutePSCommandAsync(
9293
new PSCommand().AddScript("Start-Sleep 10"),
93-
new CancellationTokenSource(1000).Token);
94+
cancellationSource.Token);
9495
}).ConfigureAwait(true);
9596
}
9697

0 commit comments

Comments
 (0)