Add stop tests and forge registration cmdlets

docs/en-US/Register-RemoteForge.md

@@ -14,7 +14,7 @@ schema: 2.0.0
 
 ### Explicit (Default)
 ```
-Register-RemoteForge [-Id] <String> [-ForgeFactory] <System.Func`2[System.String,RemoteForge.IRemoteForge]>
+Register-RemoteForge -Name <String> [-ForgeFactory] <System.Func`2[System.String,RemoteForge.IRemoteForge]>
  [-Description <String>] [-PassThru] [-ProgressAction <ActionPreference>] [<CommonParameters>]
 ```
 
@@ -83,16 +83,16 @@ Accept pipeline input: False
 Accept wildcard characters: False
 ```
 
-### -Id
-{{ Fill Id Description }}
+### -Name
+{{ Fill Name Description }}
 
 ```yaml
 Type: String
 Parameter Sets: Explicit
-Aliases:
+Aliases: Id
 
 Required: True
-Position: 0
+Position: Named
 Default value: None
 Accept pipeline input: False
 Accept wildcard characters: False

docs/en-US/Unregister-RemoteForge.md

@@ -13,7 +13,7 @@ schema: 2.0.0
 ## SYNTAX
 
 ```
-Unregister-RemoteForge [-Id] <String[]> [-ProgressAction <ActionPreference>] [<CommonParameters>]
+Unregister-RemoteForge -Name <String[]> [-ProgressAction <ActionPreference>] [<CommonParameters>]
 ```
 
 ## DESCRIPTION
@@ -30,16 +30,16 @@ PS C:\> {{ Add example code here }}
 
 ## PARAMETERS
 
-### -Id
-{{ Fill Id Description }}
+### -Name
+{{ Fill Name Description }}
 
 ```yaml
 Type: String[]
 Parameter Sets: (All)
-Aliases:
+Aliases: Id
 
 Required: True
-Position: 0
+Position: Named
 Default value: None
 Accept pipeline input: True (ByPropertyName, ByValue)
 Accept wildcard characters: False

src/RemoteForge/Commands/InvokeRemoteCommand.cs

@@ -30,7 +30,7 @@ private class DefaultVariableValue
         private static DefaultVariableValue? _instance;
 
         private DefaultVariableValue()
-        {}
+        { }
 
         public static DefaultVariableValue Value => _instance ??= new();
     }
@@ -48,6 +48,8 @@ private enum PipelineType
     private Task? _worker;
     private MethodInfo? _errorRecordSetTargetObject;
 
+    private event EventHandler? OnStopEvent;
+
     [Parameter(
         Mandatory = true,
         Position = 0
@@ -274,6 +276,7 @@ protected override void EndProcessing()
     protected override void StopProcessing()
     {
         _cancelToken.Cancel();
+        OnStopEvent?.Invoke(null, new());
     }
 
     private void WriteResult(PipelineType pipelineType, object? data)
@@ -429,7 +432,19 @@ private async Task RunScript(
             // The InvocationInfo is also useless from the remote error record
             // so returning it won't make any difference.
             PSInvocationSettings psis = new();
-            await ps.InvokeAsync(_inputPipe, outputPipe, psis, null, null);
+
+            // We cannot use Stop in case the pipeline is running something
+            // that won't respond to the stop signal. This is a best effort.
+            EventHandler stopDelegate = (s, e) => ps.BeginStop(null, null);
+            OnStopEvent += stopDelegate;
+            try
+            {
+                await ps.InvokeAsync(_inputPipe, outputPipe, psis, null, null);
+            }
+            finally
+            {
+                OnStopEvent -= stopDelegate;
+            }
         }
         finally
         {

src/RemoteForge/Commands/RemoteForgeCommand.cs

@@ -30,7 +30,7 @@ protected override void ProcessRecord()
 
     protected override void EndProcessing()
     {
-        foreach (RemoteForgeRegistration forge in RemoteForgeRegistration.Registrations)
+        foreach (RemoteForgeRegistration forge in RemoteForgeRegistration.Registrations.ToArray())
         {
             WriteVerbose($"Checking for forge '{forge.Id}' matches requested Name");
 
@@ -74,7 +74,8 @@ public sealed class RegisterRemoteForgeCommand : PSCmdlet
         ParameterSetName = "Explicit"
     )]
     [ValidateNotNullOrEmpty]
-    public string Id { get; set; } = "";
+    [Alias("Id")]
+    public string Name { get; set; } = "";
 
     [Parameter(
         Mandatory = true,
@@ -106,7 +107,7 @@ protected override void EndProcessing()
             {
                 Debug.Assert(ForgeFactory != null);
                 RemoteForgeRegistration registration = RemoteForgeRegistration.Register(
-                    Id,
+                    Name,
                     ForgeFactory,
                     description: Description);
 
@@ -153,11 +154,12 @@ public sealed class UnregisterRemoteForgeCommand : PSCmdlet
         ValueFromPipelineByPropertyName = true
     )]
     [ValidateNotNullOrEmpty]
-    public string[] Id { get; set; } = Array.Empty<string>();
+    [Alias("Id")]
+    public string[] Name { get; set; } = Array.Empty<string>();
 
     protected override void ProcessRecord()
     {
-        foreach (string forgeId in Id)
+        foreach (string forgeId in Name)
         {
             try
             {
@@ -173,6 +175,5 @@ protected override void ProcessRecord()
                 WriteError(err);
             }
         }
-        base.ProcessRecord();
     }
 }

src/RemoteForge/RemoteForgeConnectionInfo.cs

@@ -196,7 +196,7 @@ private async Task RunTransport()
             // is done or failed.
             currentStage = TransportMethodEnum.ReceiveShellOutputEx;
             Task doneTask = await Task.WhenAny(readTask, writeTask, _closeTask.Task);
-            if (doneTask == readTask && readTask.IsCompletedSuccessfully)
+            if (doneTask == writeTask && writeTask.IsCompletedSuccessfully)
             {
                 // If we reached here PowerShell doesn't know the transport is
                 // closed so we raise the exception.
@@ -241,7 +241,7 @@ private async Task RunTransport()
                     await transport.CloseConnection(_cancelSource.Token);
                 }
                 catch (OperationCanceledException)
-                {} // Dispose() was called so this is expected.
+                { } // Dispose() was called so this is expected.
             }
 
             if (transport is IDisposable disposable)

src/RemoteForge/RemoteForgeRegistration.cs

@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
-using System.Management.Automation;
 using System.Management.Automation.Runspaces;
 using System.Reflection;
 
@@ -12,7 +11,12 @@ namespace RemoteForge;
 
 public sealed class RemoteForgeRegistration
 {
-    internal static List<RemoteForgeRegistration> Registrations { get; } = new();
+    // We use a thread local storage value so the registrations are scoped to
+    // a Runspace rather than be process wide.
+    [ThreadStatic]
+    private static List<RemoteForgeRegistration>? _registrations;
+
+    internal static List<RemoteForgeRegistration> Registrations => _registrations ??= new();
 
     public string Id { get; }
     public string? Description { get; }

tests/GetRemoteForge.tests.ps1

@@ -0,0 +1,85 @@
+. (Join-Path $PSScriptRoot common.ps1)
+
+Describe "Get-RemoteForge tests" {
+    It "Gets all registered forges" {
+        $actual = Get-RemoteForge
+
+        $actual.Count | Should -BeGreaterOrEqual 2
+
+        $sshReg = $actual | Where-Object Id -EQ ssh
+        $sshReg.Description | Should -Be 'Builtin SSH transport'
+        $sshReg.IsDefault | Should -BeTrue
+
+        $pipeTestReg = $actual | Where-Object Id -EQ PipeTest
+        $pipeTestReg.Description | Should -Be 'Test pipe transport'
+        $pipeTestReg.IsDefault | Should -BeFalse
+    }
+
+    It "Gets forge with wildcard match" {
+        $actual = Get-RemoteForge Fake*, PipeTe*
+
+        $actual.Count | Should -Be 1
+        $actual.Id | Should -Be PipeTest
+        $actual.Description | Should -Be 'Test pipe transport'
+        $actual.IsDefault | Should -BeFalse
+    }
+
+    It "Gets no results with no match" {
+        $actual = Get-RemoteForge Fake
+
+        $actual | Should -BeNullOrEmpty
+    }
+}
+
+Describe "Unregister-RemoteForge tests" {
+    It "Unregisters transport by name" {
+        $ps = [PowerShell]::Create()
+        $actual = $ps.AddScript({
+                param ($RemoteForge, $TestForge)
+
+                $ErrorActionPreference = 'Stop'
+
+                Import-Module -Name $RemoteForge
+                Import-Module -Name $TestForge
+
+                Unregister-RemoteForge -Name PipeTest
+
+                Get-RemoteForge
+            }).AddParameters(@{
+                RemoteForge = (Get-Module -Name RemoteForge).Path
+                TestForge = (Get-Module -Name TestForge).Path
+            }).Invoke()
+
+        $actual | Where-Object Id -EQ PipeTest | Should -BeNullOrEmpty
+        Get-RemoteForge -Name PipeTest | Should -Not -BeNullOrEmpty
+    }
+
+    It "Unregisters transport by piped input" {
+        $ps = [PowerShell]::Create()
+        $actual = $ps.AddScript({
+                param ($RemoteForge, $TestForge)
+
+                $ErrorActionPreference = 'Stop'
+
+                Import-Module -Name $RemoteForge
+                Import-Module -Name $TestForge
+
+                Get-RemoteForge -Name PipeTest | Unregister-RemoteForge
+
+                Get-RemoteForge
+            }).AddParameters(@{
+                RemoteForge = (Get-Module -Name RemoteForge).Path
+                TestForge = (Get-Module -Name TestForge).Path
+            }).Invoke()
+
+        $actual | Where-Object Id -EQ PipeTest | Should -BeNullOrEmpty
+        Get-RemoteForge -Name PipeTest | Should -Not -BeNullOrEmpty
+    }
+
+    It "Writes error when unregistering an invalid name" {
+        Unregister-RemoteForge -Id Fake -ErrorAction SilentlyContinue -ErrorVariable err
+
+        $err.Count | Should -Be 1
+        [string]$err[0] | Should -Be "No forge has been registered with the id 'Fake'"
+    }
+}