From 17b9c45afe8bc978c6eec0d4d3e8f646419d4c70 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 6 Jan 2022 16:37:59 -0800 Subject: [PATCH 1/2] Re-enable `ExtensionCommandTests.cs` These tests actually did not require the language server nor the service provider, we just had to manually initialize the extension service itself. --- .../Extensions/ExtensionCommandTests.cs | 153 ++++++++---------- .../Session/PsesInternalHostTests.cs | 4 +- 2 files changed, 73 insertions(+), 84 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Extensions/ExtensionCommandTests.cs b/test/PowerShellEditorServices.Test/Extensions/ExtensionCommandTests.cs index 6b7d35786..cf972bde7 100644 --- a/test/PowerShellEditorServices.Test/Extensions/ExtensionCommandTests.cs +++ b/test/PowerShellEditorServices.Test/Extensions/ExtensionCommandTests.cs @@ -1,41 +1,48 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.PowerShell.EditorServices.Extensions; -using Microsoft.PowerShell.EditorServices.Extensions.Services; -using Microsoft.PowerShell.EditorServices.Services; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using Microsoft.PowerShell.EditorServices.Test.Shared; using System; using System.Collections.Generic; using System.Linq; using System.Management.Automation; +using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.PowerShell.EditorServices.Extensions; +using Microsoft.PowerShell.EditorServices.Extensions.Services; +using Microsoft.PowerShell.EditorServices.Services.Extension; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using Microsoft.PowerShell.EditorServices.Test.Shared; using Xunit; namespace Microsoft.PowerShell.EditorServices.Test.Extensions { - // TODO: - // These tests require being able to instantiate a language server and use the service provider. - // Re-enable them when we have mocked out more infrastructure for testing. - - /* + [Trait("Category", "Extensions")] public class ExtensionCommandTests : IDisposable { - private readonly PowerShellContextService _powershellContextService; - - private readonly IExtensionCommandService _extensionCommandService; + private readonly PsesInternalHost psesHost; - private readonly ExtensionService _extensionService; + private readonly ExtensionCommandService extensionCommandService; public ExtensionCommandTests() { - _powershellContextService = PowerShellContextFactory.Create(NullLogger.Instance); - _extensionCommandService = EditorObject.Instance.GetExtensionServiceProvider().ExtensionCommands; + psesHost = PsesHostFactory.Create(NullLoggerFactory.Instance); + ExtensionService extensionService = new( + languageServer: null, + serviceProvider: null, + editorOperations: null, + executionService: psesHost); + extensionService.InitializeAsync().Wait(); + extensionCommandService = new(extensionService); + } + + public void Dispose() + { + psesHost.StopAsync().Wait(); + GC.SuppressFinalize(this); } - [Trait("Category", "Extensions")] [Fact] public async Task CanRegisterAndInvokeCommandWithCmdletName() { @@ -48,34 +55,30 @@ public async Task CanRegisterAndInvokeCommandWithCmdletName() BufferRange.None); EditorCommand commandAdded = null; - _extensionCommandService.CommandAdded += (object sender, EditorCommand command) => - { - commandAdded = command; - }; - - string commandName = "test.function"; - string commandDisplayName = "Function extension"; + extensionCommandService.CommandAdded += (object _, EditorCommand command) => commandAdded = command; + const string commandName = "test.function"; + const string commandDisplayName = "Function extension"; - await _powershellContextService.ExecuteScriptStringAsync( - TestUtilities.NormalizeNewlines($@" -function Invoke-Extension {{ $global:testValue = 5 }} -Register-EditorCommand -Name {commandName} -DisplayName ""{commandDisplayName}"" -Function Invoke-Extension")); + await psesHost.ExecutePSCommandAsync( + new PSCommand().AddScript( + "function Invoke-Extension { $global:extensionValue = 5 }; " + + $"Register-EditorCommand -Name {commandName} -DisplayName \"{commandDisplayName}\" -Function Invoke-Extension"), + CancellationToken.None).ConfigureAwait(true); Assert.NotNull(commandAdded); Assert.Equal(commandAdded.Name, commandName); Assert.Equal(commandAdded.DisplayName, commandDisplayName); // Invoke the command - await _extensionCommandService.InvokeCommandAsync(commandName, editorContext); + await extensionCommandService.InvokeCommandAsync(commandName, editorContext).ConfigureAwait(true); // Assert the expected value PSCommand psCommand = new PSCommand().AddScript("$global:extensionValue"); - IEnumerable results = await _powershellContextService.ExecuteCommandAsync(psCommand); + IEnumerable results = await psesHost.ExecutePSCommandAsync(psCommand, CancellationToken.None).ConfigureAwait(true); Assert.Equal(5, results.FirstOrDefault()); } - [Trait("Category", "Extensions")] [Fact] public async Task CanRegisterAndInvokeCommandWithScriptBlock() { @@ -87,54 +90,49 @@ public async Task CanRegisterAndInvokeCommandWithScriptBlock() new BufferPosition(line: 1, column: 1), BufferRange.None); - EditorCommand commandAdded = null; - _extensionCommandService.CommandAdded += (object sender, EditorCommand command) => - { - commandAdded = command; - }; + extensionCommandService.CommandAdded += (object _, EditorCommand command) => commandAdded = command; + const string commandName = "test.scriptblock"; + const string commandDisplayName = "ScriptBlock extension"; - string commandName = "test.scriptblock"; - string commandDisplayName = "ScriptBlock extension"; - - await _powershellContextService.ExecuteCommandAsync(new PSCommand() - .AddCommand("Register-EditorCommand") - .AddParameter("Name", commandName) - .AddParameter("DisplayName", commandDisplayName) - .AddParameter("ScriptBlock", ScriptBlock.Create("$global:extensionValue = 10"))); + await psesHost.ExecutePSCommandAsync( + new PSCommand() + .AddCommand("Register-EditorCommand") + .AddParameter("Name", commandName) + .AddParameter("DisplayName", commandDisplayName) + .AddParameter("ScriptBlock", ScriptBlock.Create("$global:extensionValue = 10")), + CancellationToken.None).ConfigureAwait(true); Assert.NotNull(commandAdded); Assert.Equal(commandName, commandAdded.Name); Assert.Equal(commandDisplayName, commandAdded.DisplayName); // Invoke the command - await _extensionCommandService.InvokeCommandAsync("test.scriptblock", editorContext); + await extensionCommandService.InvokeCommandAsync("test.scriptblock", editorContext).ConfigureAwait(true); // Assert the expected value PSCommand psCommand = new PSCommand().AddScript("$global:extensionValue"); - IEnumerable results = await _powershellContextService.ExecuteCommandAsync(psCommand); + IEnumerable results = await psesHost.ExecutePSCommandAsync(psCommand, CancellationToken.None).ConfigureAwait(true); Assert.Equal(10, results.FirstOrDefault()); } - [Trait("Category", "Extensions")] [Fact] public async Task CanUpdateRegisteredCommand() { EditorCommand updatedCommand = null; - _extensionCommandService.CommandUpdated += (object sender, EditorCommand command) => - { - updatedCommand = command; - }; + extensionCommandService.CommandUpdated += (object _, EditorCommand command) => updatedCommand = command; - string commandName = "test.function"; - string commandDisplayName = "Updated function extension"; + const string commandName = "test.function"; + const string commandDisplayName = "Updated function extension"; // Register a command and then update it - await _powershellContextService.ExecuteScriptStringAsync(TestUtilities.NormalizeNewlines( - "function Invoke-Extension { Write-Output \"Extension output!\" }\n" + - $"Register-EditorCommand -Name \"{commandName}\" -DisplayName \"Old function extension\" -Function \"Invoke-Extension\"\n" + - $"Register-EditorCommand -Name \"{commandName}\" -DisplayName \"{commandDisplayName}\" -Function \"Invoke-Extension\"")); + await psesHost.ExecutePSCommandAsync( + new PSCommand().AddScript( + "function Invoke-Extension { Write-Output \"Extension output!\" }; " + + $"Register-EditorCommand -Name {commandName} -DisplayName \"Old function extension\" -Function Invoke-Extension; " + + $"Register-EditorCommand -Name {commandName} -DisplayName \"{commandDisplayName}\" -Function Invoke-Extension"), + CancellationToken.None).ConfigureAwait(true); // Wait for the add and update events Assert.NotNull(updatedCommand); @@ -142,7 +140,6 @@ await _powershellContextService.ExecuteScriptStringAsync(TestUtilities.Normalize Assert.Equal(commandDisplayName, updatedCommand.DisplayName); } - [Trait("Category", "Extensions")] [Fact] public async Task CanUnregisterCommand() { @@ -154,41 +151,33 @@ public async Task CanUnregisterCommand() new BufferPosition(line: 1, column: 1), BufferRange.None); - string commandName = "test.scriptblock"; - string commandDisplayName = "ScriptBlock extension"; + const string commandName = "test.scriptblock"; + const string commandDisplayName = "ScriptBlock extension"; EditorCommand removedCommand = null; - _extensionCommandService.CommandRemoved += (object sender, EditorCommand command) => - { - removedCommand = command; - }; + extensionCommandService.CommandRemoved += (object _, EditorCommand command) => removedCommand = command; // Add the command and wait for the add event - await _powershellContextService.ExecuteCommandAsync(new PSCommand() - .AddCommand("Register-EditorCommand") - .AddParameter("Name", commandName) - .AddParameter("DisplayName", commandDisplayName) - .AddParameter("ScriptBlock", ScriptBlock.Create("Write-Output \"Extension output!\""))); + await psesHost.ExecutePSCommandAsync( + new PSCommand() + .AddCommand("Register-EditorCommand") + .AddParameter("Name", commandName) + .AddParameter("DisplayName", commandDisplayName) + .AddParameter("ScriptBlock", ScriptBlock.Create("Write-Output \"Extension output!\"")), + CancellationToken.None).ConfigureAwait(true); // Remove the command and wait for the remove event - await _powershellContextService.ExecuteCommandAsync(new PSCommand() - .AddCommand("Unregister-EditorCommand") - .AddParameter("Name", commandName)); + await psesHost.ExecutePSCommandAsync( + new PSCommand().AddCommand("Unregister-EditorCommand").AddParameter("Name", commandName), + CancellationToken.None).ConfigureAwait(true); Assert.NotNull(removedCommand); Assert.Equal(commandName, removedCommand.Name); Assert.Equal(commandDisplayName, removedCommand.DisplayName); // Ensure that the command has been unregistered - await Assert.ThrowsAsync(() => - _extensionCommandService.InvokeCommandAsync("test.scriptblock", editorContext)); - } - - public void Dispose() - { - _powershellContextService.Dispose(); + await Assert.ThrowsAsync( + () => extensionCommandService.InvokeCommandAsync("test.scriptblock", editorContext)).ConfigureAwait(true); } } - */ } - diff --git a/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs b/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs index 87ef20bf1..dffc444c4 100644 --- a/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs +++ b/test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs @@ -85,7 +85,7 @@ public async Task CanQueueParallelPSCommands() [Fact] public async Task CanCancelExecutionWithToken() { - _ = await Assert.ThrowsAsync(() => + await Assert.ThrowsAsync(() => { return psesHost.ExecutePSCommandAsync( new PSCommand().AddScript("Start-Sleep 10"), @@ -103,7 +103,7 @@ public async Task CanCancelExecutionWithMethod() // Wait until our task has started. Thread.Sleep(2000); psesHost.CancelCurrentTask(); - _ = await Assert.ThrowsAsync(() => executeTask).ConfigureAwait(true); + await Assert.ThrowsAsync(() => executeTask).ConfigureAwait(true); Assert.True(executeTask.IsCanceled); } From 3bf231e29bd73c8f59744ca727a974949a0c1625 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 7 Jan 2022 11:09:03 -0800 Subject: [PATCH 2/2] Clean up `ExtensionService.cs` --- .../Services/Extension/ExtensionService.cs | 114 ++++++++---------- 1 file changed, 52 insertions(+), 62 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs index bf164206b..cb4562c43 100644 --- a/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs +++ b/src/PowerShellEditorServices/Services/Extension/ExtensionService.cs @@ -24,12 +24,11 @@ internal sealed class ExtensionService #region Fields - private readonly Dictionary editorCommands = - new Dictionary(); + private readonly Dictionary editorCommands = new(); private readonly ILanguageServerFacade _languageServer; - private IdempotentLatch _initializedLatch = new(); + private readonly IdempotentLatch _initializedLatch = new(); #endregion @@ -39,18 +38,18 @@ internal sealed class ExtensionService /// Gets the IEditorOperations implementation used to invoke operations /// in the host editor. /// - public IEditorOperations EditorOperations { get; private set; } + public IEditorOperations EditorOperations { get; } /// /// Gets the EditorObject which exists in the PowerShell session as the /// '$psEditor' variable. /// - public EditorObject EditorObject { get; private set; } + public EditorObject EditorObject { get; } /// /// Gets the PowerShellContext in which extension code will be executed. /// - internal IInternalPowerShellExecutionService ExecutionService { get; private set; } + internal IInternalPowerShellExecutionService ExecutionService { get; } #endregion @@ -62,7 +61,7 @@ internal sealed class ExtensionService /// /// The PSES language server instance. /// Services for dependency injection into the editor object. - /// Options object to configure the editor. + /// The interface for operating an editor. /// PowerShell execution service to run PowerShell execution requests. internal ExtensionService( ILanguageServerFacade languageServer, @@ -73,16 +72,15 @@ internal ExtensionService( ExecutionService = executionService; _languageServer = languageServer; - EditorObject = - new EditorObject( - serviceProvider, - this, - editorOperations); + EditorObject = new EditorObject( + serviceProvider, + this, + editorOperations); // Attach to ExtensionService events - CommandAdded += ExtensionService_ExtensionAddedAsync; - CommandUpdated += ExtensionService_ExtensionUpdatedAsync; - CommandRemoved += ExtensionService_ExtensionRemovedAsync; + CommandAdded += ExtensionService_ExtensionAdded; + CommandUpdated += ExtensionService_ExtensionUpdated; + CommandRemoved += ExtensionService_ExtensionRemoved; } #endregion @@ -93,7 +91,6 @@ internal ExtensionService( /// Initializes this ExtensionService using the provided IEditorOperations /// implementation for future interaction with the host editor. /// - /// An IEditorOperations implementation. /// A Task that can be awaited for completion. internal Task InitializeAsync() { @@ -121,28 +118,29 @@ internal Task InitializeAsync() /// The unique name of the command to be invoked. /// The context in which the command is being invoked. /// A Task that can be awaited for completion. + /// The command being invoked was not registered. public async Task InvokeCommandAsync(string commandName, EditorContext editorContext) { - - if (this.editorCommands.TryGetValue(commandName, out EditorCommand editorCommand)) + if (editorCommands.TryGetValue(commandName, out EditorCommand editorCommand)) { - PSCommand executeCommand = new PSCommand(); - executeCommand.AddCommand("Invoke-Command"); - executeCommand.AddParameter("ScriptBlock", editorCommand.ScriptBlock); - executeCommand.AddParameter("ArgumentList", new object[] { editorContext }); + PSCommand executeCommand = new PSCommand() + .AddCommand("Invoke-Command") + .AddParameter("ScriptBlock", editorCommand.ScriptBlock) + .AddParameter("ArgumentList", new object[] { editorContext }); await ExecutionService.ExecutePSCommandAsync( executeCommand, CancellationToken.None, - new PowerShellExecutionOptions { WriteOutputToHost = !editorCommand.SuppressOutput, ThrowOnError = false, AddToHistory = !editorCommand.SuppressOutput }) - .ConfigureAwait(false); + new PowerShellExecutionOptions + { + WriteOutputToHost = !editorCommand.SuppressOutput, + ThrowOnError = false, + AddToHistory = !editorCommand.SuppressOutput + }).ConfigureAwait(false); } else { - throw new KeyNotFoundException( - string.Format( - "Editor command not found: '{0}'", - commandName)); + throw new KeyNotFoundException($"Editor command not found: '{commandName}'"); } } @@ -156,20 +154,18 @@ public bool RegisterCommand(EditorCommand editorCommand) { Validate.IsNotNull(nameof(editorCommand), editorCommand); - bool commandExists = - this.editorCommands.ContainsKey( - editorCommand.Name); + bool commandExists = editorCommands.ContainsKey(editorCommand.Name); // Add or replace the editor command - this.editorCommands[editorCommand.Name] = editorCommand; + editorCommands[editorCommand.Name] = editorCommand; if (!commandExists) { - this.OnCommandAdded(editorCommand); + OnCommandAdded(editorCommand); } else { - this.OnCommandUpdated(editorCommand); + OnCommandUpdated(editorCommand); } return !commandExists; @@ -179,19 +175,17 @@ public bool RegisterCommand(EditorCommand editorCommand) /// Unregisters an existing EditorCommand based on its registered name. /// /// The name of the command to be unregistered. + /// The command being unregistered was not registered. public void UnregisterCommand(string commandName) { - if (this.editorCommands.TryGetValue(commandName, out EditorCommand existingCommand)) + if (editorCommands.TryGetValue(commandName, out EditorCommand existingCommand)) { - this.editorCommands.Remove(commandName); - this.OnCommandRemoved(existingCommand); + editorCommands.Remove(commandName); + OnCommandRemoved(existingCommand); } else { - throw new KeyNotFoundException( - string.Format( - "Command '{0}' is not registered", - commandName)); + throw new KeyNotFoundException($"Command '{commandName}' is not registered"); } } @@ -201,8 +195,8 @@ public void UnregisterCommand(string commandName) /// An Array of all registered EditorCommands. public EditorCommand[] GetCommands() { - EditorCommand[] commands = new EditorCommand[this.editorCommands.Count]; - this.editorCommands.Values.CopyTo(commands,0); + EditorCommand[] commands = new EditorCommand[editorCommands.Count]; + editorCommands.Values.CopyTo(commands, 0); return commands; } @@ -217,7 +211,7 @@ public EditorCommand[] GetCommands() private void OnCommandAdded(EditorCommand command) { - this.CommandAdded?.Invoke(this, command); + CommandAdded?.Invoke(this, command); } /// @@ -227,7 +221,7 @@ private void OnCommandAdded(EditorCommand command) private void OnCommandUpdated(EditorCommand command) { - this.CommandUpdated?.Invoke(this, command); + CommandUpdated?.Invoke(this, command); } /// @@ -237,35 +231,31 @@ private void OnCommandUpdated(EditorCommand command) private void OnCommandRemoved(EditorCommand command) { - this.CommandRemoved?.Invoke(this, command); + CommandRemoved?.Invoke(this, command); } - private void ExtensionService_ExtensionAddedAsync(object sender, EditorCommand e) + private void ExtensionService_ExtensionAdded(object sender, EditorCommand e) { - _languageServer?.SendNotification("powerShell/extensionCommandAdded", + _languageServer?.SendNotification( + "powerShell/extensionCommandAdded", new ExtensionCommandAddedNotification - { - Name = e.Name, - DisplayName = e.DisplayName - }); + { Name = e.Name, DisplayName = e.DisplayName }); } - private void ExtensionService_ExtensionUpdatedAsync(object sender, EditorCommand e) + private void ExtensionService_ExtensionUpdated(object sender, EditorCommand e) { - _languageServer?.SendNotification("powerShell/extensionCommandUpdated", + _languageServer?.SendNotification( + "powerShell/extensionCommandUpdated", new ExtensionCommandUpdatedNotification - { - Name = e.Name, - }); + { Name = e.Name, }); } - private void ExtensionService_ExtensionRemovedAsync(object sender, EditorCommand e) + private void ExtensionService_ExtensionRemoved(object sender, EditorCommand e) { - _languageServer?.SendNotification("powerShell/extensionCommandRemoved", + _languageServer?.SendNotification( + "powerShell/extensionCommandRemoved", new ExtensionCommandRemovedNotification - { - Name = e.Name, - }); + { Name = e.Name, }); } #endregion