Compare-DbaLogin - Fix stale destination reuse on connection failure (review of #10319)

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Author: potatoqualitee

docs/trackers/features/commit-bug-review-TRACKER.md

@@ -138,7 +138,7 @@ Find real bugs (logic errors, null refs, incorrect behavior) and fix them. Skip
 | 9899bd274 | Copy-DbaDbTableData - Add -ScriptingOptionsObject parameter (#10317) | DONE | Propagated ScriptingOptionsObject to Copy-DbaDbViewData and added unit coverage. |
 | 0c486b964 | Backup-DbaDbCertificate: Don't use decryption password if cert encrypted by master key (#10329) | DONE | Reviewed SMO export overload selection for master-key-encrypted certs; no bugs found. |
 | db77a3476 | Find-DbaObject - Add unified command to search database objects by name (#10321) | DONE | Added database DDL trigger name search via sys.triggers and covered it with an integration regression test. |
-| 6416b4e91 | Add Compare-DbaLogin command (#10319) | PENDING | |
+| 6416b4e91 | Add Compare-DbaLogin command (#10319) | DONE | Skipped failed destination connections instead of reusing the previous server; added unit regression coverage. |
 | bee08f8e7 | Copy-DbaSsisCatalog - Add standard MigrationObject output, integrate with Start-DbaMigration (#10311) | PENDING | |
 | 21e4795ee | Get-DbaService - Add PowerBI Report Server detection (#10298) | PENDING | |
 | 828ebc3b3 | Get-DbaBackupInformation - Fix inconsistencies with Get-DbaDbBackupHistory (#10308) | PENDING | |

public/Compare-DbaLogin.ps1

@@ -86,7 +86,7 @@ function Compare-DbaLogin {
         Compares user-created logins between sql1 and sql2, excluding built-in system logins.
 
     .EXAMPLE
-        PS C:\> Compare-DbaLogin -Source sql1 -Destination sql2, sql3 -Login 'appuser', 'reportuser'
+        PS C:\> Compare-DbaLogin -Source sql1 -Destination sql2, sql3 -Login "appuser", "reportuser"
 
         Compares the specified logins between sql1 and both sql2 and sql3.
     #>
@@ -129,6 +129,7 @@ function Compare-DbaLogin {
         if (Test-FunctionInterrupt) { return }
 
         foreach ($destInstance in $Destination) {
+            $destServer = $null
             try {
                 $destServer = Connect-DbaInstance -SqlInstance $destInstance -SqlCredential $DestinationSqlCredential
             } catch {

tests/Compare-DbaLogin.Tests.ps1

@@ -1,6 +1,6 @@
 #Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0" }
 param(
-    $ModuleName  = "dbatools",
+    $ModuleName = "dbatools",
     $CommandName = "Compare-DbaLogin",
     $PSDefaultParameterValues = $TestConfig.Defaults
 )
@@ -25,6 +25,71 @@ Describe $CommandName -Tag UnitTests {
     }
 }
 
+Describe $CommandName -Tag UnitTests {
+    InModuleScope dbatools {
+        Context "Destination connection failures" {
+            BeforeEach {
+                function New-MockCompareDbaLoginInstance {
+                    param(
+                        [string]$Name
+                    )
+
+                    $instance = [Dataplat.Dbatools.Parameter.DbaInstanceParameter]$Name
+                    $instance | Add-Member -NotePropertyName Name -NotePropertyValue $Name -Force
+                    $instance
+                }
+
+                Mock Test-FunctionInterrupt { $false }
+                Mock Stop-Function { }
+                Mock Connect-DbaInstance {
+                    switch ("$SqlInstance") {
+                        "source1" {
+                            New-MockCompareDbaLoginInstance -Name "source1"
+                        }
+                        "dest1" {
+                            New-MockCompareDbaLoginInstance -Name "dest1"
+                        }
+                        "dest2" {
+                            throw "dest2 unavailable"
+                        }
+                    }
+                }
+                Mock Get-DbaLogin {
+                    switch ($SqlInstance.Name) {
+                        "source1" {
+                            [PSCustomObject]@{
+                                Name      = "login1"
+                                LoginType = "SqlLogin"
+                            }
+                        }
+                        "dest1" {
+                            [PSCustomObject]@{
+                                Name      = "login1"
+                                LoginType = "SqlLogin"
+                            }
+                        }
+                        default {
+                            throw "Unexpected Get-DbaLogin call for $($SqlInstance.Name)"
+                        }
+                    }
+                }
+            }
+
+            It "skips failed destinations without reusing the previous connection" {
+                $result = Compare-DbaLogin -Source "source1" -Destination "dest1", "dest2"
+
+                $result.Count | Should -Be 1
+                $result.SourceServer | Should -Be "source1"
+                $result.DestinationServer | Should -Be "dest1"
+                Should -Invoke Get-DbaLogin -Times 2 -Exactly
+                Should -Invoke Stop-Function -Times 1 -Exactly -ParameterFilter {
+                    $Message -eq "Failure connecting to dest2" -and $Continue
+                }
+            }
+        }
+    }
+}
+
 Describe $CommandName -Tag IntegrationTests {
     BeforeAll {
         $PSDefaultParameterValues["*-Dba*:EnableException"] = $true