Skip to content

Commit 99ad254

Browse files
johljujohlju
authored
Remove Should -Not -Throw (#2302)
1 parent a2d3b29 commit 99ad254

122 files changed

Lines changed: 4165 additions & 5058 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ applyTo: "tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1"
1414
- Avoid `ExpectedMessage` for `Should -Throw` assertions
1515
- Only run integration tests in CI unless explicitly instructed.
1616
- Call commands with `-Force` parameter where applicable (avoids prompting).
17-
- Use `-ErrorAction Stop` on commands so failures surface immediately
17+
- Use `-ErrorAction 'Stop'` on commands so failures surface immediately
1818

1919
## Required Setup Block
2020

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ jobs:
215215
Write-Host 'Resolving project dependencies...'
216216
217217
# Run dependency resolution
218-
./build.ps1 -ResolveDependency -Tasks 'noop' -UseModuleFast -ErrorAction Stop
218+
./build.ps1 -ResolveDependency -Tasks 'noop' -UseModuleFast -ErrorAction 'Stop'
219219
220220
Write-Host 'Dependencies resolved successfully.'
221221
@@ -225,7 +225,7 @@ jobs:
225225
Write-Host "Building $env:MODULE_NAME module..."
226226
227227
# Build the module
228-
./build.ps1 -Tasks 'build' -ErrorAction Stop
228+
./build.ps1 -Tasks 'build' -ErrorAction 'Stop'
229229
230230
# Verify build output
231231
$builtPath = "output/builtModule/$env:MODULE_NAME"

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
317317
from managed computer and instance objects.
318318
- Enhanced to optionally return all protocols when ProtocolName parameter
319319
is not specified.
320+
- All tests was changed to no longer use `Should -Not -Throw` for testing
321+
that commands do not throw errors. Instead, tests now simply call the command
322+
and will fail if an error is thrown, following best practices (follows
323+
Pester 6 syntax).
320324

321325
### Changed
322326

‎source/DSCResources/DSC_SqlAG/DSC_SqlAG.psm1‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ function Set-TargetResource
294294
$script:localizedData.RemoveAvailabilityGroup -f $Name, $InstanceName
295295
)
296296

297-
Remove-SqlAvailabilityGroup -InputObject $availabilityGroup -ErrorAction Stop
297+
Remove-SqlAvailabilityGroup -InputObject $availabilityGroup -ErrorAction 'Stop'
298298
}
299299
catch
300300
{
@@ -368,7 +368,7 @@ function Set-TargetResource
368368
$script:localizedData.CreateAvailabilityGroupReplica -f $newReplicaParams.Name, $Name, $InstanceName
369369
)
370370

371-
$primaryReplica = New-SqlAvailabilityReplica @newReplicaParams -ErrorAction Stop
371+
$primaryReplica = New-SqlAvailabilityReplica @newReplicaParams -ErrorAction 'Stop'
372372
}
373373
catch
374374
{
@@ -412,7 +412,7 @@ function Set-TargetResource
412412
$script:localizedData.CreateAvailabilityGroup -f $Name, $InstanceName
413413
)
414414

415-
New-SqlAvailabilityGroup @newAvailabilityGroupParams -ErrorAction Stop
415+
New-SqlAvailabilityGroup @newAvailabilityGroupParams -ErrorAction 'Stop'
416416
}
417417
catch
418418
{

‎source/DSCResources/DSC_SqlAGListener/DSC_SqlAGListener.psm1‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ function Set-TargetResource
259259
Write-Verbose -Message $script:localizedData.SetAvailabilityGroupListenerDhcpDefaultSubnet
260260
}
261261

262-
New-SqlAvailabilityGroupListener @newListenerParams -ErrorAction Stop | Out-Null
262+
New-SqlAvailabilityGroupListener @newListenerParams -ErrorAction 'Stop' | Out-Null
263263
}
264264
else
265265
{
@@ -355,7 +355,7 @@ function Set-TargetResource
355355
Port = $Port
356356
}
357357

358-
Set-SqlAvailabilityGroupListener @setListenerParams -ErrorAction Stop | Out-Null
358+
Set-SqlAvailabilityGroupListener @setListenerParams -ErrorAction 'Stop' | Out-Null
359359
}
360360

361361
if (-not $ipAddressEqual)
@@ -379,7 +379,7 @@ function Set-TargetResource
379379
StaticIp = $newIpAddress
380380
}
381381

382-
Add-SqlAvailabilityGroupListenerStaticIp @setListenerParams -ErrorAction Stop | Out-Null
382+
Add-SqlAvailabilityGroupListenerStaticIp @setListenerParams -ErrorAction 'Stop' | Out-Null
383383
}
384384
}
385385
else

‎source/DSCResources/DSC_SqlAGReplica/DSC_SqlAGReplica.psm1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ function Set-TargetResource
312312
$script:localizedData.RemoveAvailabilityReplica -f $Name, $AvailabilityGroupName, $InstanceName
313313
)
314314

315-
Remove-SqlAvailabilityReplica -InputObject $availabilityGroupReplica -Confirm:$false -ErrorAction Stop
315+
Remove-SqlAvailabilityReplica -InputObject $availabilityGroupReplica -Confirm:$false -ErrorAction 'Stop'
316316
}
317317
catch
318318
{

‎source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ function Set-TargetResource
319319
$LoginCreateOptions = [Microsoft.SqlServer.Management.Smo.LoginCreateOptions]::None
320320
}
321321

322-
New-SQLServerLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction Stop
322+
New-SQLServerLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction 'Stop'
323323
}
324324

325325
default
@@ -674,7 +674,7 @@ function Update-SQLServerLogin
674674
The SecureString object that contains the password for a SQL login.
675675
676676
.EXAMPLE
677-
CreateLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction Stop
677+
CreateLogin -Login $login -LoginCreateOptions $LoginCreateOptions -SecureString $LoginCredential.Password -ErrorAction 'Stop'
678678
679679
.EXAMPLE
680680
CreateLogin -Login $login

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function Copy-ItemWithRobocopy
3232

3333
$quotedPath = '"{0}"' -f $Path
3434
$quotedDestinationPath = '"{0}"' -f $DestinationPath
35-
$robocopyExecutable = Get-Command -Name 'Robocopy.exe' -ErrorAction Stop
35+
$robocopyExecutable = Get-Command -Name 'Robocopy.exe' -ErrorAction 'Stop'
3636

3737
$robocopyArgumentSilent = '/njh /njs /ndl /nc /ns /nfl'
3838
$robocopyArgumentCopySubDirectoriesIncludingEmpty = '/e'
@@ -284,11 +284,11 @@ function Start-SqlSetupProcess
284284
ArgumentList = $ArgumentList
285285
}
286286

287-
$sqlSetupProcess = Start-Process @startProcessParameters -PassThru -NoNewWindow -ErrorAction Stop
287+
$sqlSetupProcess = Start-Process @startProcessParameters -PassThru -NoNewWindow -ErrorAction 'Stop'
288288

289289
Write-Verbose -Message ($script:localizedData.StartSetupProcess -f $sqlSetupProcess.Id, $startProcessParameters.FilePath, $Timeout) -Verbose
290290

291-
Wait-Process -InputObject $sqlSetupProcess -Timeout $Timeout -ErrorAction Stop
291+
Wait-Process -InputObject $sqlSetupProcess -Timeout $Timeout -ErrorAction 'Stop'
292292

293293
return $sqlSetupProcess.ExitCode
294294
}

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

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
6363

6464
It 'Should add a single trace flag without error' {
6565
# Act - Add the trace flag
66-
{
67-
Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
68-
} | Should -Not -Throw
66+
$null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
6967

7068
# Assert - Verify the trace flag was added
7169
$currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -74,9 +72,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
7472

7573
It 'Should add multiple trace flags without error' {
7674
# Act - Add the trace flags
77-
{
78-
Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
79-
} | Should -Not -Throw
75+
$null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
8076

8177
# Assert - Verify the trace flags were added
8278
$currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -94,9 +90,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
9490
$beforeCount = ($beforeAddTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count
9591

9692
# Act - Try to add the same trace flag again
97-
{
98-
Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
99-
} | Should -Not -Throw
93+
$null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
10094

10195
# Assert - Verify no duplicate was created
10296
$afterAddTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -111,9 +105,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
111105
Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
112106

113107
# Act - Add additional trace flags
114-
{
115-
Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:additionalTestTraceFlag -Force -ErrorAction 'Stop'
116-
} | Should -Not -Throw
108+
$null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:additionalTestTraceFlag -Force -ErrorAction 'Stop'
117109

118110
# Assert - Verify both old and new trace flags exist
119111
$currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
@@ -158,9 +150,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
158150

159151
It 'Should add a single trace flag using ServiceObject parameter' {
160152
# Act - Add the trace flag
161-
{
162-
Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
163-
} | Should -Not -Throw
153+
$null = Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
164154

165155
# Assert - Verify the trace flag was added
166156
$currentTraceFlags = Get-SqlDscTraceFlag -ServiceObject $script:serviceObject -ErrorAction 'Stop'
@@ -169,9 +159,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
169159

170160
It 'Should add multiple trace flags using ServiceObject parameter' {
171161
# Act - Add the trace flags
172-
{
173-
Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
174-
} | Should -Not -Throw
162+
$null = Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag $script:testTraceFlags -Force -ErrorAction 'Stop'
175163

176164
# Assert - Verify the trace flags were added
177165
$currentTraceFlags = Get-SqlDscTraceFlag -ServiceObject $script:serviceObject -ErrorAction 'Stop'

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ BeforeAll {
3535
}
3636

3737
AfterAll {
38-
Remove-Item -Path 'Env:\SqlServerDscCI' -ErrorAction 'SilentlyContinue'
38+
$null = Remove-Item -Path 'Env:\SqlServerDscCI' -ErrorAction 'Stop'
3939

4040
# Unload the module being tested so that it doesn't impact any other tests.
4141
Get-Module -Name $script:moduleName -All | Remove-Module -Force
@@ -52,34 +52,34 @@ Describe 'Assert-SqlDscAgentOperator' -Tag 'Integration_SQL2017', 'Integration_S
5252
$script:sqlServerObject = Connect-SqlDscDatabaseEngine -InstanceName $script:sqlServerInstance -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop'
5353

5454
# Create a test operator for assertion tests
55-
$script:sqlServerObject | New-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -EmailAddress 'assert@contoso.com' -ErrorAction Stop
55+
$null = $script:sqlServerObject | New-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -EmailAddress 'assert@contoso.com' -ErrorAction 'Stop'
5656
}
5757

5858
AfterAll {
59-
$script:sqlServerObject | Remove-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -Force -ErrorAction 'SilentlyContinue'
59+
$null = $script:sqlServerObject | Remove-SqlDscAgentOperator -Name 'IntegrationTest_AssertOperator' -Force -ErrorAction 'Stop'
6060

6161
# Disconnect from the SQL Server
6262
Disconnect-SqlDscDatabaseEngine -ServerObject $script:sqlServerObject
6363
}
6464

6565
Context 'When operator exists' {
6666
It 'Should not throw when asserting existing operator' {
67-
$null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction Stop
67+
$null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction 'Stop'
6868
}
6969

7070
It 'Should not throw when asserting persistent operator created by New-SqlDscAgentOperator' {
71-
$null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'SqlDscIntegrationTestOperator_Persistent' -ErrorAction Stop
71+
$null = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'SqlDscIntegrationTestOperator_Persistent' -ErrorAction 'Stop'
7272
}
7373

7474
It 'Should not return anything when operator exists' {
75-
$result = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction Stop
75+
$result = Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'IntegrationTest_AssertOperator' -ErrorAction 'Stop'
7676
$result | Should -BeNullOrEmpty
7777
}
7878
}
7979

8080
Context 'When operator does not exist' {
8181
It 'Should throw terminating error when operator does not exist' {
82-
{ Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'NonExistentOperator' -ErrorAction Stop } | Should -Throw
82+
{ Assert-SqlDscAgentOperator -ServerObject $script:sqlServerObject -Name 'NonExistentOperator' -ErrorAction 'Stop' } | Should -Throw
8383
}
8484

8585
}

0 commit comments

Comments
 (0)