Refactor integration tests to optimize SQL Server service management #2291
Conversation
|
Warning Rate limit exceeded@johlju has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
WalkthroughRemoved per-test Start-Service/Stop-Service calls for the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CI as CI Runner
participant Install as Install test
participant Suite as Integration suite
participant S as MSSQL$DSCSQLTEST
Note over Install,S: New flow — service started/installed once and kept running
CI->>Install: run install test
Install->>S: ensure Running (assert)
CI->>Suite: run integration tests
Suite->>S: assume Running (no Start/Stop)
Suite->>Suite: execute assertions and cleanup (no service stop)
Note over S: Service remains running between tests
sequenceDiagram
autonumber
actor CI as CI Runner
participant T as Single integration test
participant S as MSSQL$DSCSQLTEST
Note over T,S: Previous per-test lifecycle
CI->>T: run test
T->>S: Start-Service
T->>T: execute assertions
T->>S: Stop-Service
T-->>CI: report results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (21)
tests/Integration/Commands/Get-SqlDscConfigurationOption.Integration.Tests.ps1 (1)
34-44: Start MSSQL$DSCSQLTEST in BeforeAll and make Connect-SqlDscDatabaseEngine fail-fastFollow integration-test guidelines: start the MSSQL$DSCSQLTEST service in BeforeAll (do not rely on other tests) and add -ErrorAction 'Stop' to Connect-SqlDscDatabaseEngine to remove test-order coupling and flakiness.
Location: tests/Integration/Commands/Get-SqlDscConfigurationOption.Integration.Tests.ps1 — BeforeAll
BeforeAll { - # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # Ensure SQL Server service is running for this test file + $serviceName = 'MSSQL$DSCSQLTEST' + $service = Get-Service -Name $serviceName -ErrorAction 'Stop' + if ($service.Status -ne 'Running') + { + Start-Service -Name $serviceName -ErrorAction 'Stop' + (Get-Service -Name $serviceName).WaitForStatus('Running', '00:02:00') + } $script:mockInstanceName = 'DSCSQLTEST' $mockSqlAdministratorUserName = 'SqlAdmin' # Using computer name as NetBIOS name throw exception. $mockSqlAdministratorPassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force $script:mockSqlAdminCredential = [System.Management.Automation.PSCredential]::new($mockSqlAdministratorUserName, $mockSqlAdministratorPassword) - $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' }tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1 (1)
133-133: Bug: Using undefined $roleObject in BeforeEach.BeforeEach sets $script:roleObject, but the revoke uses $roleObject (undefined). This can make the cleanup a no-op or error.
- Revoke-SqlDscServerPermission -ServerRole $roleObject -Permission 'ViewServerState' -Force -ErrorAction 'SilentlyContinue' + Revoke-SqlDscServerPermission -ServerRole $script:roleObject -Permission 'ViewServerState' -Force -ErrorAction 'SilentlyContinue'tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1 (1)
34-48: Restore test independence: ensure MSSQL$DSCSQLTEST is running in BeforeAll (no cross‑test assumptions).Per our guidelines, integration tests that require SQL Server must start the service in BeforeAll and stop it in AfterAll. Assuming it was started by another test couples order and breaks standalone runs. Use an idempotent start that preserves the PR’s perf goal by only stopping if this test started it.
- # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # Ensure SQL Server service is running; stop only if we started it (preserves perf and independence) + $script:serviceName = 'MSSQL$DSCSQLTEST' + $script:stopServiceInAfterAll = $false + $service = Get-Service -Name $script:serviceName -ErrorAction 'Stop' + if ($service.Status -ne 'Running') + { + Start-Service -Name $script:serviceName -ErrorAction 'Stop' + $script:stopServiceInAfterAll = $true + }tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1 (2)
37-47: Do not rely on other tests for service state; ensure isolation by starting service in BeforeAllRelying on Install-SqlDscServer to have started MSSQL$DSCSQLTEST violates the integration test guideline and hurts test-order independence. Start the service if needed and remember prior state.
- # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # Ensure SQL Server service is running for test isolation (do not rely on other tests) + + $script:serviceName = 'MSSQL$DSCSQLTEST' + $script:wasServiceRunning = $false + + $service = Get-Service -Name $script:serviceName -ErrorAction 'Stop' + if ($service.Status -ne 'Running') + { + Start-Service -Name $script:serviceName -ErrorAction 'Stop' + } + else + { + $script:wasServiceRunning = $true + }
49-64: Restore logic can leak state; handle null vs empty flags and use Stop on errors. Also revert service to original state.
- Passing $null to -TraceFlag/-InternalTraceFlag will not clear flags; you must pass @() when original flags are absent.
- Using -ErrorAction SilentlyContinue inside try/catch defeats the catch.
- Leave service running only if it was running before (optionally gate with env var for perf).
- # Restore original startup parameters - if ($script:originalStartupParameters) + # Restore original startup parameters + if ($null -ne $script:originalStartupParameters) { try { - Set-SqlDscStartupParameter -ServiceObject $script:serviceObject -TraceFlag $script:originalStartupParameters.TraceFlag -InternalTraceFlag $script:originalStartupParameters.InternalTraceFlag -Force -ErrorAction 'SilentlyContinue' + $restoredTraceFlags = if ($null -ne $script:originalStartupParameters.TraceFlag -and $script:originalStartupParameters.TraceFlag.Count -gt 0) { $script:originalStartupParameters.TraceFlag } else { @() } + $restoredInternalTraceFlags = if ($null -ne $script:originalStartupParameters.InternalTraceFlag -and $script:originalStartupParameters.InternalTraceFlag.Count -gt 0) { $script:originalStartupParameters.InternalTraceFlag } else { @() } + + Set-SqlDscStartupParameter -ServiceObject $script:serviceObject -TraceFlag $restoredTraceFlags -InternalTraceFlag $restoredInternalTraceFlags -Force -ErrorAction 'Stop' } catch { Write-Warning -Message "Failed to restore original startup parameters: $($_.Exception.Message)" } } - # Note: SQL Server service is left running for subsequent tests for performance optimization + # Revert service to original state unless explicitly kept for CI perf + if (-not $script:wasServiceRunning -and $env:SqlServerDscKeepServiceRunning -ne '1') + { + Stop-Service -Name $script:serviceName -Force -ErrorAction 'Stop' + } + elseif ($env:SqlServerDscKeepServiceRunning -eq '1') + { + Write-Verbose -Message "Keeping service '$script:serviceName' running due to SqlServerDscKeepServiceRunning=1." -Verbose + }tests/Integration/Commands/Enable-SqlDscLogin.Integration.Tests.ps1 (1)
45-47: Ensure the test login exists; avoid hidden dependency on prior tests.Relying on a “persistent login” makes this test order-dependent. Create it if missing.
# Use existing persistent login for testing $script:testLoginName = 'IntegrationTestSqlLogin' + # Ensure precondition: create the login if it does not exist + if (-not (Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -ErrorAction 'SilentlyContinue')) + { + $testPassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force + $null = New-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -LoginType 'SqlLogin' -Password $testPassword -Force -ErrorAction 'Stop' + }tests/Integration/Commands/Remove-SqlDscAgentAlert.Integration.Tests.ps1 (1)
26-46: Start or verify MSSQL$DSCSQLTEST in BeforeAll for all integration testsrg found 74 integration command tests that assume the SQL service is pre-started; several (e.g. tests/Integration/Commands/Test-SqlDscIsAgentOperator.Integration.Tests.ps1) call Stop-Service 'MSSQL$DSCSQLTEST' in AfterAll. Per project guidelines, integration tests that require SQL Server must explicitly start the Windows service in BeforeAll and stop it in AfterAll (and call Connect-/Disconnect-SqlDscDatabaseEngine). Update these BeforeAll blocks to verify-and-start (or Start-Service) and ensure AfterAll disconnects then stops the service.
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1 (2)
43-44: Use -ErrorAction Stop on Connect to fail fast (per test guidelines).Ensures environment/setup failures are surfaced immediately.
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop'
58-71: Add -ErrorAction Stop on command invocations inside Its.Per integration test rules, add -ErrorAction 'Stop' so failures surface immediately. Apply consistently to New‑SqlDscLogin, Remove‑SqlDscLogin, Get‑SqlDscLogin, Test‑SqlDscIsLogin (and any other commands).
Example edits (apply pattern throughout):
- $null = $script:serverObject | New-SqlDscLogin -Name $script:testLoginName -SqlLogin -SecurePassword $script:testLoginPassword -Force + $null = $script:serverObject | New-SqlDscLogin -Name $script:testLoginName -SqlLogin -SecurePassword $script:testLoginPassword -Force -ErrorAction 'Stop' - $loginExists = Test-SqlDscIsLogin -ServerObject $script:serverObject -Name $script:testLoginName + $loginExists = Test-SqlDscIsLogin -ServerObject $script:serverObject -Name $script:testLoginName -ErrorAction 'Stop' - $script:serverObject | Remove-SqlDscLogin -Name $script:testLoginName -Force + $script:serverObject | Remove-SqlDscLogin -Name $script:testLoginName -Force -ErrorAction 'Stop' - $loginObject = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName2 + $loginObject = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName2 -ErrorAction 'Stop'Also applies to: 74-84, 86-97, 106-123, 125-139, 147-162
tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1 (2)
43-43: Add -ErrorAction 'Stop' to all command invocations (integration test requirement).Ensures failures surface immediately and don’t get swallowed by pipelines/assertions.
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop' - $result = Get-SqlDscRole -ServerObject $script:serverObject + $result = Get-SqlDscRole -ServerObject $script:serverObject -ErrorAction 'Stop' - $result = Get-SqlDscRole -ServerObject $script:serverObject + $result = Get-SqlDscRole -ServerObject $script:serverObject -ErrorAction 'Stop' - $result = Get-SqlDscRole -ServerObject $script:serverObject -Name 'sysadmin' + $result = Get-SqlDscRole -ServerObject $script:serverObject -Name 'sysadmin' -ErrorAction 'Stop' - $result = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForIntegrationTests + $result = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForIntegrationTests -ErrorAction 'Stop' - $result = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole + $result = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole -ErrorAction 'Stop' - $resultWithoutRefresh = Get-SqlDscRole -ServerObject $script:serverObject + $resultWithoutRefresh = Get-SqlDscRole -ServerObject $script:serverObject -ErrorAction 'Stop' - $resultWithRefresh = Get-SqlDscRole -ServerObject $script:serverObject -Refresh + $resultWithRefresh = Get-SqlDscRole -ServerObject $script:serverObject -Refresh -ErrorAction 'Stop' - $result = $script:serverObject | Get-SqlDscRole -Name $script:sharedTestRoleForIntegrationTests + $result = $script:serverObject | Get-SqlDscRole -Name $script:sharedTestRoleForIntegrationTests -ErrorAction 'Stop'Also applies to: 51-51, 58-58, 69-69, 77-77, 85-85, 93-93, 115-116, 124-124
101-104: Remove -ExpectedMessage assertions from integration tests — assert Should -Throw (or exception type) instead.Repo guideline: don't assert error messages in integration tests; remove -ExpectedMessage.
File: tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1 (lines 101–104)
- { Get-SqlDscRole -ServerObject $script:serverObject -Name 'NonExistentRole' -ErrorAction 'Stop' } | - Should -Throw -ExpectedMessage 'Server role ''NonExistentRole'' was not found.' + { Get-SqlDscRole -ServerObject $script:serverObject -Name 'NonExistentRole' -ErrorAction 'Stop' } | + Should -ThrowAlso update other integration tests that use -ExpectedMessage (examples found: Get-SqlDscLogin.Integration.Tests.ps1, Assert-SqlDscLogin.Integration.Tests.ps1, Remove-SqlDscRole.Integration.Tests.ps1, New-SqlDscRole.Integration.Tests.ps1, Set-SqlDscDatabasePermission.Integration.Tests.ps1).
tests/Integration/Commands/Set-SqlDscDatabaseDefault.Integration.Tests.ps1 (1)
44-45: Use -ErrorAction Stop on all command invocations so failures surface immediatelyIntegration tests should fail fast on non-terminating errors.
Apply this diff:
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' - $script:testDatabaseObject = New-SqlDscDatabase -ServerObject $script:serverObject -Name $script:testDatabaseName -Force + $script:testDatabaseObject = New-SqlDscDatabase -ServerObject $script:serverObject -Name $script:testDatabaseName -Force -ErrorAction 'Stop' - $null = Set-SqlDscDatabaseDefault -DatabaseObject $script:testDatabaseObject -DefaultFileGroup $script:testFileGroupName -Force + $null = Set-SqlDscDatabaseDefault -DatabaseObject $script:testDatabaseObject -DefaultFileGroup $script:testFileGroupName -Force -ErrorAction 'Stop' - $null = Set-SqlDscDatabaseDefault -DatabaseObject $script:testDatabaseObject -DefaultFileGroup 'PRIMARY' -Force + $null = Set-SqlDscDatabaseDefault -DatabaseObject $script:testDatabaseObject -DefaultFileGroup 'PRIMARY' -Force -ErrorAction 'Stop' - $null = Set-SqlDscDatabaseDefault -ServerObject $script:serverObject -Name $script:testDatabaseName -DefaultFileGroup $script:testFileGroupName -Force + $null = Set-SqlDscDatabaseDefault -ServerObject $script:serverObject -Name $script:testDatabaseName -DefaultFileGroup $script:testFileGroupName -Force -ErrorAction 'Stop' - $result = Set-SqlDscDatabaseDefault -ServerObject $script:serverObject -Name $script:testDatabaseName -DefaultFileGroup 'PRIMARY' -PassThru -Force + $result = Set-SqlDscDatabaseDefault -ServerObject $script:serverObject -Name $script:testDatabaseName -DefaultFileGroup 'PRIMARY' -PassThru -Force -ErrorAction 'Stop' - { Set-SqlDscDatabaseDefault -DatabaseObject $script:testDatabaseObject -DefaultFileGroup 'NonExistentFileGroup' -Force } | Should -Throw + { Set-SqlDscDatabaseDefault -DatabaseObject $script:testDatabaseObject -DefaultFileGroup 'NonExistentFileGroup' -Force -ErrorAction 'Stop' } | Should -Throw - { Set-SqlDscDatabaseDefault -ServerObject $script:serverObject -Name 'NonExistentDatabase' -DefaultFileGroup 'PRIMARY' -Force } | Should -Throw + { Set-SqlDscDatabaseDefault -ServerObject $script:serverObject -Name 'NonExistentDatabase' -DefaultFileGroup 'PRIMARY' -Force -ErrorAction 'Stop' } | Should -ThrowAlso applies to: 50-51, 75-76, 83-84, 93-94, 101-102, 111-112, 115-116
tests/Integration/Commands/Connect-SqlDscDatabaseEngine.Integration.Tests.ps1 (4)
51-58: Make default-instance service management idempotent; avoid Start-Service failure when already running.Start-Service throws if the service is already running (you’re using -ErrorAction Stop). Track whether this test started the service and only stop it then.
- BeforeAll { - # Starting the default instance SQL Server service prior to running tests. - Start-Service -Name 'MSSQLSERVER' -Verbose -ErrorAction 'Stop' - } + BeforeAll { + $script:defaultServiceName = 'MSSQLSERVER' + $script:didStartDefaultInstance = $false + + $service = Get-Service -Name $script:defaultServiceName -ErrorAction 'Stop' + if ($service.Status -ne 'Running') + { + Start-Service -Name $script:defaultServiceName -Verbose -ErrorAction 'Stop' + $script:didStartDefaultInstance = $true + } + } @@ - AfterAll { - # Stop the default instance SQL Server service to save memory on the build worker. - Stop-Service -Name 'MSSQLSERVER' -Verbose -ErrorAction 'Stop' - } + AfterAll { + if ($script:didStartDefaultInstance) + { + Stop-Service -Name $script:defaultServiceName -Verbose -ErrorAction 'Stop' + } + }
102-119: Same fix: no Should -Not -Throw; add Disconnect after Connect (named instance, Windows auth).- It 'Should return the correct result' { - { + It 'Should return the correct result' { $sqlAdministratorUserName = 'SqlAdmin' # Using computer name as NetBIOS name throw exception. $sqlAdministratorPassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force @@ - $sqlServerObject = Connect-SqlDscDatabaseEngine @connectSqlDscDatabaseEngineParameters - - $sqlServerObject.Status.ToString() | Should -Match '^Online$' - } | Should -Not -Throw - } + $sqlServerObject = Connect-SqlDscDatabaseEngine @connectSqlDscDatabaseEngineParameters + $sqlServerObject.Status.ToString() | Should -Match '^Online$' + Disconnect-SqlDscDatabaseEngine -InputObject $sqlServerObject + }
122-140: Same fix: no Should -Not -Throw; add Disconnect after Connect (named instance, SQL login).- It 'Should return the correct result' { - { + It 'Should return the correct result' { $sqlAdministratorUserName = 'sa' $sqlAdministratorPassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force @@ - $sqlServerObject = Connect-SqlDscDatabaseEngine @connectSqlDscDatabaseEngineParameters - - $sqlServerObject.Status.ToString() | Should -Match '^Online$' - } | Should -Not -Throw - } + $sqlServerObject = Connect-SqlDscDatabaseEngine @connectSqlDscDatabaseEngineParameters + $sqlServerObject.Status.ToString() | Should -Match '^Online$' + Disconnect-SqlDscDatabaseEngine -InputObject $sqlServerObject + }
67-83: Remove Should -Not -Throw; explicitly disconnect the SMO session using -ServerObjectDo not wrap the test in Should -Not -Throw — use ErrorAction='Stop' to surface failures, and after Connect-SqlDscDatabaseEngine call Disconnect-SqlDscDatabaseEngine -ServerObject $sqlServerObject (or pipe: Connect-SqlDscDatabaseEngine | Disconnect-SqlDscDatabaseEngine). (powershellgallery.com)
tests/Integration/Commands/New-SqlDscRole.Integration.Tests.ps1 (2)
33-53: Avoid cross-test coupling; gate service start locally (start-if-needed) per guidelines.Relying on "Install-SqlDscServer test" to have started MSSQL$DSCSQLTEST makes this suite order-dependent and conflicts with our integration-test guideline to start the service in BeforeAll and stop it in AfterAll. Gate the service: start it only if not already running, track that, and only stop it here if we started it. This keeps tests independent and still preserves the perf win when the service is pre-started.
Apply:
BeforeAll { - # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # Ensure SQL Server service is running (start only if needed) + $script:sqlServiceName = 'MSSQL$DSCSQLTEST' + $script:serviceStartedHere = $false + + $service = Get-Service -Name $script:sqlServiceName -ErrorAction 'Stop' + if ($service.Status -ne 'Running') + { + Start-Service -Name $script:sqlServiceName -ErrorAction 'Stop' + $script:serviceStartedHere = $true + } $script:mockInstanceName = 'DSCSQLTEST' $script:mockComputerName = Get-ComputerNameAlso consider updating the repo’s integration-test instructions if we’re standardizing on pre-started service across suites.
121-165: Make shared/persistent-role creation idempotent.These Its unconditionally create roles that are intentionally left behind. Re-runs or parallelization will fail with “already exists.” Ensure-exists pattern avoids flakiness while preserving semantics.
Apply:
It 'Should create a shared role for integration tests' { - # Create shared role for Get-SqlDscRole integration tests - $result = New-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForIntegrationTests -Force + # Ensure shared role for Get-SqlDscRole integration tests exists + $existing = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForIntegrationTests -ErrorAction 'SilentlyContinue' + if (-not $existing) + { + $result = New-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForIntegrationTests -Force + } + else + { + $result = $existing + } $result | Should -BeOfType 'Microsoft.SqlServer.Management.Smo.ServerRole' $result.Name | Should -Be $script:sharedTestRoleForIntegrationTests $result.IsFixedRole | Should -BeFalse @@ It 'Should create a shared role for removal tests' { - # Create shared role for Remove-SqlDscRole integration tests - $result = New-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForRemoval -Force + # Ensure shared role for Remove-SqlDscRole integration tests exists + $existing = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForRemoval -ErrorAction 'SilentlyContinue' + if (-not $existing) + { + $result = New-SqlDscRole -ServerObject $script:serverObject -Name $script:sharedTestRoleForRemoval -Force + } + else + { + $result = $existing + } @@ It 'Should create a persistent role that remains on the instance' { - # Create persistent role with sa owner that will remain on the instance - $result = New-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole -Owner 'sa' -Force + # Ensure persistent role with sa owner exists (remains on the instance) + $existing = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole -ErrorAction 'SilentlyContinue' + if (-not $existing) + { + $result = New-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole -Owner 'sa' -Force + } + else + { + $result = $existing + }tests/Integration/Commands/Test-SqlDscIsAgentOperator.Integration.Tests.ps1 (1)
58-60: Inconsistent service management detected.Line 59 still contains an explicit
Stop-Servicecall for MSSQL$DSCSQLTEST, which contradicts the PR's performance optimization strategy. This service stop should be removed and replaced with a note indicating the service is left running for subsequent tests, consistent with all other files in this PR.Apply this diff to align with the PR's optimization strategy:
- # Stopping the named instance SQL Server service after running tests. - Stop-Service -Name 'MSSQL$DSCSQLTEST' -Verbose -ErrorAction 'Stop' + # Note: SQL Server service is left running for subsequent tests for performance optimizationtests/Integration/Commands/Get-SqlDscLogin.Integration.Tests.ps1 (1)
43-44: Add -ErrorAction Stop and ensure disconnect
- Connect lacks -ErrorAction 'Stop'.
- Missing Disconnect-SqlDscDatabaseEngine in AfterAll.
Apply:
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' @@ - AfterAll { - # Note: SQL Server service is left running for subsequent tests for performance optimization - } + AfterAll { + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop' + # Note: SQL Server service is left running for subsequent tests for performance optimization + }Also applies to: 46-48
tests/Integration/Commands/Disable-SqlDscAgentOperator.Integration.Tests.ps1 (1)
64-66: Stop-Service here violates the PR objective and risks flakinessThis stops MSSQL$DSCSQLTEST, contradicting the new strategy and may break subsequent tests assuming the service is Running.
Apply:
- # Stopping the named instance SQL Server service after running tests. - Stop-Service -Name 'MSSQL$DSCSQLTEST' -Verbose -ErrorAction 'Stop'
🧹 Nitpick comments (39)
tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1 (2)
38-41: Plaintext test credential: verify scope and rotation.Hardcoded 'SqlAdmin'/'P@ssw0rd1' is acceptable only if CI-scoped and rotated. Consider pulling from environment variables with a safe default for local CI.
Example tweak:
- $mockSqlAdministratorUserName = 'SqlAdmin' # Using computer name as NetBIOS name throw exception. - $mockSqlAdministratorPassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force + $mockSqlAdministratorUserName = $env:SqlServerDsc_CI_SqlUserName + if (-not $mockSqlAdministratorUserName) { $mockSqlAdministratorUserName = 'SqlAdmin' } + + $mockSqlAdministratorPasswordPlain = $env:SqlServerDsc_CI_SqlPassword + if (-not $mockSqlAdministratorPasswordPlain) { $mockSqlAdministratorPasswordPlain = 'P@ssw0rd1' } + $mockSqlAdministratorPassword = ConvertTo-SecureString -String $mockSqlAdministratorPasswordPlain -AsPlainText -Force
152-166: Avoid cross-test coupling (“persistent” permission).Leaving CreateEndpoint granted for “other tests” makes order matter. Either gate this behind the same keep-running flag or create/verify within those tests.
tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1 (1)
45-48: Avoid cross‑test data dependencies (login/role).Relying on logins/roles “created by earlier integration tests” couples test order. Either ensure they exist here (create if missing) or fail with an explicit precondition check so the test can run standalone.
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1 (1)
179-181: Add -ErrorAction Stop to WhatIf invocation for consistency with integration test guidelinesKeeps failures surfacing immediately even in simulation.
- Set-SqlDscStartupParameter -ServerName $script:mockServerName -InstanceName $script:mockInstanceName -TraceFlag @(9999) -WhatIf + Set-SqlDscStartupParameter -ServerName $script:mockServerName -InstanceName $script:mockInstanceName -TraceFlag @(9999) -WhatIf -ErrorAction 'Stop'tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1 (1)
63-67: Add an assertion or merge with the next test.This It block performs actions without asserting outcomes. Either assert the revoke result here or remove and rely on the following test.
Possible tweak:
It 'Should revoke permissions' { $loginObject = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -ErrorAction 'Stop' $null = Revoke-SqlDscServerPermission -Login $loginObject -Permission @('ViewServerState') -Force -ErrorAction 'Stop' + $result = Test-SqlDscServerPermission -Login $loginObject -Grant -Permission @('ViewServerState') -ErrorAction 'Stop' + $result | Should -BeFalse }tests/Integration/Commands/Enable-SqlDscLogin.Integration.Tests.ps1 (3)
43-50: Add -ErrorAction 'Stop' to connect/disconnect so failures surface immediately.Keeps failures deterministic per test guidelines.
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' @@ - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'
56-100: Assign unused returns to $null inside It/BeforeEach blocks.Aligns with test hygiene rule: assign unused outputs unless part of a pipeline.
- Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' + $null = Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' @@ - Enable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' + $null = Enable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' @@ - Enable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -Force -ErrorAction 'Stop' + $null = Enable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -Force -ErrorAction 'Stop' @@ - Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' + $null = Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' @@ - Enable-SqlDscLogin -LoginObject $loginObject -Force -ErrorAction 'Stop' + $null = Enable-SqlDscLogin -LoginObject $loginObject -Force -ErrorAction 'Stop'
34-53: If perf is required, gate service teardown via CI flag (requires guideline update).If you want to keep the service hot in CI, propose an opt-in env flag and an ADR/guideline change; otherwise keep per-test start/stop.
Example pattern (optional, pending consensus):
- Stop-Service -Name $script:serviceName -Force -ErrorAction 'Stop' + if (-not $env:SQLDSC_KEEP_SERVICE_RUNNING) + { + Stop-Service -Name $script:serviceName -Force -ErrorAction 'Stop' + }Want me to open an issue to discuss updating the integration test guideline and CI bootstrap to manage the service once per job?
tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1 (3)
43-43: Add -ErrorAction Stop to Connect/Disconnect for fail‑fast behavior.Ensures failures surface immediately as required for integration tests.
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' @@ - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'Also applies to: 47-47
119-119: Avoid SilentlyContinue on Get-SqlDscDatabasePermission; prefer Stop.Silencing errors can mask genuine failures; the subsequent null/empty checks handle “no data” scenarios.
- $databasePermissionInfo = Get-SqlDscDatabasePermission -ServerObject $script:serverObject -DatabaseName 'master' -Name 'guest' -ErrorAction 'SilentlyContinue' + $databasePermissionInfo = Get-SqlDscDatabasePermission -ServerObject $script:serverObject -DatabaseName 'master' -Name 'guest' -ErrorAction 'Stop' @@ - $databasePermissionInfo = Get-SqlDscDatabasePermission -ServerObject $script:serverObject -DatabaseName 'msdb' -Name 'dbo' -ErrorAction 'SilentlyContinue' + $databasePermissionInfo = Get-SqlDscDatabasePermission -ServerObject $script:serverObject -DatabaseName 'msdb' -Name 'dbo' -ErrorAction 'Stop'Also applies to: 143-143
38-42: Confirm CI credential source (avoid hardcoding if a helper exists).If DscResource.Test exposes a helper for CI SQL credentials, prefer that over inline 'SqlAdmin'/'P@ssw0rd1'. Otherwise, consider env‑based injection.
I can search the repo for an existing credential helper and update this file accordingly.
tests/Integration/Commands/Set-SqlDscDatabaseDefault.Integration.Tests.ps1 (3)
65-71: Move DB cleanup to AfterAll and gate service stop; remove cleanup ItCleanup should be in AfterAll (not an It) per guidelines, and stopping the service should be conditional to preserve the perf optimization.
Apply these diffs:
@@ - # Disconnect from the database engine. - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject - - # Stopping the named instance SQL Server service after running tests. - Stop-Service -Name 'MSSQL$DSCSQLTEST' -Verbose -ErrorAction 'Stop' + # Remove test database, then disconnect. + $null = Remove-SqlDscDatabase -ServerObject $script:serverObject -Name $script:testDatabaseName -Force -ErrorAction 'Stop' + + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop' + + if (-not $script:keepServiceRunning) + { + Stop-Service -Name 'MSSQL$DSCSQLTEST' -ErrorAction 'Stop' + }- Context 'After all tests are completed' { - It 'Should clean up the test database' { - $null = Remove-SqlDscDatabase -ServerObject $script:serverObject -Name $script:testDatabaseName -Force - } - } + # (Removed) Cleanup moved to AfterAll for reliability and to align with guidelines.Also applies to: 119-124
37-38: Remove unused variable$script:mockComputerName is declared but never used.
Apply this diff:
- $script:mockComputerName = Get-ComputerName -
39-43: Credential handling approved; standardize MSSQL service gatingConvertTo-SecureString -AsPlainText is acceptable confined to tests (do not move to production). Verification found multiple integration tests stopping 'MSSQL$DSCSQLTEST' (e.g., Test-SqlDscIsAgentOperator, Set-SqlDscDatabaseDefault, Set-/Remove-/Enable-/Disable-SqlDscAgentOperator) and Connect-SqlDscDatabaseEngine.Integration.Tests.ps1 starts/stops 'MSSQLSERVER' — standardize on a single gating pattern (SqlServerDscKeepService) or a consistent Start/Stop approach across tests.
tests/Integration/Commands/Connect-SqlDscDatabaseEngine.Integration.Tests.ps1 (1)
37-47: Remove commented-out code per repo policy (no commented-out code).These blocks add noise; prefer deleting. If needed later, rely on git history.
- # $previouslyErrorViewPreference = $ErrorView - # $ErrorView = 'DetailedView' - # $Error.Clear() @@ - # AfterAll { - # $ErrorView = $previouslyErrorViewPreference - - # Write-Verbose -Message ('Error count: {0}' -f $Error.Count) -Verbose - # Write-Verbose -Message ($Error | Out-String) -Verbose - # }tests/Integration/Commands/New-SqlDscRole.Integration.Tests.ps1 (3)
44-45: Surface connection failures immediately.Add -ErrorAction Stop to Connect-SqlDscDatabaseEngine to fail fast in CI.
Apply:
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop'
37-37: Remove unused variable.$script:mockComputerName is never used. Drop it.
Apply:
- $script:mockComputerName = Get-ComputerName
169-172: Avoid ExpectedMessage in Throw assertions.Per test guidelines, don’t assert on error messages; assert throw only (or use -ErrorType/-ErrorId if stable). Drop -ExpectedMessage.
Apply:
- { New-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole -Force -ErrorAction 'Stop' } | - Should -Throw -ExpectedMessage "*already exists*" + { New-SqlDscRole -ServerObject $script:serverObject -Name $script:persistentTestRole -Force -ErrorAction 'Stop' } | + Should -Throwtests/Integration/Commands/New-SqlDscDatabase.Integration.Tests.ps1 (1)
106-121: Move cleanup out of It; avoid try/finally per test guidelines.Use BeforeEach/AfterEach for setup/teardown instead of try/finally inside It.
- It 'Should refresh the database collection before creating' { - $uniqueName = 'SqlDscTestRefresh_' + (Get-Random) - - try - { - $result = New-SqlDscDatabase -ServerObject $script:serverObject -Name $uniqueName -Refresh -Force -ErrorAction Stop - $result | Should -Not -BeNullOrEmpty - $result.Name | Should -Be $uniqueName - } - finally - { - # Clean up - $dbToRemove = Get-SqlDscDatabase -ServerObject $script:serverObject -Name $uniqueName -ErrorAction 'SilentlyContinue' - if ($dbToRemove) - { - $null = Remove-SqlDscDatabase -DatabaseObject $dbToRemove -Force - } - } - } + BeforeEach { + $script:uniqueName = 'SqlDscTestRefresh_' + (Get-Random) + } + + AfterEach { + $dbToRemove = Get-SqlDscDatabase -ServerObject $script:serverObject -Name $script:uniqueName -ErrorAction 'SilentlyContinue' + if ($dbToRemove) + { + $null = Remove-SqlDscDatabase -DatabaseObject $dbToRemove -Force -ErrorAction Stop + } + } + + It 'Should refresh the database collection before creating' { + $result = New-SqlDscDatabase -ServerObject $script:serverObject -Name $script:uniqueName -Refresh -Force -ErrorAction Stop + $result | Should -Not -BeNullOrEmpty + $result.Name | Should -Be $script:uniqueName + }tests/Integration/Commands/Set-SqlDscAgentAlert.Integration.Tests.ps1 (2)
69-69: Fix indentation (4‑space levels).Extra indentation here; align with surrounding 8‑space body indent.
- # Add SQL Server system message for testing message ID alerts + # Add SQL Server system message for testing message ID alerts
51-56: Optional: Clean up the test system message (avoid state leakage).Not required for idempotency (you guarded creation), but dropping the message keeps the environment tidy for adjacent tests.
AfterAll { $null = $script:sqlServerObject | Remove-SqlDscAgentAlert -Name 'IntegrationTest_UpdateAlert' -Force -ErrorAction 'SilentlyContinue' + # Clean up the test system message if present + $dropMessageQuery = @' +IF EXISTS (SELECT 1 FROM sys.messages WHERE message_id = 50003 AND language_id = 1033) +BEGIN + EXECUTE sp_dropmessage 50003, 'us_english'; +END +'@ + $script:sqlServerObject | Invoke-SqlDscQuery -DatabaseName 'master' -Query $dropMessageQuery -ErrorAction 'Stop' + # Disconnect from the SQL Server Disconnect-SqlDscDatabaseEngine -ServerObject $script:sqlServerObjecttests/Integration/Commands/Set-SqlDscServerPermission.Integration.Tests.ps1 (1)
34-34: Style nit: prefer [Type]::new() over New-Object for .NET typesNot blocking, but repository guidelines prefer ::new(). Consider updating ServerPermissionSet constructions.
Example:
- $permissionSet = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.ServerPermissionSet' + $permissionSet = [Microsoft.SqlServer.Management.Smo.ServerPermissionSet]::new()tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1 (1)
37-37: Order dependency risk: asserts service Running without local ensureIf this suite is executed standalone, it will fail. Add a lightweight guard to start the service if present but stopped, or explicitly document enforced run order in CI.
Example:
BeforeAll { Write-Verbose -Message ('Running integration test as user ''{0}''.' -f $env:UserName) -Verbose - # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # Prefer pre-running service for perf; if executed standalone, ensure it's running. + $svc = Get-Service -Name 'MSSQL$DSCSQLTEST' -ErrorAction 'SilentlyContinue' + if ($svc -and $svc.Status -ne 'Running') { Start-Service -Name 'MSSQL$DSCSQLTEST' -ErrorAction 'Stop' } }Please confirm CI enforces Install -> Uninstall ordering for this group.
tests/Integration/Commands/Test-SqlDscAgentAlertProperty.Integration.Tests.ps1 (1)
37-37: Service lifecycle deviates from stated test guideline; pick one approach and enforce globallyCurrent note to "assume running/leave running" conflicts with our guideline that integration tests start the service in BeforeAll and stop it in AfterAll. Either update the guideline and add a one‑time suite/service manager, or keep start/stop in each file. At minimum, make tests defensive by ensuring the service is running.
Apply this minimal defensive addition in BeforeAll:
- # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # Note: Prefer keeping service running across tests for perf; ensure it's running defensively + $svc = Get-Service -Name 'MSSQL$DSCSQLTEST' -ErrorAction 'SilentlyContinue' + if (-not $svc -or $svc.Status -ne 'Running') + { + Start-Service -Name 'MSSQL$DSCSQLTEST' -ErrorAction 'Stop' + }Also applies to: 60-60
tests/Integration/Commands/Set-SqlDscAgentOperator.Integration.Tests.ps1 (1)
44-44: Minor: env var cleanup is good; consider centralizing to a single suite bootstrapNon‑blocking: you set/remove $env:SqlServerDscCI here; consider moving CI/test-session concerns (env vars, one‑time start/stop) into a central test bootstrap to cut duplication.
tests/Integration/Commands/Install-SqlDscServer.Integration.Tests.ps1 (1)
41-151: Refactor away from Should -Not -Throw per test guidelines (non‑blocking)Prefer calling the command directly with -ErrorAction Stop; assertions can then focus on outcomes, not absence of exceptions.
Example for one It:
- It 'Should run the command without throwing' { - { - Install-SqlDscServer @installSqlDscServerParameters - } | Should -Not -Throw - } + It 'Should install without errors' { + Install-SqlDscServer @installSqlDscServerParameters + }tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1 (2)
43-43: Add -ErrorAction Stop to Connect-SqlDscDatabaseEngineGuideline: use -ErrorAction Stop in integration tests so failures surface immediately.
Apply:
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop'
83-83: Add -ErrorAction Stop to state‑changing Set‑SqlDscConfigurationOption callsEnsure configuration failures fail fast inside tests.
Apply:
- Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force + Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force -ErrorAction 'Stop'- Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force + Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force -ErrorAction 'Stop'- Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force + Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force -ErrorAction 'Stop'- Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force + Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force -ErrorAction 'Stop'Also applies to: 96-96, 162-162, 171-171
tests/Integration/Commands/Get-SqlDscLogin.Integration.Tests.ps1 (1)
53-61: Use -ErrorAction Stop on command invocationsAdd -ErrorAction 'Stop' to make failures surface immediately (repo rule).
- $result = Get-SqlDscLogin -ServerObject $script:serverObject + $result = Get-SqlDscLogin -ServerObject $script:serverObject -ErrorAction 'Stop' @@ - $result = Get-SqlDscLogin -ServerObject $script:serverObject + $result = Get-SqlDscLogin -ServerObject $script:serverObject -ErrorAction 'Stop' @@ - $result = Get-SqlDscLogin -ServerObject $script:serverObject -Name 'sa' + $result = Get-SqlDscLogin -ServerObject $script:serverObject -Name 'sa' -ErrorAction 'Stop' @@ - $resultWithoutRefresh = Get-SqlDscLogin -ServerObject $script:serverObject - $resultWithRefresh = Get-SqlDscLogin -ServerObject $script:serverObject -Refresh + $resultWithoutRefresh = Get-SqlDscLogin -ServerObject $script:serverObject -ErrorAction 'Stop' + $resultWithRefresh = Get-SqlDscLogin -ServerObject $script:serverObject -Refresh -ErrorAction 'Stop' @@ - $result = $script:serverObject | Get-SqlDscLogin -Name 'sa' + $result = $script:serverObject | Get-SqlDscLogin -Name 'sa' -ErrorAction 'Stop'Also applies to: 64-67, 71-77, 93-97, 101-106
tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1 (1)
37-39: Comment may be misleading hereThis test exercises WMI-managed computer APIs and does not require the DB engine to be running. Consider removing or clarifying the “service already running” note to avoid implying an ordering dependency.
tests/Integration/Commands/Disable-SqlDscAudit.Integration.Tests.ps1 (1)
48-51: Add -ErrorAction Stop on disconnectMinor consistency fix.
- Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'tests/Integration/Commands/Get-SqlDscServerPermission.Integration.Tests.ps1 (2)
43-44: Add -ErrorAction Stop on Connect/DisconnectMatch repo rule for integration tests.
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' @@ - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'Also applies to: 46-50
55-66: Add -ErrorAction Stop to command invocations (apply across file)Many Get-SqlDscServerPermission/Get-SqlDscLogin/Get-SqlDscRole calls lack -ErrorAction 'Stop'. Add it to surface failures immediately.
Example edits (apply pattern everywhere in this file):
- $result = Get-SqlDscServerPermission -ServerObject $script:serverObject -Name 'sa' + $result = Get-SqlDscServerPermission -ServerObject $script:serverObject -Name 'sa' -ErrorAction 'Stop' @@ - $result = $script:serverObject | Get-SqlDscServerPermission -Name 'sa' + $result = $script:serverObject | Get-SqlDscServerPermission -Name 'sa' -ErrorAction 'Stop' @@ - $result = Get-SqlDscRole -ServerObject $script:serverObject -Name 'public' + $result = Get-SqlDscRole -ServerObject $script:serverObject -Name 'public' -ErrorAction 'Stop'Also applies to: 71-76, 79-83, 88-92, 95-104, 109-116, 120-128, 134-135, 157-168, 171-175, 188-191, 194-197, 202-207, 210-215, 221-224, 232-236, 243-246, 249-254, 265-274, 279-287, 296-312
tests/Integration/Commands/Get-SqlDscDatabase.Integration.Tests.ps1 (2)
44-45: Add -ErrorAction Stop on Connect/DisconnectMatch integration test rules.
- $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential + $script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop' @@ - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'Also applies to: 47-51
55-63: Use -ErrorAction Stop on Get-SqlDscDatabase callsAdd -ErrorAction 'Stop' for consistency and faster failure surfacing.
- $result = Get-SqlDscDatabase -ServerObject $script:serverObject + $result = Get-SqlDscDatabase -ServerObject $script:serverObject -ErrorAction 'Stop' @@ - $result = Get-SqlDscDatabase -ServerObject $script:serverObject + $result = Get-SqlDscDatabase -ServerObject $script:serverObject -ErrorAction 'Stop' @@ - $result = Get-SqlDscDatabase -ServerObject $script:serverObject -Name 'master' + $result = Get-SqlDscDatabase -ServerObject $script:serverObject -Name 'master' -ErrorAction 'Stop' @@ - $result = Get-SqlDscDatabase -ServerObject $script:serverObject -Refresh + $result = Get-SqlDscDatabase -ServerObject $script:serverObject -Refresh -ErrorAction 'Stop'Also applies to: 66-72, 77-82, 98-102
tests/Integration/Commands/Get-SqlDscManagedComputerInstance.Integration.Tests.ps1 (1)
43-45: AfterAll is a no-op; either remove it or optionally restore local state only when we started the serviceTo keep CI fast while being polite in local runs, you can (optionally) stop the service only if this test started it.
Optional follow-up inside AfterAll:
- # Note: SQL Server service is left running for subsequent tests for performance optimization + # Leave running in CI for perf; restore local state if we started it. + if (-not $script:originalServiceWasRunning -and $env:GITHUB_ACTIONS -ne 'true') + { + Stop-Service -Name $script:instanceServiceName -Force -ErrorAction 'Stop' + Write-Verbose -Message ("Stopped service '{0}' to restore local state." -f $script:instanceServiceName) -Verbose + }If you don’t want local restoration, consider removing the empty AfterAll block to avoid confusion.
tests/Integration/Commands/Disable-SqlDscLogin.Integration.Tests.ps1 (3)
66-66: Assign unused returns to $null inside It blocks.Keep output clean and follow test guidelines.
- Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' + $null = Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Force -ErrorAction 'Stop' @@ - Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -Force -ErrorAction 'Stop' + $null = Disable-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -Force -ErrorAction 'Stop' @@ - Disable-SqlDscLogin -LoginObject $loginObject -Force -ErrorAction 'Stop' + $null = Disable-SqlDscLogin -LoginObject $loginObject -Force -ErrorAction 'Stop'Also applies to: 75-75, 100-100
62-62: Add -ErrorAction 'Stop' to Get-SqlDscLogin (and use -Refresh for initial read).Surface failures immediately and avoid stale objects.
- $loginBefore = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName + $loginBefore = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -ErrorAction 'Stop' @@ - $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh + $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -ErrorAction 'Stop' @@ - $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh + $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -ErrorAction 'Stop' @@ - $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh + $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -ErrorAction 'Stop' @@ - $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh + $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -ErrorAction 'Stop' @@ - $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh + $loginAfter = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testLoginName -Refresh -ErrorAction 'Stop'Also applies to: 69-69, 78-78, 87-87, 103-103, 113-113
38-38: Polish comment grammar.- $mockSqlAdministratorUserName = 'SqlAdmin' # Using computer name as NetBIOS name throw exception. + $mockSqlAdministratorUserName = 'SqlAdmin' # Using the computer name as the NetBIOS name throws an exception.
tests/Integration/Commands/Connect-SqlDscDatabaseEngine.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
Show resolved
Hide resolved
tests/Integration/Commands/Disable-SqlDscLogin.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/Set-SqlDscAgentAlert.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/Set-SqlDscConfigurationOption.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/Set-SqlDscDatabase.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/Set-SqlDscDatabase.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
tests/Integration/Commands/Set-SqlDscDatabaseDefault.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
…erver service state - Removed comments indicating that the SQL Server service is already running for performance optimization from multiple integration test files. - Cleaned up AfterAll sections to enhance readability and maintainability of the test scripts.
…SQL Server instance
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1 (1)
110-132: Replace try/finally cleanup with AfterEach per guidelinesTests should not use try/finally for cleanup; rely on AfterEach. Reuse $script:testAuditName so existing AfterEach removes the audit.
- $securityLogAuditName = 'SqlDscTestAudit_SecLog_' + (Get-Random) - - try - { - $result = New-SqlDscAudit -ServerObject $script:serverObject -Name $securityLogAuditName -LogType 'SecurityLog' -PassThru -Force -ErrorAction Stop - - $result | Should -Not -BeNullOrEmpty - $result.Name | Should -Be $securityLogAuditName - $result.DestinationType | Should -Be 'SecurityLog' - - # Verify the audit exists in the server - $createdAudit = Get-SqlDscAudit -ServerObject $script:serverObject -Name $securityLogAuditName -ErrorAction Stop - $createdAudit | Should -Not -BeNullOrEmpty - $createdAudit.DestinationType | Should -Be 'SecurityLog' - } - finally - { - # Clean up - $auditToRemove = Get-SqlDscAudit -ServerObject $script:serverObject -Name $securityLogAuditName -ErrorAction 'SilentlyContinue' - if ($auditToRemove) - { - Remove-SqlDscAudit -AuditObject $auditToRemove -Force -ErrorAction 'SilentlyContinue' - } - } + # Use shared AfterEach cleanup by reusing $script:testAuditName + $script:testAuditName = 'SqlDscTestAudit_SecLog_' + (Get-Random) + + $result = New-SqlDscAudit -ServerObject $script:serverObject -Name $script:testAuditName -LogType 'SecurityLog' -PassThru -Force -ErrorAction Stop + + $result | Should -Not -BeNullOrEmpty + $result.Name | Should -Be $script:testAuditName + $result.DestinationType | Should -Be 'SecurityLog' + + # Verify the audit exists in the server + $createdAudit = Get-SqlDscAudit -ServerObject $script:serverObject -Name $script:testAuditName -ErrorAction Stop + $createdAudit | Should -Not -BeNullOrEmpty + $createdAudit.DestinationType | Should -Be 'SecurityLog'Additionally, DB‑requiring integration tests must ensure the service is running in BeforeAll and stop it in AfterAll (only if this test started it) to avoid order coupling while keeping the PR’s perf goal:
# Add to BeforeAll (before Connect-SqlDscDatabaseEngine) $script:serviceName = 'MSSQL$DSCSQLTEST' $script:startedService = $false $service = Get-Service -Name $script:serviceName -ErrorAction Stop if ($service.Status -ne 'Running') { Start-Service -Name $script:serviceName -ErrorAction Stop $service.WaitForStatus('Running', (New-TimeSpan -Minutes 2)) $script:startedService = $true } # Add to AfterAll (after Disconnect-SqlDscDatabaseEngine) if ($script:startedService) { Stop-Service -Name $script:serviceName -Force -ErrorAction SilentlyContinue (Get-Service -Name $script:serviceName).WaitForStatus('Stopped', (New-TimeSpan -Minutes 2)) }
🧹 Nitpick comments (3)
tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1 (2)
50-50: Minor: Unnecessary blank lines.These blank lines appear to be formatting artifacts and don't add value to the test structure.
- $emptyCollection = [Microsoft.SqlServer.Management.Smo.DatabasePermissionInfo[]] @() - + $emptyCollection = [Microsoft.SqlServer.Management.Smo.DatabasePermissionInfo[]] @()- $emptyCollection = [Microsoft.SqlServer.Management.Smo.DatabasePermissionInfo[]] @() - + $emptyCollection = [Microsoft.SqlServer.Management.Smo.DatabasePermissionInfo[]] @()Also applies to: 58-58
76-76: Minor: Unnecessary blank lines in test blocks.These blank lines don't follow the established pattern and don't improve readability within the assertion blocks.
# Validate the result structure $result | Should -Not -BeNullOrEmpty - # Each result should have State and Permission properties$permission.State | Should -Not -BeNullOrEmpty $permission.Permission | Should -Not -BeNullOrEmpty - # Validate that permission state is one of the expected valuesApply similar changes to remove unnecessary blank lines at lines 98, 103, 122, 127, and 147.
Also applies to: 81-81, 98-98, 103-103, 122-122, 127-127, 147-147
tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1 (1)
37-37: Avoid cross-file service-state couplingThis test doesn’t require the DB Engine to be running. The comment suggests an order dependency on another test; that’s brittle. Reword to avoid implying a prerequisite.
- # Note: SQL Server service is already running from Install-SqlDscServer test for performance optimization + # This test does not depend on the Database Engine service state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
tests/Integration/Commands/Assert-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1(5 hunks)tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Disable-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Disable-SqlDscAudit.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Disable-SqlDscLogin.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Enable-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Enable-SqlDscLogin.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscAgentAlert.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Get-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscConfigurationOption.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscDatabase.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscLogin.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Get-SqlDscManagedComputerInstance.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Get-SqlDscManagedComputerService.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscServerPermission.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Get-SqlDscServerProtocol.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/New-SqlDscAgentAlert.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/New-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1(3 hunks)tests/Integration/Commands/New-SqlDscDatabase.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/New-SqlDscRole.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Remove-SqlDscAgentAlert.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Remove-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Remove-SqlDscAudit.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Remove-SqlDscDatabase.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Remove-SqlDscRole.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Set-SqlDscAgentAlert.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Set-SqlDscAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Set-SqlDscAudit.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Set-SqlDscConfigurationOption.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Set-SqlDscDatabase.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Set-SqlDscDatabaseDefault.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Set-SqlDscDatabasePermission.Integration.Tests.ps1(2 hunks)tests/Integration/Commands/Set-SqlDscServerPermission.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1(2 hunks)tests/Integration/Commands/Set-SqlDscTraceFlag.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Test-SqlDscAgentAlertProperty.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Test-SqlDscDatabase.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Test-SqlDscIsAgentAlert.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Test-SqlDscIsAgentOperator.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Test-SqlDscIsLogin.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Test-SqlDscIsLoginEnabled.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Test-SqlDscIsRole.Integration.Tests.ps1(0 hunks)tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1(0 hunks)
💤 Files with no reviewable changes (36)
- tests/Integration/Commands/Get-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscDatabaseDefault.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscDatabase.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscIsRole.Integration.Tests.ps1
- tests/Integration/Commands/Remove-SqlDscDatabase.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscServerPermission.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscRole.Integration.Tests.ps1
- tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
- tests/Integration/Commands/Assert-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/New-SqlDscRole.Integration.Tests.ps1
- tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
- tests/Integration/Commands/New-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscDatabase.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscConfigurationOption.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscAudit.Integration.Tests.ps1
- tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
- tests/Integration/Commands/Remove-SqlDscRole.Integration.Tests.ps1
- tests/Integration/Commands/New-SqlDscDatabase.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscIsAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscTraceFlag.Integration.Tests.ps1
- tests/Integration/Commands/Disable-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/Enable-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscIsLoginEnabled.Integration.Tests.ps1
- tests/Integration/Commands/Disable-SqlDscLogin.Integration.Tests.ps1
- tests/Integration/Commands/Remove-SqlDscAudit.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscConfigurationOption.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscLogin.Integration.Tests.ps1
- tests/Integration/Commands/Enable-SqlDscLogin.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscDatabase.Integration.Tests.ps1
- tests/Integration/Commands/Remove-SqlDscAgentOperator.Integration.Tests.ps1
- tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1
🚧 Files skipped from review as they are similar to previous changes (13)
- tests/Integration/Commands/Get-SqlDscManagedComputerService.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscAgentAlert.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscServerProtocol.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscAgentAlertProperty.Integration.Tests.ps1
- tests/Integration/Commands/New-SqlDscAgentAlert.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscDatabasePermission.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscAgentAlert.Integration.Tests.ps1
- tests/Integration/Commands/Disable-SqlDscAudit.Integration.Tests.ps1
- tests/Integration/Commands/Get-SqlDscManagedComputerInstance.Integration.Tests.ps1
- tests/Integration/Commands/Remove-SqlDscAgentAlert.Integration.Tests.ps1
- tests/Integration/Commands/Set-SqlDscServerPermission.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscIsAgentAlert.Integration.Tests.ps1
- tests/Integration/Commands/Test-SqlDscIsLogin.Integration.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (11)
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
**/*.{ps1,psm1}: Public PowerShell commands must use the naming format: Verb-SqlDscNoun
Private PowerShell functions must use the naming format: Verb-Noun
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place newline before opening brace (except variable assignments)
One newline after opening brace
Two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Wr...
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
tests/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
tests/Integration/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
tests/Integration/**/*.ps1: Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
tests/[iI]ntegration/Commands/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: Do not use mocking in integration tests; run against a real environment
In CI, use Get-ComputerName for computer names
Avoid using ExpectedMessage with Should -Throw assertions
When invoking commands in integration tests, pass -Force where applicable to avoid prompts
Use -ErrorAction Stop on commands so failures surface immediately
At the top of each integration test file, include SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments') with an empty justification parameter and param ()
In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Catch [System.IO.FileNotFoundException] during setup and throw: "DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first."
In BeforeAll, set $script:moduleName and Import-Module -Name $script:moduleName -Force -ErrorAction Stop
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).- Use
-ErrorAction Stopon commands so failures surface immediatelyRequired Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Place integration tests for public commands in tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Add integration tests for all public commands (and resources)
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/**/*.ps1 : When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Unit/**/*.ps1 : Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/**/*.ps1 : When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Stubs/SMO.cs : After changing SMO stub types, run tests in a new PowerShell session for changes to take effect
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/Commands/*.[iI]ntegration.[tT]ests.ps1 : Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands so failures surface immediately
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : At the top of each integration test file, include SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments') with an empty justification parameter and param ()
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Perform mocking in BeforeAll (use BeforeEach only when required)
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeAll, set $script:moduleName and Import-Module -Name $script:moduleName -Force -ErrorAction Stop
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Do not use mocking in integration tests; run against a real environment
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Unit/**/*.ps1 : In unit tests, use SMO stub types from tests/Stubs/SMO.cs; never mock real SMO types
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:34:27.319Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-09-16T16:34:27.319Z
Learning: Applies to **/*.{ps1,psm1} : Place $null on the left side of comparisons
Applied to files:
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In CI, use Get-ComputerName for computer names
Applied to files:
tests/Integration/Commands/Get-SqlDscManagedComputer.Integration.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use BeforeEach and AfterEach sparingly
Applied to files:
tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Applied to files:
tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (5)
tests/Integration/Commands/ConvertTo-SqlDscDatabasePermission.Integration.Tests.ps1 (2)
33-42: Service management inconsistency with established guidelines.Based on the retrieved learnings and the PR context, this test file has removed SQL service management (
Start-ServiceandStop-ServiceforMSSQL$DSCSQLTEST) that was previously required. While the PR aims to optimize performance by assuming the service is already running, this creates a dependency on external test setup and violates the established integration test guidelines that require explicit service lifecycle management.The existing comment from the previous review correctly identified this issue. The current approach couples tests together and reduces test isolation, which can lead to flakiness when tests are run individually or in different orders.
32-157: Test structure and logic look solid.The integration tests properly:
- Use the correct instance name
DSCSQLTESTas per guidelines- Connect and disconnect from SQL Server properly
- Test both parameter and pipeline input methods
- Include comprehensive validation of the conversion functionality
- Handle cases where permission data might not be available
- Test edge cases like empty collections and different permission states
The test implementation follows Pester v5 syntax and adheres to the coding guidelines for test structure.
tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1 (2)
54-56: Cleanup query looks goodThe filtered cleanup for test audits is fine and scoped. No issues.
42-50: Verify CI credentials sourceHardcoded "SqlAdmin"/"P@ssw0rd1" is present across integration tests and resources (e.g. tests/Integration/Commands/New-SqlDscAudit.Integration.Tests.ps1; tests/Integration/Resources/README.md and many DSC_*.config.ps1 files). Confirm DSCSQLTEST in CI uses these credentials or that the pipeline injects/overrides them via secrets/env vars — otherwise update tests to read credentials from pipeline secrets or document the required CI setup.
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1 (1)
182-182: No action needed.The added blank line is fine and within formatting guidelines.
tests/Integration/Commands/Set-SqlDscStartupParameter.Integration.Tests.ps1
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1 (2)
32-60: Service lifecycle: add a guard or update guidelines to avoid brittle tests.This test assumes MSSQL$DSCSQLTEST is already running. Our guidelines require starting in BeforeAll and stopping in AfterAll for DB‑backed integration tests. To keep the PR’s perf win while maintaining reliability for local runs/partial CI, add a lightweight guard that either starts the service when explicitly allowed or fails early with a clear message. Also mirror a conditional stop in AfterAll. Example:
Describe 'Invoke-SqlDscQuery' -Tag @('Integration_SQL2017', 'Integration_SQL2019', 'Integration_SQL2022') { BeforeAll { $script:mockInstanceName = 'DSCSQLTEST' $script:mockComputerName = Get-ComputerName + # Ensure SQL Server service is running (guard for local/dev runs). + $script:serviceName = "MSSQL`$$script:mockInstanceName" + $service = Get-Service -Name $script:serviceName -ErrorAction 'SilentlyContinue' + if (-not $service) { + throw "SQL Server service '$($script:serviceName)' not found." + } + if ($env:SqlServerDscCI_ManageService -eq '1') { + if ($service.Status -ne 'Running') { + Start-Service -Name $script:serviceName -ErrorAction 'Stop' + (Get-Service -Name $script:serviceName).WaitForStatus('Running', '00:00:30') + } + } + elseif ($service.Status -ne 'Running') { + throw "SQL Server service '$($script:serviceName)' is not running. Start it before running integration tests." + }And in AfterAll:
AfterAll { # Clean up test database if ($script:testDatabaseName) { try { Remove-SqlDscDatabase -ServerObject $script:serverObject -Name $script:testDatabaseName -Force -ErrorAction 'Stop' } catch { Write-Warning -Message "Failed to remove test database '$($script:testDatabaseName)': $($_.Exception.Message)" } } - Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop' + + if ($env:SqlServerDscCI_ManageService -eq '1') { + try { + Stop-Service -Name $script:serviceName -ErrorAction 'Stop' + (Get-Service -Name $script:serviceName).WaitForStatus('Stopped', '00:00:30') + } + catch { + Write-Warning -Message "Failed to stop SQL Server service '$($script:serviceName)': $($_.Exception.Message)" + } + } }If the new repo‑wide policy is to keep DSCSQLTEST running, please also update the written guidelines accordingly in a follow‑up PR.
81-85: Replace all{ ... } | Should -Not -Throwin tests/Integration with direct invocationPer Pester v5, remove non‑throw assertions; invoke the command directly (or assign to $null to discard output). Example:
{ Invoke-SqlDscQuery ... } | Should -Not -Throw → $null = Invoke-SqlDscQuery ...
🧹 Nitpick comments (2)
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1 (2)
100-100: Use type literal with BeOfType for clarity.Prefer passing a type rather than a string to BeOfType.
-$result | Should -BeOfType 'System.Data.DataSet' +$result | Should -BeOfType ([System.Data.DataSet])Also applies to: 107-107, 152-152, 208-208, 217-217
76-77: Add -ErrorAction Stop on Disconnect to surface failures.Keep error handling consistent so cleanup failures fail the test.
- Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
**/*.{ps1,psm1}: Public PowerShell commands must use the naming format: Verb-SqlDscNoun
Private PowerShell functions must use the naming format: Verb-Noun
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place newline before opening brace (except variable assignments)
One newline after opening brace
Two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Wr...
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
tests/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
tests/Integration/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
tests/Integration/**/*.ps1: Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
tests/[iI]ntegration/Commands/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: Do not use mocking in integration tests; run against a real environment
In CI, use Get-ComputerName for computer names
Avoid using ExpectedMessage with Should -Throw assertions
When invoking commands in integration tests, pass -Force where applicable to avoid prompts
Use -ErrorAction Stop on commands so failures surface immediately
At the top of each integration test file, include SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments') with an empty justification parameter and param ()
In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Catch [System.IO.FileNotFoundException] during setup and throw: "DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first."
In BeforeAll, set $script:moduleName and Import-Module -Name $script:moduleName -Force -ErrorAction Stop
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).- Use
-ErrorAction Stopon commands so failures surface immediatelyRequired Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Place integration tests for public commands in tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Add integration tests for all public commands (and resources)
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/**/*.ps1 : When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Unit/**/*.ps1 : Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/**/*.ps1 : When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/Commands/*.[iI]ntegration.[tT]ests.ps1 : Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Stubs/SMO.cs : After changing SMO stub types, run tests in a new PowerShell session for changes to take effect
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : At the top of each integration test file, include SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments') with an empty justification parameter and param ()
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Perform mocking in BeforeAll (use BeforeEach only when required)
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeAll, set $script:moduleName and Import-Module -Name $script:moduleName -Force -ErrorAction Stop
Applied to files:
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (1)
tests/Integration/Commands/Invoke-SqlDscQuery.Integration.Tests.ps1 (1)
58-58: LGTM on whitespace tweak.The added newline improves readability around the here‑string terminator.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2291 +/- ##
====================================
Coverage 94% 94%
====================================
Files 149 149
Lines 9163 9163
====================================
Hits 8687 8687
Misses 476 476
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1 (2)
41-41: Add -ErrorAction Stop to Connect-SqlDscDatabaseEngine.Connection issues should terminate the test immediately.
-$script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential +$script:serverObject = Connect-SqlDscDatabaseEngine -InstanceName $script:mockInstanceName -Credential $script:mockSqlAdminCredential -ErrorAction 'Stop'
44-45: Use -ErrorAction Stop on all Get- calls used to set expectations.Avoids masking non-terminating errors and ensures deterministic failures.
- $script:originalAgentXPsValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' + $script:originalAgentXPsValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -ErrorAction 'Stop' @@ - $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' + $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -ErrorAction 'Stop' @@ - $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' + $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -ErrorAction 'Stop' @@ - $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'cost threshold for parallelism' + $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'cost threshold for parallelism' -ErrorAction 'Stop' @@ - $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'max degree of parallelism' + $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'max degree of parallelism' -ErrorAction 'Stop' @@ - $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' + $currentValue = Get-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -ErrorAction 'Stop'Also applies to: 60-60, 69-69, 112-112, 126-126, 142-142
🧹 Nitpick comments (3)
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1 (3)
54-55: Also fail fast on disconnect errors.Surface teardown failures instead of continuing.
- Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject + Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'
79-79: Assign unused returns to $null and add -ErrorAction Stop for Set- commands.Meets test style guidelines and fails on non-terminating errors.
- Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force + $null = Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force -ErrorAction 'Stop' @@ - Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force + $null = Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force -ErrorAction 'Stop' @@ - Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force + $null = Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 0 -Force -ErrorAction 'Stop' @@ - Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force + $null = Set-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value 1 -Force -ErrorAction 'Stop'Also applies to: 92-92, 158-158, 167-167
63-87: Optional: add -ErrorAction Stop to Test-SqlDscConfigurationOption invocations.Keeps behavior consistent with “fail fast” across all commands.
Example:
- $result = Test-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value $currentValue.RunValue + $result = Test-SqlDscConfigurationOption -ServerObject $script:serverObject -Name 'Agent XPs' -Value $currentValue.RunValue -ErrorAction 'Stop'Also applies to: 95-101, 115-121, 129-135, 145-151, 161-171
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/Integration/Commands/Set-SqlDscConfigurationOption.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Integration/Commands/Set-SqlDscConfigurationOption.Integration.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (11)
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
**/*.{ps1,psm1}: Public PowerShell commands must use the naming format: Verb-SqlDscNoun
Private PowerShell functions must use the naming format: Verb-Noun
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place newline before opening brace (except variable assignments)
One newline after opening brace
Two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Wr...
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
tests/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
tests/Integration/**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
tests/Integration/**/*.ps1: Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
tests/[iI]ntegration/Commands/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md)
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: Do not use mocking in integration tests; run against a real environment
In CI, use Get-ComputerName for computer names
Avoid using ExpectedMessage with Should -Throw assertions
When invoking commands in integration tests, pass -Force where applicable to avoid prompts
Use -ErrorAction Stop on commands so failures surface immediately
At the top of each integration test file, include SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments') with an empty justification parameter and param ()
In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Catch [System.IO.FileNotFoundException] during setup and throw: "DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first."
In BeforeAll, set $script:moduleName and Import-Module -Name $script:moduleName -Force -ErrorAction Stop
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).- Use
-ErrorAction Stopon commands so failures surface immediatelyRequired Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Place integration tests for public commands in tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Add integration tests for all public commands (and resources)
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/**/*.ps1 : When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests requiring SQL Server DB must start the Windows service in BeforeAll and stop it in AfterAll
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Unit/**/*.ps1 : Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : After Connect-SqlDscDatabaseEngine, integration tests must call Disconnect-SqlDscDatabaseEngine
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Integration/**/*.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to create SQL Server DB sessions
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/**/*.ps1 : When referencing CI SQL instances in tests, use: Database Engine=DSCSQLTEST, Reporting Services=SSRS, Power BI Report Server=PBIRS
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/Commands/*.[iI]ntegration.[tT]ests.ps1 : Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Use -ErrorAction Stop on commands so failures surface immediately
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeAll, set $script:moduleName and Import-Module -Name $script:moduleName -Force -ErrorAction Stop
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-14T19:16:56.215Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-14T19:16:56.215Z
Learning: Applies to tests/Stubs/SMO.cs : After changing SMO stub types, run tests in a new PowerShell session for changes to take effect
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : At the top of each integration test file, include SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments') with an empty justification parameter and param ()
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Perform mocking in BeforeAll (use BeforeEach only when required)
Applied to files:
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (2)
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1 (2)
49-52: Good guard: restore only when original value is present.The explicit null-check avoids throwing when the pre-read failed.
32-45: Ensure DSCSQLTEST SQL service is started in BeforeAll or by CIRelying on a pre-running DSCSQLTEST creates order/environment dependencies; add a lightweight BeforeAll guard to start MSSQL$DSCSQLTEST if needed (do not stop it in teardown), or update CI/bootstrap to start it once per job.
BeforeAll { $script:mockInstanceName = 'DSCSQLTEST' + # Ensure SQL service is running (start only if needed; do not stop in teardown) + $serviceName = 'MSSQL$DSCSQLTEST' + $service = Get-Service -Name $serviceName -ErrorAction 'Stop' + if ($service.Status -ne 'Running') + { + Start-Service -Name $serviceName -ErrorAction 'Stop' + } + $mockSqlAdministratorUserName = 'SqlAdmin' # Using computer name as NetBIOS name throw exception.Verification scan returned no matches for explicit MSSQL$DSCSQLTEST Start/Stop calls; confirm CI/bootstrap or run a repo-wide check.
tests/Integration/Commands/Test-SqlDscConfigurationOption.Integration.Tests.ps1
Show resolved
Hide resolved
…correct values for Agent XPs configuration option
…erver service state for named instance context
…scAgentAlert integration tests
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 1 of 57 files at r1, 40 of 54 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 2 of 2 files at r5, 1 of 1 files at r6, 1 of 1 files at r7, 11 of 11 files at r8, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @johlju)
1 participant
Pull Request (PR) description
for subsequent tests, significantly improving CI build times.
This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is