Skip to content

SqlProtocol/SqlProtocolTcpIp: Refactor to use public commands#2368

Merged
johlju merged 4 commits intodsccommunity:mainfrom
johlju:fix/issue-#2104
Dec 12, 2025
Merged

SqlProtocol/SqlProtocolTcpIp: Refactor to use public commands#2368
johlju merged 4 commits intodsccommunity:mainfrom
johlju:fix/issue-#2104

Conversation

@johlju
Copy link
Copy Markdown
Member

@johlju johlju commented Dec 11, 2025

Pull Request (PR) description

  • SqlProtocol
    • Refactored to use the public command Get-SqlDscServerProtocolName instead
      of the deprecated private function Get-ProtocolNameProperties
      (issue #2104).
    • Refactored to use the public command Get-SqlDscServerProtocol instead
      of the deprecated private function Get-ServerProtocolObject
      (issue #2104).
  • SqlProtocolTcpIp
    • Refactored to use the public command Get-SqlDscServerProtocol instead
      of the deprecated private function Get-ServerProtocolObject
      (issue #2104).

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation updated in the resource's README.md.
  • Resource parameter descriptions updated in schema.mof.
  • Comment-based help updated, including parameter descriptions.
  • Localization strings updated.
  • Examples updated.
  • Unit tests updated. See DSC Community Testing Guidelines.
  • Integration tests updated (where possible). See DSC Community Testing Guidelines.
  • Code changes adheres to DSC Community Style Guidelines.

This change is Reviewable

@johlju johlju requested a review from a team as a code owner December 11, 2025 16:56
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 11, 2025

Walkthrough

Replaced two removed private helpers with new public commands: callers updated to use Get-SqlDscServerProtocol and Get-SqlDscServerProtocolName (parameter key changes InstanceInstanceName where applicable), removed exports and implementations for Get-ProtocolNameProperties and Get-ServerProtocolObject, and updated tests to mock the new cmdlets and load SMO stubs.

Changes

Cohort / File(s) Summary
Removed private helpers
source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psd1, source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1
Removed Get-ProtocolNameProperties and Get-ServerProtocolObject from the module implementation and from the FunctionsToExport manifest.
DSC resources updated to public commands
source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1, source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
Replaced calls to Get-ProtocolNamePropertiesGet-SqlDscServerProtocolName and Get-ServerProtocolObjectGet-SqlDscServerProtocol; renamed parameter hashtables/keys from InstanceInstanceName and aligned parameter names across Get/Set/Test flows.
Tests updated / removed
tests/Unit/DSC_SqlProtocol.Tests.ps1, tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1, tests/Unit/SqlServerDsc.Common.Tests.ps1
Added SMO stub type loading in BeforeAll; replaced mocks of Get-ServerProtocolObject with Get-SqlDscServerProtocol; removed test suites for the deleted helper functions; adjusted mock return structures and some assertions.
Localization resource change
source/Modules/SqlServerDsc.Common/en-US/SqlServerDsc.Common.strings.psd1, source/Modules/SqlServerDsc.Common/sv-SE/SqlServerDsc.Common.strings.psd1
Removed the FailedToObtainServerInstance string entry from both en-US and sv-SE resource files.
Changelog
CHANGELOG.md
Documented replacing private helpers with public commands and corresponding test/manifest updates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Verify all references to Get-ServerProtocolObject and Get-ProtocolNameProperties are removed or replaced.
  • Confirm parameter key/name changes (InstanceInstanceName) are consistent in all callsites.
  • Validate mocks in tests produce the same shape (properties/methods) expected by consumers for Get-SqlDscServerProtocol.
  • Review removal from FunctionsToExport for unintended API breakage and update release notes if needed.
  • Check no remaining localization references expect the deleted FailedToObtainServerInstance key.

Possibly related PRs

  • Add new public commands #2108 — Introduces the new public commands Get-SqlDscServerProtocol and Get-SqlDscServerProtocolName that directly correspond to the replacements applied in this PR.

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR partially addresses issue #2104: it refactors code to use Get-SqlDscServerProtocol and Get-SqlDscServerProtocolName, but does not migrate from New-Object to CIM-based retrieval as suggested in the issue. Implement CIM-based retrieval using Get-CimInstance for SQL Server network protocol properties as specified in issue #2104 to fully modernize the implementation.
Out of Scope Changes check ⚠️ Warning The PR includes scope-appropriate changes: refactoring to use public commands, updating tests and localization strings. However, removal of two private functions (Get-ProtocolNameProperties, Get-ServerProtocolObject) from SqlServerDsc.Common may impact external consumers using these functions. Verify that the removal of Get-ProtocolNameProperties and Get-ServerProtocolObject from public exports does not break external dependencies, or document breaking changes appropriately.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: refactoring SqlProtocol and SqlProtocolTcpIp resources to use public commands instead of private functions.
Description check ✅ Passed The description is well-related to the changeset, clearly documenting which resources were refactored, which functions were replaced, and which issue is being addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@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)
tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (1)

1118-1125: Fix Should -BeTru typo to avoid test failure

$script:wasMethodAlterCalled | Should -BeTru uses a non-existent assertion (-BeTru), which will cause this test to fail even when the behavior is correct. It should be -BeTrue.

-                    $script:wasMethodAlterCalled | Should -BeTru
+                    $script:wasMethodAlterCalled | Should -BeTrue
🧹 Nitpick comments (3)
CHANGELOG.md (1)

110-118: Optional: consolidate “Removed” bullets under module heading for brevity

To reduce repetition and keep items concise, consider grouping both removals under SqlServerDsc.Common with both function names and replacement commands in one item.

- - Removed deprecated private function `Get-ProtocolNameProperties` from the
-   SqlServerDsc.Common module. Use the public command `Get-SqlDscServerProtocolName`
-   instead ([issue #2104](https://github.com/dsccommunity/SqlServerDsc/issues/2104)).
- - Removed deprecated private function `Get-ServerProtocolObject` from the
-   SqlServerDsc.Common module. Use the public command `Get-SqlDscServerProtocol`
-   instead ([issue #2104](https://github.com/dsccommunity/SqlServerDsc/issues/2104)).
+ - SqlServerDsc.Common
+   - Removed deprecated private functions `Get-ProtocolNameProperties` and
+     `Get-ServerProtocolObject`. Use `Get-SqlDscServerProtocolName` and
+     `Get-SqlDscServerProtocol` instead
+     ([issue #2104](https://github.com/dsccommunity/SqlServerDsc/issues/2104)).
tests/Unit/DSC_SqlProtocol.Tests.ps1 (1)

92-95: Mocks correctly switched to Get-SqlDscServerProtocol (minor style nit)

The refactor to mock Get-SqlDscServerProtocol instead of the old private Get-ServerProtocolObject is consistent and preserves the expected shapes (IsEnabled, HasMultiIPAddresses, ProtocolProperties, and Alter() behavior) that DSC_SqlProtocol consumes. This keeps the resource behavior and tests aligned with the new public helper.

Minor style note: in a few places you still use -MockWith { return $null } when the mock only returns $null. Per the Pester/SqlServerDsc guidelines, you can omit -MockWith entirely in those cases for brevity. This is optional and can be cleaned up later.

As per coding guidelines, omitting -MockWith on $null-only mocks is preferred but not required.

Also applies to: 138-151, 185-194, 229-235, 266-272, 741-743, 819-836, 880-897, 939-956, 1004-1021, 1065-1082, 1134-1149, 1192-1207, 1249-1264

tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (1)

82-85: Get-SqlDscServerProtocol mocks look correct (optional cleanup)

The mocks have been consistently updated to Get-SqlDscServerProtocol and return IPAddress/IpAddressProperties structures compatible with DSC_SqlProtocolTcpIp (including IPAll vs IPx cases and Alter()-time validation), so the refactor to the new public helper keeps behavior intact.

Similar to the other test file, a few mocks that only return $null still specify -MockWith; you can drop -MockWith there to follow the Pester style guidance, but this is non-blocking.

As per coding guidelines, omitting -MockWith for $null-only mocks is preferred but not mandatory.

Also applies to: 123-135, 158-175, 207-223, 258-285, 317-343, 965-968, 997-1008, 1072-1099, 1145-1172, 1219-1243, 1289-1313, 1361-1385, 1436-1447

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 443a52f and 9bbfc19.

📒 Files selected for processing (8)
  • CHANGELOG.md (2 hunks)
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1 (5 hunks)
  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1 (3 hunks)
  • source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psd1 (0 hunks)
  • source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 (0 hunks)
  • tests/Unit/DSC_SqlProtocol.Tests.ps1 (15 hunks)
  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (15 hunks)
  • tests/Unit/SqlServerDsc.Common.Tests.ps1 (0 hunks)
💤 Files with no reviewable changes (3)
  • source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psd1
  • tests/Unit/SqlServerDsc.Common.Tests.ps1
  • source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1
🧰 Additional context used
📓 Path-based instructions (11)
**/*.ps1

📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)

**/*.ps1: Format public commands as {Verb}-SqlDsc{Noun} following PowerShell naming conventions
Format private functions as {Verb}-{Noun} following PowerShell naming conventions

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
**/*.[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 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
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'
Public commands: Never use InModuleScope (unless retrieving localized strings or creating an object using an internal class)
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
Spacing between blocks, arrange, act, and assert for readability
PascalCase: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse never -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
Never use param() inside -MockWith scriptblock...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.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 or creating an object using an internal class)
  • 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
  • Spacing between blocks, arrange, act, and assert for readability

Syntax Rules

  • PascalCase: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
  • Use -BeTrue/-BeFalse never -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, `...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)

tests/[Uu]nit/**/*.[Tt]ests.ps1: Test with localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
Mock files using the $TestDrive variable (path to the test drive)
All public commands require parameter set validation tests
Use the exact setup block with BeforeDiscovery, BeforeAll, and AfterAll blocks including DscResource.Test module import and PSDefaultParameterValues configuration
Parameter set validation tests should use Get-Command with Where-Object filtering and Should assertions to verify ParameterSetName and ParameterListAsString
Parameter property tests should verify mandatory parameter attributes using Get-Command Parameters and Should -BeTrue assertions

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1

⚙️ CodeRabbit configuration file

tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines

  • Test with localized strings: Use InModuleScope -ScriptBlock { $script:localizedData.Key }
  • Mock files: Use $TestDrive variable (path to the test drive)
  • All public commands require parameter set validation tests
  • After modifying classes, always run tests in new session (for changes to take effect)

Test Setup Requirements

Use this exact setup block before Describe:

[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'

    $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName
    $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName
    $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName
}

AfterAll {
    $PSDefaultParameterValues.Remove('InModuleScope:ModuleName')
    $PSDefaultParameterValues.Remove('Mock:ModuleNam...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
tests/Unit/**/*.Tests.ps1

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)

Unit tests should be located in tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
**/*.{ps1,psm1,psd1}

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)

**/*.{ps1,psm1,psd1}: Use descriptive names with 3+ characters and no abbreviations for functions, parameters, variables, keywords, and classes
Use PascalCase with Verb-Noun format for function names, using approved PowerShell verbs
Use PascalCase for parameter names
Use camelCase for variable names
Use lower-case for keywords
Use PascalCase for class names
Include scope for script/global/environment variables using $script:, $global:, or $env: prefixes
Use 4 spaces for indentation, not tabs
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))
Avoid spaces on empty lines
Limit lines to 120 characters
Place opening brace on a new line, except for variable assignments
Add one newline after opening brace
Add two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed (e.g., 'text' vs "text $variable")
Use single-line array syntax for simple arrays: @('one', 'two', 'three')
Use multi-line array syntax with each element on a separate line with proper indentation
Do not use the unary comma operator (,) in return statements to force an array
Use empty hashtable syntax: @{}
Place each hashtable property on a separate line with proper indentation
Use PascalCase for hashtable properties
Use single-line comment format # Comment (capitalized, on own line)
Use multi-line comment format <# Comment #> with opening and closing brackets on own lines and indented text
Never include commented-out code
Always add comment-based help to all functions and scripts with SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class declaration
Use comment-based help indentation with keywords at 4 spaces and text at 8 spaces
Include examples in comment-based help for all parameter sets and combinations
In comment-based help INPUTS sec...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1
**/*.{ps1,psm1}

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)

Use $PSCmdlet.ThrowTerminatingError() for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • 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 pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File Organization

  • Public commands: source/Public/{CommandName}.ps1
  • Private functions: source/Private/{FunctionName}.ps1
  • Classes: source/Classes/{DependencyGroupNumber}.{ClassName}.ps1
  • Enums: source/Enum/{DependencyGroupNumber}.{EnumName}.ps1
  • Unit tests: tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1
  • Integration tests: tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Requirements

  • 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/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1
  • CHANGELOG.md
{**/*.ps1,**/*.psm1,**/*.psd1}

⚙️ CodeRabbit configuration file

{**/*.ps1,**/*.psm1,**/*.psd1}: # 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 as inline code with a 1‑line description...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1
source/DSCResources/**/*.psm1

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)

source/DSCResources/**/*.psm1: Every DSC resource must define the three required functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource
Export functions using the *-TargetResource pattern
Get-TargetResource function must return a hashtable with all resource properties
Test-TargetResource function must return a boolean value ($true or $false)
Set-TargetResource function must not return anything (void)
Get-TargetResource should only include parameters needed to retrieve actual current state values
Get-TargetResource should remove non-mandatory parameters that are never used
Set-TargetResource and Test-TargetResource must have identical parameters
Unused mandatory parameters in Set-TargetResource and Test-TargetResource should include "Not used in <function_name>" in the help comment
Each DSC resource function must include at least one Write-Verbose statement
Get-TargetResource verbose messages must start with "Getting the current state of..."
Set-TargetResource verbose messages must start with "Setting the desired state of..."
Test-TargetResource verbose messages must start with "Determining the current state of..."
Use localized strings for all messages including Write-Verbose, Write-Error, and other messaging commands
Import localized strings using Get-LocalizedData at the module top level
Use try/catch blocks to handle exceptions in MOF-based DSC resources
Use New-InvalidDataException for invalid data errors instead of throw in MOF-based DSC resources
Use New-ArgumentException for argument-related errors instead of throw in MOF-based DSC resources
Use New-InvalidOperationException for invalid operation errors instead of throw in MOF-based DSC resources
Use New-ObjectNotFoundException for object not found errors instead of throw in MOF-based DSC resources
Use New-InvalidResultException for invalid result errors instead of throw in MOF-based DSC resources
Use New-NotImplementedException...

Files:

  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1

⚙️ CodeRabbit configuration file

source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources Guidelines

Required Functions

  • Every DSC resource must define: Get-TargetResource, Set-TargetResource, Test-TargetResource
  • Export using *-TargetResource pattern

Function Return Types

  • Get-TargetResource: Must return hashtable with all resource properties
  • Test-TargetResource: Must return boolean ($true/$false)
  • Set-TargetResource: Must not return anything (void)

Parameter Guidelines

  • Get-TargetResource: Only include parameters needed to retrieve actual current state values
  • Get-TargetResource: Remove non-mandatory parameters that are never used
  • Set-TargetResource and Test-TargetResource: Must have identical parameters
  • Set-TargetResource and Test-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help comment

Required Elements

  • Each function must include Write-Verbose at least once
    • Get-TargetResource: Use verbose message starting with "Getting the current state of..."
    • Set-TargetResource: Use verbose message starting with "Setting the desired state of..."
    • Test-TargetResource: Use verbose message starting with "Determining the current state of..."
  • Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
  • Import localized strings using Get-LocalizedData at module top

Error Handling

Files:

  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1
CHANGELOG.md

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)

CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.md

Always update CHANGELOG.md Unreleased section

Files:

  • 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
  • Skip adding entry if same change already exists in Unreleased section
  • No duplicate sections or items in Unreleased section

Files:

  • CHANGELOG.md
**/*.md

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)

**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
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
Format parameters using bold in Markdown documentation
Format values and literals using inline code in Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths using inline code in Markdown documentation

Files:

  • CHANGELOG.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
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : In unit tests: use SMO stub types from SMO.cs, never mock SMO types

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : When unit tests test classes or commands containing SMO types like `[Microsoft.SqlServer.Management.Smo.*]`, ensure they are properly stubbed in SMO.cs

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: After changing SMO stub types in SMO.cs, run tests in a new PowerShell session for changes to take effect

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Load SMO stub types in unit test files using `Add-Type -Path "$PSScriptRoot/../Stubs/SMO.cs"`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Add `$env:SqlServerDscCI = $true` in `BeforeAll` block and remove in `AfterAll` block of unit tests

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Mocking in `BeforeAll` (`BeforeEach` only when required)

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T18:00:20.934Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-11-27T18:00:20.934Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Use the exact setup block with BeforeDiscovery, BeforeAll, and AfterAll blocks including DscResource.Test module import and PSDefaultParameterValues configuration

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Mock variables prefix: 'mock'

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-10-10T14:01:42.703Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.703Z
Learning: For script-based (MOF) DSC resources in UpdateServicesDsc, unit tests should use Initialize-TestEnvironment with variables $script:dscModuleName and $script:dscResourceName, and set -ResourceType 'Mof'. This differs from class-based resource tests which use the simpler $script:moduleName template without Initialize-TestEnvironment.

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Set `$PSDefaultParameterValues` for `Mock:ModuleName`, `Should:ModuleName`, `InModuleScope:ModuleName`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Integration tests must include the required setup block with BeforeDiscovery that imports DscResource.Test module and BeforeAll that imports the module being tested

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use `Mock` inside `InModuleScope`-block

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Private functions/class resources: Always use `InModuleScope`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T18:56:46.759Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 1622
File: tests/Integration/DSC_SqlDatabase.Integration.Tests.ps1:1-30
Timestamp: 2025-11-27T18:56:46.759Z
Learning: Integration tests for MOF-based DSC resources (located in source/DSCResources/**) should use the Initialize-TestEnvironment pattern with -ResourceType 'Mof', not the BeforeDiscovery/BeforeAll pattern. The BeforeDiscovery/BeforeAll pattern is for class-based resources only.

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Omit `-MockWith` when returning `$null`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never add an empty `-MockWith` block

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use `param()` inside `-MockWith` scriptblocks, parameters are auto-bound

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T18:00:20.934Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-11-27T18:00:20.934Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Parameter set validation tests should use Get-Command with Where-Object filtering and Should assertions to verify ParameterSetName and ParameterListAsString

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : `Get-TargetResource` should only include parameters needed to retrieve actual current state values

Applied to files:

  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources

Applied to files:

  • source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1
  • source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1
  • CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions

Applied to files:

  • CHANGELOG.md
⏰ 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)
CHANGELOG.md (1)

40-50: LGTM: clear, scoped “Changed” entries for SqlProtocol/SqlProtocolTcpIp

Accurately records the refactor to public commands with correct naming and issue links.

source/DSCResources/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1 (1)

106-113: New Get-SqlDscServerProtocol usage is consistent and correct

Both Get-/Set-TargetResource now build $getSqlDscServerProtocolParameters with ServerName = $computerName, InstanceName = $InstanceName, and ProtocolName = 'TcpIp', and call Get-SqlDscServerProtocol. This cleanly replaces the old helper, preserves the existing semantics around using the local machine name, and keeps the null/error handling logic intact.

Also applies to: 297-304

source/DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1 (2)

83-90: Good reuse of Get-SqlDscServerProtocolName across Get/Set/Test

Switching from the private protocol-name helper to the public Get-SqlDscServerProtocolName in all three paths (Get-/Set-/Test-TargetResource) centralizes protocol metadata resolution and keeps verbose messages in sync with the public API. This is a clean refactor and aligns with the {Verb}-SqlDsc{Noun} naming convention.

Also applies to: 232-249, 460-464


98-105: Get-SqlDscServerProtocol integration looks correct

The new $getSqlDscServerProtocolParameters hashtables in Get-/Set-TargetResource use ServerName = $computerName, InstanceName = $InstanceName, and ProtocolName = $ProtocolName and then call Get-SqlDscServerProtocol. This maintains the prior behavior (local host for SMO/CIM access) while moving to the public helper. The downstream usage (IsEnabled, HasMultiIPAddresses, ProtocolProperties, Alter()) remains the same, so behavior should be preserved.

Also applies to: 255-262

tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (1)

46-48: No action required. The SMO stub path is correct.

The code correctly loads SMO stubs from tests/Unit/Stubs/SMO.cs using Join-Path -Path $PSScriptRoot -ChildPath 'Stubs\SMO.cs'. The stub file exists at this location, so the path resolves properly and tests will load the stub types without issues.

@johlju johlju added the needs review The pull request needs a code review. label Dec 11, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94%. Comparing base (443a52f) to head (472f317).

Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff          @@
##           main   #2368   +/-   ##
====================================
- Coverage    94%     94%   -1%     
====================================
  Files       167     167           
  Lines      9864    9843   -21     
====================================
- Hits       9303    9282   -21     
  Misses      561     561           
Flag Coverage Δ
unit 94% <100%> (-1%) ⬇️
Files with missing lines Coverage Δ
.../DSCResources/DSC_SqlProtocol/DSC_SqlProtocol.psm1 100% <100%> (ø)
...ces/DSC_SqlProtocolTcpIp/DSC_SqlProtocolTcpIp.psm1 100% <100%> (ø)
...dules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 96% <ø> (-1%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member Author

@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 10 of 10 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @johlju)

Copy link
Copy Markdown

@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

🧹 Nitpick comments (2)
tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (2)

46-48: Make SMO stub loading idempotent (avoid Add-Type collisions across test files).
Loading SMO stubs is the right move (per repo learnings), but Add-Type -Path ...SMO.cs can intermittently fail when multiple unit test files load the same stub types in the same PowerShell session.

Suggested tweak:

 # Load the SMO stub types
-Add-Type -Path (Join-Path -Path $PSScriptRoot -ChildPath 'Stubs\SMO.cs')
+if (-not ('Microsoft.SqlServer.Management.Smo.Server' -as [System.Type]))
+{
+    Add-Type -Path (Join-Path -Path $PSScriptRoot -ChildPath 'Stubs\SMO.cs')
+}

Based on learnings, also ensure unit tests are executed in a new PowerShell session after SMO.cs changes so the new types are picked up consistently.


1068-1096: Simplify the heavy New-Object | Add-Member mock objects; avoid backtick continuations.
These mocks work, but they’re hard to read/maintain and they rely on backticks inside the Alter method conditionals. Consider using [pscustomobject]@{} for the shape and only Add-Member for the Alter scriptmethod.

Example rewrite for one block (apply similarly to the other repeated ones):

 Mock -CommandName Get-SqlDscServerProtocol -MockWith {
-    return New-Object -TypeName PSObject |
-            Add-Member -MemberType NoteProperty -Name 'IPAddresses' -Value @{
-                Name  = 'IPAll'
-                IPAll = New-Object -TypeName PSObject |
-                    Add-Member -MemberType NoteProperty -Name 'IPAddressProperties' -Value @{
-                        TcpPort = New-Object -TypeName PSObject |
-                            Add-Member -MemberType NoteProperty -Name 'Value' -Value '1433' -PassThru -Force
-                        TcpDynamicPorts = New-Object -TypeName PSObject |
-                            Add-Member -MemberType NoteProperty -Name 'Value' -Value '' -PassThru -Force
-                    } -PassThru -Force
-            } -PassThru |
-            Add-Member -MemberType ScriptMethod -Name 'Alter' -Value {
+    $protocol = [pscustomobject]@{
+        IPAddresses = @{
+            Name  = 'IPAll'
+            IPAll = [pscustomobject]@{
+                IPAddressProperties = @{
+                    TcpPort        = [pscustomobject]@{ Value = '1433' }
+                    TcpDynamicPorts = [pscustomobject]@{ Value = '' }
+                }
+            }
+        }
+    }
+
+    return ($protocol | Add-Member -MemberType ScriptMethod -Name 'Alter' -Value {
         $ipAddressProperties = $this.IPAddresses['IPAll'].IPAddressProperties
-
-        if ($ipAddressProperties['TcpDynamicPorts'].Value -eq '0' `
-            -and $ipAddressProperties['TcpPort'].Value -eq '')
+        if (
+            ($ipAddressProperties['TcpDynamicPorts'].Value -eq '0') -and
+            ($ipAddressProperties['TcpPort'].Value -eq '')
+        )
         {
             InModuleScope -ScriptBlock {
                 $script:wasMethodAlterCalled = $true
             }
         }
-    } -PassThru -Force
+    } -PassThru -Force)
 }

The assertion update to $script:wasMethodAlterCalled | Should -BeTrue is a good, direct signal that the mock Alter() was invoked.

Also applies to: 1120-1121, 1141-1169, 1215-1240, 1285-1310, 1357-1382, 1432-1444

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 472f317 and 09b2137.

📒 Files selected for processing (2)
  • tests/Unit/DSC_SqlProtocol.Tests.ps1 (15 hunks)
  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/Unit/DSC_SqlProtocol.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (8)
**/*.ps1

📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)

**/*.ps1: Format public commands as {Verb}-SqlDsc{Noun} following PowerShell naming conventions
Format private functions as {Verb}-{Noun} following PowerShell naming conventions

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
**/*.[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 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
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'
Public commands: Never use InModuleScope (unless retrieving localized strings or creating an object using an internal class)
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
Spacing between blocks, arrange, act, and assert for readability
PascalCase: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse never -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
Never use param() inside -MockWith scriptblock...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.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 or creating an object using an internal class)
  • 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
  • Spacing between blocks, arrange, act, and assert for readability

Syntax Rules

  • PascalCase: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
  • Use -BeTrue/-BeFalse never -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, `...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)

tests/[Uu]nit/**/*.[Tt]ests.ps1: Test with localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
Mock files using the $TestDrive variable (path to the test drive)
All public commands require parameter set validation tests
Use the exact setup block with BeforeDiscovery, BeforeAll, and AfterAll blocks including DscResource.Test module import and PSDefaultParameterValues configuration
Parameter set validation tests should use Get-Command with Where-Object filtering and Should assertions to verify ParameterSetName and ParameterListAsString
Parameter property tests should verify mandatory parameter attributes using Get-Command Parameters and Should -BeTrue assertions

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1

⚙️ CodeRabbit configuration file

tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines

  • Test with localized strings: Use InModuleScope -ScriptBlock { $script:localizedData.Key }
  • Mock files: Use $TestDrive variable (path to the test drive)
  • All public commands require parameter set validation tests
  • After modifying classes, always run tests in new session (for changes to take effect)

Test Setup Requirements

Use this exact setup block before Describe:

[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'

    $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName
    $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName
    $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName
}

AfterAll {
    $PSDefaultParameterValues.Remove('InModuleScope:ModuleName')
    $PSDefaultParameterValues.Remove('Mock:ModuleNam...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
tests/Unit/**/*.Tests.ps1

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)

Unit tests should be located in tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
**/*.{ps1,psm1,psd1}

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)

**/*.{ps1,psm1,psd1}: Use descriptive names with 3+ characters and no abbreviations for functions, parameters, variables, keywords, and classes
Use PascalCase with Verb-Noun format for function names, using approved PowerShell verbs
Use PascalCase for parameter names
Use camelCase for variable names
Use lower-case for keywords
Use PascalCase for class names
Include scope for script/global/environment variables using $script:, $global:, or $env: prefixes
Use 4 spaces for indentation, not tabs
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))
Avoid spaces on empty lines
Limit lines to 120 characters
Place opening brace on a new line, except for variable assignments
Add one newline after opening brace
Add two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed (e.g., 'text' vs "text $variable")
Use single-line array syntax for simple arrays: @('one', 'two', 'three')
Use multi-line array syntax with each element on a separate line with proper indentation
Do not use the unary comma operator (,) in return statements to force an array
Use empty hashtable syntax: @{}
Place each hashtable property on a separate line with proper indentation
Use PascalCase for hashtable properties
Use single-line comment format # Comment (capitalized, on own line)
Use multi-line comment format <# Comment #> with opening and closing brackets on own lines and indented text
Never include commented-out code
Always add comment-based help to all functions and scripts with SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class declaration
Use comment-based help indentation with keywords at 4 spaces and text at 8 spaces
Include examples in comment-based help for all parameter sets and combinations
In comment-based help INPUTS sec...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
**/*.{ps1,psm1}

📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)

Use $PSCmdlet.ThrowTerminatingError() for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • 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 pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File Organization

  • Public commands: source/Public/{CommandName}.ps1
  • Private functions: source/Private/{FunctionName}.ps1
  • Classes: source/Classes/{DependencyGroupNumber}.{ClassName}.ps1
  • Enums: source/Enum/{DependencyGroupNumber}.{EnumName}.ps1
  • Unit tests: tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1
  • Integration tests: tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Requirements

  • 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/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}

⚙️ CodeRabbit configuration file

{**/*.ps1,**/*.psm1,**/*.psd1}: # 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 as inline code with a 1‑line description...

Files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
🧠 Learnings (32)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: After changing SMO stub types in SMO.cs, run tests in a new PowerShell session for changes to take effect
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : When unit tests test classes or commands containing SMO types like `[Microsoft.SqlServer.Management.Smo.*]`, ensure they are properly stubbed in SMO.cs
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : In unit tests: use SMO stub types from SMO.cs, never mock SMO types
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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.
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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.
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : When unit tests test classes or commands containing SMO types like `[Microsoft.SqlServer.Management.Smo.*]`, ensure they are properly stubbed in SMO.cs

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : In unit tests: use SMO stub types from SMO.cs, never mock SMO types

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: After changing SMO stub types in SMO.cs, run tests in a new PowerShell session for changes to take effect

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Load SMO stub types in unit test files using `Add-Type -Path "$PSScriptRoot/../Stubs/SMO.cs"`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/**/*.tests.ps1 : Add `$env:SqlServerDscCI = $true` in `BeforeAll` block and remove in `AfterAll` block of unit tests

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Mocking in `BeforeAll` (`BeforeEach` only when required)

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T18:00:20.934Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-11-27T18:00:20.934Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Use the exact setup block with BeforeDiscovery, BeforeAll, and AfterAll blocks including DscResource.Test module import and PSDefaultParameterValues configuration

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T18:56:46.759Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 1622
File: tests/Integration/DSC_SqlDatabase.Integration.Tests.ps1:1-30
Timestamp: 2025-11-27T18:56:46.759Z
Learning: Integration tests for MOF-based DSC resources (located in source/DSCResources/**) should use the Initialize-TestEnvironment pattern with -ResourceType 'Mof', not the BeforeDiscovery/BeforeAll pattern. The BeforeDiscovery/BeforeAll pattern is for class-based resources only.

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-10-10T14:01:42.703Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.703Z
Learning: For script-based (MOF) DSC resources in UpdateServicesDsc, unit tests should use Initialize-TestEnvironment with variables $script:dscModuleName and $script:dscResourceName, and set -ResourceType 'Mof'. This differs from class-based resource tests which use the simpler $script:moduleName template without Initialize-TestEnvironment.

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Set `$PSDefaultParameterValues` for `Mock:ModuleName`, `Should:ModuleName`, `InModuleScope:ModuleName`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Integration tests must include the required setup block with BeforeDiscovery that imports DscResource.Test module and BeforeAll that imports the module being tested

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use `Mock` inside `InModuleScope`-block

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Private functions/class resources: Always use `InModuleScope`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Omit `-MockWith` when returning `$null`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-08-16T13:35:08.323Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2134
File: source/Public/Get-SqlDscLogin.ps1:0-0
Timestamp: 2025-08-16T13:35:08.323Z
Learning: In PowerShell, users expect to receive $null when no objects are found or when a non-terminating error occurs, rather than empty arrays. This is normal PowerShell behavior and should be maintained in DSC commands.

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T18:00:20.934Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-11-27T18:00:20.934Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Parameter set validation tests should use Get-Command with Where-Object filtering and Should assertions to verify ParameterSetName and ParameterListAsString

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-09-25T16:38:08.867Z
Learnt from: johlju
Repo: dsccommunity/ActiveDirectoryDsc PR: 741
File: tests/Integration/MSFT_ADReadOnlyDomainControllerAccount.Integration.Tests.ps1:102-104
Timestamp: 2025-09-25T16:38:08.867Z
Learning: Test-DscConfiguration cmdlet returns string values 'True' or 'False', not boolean values. Therefore, Should -Be 'True' is correct, and Should -BeTrue would be incorrect for this cmdlet.

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use `Assert-MockCalled`, use `Should -Invoke` instead

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never add an empty `-MockWith` block

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use `param()` inside `-MockWith` scriptblocks, parameters are auto-bound

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Mock variables prefix: 'mock'

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : No `Should -Not -Throw` - invoke commands directly

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:58:31.910Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-11-27T17:58:31.910Z
Learning: Applies to tests/Integration/**/*.Integration.Tests.ps1 : Avoid `ExpectedMessage` parameter for `Should -Throw` assertions in integration tests

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ObjectNotFoundException` for object not found errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T18:00:20.934Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-11-27T18:00:20.934Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Parameter property tests should verify mandatory parameter attributes using Get-Command Parameters and Should -BeTrue assertions

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T17:59:27.205Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-11-27T17:59:27.205Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use `-BeTrue`/`-BeFalse` never `-Be $true`/`-Be $false`

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1
📚 Learning: 2025-11-27T18:00:35.078Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Follow PowerShell style and test guideline instructions strictly

Applied to files:

  • tests/Unit/DSC_SqlProtocolTcpIp.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 (1)
tests/Unit/DSC_SqlProtocolTcpIp.Tests.ps1 (1)

82-83: Mocks updated to Get-SqlDscServerProtocol look consistent with the refactor.
The new mock shapes (returning $null, IPAddresses as an array for “missing group”, and hashtable-style IPAddresses for specific groups) read as reasonable coverage for Get-TargetResource / Set-TargetResource call paths.

Only thing to keep an eye on: keep the mocked IPAddresses structure consistent with what the resource actually expects for each path (array-vs-hashtable), since the tests use both patterns now.

Also applies to: 121-133, 156-172, 205-222, 256-283, 315-342, 963-964, 993-1005

@johlju johlju merged commit 5202a10 into dsccommunity:main Dec 12, 2025
6 of 11 checks passed
@johlju johlju deleted the fix/issue-#2104 branch December 12, 2025 07:18
@johlju johlju removed the needs review The pull request needs a code review. label Dec 12, 2025
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.

Get-SqlDscServerProtocol: Add new command

2 participants