Remove Should -Not -Throw

.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md

@@ -14,7 +14,7 @@ applyTo: "tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1"
 - Avoid `ExpectedMessage` for `Should -Throw` assertions
 - Only run integration tests in CI unless explicitly instructed.
 - Call commands with `-Force` parameter where applicable (avoids prompting).
-- Use `-ErrorAction Stop` on commands so failures surface immediately
+- Use `-ErrorAction 'Stop'` on commands so failures surface immediately
 
 ## Required Setup Block
 

.github/workflows/copilot-setup-steps.yml

@@ -215,7 +215,7 @@ jobs:
           Write-Host 'Resolving project dependencies...'
 
           # Run dependency resolution
-          ./build.ps1 -ResolveDependency -Tasks 'noop' -UseModuleFast -ErrorAction Stop
+          ./build.ps1 -ResolveDependency -Tasks 'noop' -UseModuleFast -ErrorAction 'Stop'
 
           Write-Host 'Dependencies resolved successfully.'
 
@@ -225,7 +225,7 @@ jobs:
           Write-Host "Building $env:MODULE_NAME module..."
 
           # Build the module
-          ./build.ps1 -Tasks 'build' -ErrorAction Stop
+          ./build.ps1 -Tasks 'build' -ErrorAction 'Stop'
 
           # Verify build output
           $builtPath = "output/builtModule/$env:MODULE_NAME"

CHANGELOG.md

@@ -317,6 +317,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     from managed computer and instance objects.
   - Enhanced to optionally return all protocols when ProtocolName parameter
     is not specified.
+- All tests was changed to no longer use `Should -Not -Throw` for testing
+  that commands do not throw errors. Instead, tests now simply call the command
+  and will fail if an error is thrown, following best practices (follows
+  Pester 6 syntax).
 
 ### Changed
 

source/DSCResources/DSC_SqlAG/DSC_SqlAG.psm1

@@ -294,7 +294,7 @@ function Set-TargetResource
                             $script:localizedData.RemoveAvailabilityGroup -f $Name, $InstanceName
                         )
 
-                        Remove-SqlAvailabilityGroup -InputObject $availabilityGroup -ErrorAction Stop
+                        Remove-SqlAvailabilityGroup -InputObject $availabilityGroup -ErrorAction 'Stop'
                     }
                     catch
                     {
@@ -368,7 +368,7 @@ function Set-TargetResource
                         $script:localizedData.CreateAvailabilityGroupReplica -f $newReplicaParams.Name, $Name, $InstanceName
                     )
 
-                    $primaryReplica = New-SqlAvailabilityReplica @newReplicaParams -ErrorAction Stop
+                    $primaryReplica = New-SqlAvailabilityReplica @newReplicaParams -ErrorAction 'Stop'
                 }
                 catch
                 {
@@ -412,7 +412,7 @@ function Set-TargetResource
                         $script:localizedData.CreateAvailabilityGroup -f $Name, $InstanceName
                     )
 
-                    New-SqlAvailabilityGroup @newAvailabilityGroupParams -ErrorAction Stop
+                    New-SqlAvailabilityGroup @newAvailabilityGroupParams -ErrorAction 'Stop'
                 }
                 catch
                 {

source/DSCResources/DSC_SqlAGListener/DSC_SqlAGListener.psm1

@@ -259,7 +259,7 @@ function Set-TargetResource
                     Write-Verbose -Message $script:localizedData.SetAvailabilityGroupListenerDhcpDefaultSubnet
                 }
 
-                New-SqlAvailabilityGroupListener @newListenerParams -ErrorAction Stop | Out-Null
+                New-SqlAvailabilityGroupListener @newListenerParams -ErrorAction 'Stop' | Out-Null
             }
             else
             {
@@ -355,7 +355,7 @@ function Set-TargetResource
                                 Port        = $Port
                             }
 
-                            Set-SqlAvailabilityGroupListener @setListenerParams -ErrorAction Stop | Out-Null
+                            Set-SqlAvailabilityGroupListener @setListenerParams -ErrorAction 'Stop' | Out-Null
                         }
 
                         if (-not $ipAddressEqual)
@@ -379,7 +379,7 @@ function Set-TargetResource
                                 StaticIp    = $newIpAddress
                             }
 
-                            Add-SqlAvailabilityGroupListenerStaticIp @setListenerParams -ErrorAction Stop | Out-Null
+                            Add-SqlAvailabilityGroupListenerStaticIp @setListenerParams -ErrorAction 'Stop' | Out-Null
                         }
                     }
                     else

source/DSCResources/DSC_SqlAGReplica/DSC_SqlAGReplica.psm1

@@ -312,7 +312,7 @@ function Set-TargetResource
                             $script:localizedData.RemoveAvailabilityReplica -f $Name, $AvailabilityGroupName, $InstanceName
                         )
 
-                        Remove-SqlAvailabilityReplica -InputObject $availabilityGroupReplica -Confirm:$false -ErrorAction Stop
+                        Remove-SqlAvailabilityReplica -InputObject $availabilityGroupReplica -Confirm:$false -ErrorAction 'Stop'
                     }
                     catch
                     {

source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1

@@ -319,7 +319,7 @@ function Set-TargetResource
                             $LoginCreateOptions = [Microsoft.SqlServer.Management.Smo.LoginCreateOptions]::None
                         }
 
-                        New-SQLServerLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction Stop
+                        New-SQLServerLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction 'Stop'
                     }
 
                     default
@@ -674,7 +674,7 @@ function Update-SQLServerLogin
         The SecureString object that contains the password for a SQL login.
 
     .EXAMPLE
-        CreateLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction Stop
+        CreateLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction 'Stop'
 
     .EXAMPLE
         CreateLogin -Login $login

source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1

@@ -32,7 +32,7 @@ function Copy-ItemWithRobocopy
 
     $quotedPath = '"{0}"' -f $Path
     $quotedDestinationPath = '"{0}"' -f $DestinationPath
-    $robocopyExecutable = Get-Command -Name 'Robocopy.exe' -ErrorAction Stop
+    $robocopyExecutable = Get-Command -Name 'Robocopy.exe' -ErrorAction 'Stop'
 
     $robocopyArgumentSilent = '/njh /njs /ndl /nc /ns /nfl'
     $robocopyArgumentCopySubDirectoriesIncludingEmpty = '/e'
@@ -284,11 +284,11 @@ function Start-SqlSetupProcess
         ArgumentList = $ArgumentList
     }
 
-    $sqlSetupProcess = Start-Process @startProcessParameters -PassThru -NoNewWindow -ErrorAction Stop
+    $sqlSetupProcess = Start-Process @startProcessParameters -PassThru -NoNewWindow -ErrorAction 'Stop'
 
     Write-Verbose -Message ($script:localizedData.StartSetupProcess -f $sqlSetupProcess.Id, $startProcessParameters.FilePath, $Timeout) -Verbose
 
-    Wait-Process -InputObject $sqlSetupProcess -Timeout $Timeout -ErrorAction Stop
+    Wait-Process -InputObject $sqlSetupProcess -Timeout $Timeout -ErrorAction 'Stop'
 
     return $sqlSetupProcess.ExitCode
 }

tests/Integration/Commands/Add-SqlDscTraceFlag.Integration.Tests.ps1

@@ -63,9 +63,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
 
         It 'Should add a single trace flag without error' {
             # Act - Add the trace flag
-            {
-                Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
-            } | Should -Not -Throw
+            $null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
 
             # Assert - Verify the trace flag was added
             $currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -74,9 +72,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
 
         It 'Should add multiple trace flags without error' {
             # Act - Add the trace flags
-            {
-                Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
-            } | Should -Not -Throw
+            $null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
 
             # Assert - Verify the trace flags were added
             $currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -94,9 +90,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
             $beforeCount = ($beforeAddTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count
 
             # Act - Try to add the same trace flag again
-            {
-                Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
-            } | Should -Not -Throw
+            $null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
 
             # Assert - Verify no duplicate was created
             $afterAddTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -111,9 +105,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
             Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
 
             # Act - Add additional trace flags
-            {
-                Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:additionalTestTraceFlag -Force -ErrorAction 'Stop'
-            } | Should -Not -Throw
+            $null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:additionalTestTraceFlag -Force -ErrorAction 'Stop'
 
             # Assert - Verify both old and new trace flags exist
             $currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -158,9 +150,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
 
         It 'Should add a single trace flag using ServiceObject parameter' {
             # Act - Add the trace flag
-            {
-                Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
-            } | Should -Not -Throw
+            $null = Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
 
             # Assert - Verify the trace flag was added
             $currentTraceFlags = Get-SqlDscTraceFlag -ServiceObject $script:serviceObject -ErrorAction 'Stop'
@@ -169,9 +159,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
 
         It 'Should add multiple trace flags using ServiceObject parameter' {
             # Act - Add the trace flags
-            {
-                Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
-            } | Should -Not -Throw
+            $null = Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
 
             # Assert - Verify the trace flags were added
             $currentTraceFlags = Get-SqlDscTraceFlag -ServiceObject $script:serviceObject -ErrorAction 'Stop'

tests/Integration/Commands/Assert-SqlDscAgentOperator.Integration.Tests.ps1

@@ -35,7 +35,7 @@ BeforeAll {
 }
 
 AfterAll {
-    Remove-Item -Path 'Env:\SqlServerDscCI' -ErrorAction 'SilentlyContinue'
+    $null = Remove-Item -Path 'Env:\SqlServerDscCI' -ErrorAction 'Stop'
 
     # Unload the module being tested so that it doesn't impact any other tests.
     Get-Module -Name $script:moduleName -All | Remove-Module -Force
@@ -52,34 +52,34 @@ Describe 'Assert-SqlDscAgentOperator' -Tag 'Integration_SQL2017', 'Integration_S
         $script:sqlServerObject = Connect-SqlDscDatabaseEngine -InstanceName $script:sqlServerInstance -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop'
 
         # Create a test operator for assertion tests
-        $script:sqlServerObject | New-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -EmailAddress '[email protected]' -ErrorAction Stop
+        $null = $script:sqlServerObject | New-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -EmailAddress '[email protected]' -ErrorAction 'Stop'
     }
 
     AfterAll {
-        $script:sqlServerObject | Remove-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -Force -ErrorAction 'SilentlyContinue'
+        $null = $script:sqlServerObject | Remove-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -Force -ErrorAction 'Stop'
 
         # Disconnect from the SQL Server
         Disconnect-SqlDscDatabaseEngine -ServerObject $script:sqlServerObject
     }
 
     Context 'When operator exists' {
         It 'Should not throw when asserting existing operator' {
-            $null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction Stop
+            $null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction 'Stop'
         }
 
         It 'Should not throw when asserting persistent operator created by New-SqlDscAgentOperator' {
-            $null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'SqlDscIntegrationTestOperator_Persistent' -ErrorAction Stop
+            $null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'SqlDscIntegrationTestOperator_Persistent' -ErrorAction 'Stop'
         }
 
         It 'Should not return anything when operator exists' {
-            $result = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction Stop
+            $result = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction 'Stop'
             $result | Should -BeNullOrEmpty
         }
     }
 
     Context 'When operator does not exist' {
         It 'Should throw terminating error when operator does not exist' {
-            { Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'NonExistentOperator' -ErrorAction Stop } | Should -Throw
+            { Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'NonExistentOperator' -ErrorAction 'Stop' } | Should -Throw
         }
 
     }

tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1

@@ -51,19 +51,19 @@ Describe 'Assert-SqlDscLogin' -Tag @('Integration_SQL2017', 'Integration_SQL2019
 
         Context 'When a login exists' {
             It 'Should not throw an error for sa login' {
-                { Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'sa' } | Should -Not -Throw
+                $null = Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'sa' -ErrorAction 'Stop'
             }
 
             It 'Should not throw an error when using pipeline' {
-                { $script:serverObject | Assert-SqlDscLogin -Name 'sa' } | Should -Not -Throw
+                $null = $script:serverObject | Assert-SqlDscLogin -Name 'sa' -ErrorAction 'Stop'
             }
 
             It 'Should not throw an error for NT AUTHORITY\SYSTEM login' {
-                { Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'NT AUTHORITY\SYSTEM' } | Should -Not -Throw
+                $null = Assert-SqlDscLogin -ServerObject $script:serverObject -Name 'NT AUTHORITY\SYSTEM' -ErrorAction 'Stop'
             }
 
             It 'Should not throw an error for SqlAdmin login' {
-                { Assert-SqlDscLogin -ServerObject $script:serverObject -Name ('{0}\SqlAdmin' -f $script:computerName) } | Should -Not -Throw
+                $null = Assert-SqlDscLogin -ServerObject $script:serverObject -Name ('{0}\SqlAdmin' -f $script:computerName) -ErrorAction 'Stop'
             }
         }