Merge pull request #1848 from PowerShell/andschwa/ci-stability

Improve CI stability

Author: andyleejordan

PowerShellEditorServices.build.ps1

@@ -11,12 +11,13 @@ param(
 
     [string]$DefaultModuleRepository = "PSGallery",
 
+    [string[]]$VerbosityArgs = @("--verbosity", "quiet", "--nologo"),
+
     # See: https://docs.microsoft.com/en-us/dotnet/core/testing/selective-unit-tests
     [string]$TestFilter = '',
 
     # See: https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-test
-    # E.g. use @("--logger", "console;verbosity=detailed") for detailed console output instead
-    [string[]]$TestArgs = @("--logger", "trx")
+    [string[]]$TestArgs = $VerbosityArgs + @("--logger", "console;verbosity=normal", "--logger", "trx")
 )
 
 #Requires -Modules @{ModuleName="InvokeBuild"; ModuleVersion="5.0.0"}
@@ -38,9 +39,9 @@ $script:BuildInfoPath = [System.IO.Path]::Combine($PSScriptRoot, "src", "PowerSh
 $script:PsesCommonProps = [xml](Get-Content -Raw "$PSScriptRoot/PowerShellEditorServices.Common.props")
 
 $script:NetRuntime = @{
-    PS7 = 'netcoreapp3.1'
-    PS72 = 'net6.0'
-    Desktop = 'net462'
+    PS7      = 'netcoreapp3.1'
+    PS72     = 'net6.0'
+    Desktop  = 'net462'
     Standard = 'netstandard2.0'
 }
 
@@ -54,31 +55,31 @@ if (Get-Command git -ErrorAction SilentlyContinue) {
     git update-index --assume-unchanged "$PSScriptRoot/src/PowerShellEditorServices.Hosting/BuildInfo.cs"
 }
 
-task FindDotNet {
-    assert (Get-Command dotnet -ErrorAction SilentlyContinue) "dotnet not found, please install it: https://aka.ms/dotnet-cli"
+Task FindDotNet {
+    Assert (Get-Command dotnet -ErrorAction SilentlyContinue) "dotnet not found, please install it: https://aka.ms/dotnet-cli"
 
     # Strip out semantic version metadata so it can be cast to `Version`
     $existingVersion, $null = (dotnet --version) -split '-'
-    assert ([Version]$existingVersion -ge [Version]("6.0")) ".NET SDK 6.0 or higher is required, please update it: https://aka.ms/dotnet-cli"
+    Assert ([Version]$existingVersion -ge [Version]("6.0")) ".NET SDK 6.0 or higher is required, please update it: https://aka.ms/dotnet-cli"
 
     # Anywhere other than on a Mac with an M1 processor, we additionally
     # need the .NET 3.1 runtime for our netcoreapp3.1 framework.
     if (-not $script:IsAppleM1 -and -not $script:IsArm64) {
         $runtimes = dotnet --list-runtimes
-        assert ($runtimes -match "Microsoft.NETCore.App 3.1") ".NET Runtime 3.1 required but not found!"
+        Assert ($runtimes -match "Microsoft.NETCore.App 3.1") ".NET Runtime 3.1 required but not found!"
     }
 
     Write-Host "Using dotnet v$(dotnet --version) at path $((Get-Command dotnet).Source)" -ForegroundColor Green
 }
 
-task BinClean {
+Task BinClean {
     Remove-Item $PSScriptRoot\.tmp -Recurse -Force -ErrorAction Ignore
     Remove-Item $PSScriptRoot\module\PowerShellEditorServices\bin -Recurse -Force -ErrorAction Ignore
     Remove-Item $PSScriptRoot\module\PowerShellEditorServices.VSCode\bin -Recurse -Force -ErrorAction Ignore
 }
 
-task Clean FindDotNet, BinClean, {
-    exec { & dotnet clean }
+Task Clean FindDotNet, BinClean, {
+    Exec { & dotnet clean $VerbosityArgs }
     Get-ChildItem -Recurse $PSScriptRoot\src\*.nupkg | Remove-Item -Force -ErrorAction Ignore
     Get-ChildItem $PSScriptRoot\PowerShellEditorServices*.zip | Remove-Item -Force -ErrorAction Ignore
     Get-ChildItem $PSScriptRoot\module\PowerShellEditorServices\Commands\en-US\*-help.xml | Remove-Item -Force -ErrorAction Ignore
@@ -93,7 +94,7 @@ task Clean FindDotNet, BinClean, {
     }
 }
 
-task CreateBuildInfo {
+Task CreateBuildInfo {
     $buildVersion = "<development-build>"
     $buildOrigin = "Development"
     $buildCommit = git rev-parse HEAD
@@ -147,77 +148,77 @@ namespace Microsoft.PowerShell.EditorServices.Hosting
 "@
 
     if (Compare-Object $buildInfoContents.Split([Environment]::NewLine) (Get-Content $script:BuildInfoPath)) {
-        Write-Host "Updating Build Info"
+        Write-Host "Updating build info."
         Set-Content -LiteralPath $script:BuildInfoPath -Value $buildInfoContents -Force
     }
 }
 
-task SetupHelpForTests {
+Task SetupHelpForTests {
     if (-not (Get-Help Write-Host).Examples) {
-        Write-Host "Updating help for tests"
+        Write-Host "Updating help for tests."
         Update-Help -Module Microsoft.PowerShell.Utility -Force -Scope CurrentUser
     }
 }
 
 Task Build FindDotNet, CreateBuildInfo, {
-    exec { & dotnet restore }
-    exec { & dotnet publish -c $Configuration .\src\PowerShellEditorServices\PowerShellEditorServices.csproj -f $script:NetRuntime.Standard }
-    exec { & dotnet publish -c $Configuration .\src\PowerShellEditorServices.Hosting\PowerShellEditorServices.Hosting.csproj -f $script:NetRuntime.PS7 }
+    Exec { & dotnet restore $VerbosityArgs }
+    Exec { & dotnet publish $VerbosityArgs -c $Configuration .\src\PowerShellEditorServices\PowerShellEditorServices.csproj -f $script:NetRuntime.Standard }
+    Exec { & dotnet publish $VerbosityArgs -c $Configuration .\src\PowerShellEditorServices.Hosting\PowerShellEditorServices.Hosting.csproj -f $script:NetRuntime.PS7 }
     if (-not $script:IsNix) {
-        exec { & dotnet publish -c $Configuration .\src\PowerShellEditorServices.Hosting\PowerShellEditorServices.Hosting.csproj -f $script:NetRuntime.Desktop }
+        Exec { & dotnet publish $VerbosityArgs -c $Configuration .\src\PowerShellEditorServices.Hosting\PowerShellEditorServices.Hosting.csproj -f $script:NetRuntime.Desktop }
     }
 
     # Build PowerShellEditorServices.VSCode module
-    exec { & dotnet publish -c $Configuration .\src\PowerShellEditorServices.VSCode\PowerShellEditorServices.VSCode.csproj -f $script:NetRuntime.Standard }
+    Exec { & dotnet publish $VerbosityArgs -c $Configuration .\src\PowerShellEditorServices.VSCode\PowerShellEditorServices.VSCode.csproj -f $script:NetRuntime.Standard }
 }
 
-task Test TestServer, TestE2E
+Task Test TestServer, TestE2E
 
-task TestServer TestServerWinPS, TestServerPS7, TestServerPS72
+Task TestServer TestServerWinPS, TestServerPS7, TestServerPS72
 
 Task TestServerWinPS -If (-not $script:IsNix) Build, SetupHelpForTests, {
     Set-Location .\test\PowerShellEditorServices.Test\
     # TODO: See https://github.com/dotnet/sdk/issues/18353 for x64 test host
     # that is debuggable! If architecture is added, the assembly path gets an
     # additional folder, necesstiating fixes to find the commands definition
     # file and test files.
-    exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.Desktop }
+    Exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.Desktop }
 }
 
-task TestServerPS7 -If (-not $script:IsAppleM1 -and -not $script:IsArm64) Build, SetupHelpForTests, {
+Task TestServerPS7 -If (-not $script:IsAppleM1 -and -not $script:IsArm64) Build, SetupHelpForTests, {
     Set-Location .\test\PowerShellEditorServices.Test\
-    exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS7 }
+    Exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS7 }
 }
 
-task TestServerPS72 Build, SetupHelpForTests, {
+Task TestServerPS72 Build, SetupHelpForTests, {
     Set-Location .\test\PowerShellEditorServices.Test\
-    exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS72 }
+    Exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS72 }
 }
 
-task TestE2E Build, SetupHelpForTests, {
+Task TestE2E Build, SetupHelpForTests, {
     Set-Location .\test\PowerShellEditorServices.Test.E2E\
 
     $env:PWSH_EXE_NAME = if ($IsCoreCLR) { "pwsh" } else { "powershell" }
     $NetRuntime = if ($IsAppleM1 -or $script:IsArm64) { $script:NetRuntime.PS72 } else { $script:NetRuntime.PS7 }
-    exec { & dotnet $script:dotnetTestArgs $NetRuntime }
+    Exec { & dotnet $script:dotnetTestArgs $NetRuntime }
 
-    # Run E2E tests in ConstrainedLanguage mode.
     if (!$script:IsNix) {
         if (-not [Security.Principal.WindowsIdentity]::GetCurrent().Owner.IsWellKnown("BuiltInAdministratorsSid")) {
-            Write-Warning 'Skipping E2E CLM tests as they must be ran in an elevated process.'
+            Write-Warning "Skipping Constrained Language Mode tests as they must be ran in an elevated process."
             return
         }
 
         try {
+            Write-Host "Running end-to-end tests in Constrained Language Mode."
             [System.Environment]::SetEnvironmentVariable("__PSLockdownPolicy", "0x80000007", [System.EnvironmentVariableTarget]::Machine);
-            exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS7 }
+            Exec { & dotnet $script:dotnetTestArgs $script:NetRuntime.PS7 }
         } finally {
             [System.Environment]::SetEnvironmentVariable("__PSLockdownPolicy", $null, [System.EnvironmentVariableTarget]::Machine);
         }
     }
 }
 
-task LayoutModule -After Build {
+Task LayoutModule -After Build {
     $modulesDir = "$PSScriptRoot/module"
     $psesVSCodeBinOutputPath = "$modulesDir/PowerShellEditorServices.VSCode/bin"
     $psesOutputPath = "$modulesDir/PowerShellEditorServices"
@@ -226,8 +227,8 @@ task LayoutModule -After Build {
     $psesCoreHostPath = "$psesBinOutputPath/Core"
     $psesDeskHostPath = "$psesBinOutputPath/Desktop"
 
-    foreach ($dir in $psesDepsPath,$psesCoreHostPath,$psesDeskHostPath,$psesVSCodeBinOutputPath) {
-        New-Item -Force -Path $dir -ItemType Directory
+    foreach ($dir in $psesDepsPath, $psesCoreHostPath, $psesDeskHostPath, $psesVSCodeBinOutputPath) {
+        New-Item -Force -Path $dir -ItemType Directory | Out-Null
     }
 
     # Copy Third Party Notices.txt to module folder
@@ -310,7 +311,7 @@ task RestorePsesModules -After Build {
     # Save each module in the modules.json file
     foreach ($moduleName in $moduleInfos.Keys) {
         if (Test-Path -Path (Join-Path -Path $submodulePath -ChildPath $moduleName)) {
-            Write-Host "`tModule '${moduleName}' already detected. Skipping"
+            Write-Host "`tModule '${moduleName}' already detected, skipping!"
             continue
         }
 
@@ -331,9 +332,9 @@ task RestorePsesModules -After Build {
 }
 
 Task BuildCmdletHelp -After LayoutModule {
-    New-ExternalHelp -Path $PSScriptRoot\module\docs -OutputPath $PSScriptRoot\module\PowerShellEditorServices\Commands\en-US -Force
-    New-ExternalHelp -Path $PSScriptRoot\module\PowerShellEditorServices.VSCode\docs -OutputPath $PSScriptRoot\module\PowerShellEditorServices.VSCode\en-US -Force
+    New-ExternalHelp -Path $PSScriptRoot\module\docs -OutputPath $PSScriptRoot\module\PowerShellEditorServices\Commands\en-US -Force | Out-Null
+    New-ExternalHelp -Path $PSScriptRoot\module\PowerShellEditorServices.VSCode\docs -OutputPath $PSScriptRoot\module\PowerShellEditorServices.VSCode\en-US -Force | Out-Null
 }
 
 # The default task is to run the entire CI build
-task . Clean, Build, Test
+Task . Clean, Build, Test

src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs

@@ -48,8 +48,9 @@ public static IServiceCollection AddPsesLanguageServices(
                         // is ready, it will be available. NOTE: We cannot await this because it
                         // uses a lazy initialization to avoid a race with the dependency injection
                         // framework, see the EditorObject class for that!
+#pragma warning disable VSTHRD110
                         extensionService.InitializeAsync();
-
+#pragma warning restore VSTHRD110
                         return extensionService;
                     })
                 .AddSingleton<AnalysisService>();

src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs

@@ -101,7 +101,7 @@ private void OnRunspaceChanged(object sender, RunspaceChangedEventArgs e)
                         // Sends the InitializedEvent so that the debugger will continue
                         // sending configuration requests
                         _debugStateService.WaitingForAttach = false;
-                        _debugStateService.ServerStarted.SetResult(true);
+                        _debugStateService.ServerStarted.TrySetResult(true);
                     }
                     return;
 

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs

@@ -210,7 +210,7 @@ public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, Can
 
             // Sends the InitializedEvent so that the debugger will continue
             // sending configuration requests
-            _debugStateService.ServerStarted.SetResult(true);
+            _debugStateService.ServerStarted.TrySetResult(true);
 
             return new LaunchResponse();
         }
@@ -440,7 +440,7 @@ await _executionService.ExecutePSCommandAsync(
 
             if (runspaceVersion.Version.Major >= 7)
             {
-                _debugStateService.ServerStarted.SetResult(true);
+                _debugStateService.ServerStarted.TrySetResult(true);
             }
             return new AttachResponse();
         }
@@ -453,7 +453,7 @@ await _executionService.ExecutePSCommandAsync(
         // PSES sends the initialized event at the end of the Launch/Attach handler
 
         // The way that the Omnisharp server works is that this OnStarted handler runs after OnInitialized
-        // (after the Initialize DAP response is sent to the client) but before the _Initalized_ DAP event
+        // (after the Initialize DAP response is sent to the client) but before the _Initialized_ DAP event
         // gets sent to the client. Because of the way PSES handles breakpoints,
         // we can't send the Initialized event until _after_ we finish the Launch/Attach handler.
         // The flow above depicts this. To achieve this, we wait until _debugStateService.ServerStarted

src/PowerShellEditorServices/Services/PowerShell/Context/PowerShellContextFrame.cs

@@ -79,7 +79,7 @@ protected virtual void Dispose(bool disposing)
         public void Dispose()
         {
             // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
-            Dispose(disposing: true);
+            Dispose(true);
             GC.SuppressFinalize(this);
         }
 

src/PowerShellEditorServices/Services/PowerShell/Context/PowerShellVersionDetails.cs

@@ -1,11 +1,11 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-using Microsoft.Extensions.Logging;
-using Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility;
 using System;
 using System.Collections;
 using System.Linq;
+using Microsoft.Extensions.Logging;
+using Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility;
 
 namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Context
 {
@@ -91,7 +91,7 @@ public PowerShellVersionDetails(
         /// Gets the PowerShell version details for the given runspace.
         /// </summary>
         /// <param name="logger">An ILogger implementation used for writing log messages.</param>
-        /// <param name="pwsh">The PowerShell instance for which to to get the version.</param>
+        /// <param name="pwsh">The PowerShell instance for which to get the version.</param>
         /// <returns>A new PowerShellVersionDetails instance.</returns>
         public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerShell pwsh)
         {

src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs

@@ -187,7 +187,7 @@ public void ProcessDebuggerResult(DebuggerCommandResults debuggerResult)
                 RaiseDebuggerResumingEvent(new DebuggerResumingEventArgs(debuggerResult.ResumeAction.Value));
 
                 // The Terminate exception is used by the engine for flow control
-                // when it needs to unwind the callstack out of the debugger.
+                // when it needs to unwind the call stack out of the debugger.
                 if (debuggerResult.ResumeAction is DebuggerResumeAction.Stop)
                 {
                     throw new TerminateException();

src/PowerShellEditorServices/Services/PowerShell/Handlers/IGetPSHostProcessesHandler.cs

@@ -7,9 +7,9 @@
 namespace Microsoft.PowerShell.EditorServices.Handlers
 {
     [Serial, Method("powerShell/getPSHostProcesses")]
-    internal interface IGetPSHostProcessesHandler : IJsonRpcRequestHandler<GetPSHostProcesssesParams, PSHostProcessResponse[]> { }
+    internal interface IGetPSHostProcessesHandler : IJsonRpcRequestHandler<GetPSHostProcessesParams, PSHostProcessResponse[]> { }
 
-    internal class GetPSHostProcesssesParams : IRequest<PSHostProcessResponse[]> { }
+    internal class GetPSHostProcessesParams : IRequest<PSHostProcessResponse[]> { }
 
     internal class PSHostProcessResponse
     {

src/PowerShellEditorServices/Services/PowerShell/Handlers/PSHostProcessAndRunspaceHandlers.cs

@@ -9,8 +9,8 @@
 
 namespace Microsoft.PowerShell.EditorServices.Handlers
 {
-    using Microsoft.PowerShell.EditorServices.Services.PowerShell;
     using System.Management.Automation;
+    using Microsoft.PowerShell.EditorServices.Services.PowerShell;
 
     internal class PSHostProcessAndRunspaceHandlers : IGetPSHostProcessesHandler, IGetRunspaceHandler
     {
@@ -23,7 +23,7 @@ public PSHostProcessAndRunspaceHandlers(ILoggerFactory factory, IInternalPowerSh
             _executionService = executionService;
         }
 
-        public Task<PSHostProcessResponse[]> Handle(GetPSHostProcesssesParams request, CancellationToken cancellationToken)
+        public Task<PSHostProcessResponse[]> Handle(GetPSHostProcessesParams request, CancellationToken cancellationToken)
         {
             List<PSHostProcessResponse> psHostProcesses = new();
 

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

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation.
+// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
 using System;
@@ -291,11 +291,6 @@ public void TriggerShutdown()
             if (Interlocked.Exchange(ref _shuttingDown, 1) == 0)
             {
                 _cancellationContext.CancelCurrentTaskStack();
-                // NOTE: This is mostly for sanity's sake, as during debugging of tests I became
-                // concerned that the repeated creation and disposal of the host was not also
-                // joining and disposing this thread, leaving the tests in a weird state. Because
-                // the tasks have been canceled, we should be able to join this thread.
-                _pipelineThread.Join();
             }
         }
 

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

@@ -6,7 +6,6 @@
 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 +28,7 @@ public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient
             }
 
             // This will check to see if we received the Initialized event from the server.
-            await Task.Run(
-                async () => await started.Task.ConfigureAwait(true),
-                new CancellationTokenSource(2000).Token).ConfigureAwait(true);
+            await started.Task.ConfigureAwait(true);
         }
     }
 }