Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 30, 2025

  • Add new server permission management commands with object-based parameters
  • Create Grant-SqlDscServerPermission, Deny-SqlDscServerPermission, Revoke-SqlDscServerPermission commands
  • Implement Test-SqlDscServerPermission with parameter sets for Grant and Deny
  • Use Login/ServerRole objects instead of ServerObject and Name parameters
  • Create SqlServerPermission enum for type-safe permission handling
  • Update error handling to use $PSCmdlet.ThrowTerminatingError()
  • Remove unused $principalObject variables from all server permission commands
  • Remove unit test stub imports from integration tests to use real SqlServer module

Fixed integration tests by removing Import-Module statements for Unit/Stubs/SqlServer.psm1 and unit test specific $PSDefaultParameterValues from Grant-, Revoke-, and Test-SqlDscServerPermission integration tests. Integration tests now properly rely on the real SqlServer module and Connect-SqlDscDatabaseEngine as intended.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.


This change is Reviewable

Copilot AI changed the title [WIP] Based on this file @dsccommunity/SqlServerDsc/files/source/Classes/020.SqlPermission.ps1 I would like to create new public commands that does the getting (Get-SqlDscServerPermission already available), creation, testing, removing and setting of the o... Add new public commands for SQL Server permission management Aug 30, 2025
Copilot AI requested a review from johlju August 30, 2025 14:40
@codecov
Copy link

codecov bot commented Aug 30, 2025

Codecov Report

❌ Patch coverage is 97.47899% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 94%. Comparing base (5356e0c) to head (360c066).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
source/Public/Test-SqlDscServerPermission.ps1 93% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##           main   #2157    +/-   ##
=====================================
  Coverage    94%     94%            
=====================================
  Files       127     131     +4     
  Lines      8567    8686   +119     
=====================================
+ Hits       8114    8230   +116     
- Misses      453     456     +3     
Flag Coverage Δ
unit 94% <97%> (+<1%) ⬆️
Files with missing lines Coverage Δ
source/Public/Deny-SqlDscServerPermission.ps1 100% <100%> (ø)
source/Public/Grant-SqlDscServerPermission.ps1 100% <100%> (ø)
source/Public/Revoke-SqlDscServerPermission.ps1 100% <100%> (ø)
source/Public/Test-SqlDscServerPermission.ps1 93% <93%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI changed the title Add new public commands for SQL Server permission management Add new public commands for SQL Server permission management with simplified parameter structure Aug 30, 2025
Copilot AI requested a review from johlju August 30, 2025 17:17
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change the command names and add new, I think

  • New-SqlDscPermission -> Grant-SqlDscServerPermission
  • Remove-SqlDscPermission -> Revoke-SqlDscPermission
  • Add a new command Deny-SqlDscPermission
    then we don't need the State parameter in those commands.

For Test-SqlDscServerPermission we can add two parameters sets each have a switch parameter; Deny and Grant respectively plus -WithGrant for both parameter sets.

@johlju
Copy link
Member

johlju commented Aug 30, 2025

@copilot I think we should change the command names and add new, I think

New-SqlDscPermission -> Grant-SqlDscServerPermission
Remove-SqlDscServerPermission -> Revoke-SqlDscServerPermission
Add a new command Deny-SqlDscServerPermission
then we don't need the State parameter in those commands.
For Test-SqlDscServerPermission we can add two parameters sets each have a switch parameter; Deny and Grant respectively plus -WithGrant for both parameter sets.

@coderabbitai
Copy link

coderabbitai bot commented Aug 30, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds four public cmdlets (Grant/Deny/Revoke/Test-SqlDscServerPermission), a new SqlServerPermission enum, localization strings, CI test sequence updates, many unit and integration Pester tests, test stub changes for SMO.ServerRole, and updates the changelog to reflect renamed/new commands.

Changes

Cohort / File(s) Summary
Public cmdlets
source/Public/Grant-SqlDscServerPermission.ps1, source/Public/Deny-SqlDscServerPermission.ps1, source/Public/Revoke-SqlDscServerPermission.ps1, source/Public/Test-SqlDscServerPermission.ps1
Adds four exported cmdlets to grant, deny, revoke, and test server permissions for Login/ServerRole principals. Support parameter sets for Login/ServerRole, Permission[] (SqlServerPermission), WithGrant/ExactMatch/Force switches as applicable, ShouldProcess integration, localized messages, and error handling.
Enum
source/Enum/002.ServerPermission.ps1
Adds enum SqlServerPermission with 52 permission members (AdministerBulkOperations plus 51 additional server permissions).
Localization
source/en-US/SqlServerDsc.strings.psd1
Adds ShouldProcess captions/descriptions/warnings and failure/test messages for Grant, Deny, Revoke, and Test server-permission operations.
Changelog
CHANGELOG.md
Reworks Unreleased entries: replaces previous New-/Remove-SqlDscServerPermission entries with Grant/Deny/Revoke/Test cmdlets and documents parameter/signature changes.
CI pipeline
azure-pipelines.yml
Reorganizes integration test sequence: inserts server-permission integration tests (Grant/Get/Test/Deny/Revoke), repositions cleanup tests into a later group, and extends/reorders test groups accordingly.
Integration tests
tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1, .../Deny-SqlDscServerPermission.Integration.Tests.ps1, .../Revoke-SqlDscServerPermission.Integration.Tests.ps1, .../Test-SqlDscServerPermission.Integration.Tests.ps1, tests/Integration/Commands/README.md
Adds comprehensive Pester integration suites validating grant/deny/revoke/test flows against persistent principals; updates README to document persistent objects and run-order/dependencies.
Unit tests (public)
tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1, .../Deny-SqlDscServerPermission.Tests.ps1, .../Revoke-SqlDscServerPermission.Tests.ps1, .../Test-SqlDscServerPermission.Tests.ps1
Adds unit test suites covering parameter sets, required/optional params, success/failure, pipeline input, WithGrant/ExactMatch logic, and error paths; use SMO stubs and InModuleScope mocks.
Test stubs
tests/Unit/Stubs/SMO.cs
Adds public Parent field to ServerRole and initializes it in constructors to support tests.
Docs / Guidelines
.github/instructions/dsc-community-style-guidelines-pester.instructions.md
Adds/relocates Pester style guidance: new Requirements subsection for It-block usage and a Best Practices note about PSDefaultParameterValues.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User/Caller
  participant C as SqlServerDsc Cmdlet<br/>(Grant/Deny/Revoke/Test)
  participant P as Principal<br/>(Login/ServerRole)
  participant S as SMO Server
  participant R as Strings/Resources

  U->>C: Invoke with -Login/-ServerRole and -Permission[]
  C->>P: Resolve principal name and server object
  C->>C: Build ServerPermissionSet from enum values
  C->>R: Format ShouldProcess messages
  alt Grant
    C->>S: Grant(permSet, principal[, WithGrant])
    S-->>C: Success / Exception
  else Deny
    C->>S: Deny(permSet, principal)
    S-->>C: Success / Exception
  else Revoke
    C->>S: Revoke(permSet, principal[, WithGrant cascade])
    S-->>C: Success / Exception
  else Test
    C->>S: Get permissions via Get-SqlDscServerPermission
    C->>C: Compare actual vs desired (Grant/Deny[, WithGrant][, ExactMatch])
    C-->>U: [bool]
  end
  opt Error
    C->>R: Get failure message
    C-->>U: Terminating error / false (Test)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Join our Discord community for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor Author

Copilot AI commented Aug 30, 2025

I think we should change the command names and add new, I think

  • New-SqlDscPermission -> Grant-SqlDscServerPermission
  • Remove-SqlDscPermission -> Revoke-SqlDscPermission
    ...

I've implemented the requested changes:

  • Renamed New-SqlDscServerPermission to Grant-SqlDscServerPermission
  • Renamed Remove-SqlDscServerPermission to Revoke-SqlDscServerPermission
  • Added new Deny-SqlDscServerPermission command
  • Removed State parameter from all three permission commands since each command now represents a specific action
  • Updated Test-SqlDscServerPermission to use parameter sets with Grant and Deny switch parameters plus WithGrant for both parameter sets
  • Updated localized strings and test files accordingly

The module builds successfully with these changes.

…mands, add Deny command, and use parameter sets for Test command

Co-authored-by: johlju <[email protected]>
Copilot AI changed the title Add new public commands for SQL Server permission management with simplified parameter structure Add new server permission management commands with action-specific verbs Aug 30, 2025
Copilot AI requested a review from johlju August 30, 2025 18:25
Copy link
Contributor Author

Copilot AI commented Aug 30, 2025

@copilot I think we should change the command names and add new, I think

New-SqlDscPermission -> Grant-SqlDscServerPermission
...

I've implemented all the requested changes in commit 167bac9:

  • Renamed New-SqlDscServerPermissionGrant-SqlDscServerPermission
  • Renamed Remove-SqlDscServerPermissionRevoke-SqlDscServerPermission
  • Added new Deny-SqlDscServerPermission command
  • Removed State parameter from all three permission commands since each command now represents a specific action
  • Updated Test-SqlDscServerPermission to use parameter sets with Grant and Deny switch parameters, plus WithGrant for both parameter sets

The module builds successfully with these changes.

Copy link
Contributor Author

Copilot AI commented Aug 31, 2025

@copilot In
tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
around lines 35-37, remove the Import-Module of the Unit/Stubs/SqlServer.psm1
...

Removed the Import-Module statements for Unit/Stubs/SqlServer.psm1 from all three server permission integration test files (Grant-, Revoke-, and Test-SqlDscServerPermission). Also removed unit test specific $PSDefaultParameterValues settings. Integration tests now properly rely on the real SqlServer module and Connect-SqlDscDatabaseEngine as intended. Commit: eba49be

@johlju
Copy link
Member

johlju commented Aug 31, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 31, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (1)
tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1 (1)

132-145: Use object-based parameters with Revoke/Test for role.

Calls still use -ServerObject/-Name. Fetch the ServerRole object and pass it.

-        BeforeEach {
-            Revoke-SqlDscServerPermission -ServerObject $script:serverObject -Name $script:testRoleName -Permission 'ViewServerState' -Force -ErrorAction 'SilentlyContinue'
-        }
+        BeforeEach {
+            $roleObject = Get-SqlDscRole -ServerObject $script:serverObject -Name $script:testRoleName
+            Revoke-SqlDscServerPermission -ServerRole $roleObject -Permission ViewServerState -Force -ErrorAction 'SilentlyContinue'
+        }
🧹 Nitpick comments (6)
CHANGELOG.md (1)

39-45: Wrap long lines to satisfy MD013 (80-char) markdown rule.

Break overly long bullet descriptions onto a second indented line.

-  - Added new public command to grant server permissions to a principal (Login or ServerRole) on a SQL Server Database Engine instance.
+  - Added new public command to grant server permissions to a principal
+    (Login or ServerRole) on a SQL Server Database Engine instance.

-  - Added new public command to deny server permissions to a principal (Login or ServerRole).
+  - Added new public command to deny server permissions to a principal
+    (Login or ServerRole).

-  - Added new public command to revoke server permissions from a principal (Login or ServerRole).
+  - Added new public command to revoke server permissions from a principal
+    (Login or ServerRole).

-  - Added new public command with Grant/Deny parameter sets (and `-WithGrant`) to test server permissions for a principal.
+  - Added new public command with Grant/Deny parameter sets (and `-WithGrant`)
+    to test server permissions for a principal.
tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1 (2)

36-39: Avoid InModuleScope defaults in public command unit tests.

Per guidelines, don't use InModuleScope for public commands; keep Mock/Should defaults only.

-    $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:dscModuleName
     $PSDefaultParameterValues['Mock:ModuleName'] = $script:dscModuleName
     $PSDefaultParameterValues['Should:ModuleName'] = $script:dscModuleName
-    $PSDefaultParameterValues.Remove('InModuleScope:ModuleName')
     $PSDefaultParameterValues.Remove('Mock:ModuleName')
     $PSDefaultParameterValues.Remove('Should:ModuleName')

Also applies to: 41-45


117-123: Remove InModuleScope usage; invoke cmdlet directly.

Call Grant-SqlDscServerPermission with the constructed objects without wrapping in InModuleScope.

Example (apply same pattern to other It-blocks):

-            InModuleScope -Parameters @{
-                mockLogin = $mockLogin
-            } -ScriptBlock {
-                $null = Grant-SqlDscServerPermission -Login $mockLogin -Permission ConnectSql -Force
-            }
+            $null = Grant-SqlDscServerPermission -Login $mockLogin -Permission ConnectSql -Force

Also applies to: 125-131, 133-139, 181-187, 189-195

tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1 (2)

36-39: Avoid InModuleScope defaults in public command unit tests.

Keep Mock/Should defaults; drop InModuleScope default.

-    $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:dscModuleName
     $PSDefaultParameterValues['Mock:ModuleName'] = $script:dscModuleName
     $PSDefaultParameterValues['Should:ModuleName'] = $script:dscModuleName
-    $PSDefaultParameterValues.Remove('InModuleScope:ModuleName')
     $PSDefaultParameterValues.Remove('Mock:ModuleName')
     $PSDefaultParameterValues.Remove('Should:ModuleName')

Also applies to: 41-45


170-178: Remove InModuleScope usage; invoke cmdlet directly.

Invoke Test-SqlDscServerPermission with the prepared objects directly; InModuleScope isn't needed for public cmdlets.

Example (apply to all similar blocks):

-            InModuleScope -Parameters @{
-                mockLogin = $mockLogin
-            } -ScriptBlock {
-                $result = Test-SqlDscServerPermission -Login $mockLogin -Grant -Permission ConnectSql
-
-                $result | Should -BeTrue
-            }
+            $result = Test-SqlDscServerPermission -Login $mockLogin -Grant -Permission ConnectSql
+            $result | Should -BeTrue

Also applies to: 180-188, 190-198, 200-208, 210-218, 220-232, 268-276, 291-299, 317-325, 356-364, 393-401, 439-447, 449-457, 528-551

tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1 (1)

47-47: Standardize tag to 'Integration'.

Use a consistent tag across integration tests.

-Describe 'Deny-SqlDscServerPermission' -Tag 'IntegrationTest' {
+Describe 'Deny-SqlDscServerPermission' -Tag 'Integration' {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Jira integration is disabled
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2c337e3 and eba49be.

📒 Files selected for processing (9)
  • CHANGELOG.md (1 hunks)
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Unit/Public/Deny-SqlDscServerPermission.Tests.ps1 (1 hunks)
  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1 (1 hunks)
  • tests/Unit/Public/Revoke-SqlDscServerPermission.Tests.ps1 (1 hunks)
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1 (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Revoke-SqlDscServerPermission.Tests.ps1
  • tests/Unit/Public/Deny-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (6)
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • Command: Public command
  • Function: Private function
  • Resource: DSC class-based resource

Build & Test Workflow

  • Run in PowerShell, from repository root
  • Build before running tests: .\build.ps1 -Tasks build
  • Always run tests in new PowerShell session: Invoke-Pester -Path @({test paths}) -Output Detailed

File 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.ps1

Requirements

  • Follow guidelines over existing code patterns
  • 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
  • Add unit tests for all commands/functions/resources
  • Add integration tests for all public commands and resources

Files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
  • CHANGELOG.md
**/*.ps?(m|d)1

⚙️ CodeRabbit configuration file

**/*.ps?(m|d)1: # PowerShell Guidelines

Naming

  • 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.ps1 format (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 array

Hashtables

  • 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.
  • OUTPUTS: Lis...

Files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
**/*.[Tt]ests.ps1

⚙️ CodeRabbit configuration file

**/*.[Tt]ests.ps1: # Tests Guidelines

Core 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 Describe blocks
  • Assertions only in It blocks
  • Never test verbose messages, debug messages or parameter binding behavior
  • Pass all mandatory parameters to avoid prompts

Naming

  • One Describe block per file matching the tested entity name
  • Context descriptions start with 'When'
  • It descriptions 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 Context block
  • Each scenario = separate Context block
  • Use nested Context blocks for complex scenarios
  • Mocking in BeforeAll (BeforeEach only when required)
  • Setup/teardown in BeforeAll,BeforeEach/AfterAll,AfterEach close to usage

Syntax Rules

  • PascalCase: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
  • Prefer -BeTrue/-BeFalse over -Be $true/-Be $false
  • Never use Assert-MockCalled, use Should -Invoke instead
  • No Should -Not -Throw - invoke commands directly
  • Never add an empty -MockWith block
  • Omit -MockWith when returning $null
  • Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, InModuleScope:ModuleName
  • Omit -ModuleName parameter on Pester commands
  • Never use Mock inside InModuleScope-block

File Organization

  • Class resources: tests/Unit/Classes/{Name}.Tests.ps1
  • Public commands: tests/Unit/Public/{Name}.Tests.ps1
  • Private functions: tests/Unit/Private/{Name}.Tests.ps1

Data-Driven Tests

  • Define variables in separate BeforeDiscovery for -ForEach (close to usage)
  • -ForEach al...

Files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1

⚙️ CodeRabbit configuration file

tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests Guidelines

Requirements

  • 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-ComputerName for computer names in CI
  • Avoid ExpectedMessage for Should -Throw assertions
  • Only run integration tests in CI unless explicitly instructed.
  • Call commands with -Force parameter where applicable (avoids prompting).
  • Use -ErrorAction Stop on commands so failures surface immediately

Required 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 build" first.'
    }
}

BeforeAll {
    $script:moduleName = '{MyModuleName}'

    Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop'
}

Files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: # Markdown Style Guidelines

  • Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
  • Use 2 spaces for indentation
  • Use '1.' for all items in ordered lists (1/1/1 numbering style)
  • Disable MD013 rule by adding a comment for tables/code blocks exceeding 80 characters
  • Empty lines required before/after code blocks and headings (except before line 1)
  • Escape backslashes in file paths only (not in code blocks)
  • Code blocks must specify language identifiers

Text Formatting

  • Parameters: bold
  • Values/literals: inline code
  • Resource/module/product names: italic
  • Commands/files/paths: inline code

Files:

  • CHANGELOG.md
CHANGELOG.md

⚙️ CodeRabbit configuration file

CHANGELOG.md: # Changelog Guidelines

  • Always update the Unreleased section in CHANGELOG.md
  • Use Keep a Changelog format
  • Describe notable changes briefly, ≤2 items per change type
  • Reference issues using format issue #<issue_number>
  • No empty lines between list items in same section
  • Do not add item if there are already an existing item for the same change

Files:

  • CHANGELOG.md
🧠 Learnings (28)
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Add unit tests for all commands, functions, and resources

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands, private functions and classes must have unit tests

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Unit/**/*.ps1 : Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:24:39.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:24:39.268Z
Learning: Applies to tests/[u]Unit/**/*.[Tt]ests.ps1 : Add Parameter Properties tests (e.g., assert Mandatory on parameters) as shown in the template

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:24:39.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:24:39.268Z
Learning: Applies to tests/[u]Unit/**/*.[Tt]ests.ps1 : All public commands must have parameter set validation tests

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Add integration tests for all public commands and resources

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : Use DSCSQLTEST (Database Engine), SSRS (Reporting Services), and PBIRS (Power BI Report Server) instance names in CI integration tests

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*Unit.Tests.ps1 : Always add and structure Unit tests according to the unit testing guidelines

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.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/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Include the required setup block at the top of each integration test: the SuppressMessage param(), a BeforeDiscovery block that ensures DscResource.Test is available (or runs build.ps1 -Tasks 'noop' and imports it), and a BeforeAll block that sets $script:moduleName and imports the module under test with -Force -ErrorAction Stop

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:24:39.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:24:39.268Z
Learning: Applies to tests/[u]Unit/**/*.[Tt]ests.ps1 : Use the provided Parameter Set Validation test template for single-parameter-set commands

Applied to files:

  • tests/Unit/Public/Grant-SqlDscServerPermission.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : In integration tests requiring a SQL Server DB, use Connect-SqlDscDatabaseEngine with correct CI credentials to open the session

Applied to files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*Integration.Tests.ps1 : Always add and structure Integration tests according to the integration testing guidelines

Applied to files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Ensure integration tests cover all scenarios and code paths

Applied to files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : Always call Disconnect-SqlDscDatabaseEngine after Connect-SqlDscDatabaseEngine in integration tests

Applied to files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : If an integration test requires a SQL Server Database Engine, start the Windows service in BeforeAll and stop it in AfterAll

Applied to files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.

Applied to files:

  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/[cC]ommands/*.[iI]ntegration.[tT]ests.ps1 : Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Place command integration tests in tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : When calling commands in integration tests, pass -Force where applicable to avoid prompts

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Unit/**/*.ps1 : In unit tests, use SMO stub types from SMO.cs and never mock real SMO types

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
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/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Define mocks in BeforeAll; use BeforeEach only when required

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to **/*.{ps1,psm1} : Public PowerShell commands must use the naming format Verb-SqlDsc{Noun}

Applied to files:

  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
  • CHANGELOG.md
📚 Learning: 2025-08-16T13:22:15.230Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2134
File: tests/Unit/Public/Get-SqlDscLogin.Tests.ps1:78-93
Timestamp: 2025-08-16T13:22:15.230Z
Learning: In PowerShell unit tests for parameter validation in SqlServerDsc, accessing parameter attributes directly like `$cmd.Parameters['ParameterName'].Attributes.Mandatory` works correctly because PowerShell automatically iterates through the array and returns the property values. Additional filtering to ParameterAttribute is not necessary when the parameter structure is known and controlled.

Applied to files:

  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*.Tests.ps1 : Follow test patterns strictly for maintainability in Pester tests

Applied to files:

  • tests/Unit/Public/Test-SqlDscServerPermission.Tests.ps1
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md

39-39: Line length
Expected: 80; Actual: 135

(MD013, line-length)


41-41: Line length
Expected: 80; Actual: 93

(MD013, line-length)


43-43: Line length
Expected: 80; Actual: 97

(MD013, line-length)


45-45: Line length
Expected: 80; Actual: 122

(MD013, line-length)

⏰ 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). (6)
  • GitHub Check: PSScriptAnalyzer
  • GitHub Check: PSScriptAnalyzer
  • GitHub Check: PSScriptAnalyzer
  • GitHub Check: PSScriptAnalyzer
  • GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
  • GitHub Check: dsccommunity.SqlServerDsc
🔇 Additional comments (1)
CHANGELOG.md (1)

38-45: Server-permission command names and descriptions look correct.

Entries now use Grant/Deny/Revoke/Test with object-based principals. LGTM.

@johlju
Copy link
Member

johlju commented Aug 31, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 31, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
azure-pipelines.yml (1)

280-286: Critical: You remove SqlServer/SQLPS but don’t reinstall — SMO provider may be unavailable

The “Clean CI worker” step removes SqlServer/SQLPS, and this stage doesn’t re-install SqlServer nor set SMODefaultModuleName. Connect-SqlDscDatabaseEngine may fail at runtime. Add an install step (like in Build stage) right after cleaning.

           - powershell: |
               Import-Module -Name ./tests/TestHelpers/CommonTestHelper.psm1
               Remove-PowerShellModuleFromCI -Name @('SqlServer', 'SQLPS')
               Remove-Module -Name CommonTestHelper
             name: cleanCIWorker
             displayName: 'Clean CI worker'
+          - pwsh: |
+              Write-Information -MessageData 'Installing latest SqlServer on the CI worker.' -InformationAction Continue
+              Install-Module -Name SqlServer -Force -Confirm:$false -Verbose
+              Get-Module -Name SqlServer -ListAvailable |
+                Select-Object -Property Name,Version,@{Name='Prerelease'; Expression={$_.PrivateData.PSData.Prerelease}}, Path
+            displayName: 'Install SqlServer module'
♻️ Duplicate comments (1)
azure-pipelines.yml (1)

302-306: Reordering looks good — server-permission tests run before role removal

Prevents missing prerequisites from Remove-SqlDscRole. Matches earlier guidance.

🧹 Nitpick comments (4)
.github/instructions/dsc-community-style-guidelines-pester.instructions.md (2)

17-23: Fix markdownlint spacing and minor grammar in the new “Requirements” section

  • Add a blank line after the “## Requirements” heading and after the list.
  • Tweak grammar: “in the same It block”, “The tested entity”.
 ## Requirements
- - Inside `It` blocks, assign unused return objects to `$null` (unless part of pipeline)
- - Tested entity must be called from within the `It` blocks
- - Keep results and assertions in same `It` block
- - Avoid try-catch-finally for cleanup, use `AfterAll` or `AfterEach`
- - Avoid unnecessary remove/recreate cycles
+
+- Inside `It` blocks, assign unused return objects to `$null` (unless part of a pipeline)
+- The tested entity must be called 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
+

63-63: Wrap long line to satisfy MD013 (line length) and improve readability

-- Use `$PSDefaultParameterValues` only for Pester commands (`Describe`, `Context`, `It`, `Mock`, `Should`, `InModuleScope`)
+- Use `$PSDefaultParameterValues` only for Pester commands
+  (`Describe`, `Context`, `It`, `Mock`, `Should`, `InModuleScope`)
tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1 (2)

32-37: Avoid unloading SqlServer module in AfterAll (may affect subsequent tests in same session)

This test runs amid many others in the same Invoke-Pester run. Removing SqlServer can impact following tests that expect it loaded. Prefer letting Connect-SqlDscDatabaseEngine manage module import per test.

 AfterAll {
     # Unload the module being tested so that it doesn't impact any other tests.
     Get-Module -Name $script:moduleName -All | Remove-Module -Force

-    Remove-Module -Name SqlServer -Force
 }

74-76: Harden teardown with -ErrorAction Stop

Minor: ensure failures surface immediately during disconnect.

-        Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject
+        Disconnect-SqlDscDatabaseEngine -ServerObject $script:serverObject -ErrorAction 'Stop'
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Jira integration is disabled
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between eba49be and 475290e.

📒 Files selected for processing (7)
  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md (2 hunks)
  • azure-pipelines.yml (1 hunks)
  • source/Enum/002.ServerPermission.ps1 (1 hunks)
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1 (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • tests/Integration/Commands/Test-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Grant-SqlDscServerPermission.Integration.Tests.ps1
  • tests/Integration/Commands/Deny-SqlDscServerPermission.Integration.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (5)
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • Command: Public command
  • Function: Private function
  • Resource: DSC class-based resource

Build & Test Workflow

  • Run in PowerShell, from repository root
  • Build before running tests: .\build.ps1 -Tasks build
  • Always run tests in new PowerShell session: Invoke-Pester -Path @({test paths}) -Output Detailed

File 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.ps1

Requirements

  • Follow guidelines over existing code patterns
  • 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
  • Add unit tests for all commands/functions/resources
  • Add integration tests for all public commands and resources

Files:

  • source/Enum/002.ServerPermission.ps1
  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • azure-pipelines.yml
**/*.ps?(m|d)1

⚙️ CodeRabbit configuration file

**/*.ps?(m|d)1: # PowerShell Guidelines

Naming

  • 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.ps1 format (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 array

Hashtables

  • 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.
  • OUTPUTS: Lis...

Files:

  • source/Enum/002.ServerPermission.ps1
  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
source/**/*.ps1

⚙️ CodeRabbit configuration file

source/**/*.ps1: # Localization Guidelines

Requirements

  • Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
  • Use localized string keys, not hardcoded strings
  • Assume $script:localizedData is available

String Files

  • Commands/functions: source/en-US/{MyModuleName}.strings.psd1
  • Class resources: source/en-US/{ResourceClassName}.strings.psd1

Key Naming Patterns

  • Format: Verb_FunctionName_Action (underscore separators), e.g. Get_Database_ConnectingToDatabase

String Format

ConvertFrom-StringData @'
    KeyName = Message with {0} placeholder. (PREFIX0001)
'@

String IDs

  • Format: (PREFIX####)
  • PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
  • Number: Sequential from 0001

Usage

Write-Verbose -Message ($script:localizedData.KeyName -f $value1)

Files:

  • source/Enum/002.ServerPermission.ps1
**/*.[Tt]ests.ps1

⚙️ CodeRabbit configuration file

**/*.[Tt]ests.ps1: # Tests Guidelines

Core 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 Describe blocks
  • Assertions only in It blocks
  • Never test verbose messages, debug messages or parameter binding behavior
  • Pass all mandatory parameters to avoid prompts

Requirements

  • Inside It blocks, assign unused return objects to $null (unless part of pipeline)
  • Tested entity must be called from within the It blocks
  • Keep results and assertions in same It block
  • Avoid try-catch-finally for cleanup, use AfterAll or AfterEach
  • Avoid unnecessary remove/recreate cycles

Naming

  • One Describe block per file matching the tested entity name
  • Context descriptions start with 'When'
  • It descriptions 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 Context block
  • Each scenario = separate Context block
  • Use nested Context blocks for complex scenarios
  • Mocking in BeforeAll (BeforeEach only when required)
  • Setup/teardown in BeforeAll,BeforeEach/AfterAll,AfterEach close to usage

Syntax Rules

  • PascalCase: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
  • Prefer -BeTrue/-BeFalse over -Be $true/-Be $false
  • Never use Assert-MockCalled, use Should -Invoke instead
  • No Should -Not -Throw - invoke commands directly
  • Never add an empty -MockWith block
  • Omit -MockWith when returning $null
  • Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, InModuleScope:ModuleName
  • Omit -ModuleName parameter on Pester commands
  • Never use Mock inside `InModule...

Files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1

⚙️ CodeRabbit configuration file

tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests Guidelines

Requirements

  • 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-ComputerName for computer names in CI
  • Avoid ExpectedMessage for Should -Throw assertions
  • Only run integration tests in CI unless explicitly instructed.
  • Call commands with -Force parameter where applicable (avoids prompting).
  • Use -ErrorAction Stop on commands so failures surface immediately

Required 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 build" first.'
    }
}

BeforeAll {
    $script:moduleName = '{MyModuleName}'

    Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop'
}

Files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
🧠 Learnings (42)
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Add integration tests for all public commands and resources

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : Use DSCSQLTEST (Database Engine), SSRS (Reporting Services), and PBIRS (Power BI Report Server) instance names in CI integration tests

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Include the required setup block at the top of each integration test: the SuppressMessage param(), a BeforeDiscovery block that ensures DscResource.Test is available (or runs build.ps1 -Tasks 'noop' and imports it), and a BeforeAll block that sets $script:moduleName and imports the module under test with -Force -ErrorAction Stop

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : In integration tests requiring a SQL Server DB, use Connect-SqlDscDatabaseEngine with correct CI credentials to open the session

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : Always call Disconnect-SqlDscDatabaseEngine after Connect-SqlDscDatabaseEngine in integration tests

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*Integration.Tests.ps1 : Always add and structure Integration tests according to the integration testing guidelines

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
  • azure-pipelines.yml
  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
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/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Unit/**/*.ps1 : In unit tests, use SMO stub types from SMO.cs and never mock real SMO types

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
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/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Unit/**/*.ps1 : Load SMO stub types in unit tests, e.g., Add-Type -Path "$PSScriptRoot/../Stubs/SMO.cs"

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Stubs/SMO.cs : Ensure all SMO types used by unit-tested classes/commands are properly stubbed in SMO.cs

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Stubs/SMO.cs : After changing SMO.cs (SMO stubs), run tests in a new PowerShell session

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use Mock inside an InModuleScope block

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to tests/Integration/**/*.ps1 : If an integration test requires a SQL Server Database Engine, start the Windows service in BeforeAll and stop it in AfterAll

Applied to files:

  • tests/Integration/Commands/Revoke-SqlDscServerPermission.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/Revoke-SqlDscServerPermission.Integration.Tests.ps1
📚 Learning: 2025-08-29T17:20:42.266Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:20:42.266Z
Learning: Applies to azure-pipelines.yml : Add integration test script files to a group within the test stage in azure-pipelines.yml and choose the appropriate group number based on dependencies

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Place command integration tests in tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/[cC]ommands/*.[iI]ntegration.[tT]ests.ps1 : Place command integration tests at tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles in tests

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : When calling commands in integration tests, pass -Force where applicable to avoid prompts

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:22:07.610Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-08-29T17:22:07.610Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Ensure integration tests cover all scenarios and code paths

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:25:09.959Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-08-29T17:25:09.959Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Add unit tests for all commands, functions, and resources

Applied to files:

  • azure-pipelines.yml
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*Unit.Tests.ps1 : Always add and structure Unit tests according to the unit testing guidelines

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-28T15:44:12.628Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2150
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:35-35
Timestamp: 2025-08-28T15:44:12.628Z
Learning: The repository uses RequiredModules.psd1 to specify Pester version requirements with RequiredVersion = 'latest', which ensures compatibility with Should -Invoke syntax (requires Pester 5.4+).

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:19:40.951Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T17:19:40.951Z
Learning: Applies to **/*.Tests.ps1 : Follow test patterns strictly for maintainability in Pester tests

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : One Describe block per file matching the tested entity name

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Place test code only inside Describe blocks

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Context descriptions start with 'When'

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Call the tested entity from within It blocks

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use a separate Context block for each scenario

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use assertions only inside It blocks

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : It descriptions start with 'Should' and must not contain 'when'

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use nested Context blocks for complex scenarios

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use a separate Context block for each class method

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Keep variable scope close to the usage context

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use BeforeEach and AfterEach sparingly

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Define mocks in BeforeAll; use BeforeEach only when required

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
📚 Learning: 2025-08-29T17:23:02.585Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-08-29T17:23:02.585Z
Learning: Applies to **/*.[Tt]ests.ps1 : Place setup/teardown in BeforeAll/BeforeEach and AfterAll/AfterEach close to their usage

Applied to files:

  • .github/instructions/dsc-community-style-guidelines-pester.instructions.md
🪛 LanguageTool
.github/instructions/dsc-community-style-guidelines-pester.instructions.md

[grammar] ~17-~17: There might be a mistake here.
Context: ...meters to avoid prompts ## Requirements - Inside It blocks, assign unused return...

(QB_NEW_EN)


[grammar] ~18-~18: There might be a mistake here.
Context: ... return objects to $null (unless part of pipeline) - Tested entity must be calle...

(QB_NEW_EN)


[grammar] ~20-~20: There might be a mistake here.
Context: ...tblocks - Keep results and assertions in sameIt` block - Avoid try-catch-final...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)
.github/instructions/dsc-community-style-guidelines-pester.instructions.md

17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


18-18: Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


63-63: Line length
Expected: 80; Actual: 123

(MD013, line-length)

⏰ 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). (2)
  • GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
  • GitHub Check: PSScriptAnalyzer
🔇 Additional comments (3)
source/Enum/002.ServerPermission.ps1 (1)

9-63: Verify enum member names match SMO ServerPermission — LGTM on using 1-based values; manually confirm all enum members exactly match the static field names of Microsoft.SqlServer.Management.Smo.ServerPermission to avoid runtime mapping errors.

tests/Integration/Commands/Revoke-SqlDscServerPermission.Integration.Tests.ps1 (2)

26-30: Required setup pattern followed correctly

Imports module under test with -Force and -ErrorAction Stop. Good.


78-118: Test flows look solid

  • Uses persistent principals, grants then revokes, and validates via Test-SqlDscServerPermission.
  • Covers pipeline input and role scenarios.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@johlju reviewed 1 of 8 files at r6, 8 of 21 files at r11, 1 of 1 files at r12, 3 of 9 files at r13, 3 of 7 files at r15, 1 of 1 files at r16, 2 of 3 files at r17, all commit messages.
Reviewable status: 18 of 24 files reviewed, 19 unresolved discussions (waiting on @copilot)

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johlju reviewed 2 of 21 files at r11, 2 of 9 files at r13, 1 of 7 files at r15, 1 of 3 files at r17.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @copilot)

@johlju johlju changed the title Add new server permission management commands with object-based parameters *-SqlDscServerPermission: Add commands SQL Server permission management Sep 1, 2025
@johlju johlju merged commit 755ad31 into main Sep 1, 2025
26 of 27 checks passed
@johlju johlju deleted the copilot/fix-e806ea1d-2077-4c68-a272-f8dea52701f7 branch September 1, 2025 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants