Compare-DscParameterState: Fixed a bug comparing single-valued arrays in the desired state when TurnOffTypeChecking is used #185
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ings, this causes wrong result Fixes dsccommunity#184
WalkthroughCompare-DscParameterState: tightened equality so a current value that is an array does not match a non-array desired value; final result construction now uses ArrayList.ForEach. Tests and CHANGELOG updated to cover single-valued-array behavior under TurnOffTypeChecking. No public exports changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant TestDsc as Test-DscParameterState
participant Compare as Compare-DscParameterState
Caller->>TestDsc: Invoke(CurrentValues, DesiredValues, TurnOffTypeChecking=$true)
TestDsc->>Compare: Compare each parameter entry
alt Desired and Current are non-array
Note right of Compare #bfe5bf: Scalar equality short-circuit (both non-array)
Compare-->>TestDsc: match / mismatch
else
Note right of Compare #fde0d0: Array-aware path (sort/compare membership/order)
Compare-->>TestDsc: match / mismatch
end
Note left of Compare #e8f4ff: Build final result via ArrayList.ForEach -> PSCustomObject
TestDsc-->>Caller: Return overall boolean + details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)source/Public/Compare-DscParameterState.ps1(1 hunks)tests/Unit/Public/Test-DscParameterState.Tests.ps1(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.mdsource/Public/Compare-DscParameterState.ps1tests/Unit/Public/Test-DscParameterState.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
MD013rule 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
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
source/Public/Compare-DscParameterState.ps1tests/Unit/Public/Test-DscParameterState.Tests.ps1
source/**/*.ps1
⚙️ CodeRabbit configuration file
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/{MyModuleName}.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming Patterns
- Format:
Verb_FunctionName_Action(underscore separators), e.g.Get_Database_ConnectingToDatabaseString 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/Public/Compare-DscParameterState.ps1
tests/[Uu]nit/**/*.[Tt]ests.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
$TestDrivevariable (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/Public/Test-DscParameterState.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.DscResource.Common (Build Package Module)
🔇 Additional comments (4)
CHANGELOG.md (1)
14-16: LGTM! Clear and accurate changelog entry.The entry accurately describes the bug fix and properly references issue #184. It follows the Keep a Changelog format and is appropriately placed under the Unreleased/Fixed section.
source/Public/Compare-DscParameterState.ps1 (1)
362-362: Excellent fix for the single-valued array comparison bug.The addition of
-and -not $currentType.IsArraycorrectly prevents the equality short-circuit when the current value is an array. This ensures that single-element arrays in the desired state are properly compared as arrays rather than being unwrapped to scalars, which was causing the bug reported in issue #184.The logic now requires both the desired and current types to be non-arrays before taking the simple equality path, ensuring array comparisons go through the proper array comparison logic below.
tests/Unit/Public/Test-DscParameterState.Tests.ps1 (2)
297-323: Good test coverage for single-valued array handling with TurnOffTypeChecking.This test correctly verifies that when TurnOffTypeChecking is enabled and a single-valued property is missing from the current state, the comparison returns false. The test setup with
SingleValueArray = 'v1'(as a string) in desired values exercises the bug fix for single-element arrays.
431-431: LGTM! Appropriate test data addition.Adding
SingleValueArray = 'v1', 'v2'to the current values properly sets up the test data for single-value array comparison scenarios.
Contributor
|
I think Needs to be: if ($currentValue -eq $desiredValue -and (-not $desiredType.IsArray -and -not $currentType.IsArray)) |
Contributor
|
Ignore the above, the tests fail due to the |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/Unit/Public/Test-DscParameterState.Tests.ps1 (1)
976-1002: Duplicate test scenario; consider consolidation.This test has the same issue as the test at lines 297-323: it validates that the function returns
$falsewhendesiredValueshas aSingleValueArrayproperty that doesn't exist incurrentValues(defined at lines 748-758 withoutSingleValueArray). This doesn't test the actual bug from issue #184 where desired has a single-value and current has a multi-value array.Since this test is functionally identical to the one at lines 297-323 (both test: desired has property, current doesn't), consider:
- Removing this duplicate test to reduce redundancy
- OR updating
currentValuesin this context (line 748) to includeSingleValueArray = 'v1', 'v2'and test the actual bug scenario hereNote: The past review comment about a syntax error on line 978 appears to have been addressed—the current code is correctly structured.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Unit/Public/Test-DscParameterState.Tests.ps1(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.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
$TestDrivevariable (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/Public/Test-DscParameterState.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Unit/Public/Test-DscParameterState.Tests.ps1(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.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
$TestDrivevariable (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/Public/Test-DscParameterState.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.DscResource.Common (Build Package Module)
🔇 Additional comments (2)
tests/Unit/Public/Test-DscParameterState.Tests.ps1 (2)
431-431: LGTM!This correctly adds the
SingleValueArraytest data tocurrentValuesfor validating the bug fix. The test at lines 624-650 properly exercises this scenario.
624-650: LGTM!This correctly tests the bug fix for issue #184. The test validates that when
TurnOffTypeCheckingis enabled, a single‑value desired state ('v1') is properly detected as different from a multi‑value current state ('v1', 'v2'), returning$falseas expected.
Comment on lines
297
to
323
| Context 'When there is a single-valued array and TurnOffTypeChecking is used' { | ||
| BeforeAll { | ||
| $desiredValues = [PSObject] @{ | ||
| String = 'a string' | ||
| Bool = $true | ||
| Int = '99' | ||
| Array = 'a', 'b', 'c' | ||
| SingleValueArray = 'v1' #Using a string by purpose to test single value array handling | ||
| } | ||
| } | ||
|
|
||
| It 'Should not throw exception' { | ||
| { Test-DscParameterState ` | ||
| -CurrentValues $currentValues ` | ||
| -DesiredValues $desiredValues ` | ||
| -TurnOffTypeChecking ` | ||
| -Verbose:$verbose } | Should -Not -Throw | ||
| } | ||
|
|
||
| It 'Should return $false' { | ||
| Test-DscParameterState ` | ||
| -CurrentValues $currentValues ` | ||
| -DesiredValues $desiredValues ` | ||
| -TurnOffTypeChecking ` | ||
| -Verbose:$verbose | Should -BeFalse | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove duplicate test from wrong context.
This test is in the "When testing single values" context, which defines $currentValues at lines 55-66 without SingleValueArray. Therefore, this test compares desiredValues.SingleValueArray = 'v1' against a $currentValues that doesn't have the property at all—testing a missing‑property scenario, not the bug from issue #184.
The correct test for the bug (single‑value desired vs multi‑value current with TurnOffTypeChecking) already exists at lines 624-650 in the "When testing array values" context, where $currentValues includes SingleValueArray = 'v1', 'v2'.
Remove this duplicate test:
- Context 'When there is a single-valued array and TurnOffTypeChecking is used' {
- BeforeAll {
- $desiredValues = [PSObject] @{
- String = 'a string'
- Bool = $true
- Int = '99'
- Array = 'a', 'b', 'c'
- SingleValueArray = 'v1' #Using a string by purpose to test single value array handling
- }
- }
-
- It 'Should not throw exception' {
- { Test-DscParameterState `
- -CurrentValues $currentValues `
- -DesiredValues $desiredValues `
- -TurnOffTypeChecking `
- -Verbose:$verbose } | Should -Not -Throw
- }
-
- It 'Should return $false' {
- Test-DscParameterState `
- -CurrentValues $currentValues `
- -DesiredValues $desiredValues `
- -TurnOffTypeChecking `
- -Verbose:$verbose | Should -BeFalse
- }
- }
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Context 'When there is a single-valued array and TurnOffTypeChecking is used' { | |
| BeforeAll { | |
| $desiredValues = [PSObject] @{ | |
| String = 'a string' | |
| Bool = $true | |
| Int = '99' | |
| Array = 'a', 'b', 'c' | |
| SingleValueArray = 'v1' #Using a string by purpose to test single value array handling | |
| } | |
| } | |
| It 'Should not throw exception' { | |
| { Test-DscParameterState ` | |
| -CurrentValues $currentValues ` | |
| -DesiredValues $desiredValues ` | |
| -TurnOffTypeChecking ` | |
| -Verbose:$verbose } | Should -Not -Throw | |
| } | |
| It 'Should return $false' { | |
| Test-DscParameterState ` | |
| -CurrentValues $currentValues ` | |
| -DesiredValues $desiredValues ` | |
| -TurnOffTypeChecking ` | |
| -Verbose:$verbose | Should -BeFalse | |
| } | |
| } |
🤖 Prompt for AI Agents
In tests/Unit/Public/Test-DscParameterState.Tests.ps1 around lines 297-323,
remove the duplicate Context block that tests the single-valued array with
TurnOffTypeChecking (the whole Context including its BeforeAll and two It
blocks) because it lives in the "When testing single values" context and
compares against $currentValues that lack SingleValueArray; delete this Context
so the correct test in the "When testing array values" context (lines ~624-650)
remains as the single source of truth for the issue.
Comment on lines
652
to
682
| Context 'When an array in hashtable has different order but SortArrayValues is used' { | ||
| BeforeAll { | ||
| $desiredValues = [PSObject] @{ | ||
| String = 'a string' | ||
| Bool = $true | ||
| Int = 99 | ||
| Array = 'a', 'b', 'c' | ||
| Hashtable = @{ | ||
| k1 = 'Test' | ||
| k2 = 123 | ||
| k3 = 'v3', 'v2', 'v1', 99 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| It 'Should not throw exception' { | ||
| { Test-DscParameterState ` | ||
| -CurrentValues $currentValues ` | ||
| -DesiredValues $desiredValues ` | ||
| -SortArrayValues ` | ||
| -Verbose:$verbose } | Should -Not -Throw | ||
| } | ||
|
|
||
| It 'Should return $true' { | ||
| Test-DscParameterState ` | ||
| -CurrentValues $currentValues ` | ||
| -DesiredValues $desiredValues ` | ||
| -SortArrayValues ` | ||
| -Verbose:$verbose | Should -BeTrue | ||
| } | ||
| } |
There was a problem hiding this comment.
Move test to correct context.
This test is in the "When testing array values" context (line 424), which defines currentValues.Hashtable.k3 = 'v1', 'v2', 'v3' at line 435 (3 elements). However, desiredValues.Hashtable.k3 = 'v3', 'v2', 'v1', 99 at line 662 has 4 elements. The test expects $true but the arrays have different lengths, so it should fail.
The test should be moved to the "When testing hashtables" context (line 805), where currentValues.Hashtable.k3 = 'v1', 'v2', 'v3', 99 at line 815 correctly includes the 99 element.
Move this test to the "When testing hashtables" context (after line 1000) where currentValues has the matching data:
- Context 'When an array in hashtable has different order but SortArrayValues is used' {
- BeforeAll {
- $desiredValues = [PSObject] @{
- String = 'a string'
- Bool = $true
- Int = 99
- Array = 'a', 'b', 'c'
- Hashtable = @{
- k1 = 'Test'
- k2 = 123
- k3 = 'v3', 'v2', 'v1', 99
- }
- }
- }
-
- It 'Should not throw exception' {
- { Test-DscParameterState `
- -CurrentValues $currentValues `
- -DesiredValues $desiredValues `
- -SortArrayValues `
- -Verbose:$verbose } | Should -Not -Throw
- }
-
- It 'Should return $true' {
- Test-DscParameterState `
- -CurrentValues $currentValues `
- -DesiredValues $desiredValues `
- -SortArrayValues `
- -Verbose:$verbose | Should -BeTrue
- }
- }
-Then add it after line 1000 in the "When testing hashtables" context.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
tests/Unit/Public/Test-DscParameterState.Tests.ps1 lines 652-682: the test block
that assumes desiredValues.Hashtable.k3 contains four items is currently placed
in the "When testing array values" context where currentValues.Hashtable.k3 only
has three items; move this entire Context block (lines 652-682) into the "When
testing hashtables" context so it runs against the matching currentValues (the
hashtable context where currentValues.Hashtable.k3 includes the 99), placing it
after line 1000 inside that context; ensure indentation and surrounding
BeforeAll/It blocks remain consistent and that no duplicate variable definitions
are created when relocating the block.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/Unit/Public/Test-DscParameterState.Tests.ps1 (1)
596-627: LGTM! Test correctly exercises the bug fix.This test properly validates the fix for issue #184:
- Uses
currentValues.SingleValueArray = 'v1', 'v2'(multi-valued array from parent context)- Sets
desiredValues.SingleValueArray = 'v1'(single-valued, intentionally as string to test the bug scenario)- With
TurnOffTypeCheckingenabled, correctly expects$false(drift detected)The test is now correctly placed within the "When testing array values" context where the matching
$currentValuesis defined, and follows all Pester v5 syntax and coding guidelines.Minor: Redundant comment.
The comment on line 603 is redundant since the context name already clearly describes the test scenario. Consider removing it for brevity:
- SingleValueArray = 'v1' #for testing the comparison of single value arrays in the desired state + SingleValueArray = 'v1'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Unit/Public/Test-DscParameterState.Tests.ps1(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.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
$TestDrivevariable (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/Public/Test-DscParameterState.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Test-DscParameterState.Tests.ps1
🔇 Additional comments (1)
tests/Unit/Public/Test-DscParameterState.Tests.ps1 (1)
403-403: LGTM! Test data properly added.The
SingleValueArraytest data is correctly placed in the "When testing array values" context's$currentValues, where it will be accessible to child test contexts that exercise the single-valued array comparison bug.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/Unit/Public/Compare-DscParameterState.Tests.ps1 (2)
1261-1261: Clarify the comment for accuracy.The comment says "single value arrays" but
'v1', 'v2'is actually a two-element array. The comment is misleading about what's being tested.Apply this diff to clarify the comment:
- SingleValueArray = 'v1', 'v2' #for testing the comparison of single value arrays in the desired state + SingleValueArray = 'v1', 'v2' # Multi-element array to test comparison against a single string in desired state
1518-1551: Clarify comment and consider enhancing test coverage.The test correctly verifies the bug fix, but has a couple of areas for improvement:
Comment accuracy: Line 1525 says "single value arrays" but
'v1'is a string, not an array. The comment should reflect that this tests comparing a string in desired state against a multi-element array in current state.Test completeness: The test doesn't verify that other properties remain unaffected, which is a pattern used consistently in similar tests throughout this file (e.g., "Should return all InDesiredState (without X property) in $true").
Parameter coverage: Issue #184 mentions both
TurnOffTypeCheckingandSortArrayValuesparameters in the reproduction, but this test only exercisesTurnOffTypeChecking. Consider whether testing the combination is necessary to fully cover the bug scenario.Apply this diff to improve the comment:
- SingleValueArray = 'v1' #for testing the comparison of single value arrays in the desired state + SingleValueArray = 'v1' # Single string to test comparison against multi-element array in current stateConsider adding an additional assertion to verify other properties are not affected:
It 'Should not return other properties' { $script:result.Where({ $_.Property -ne 'SingleValueArray' }) | Should -BeNullOrEmpty }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/Public/Compare-DscParameterState.ps1(2 hunks)tests/Unit/Public/Compare-DscParameterState.Tests.ps1(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- source/Public/Compare-DscParameterState.ps1
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/Compare-DscParameterState.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.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
$TestDrivevariable (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/Public/Compare-DscParameterState.Tests.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Unit/Public/Compare-DscParameterState.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/Public/Compare-DscParameterState.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.DscResource.Common (Build Package Module)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
14-16: Style consistency: backticks for command, bold parameter, add “Fixes”.For consistency with surrounding entries and style guides, wrap the command name in backticks, bold the parameter name, and append “Fixes [#184]”.
- - Compare-DscParameterState - - Fixed a bug comparing single-valued arrays in the desired state when - TurnOffTypeChecking is used [#184](https://github.com/dsccommunity/DscResource.Common/issues/184). + - `Compare-DscParameterState` + - Fixed a bug comparing single-valued arrays in the desired state when + **TurnOffTypeChecking** is used. Fixes [#184](https://github.com/dsccommunity/DscResource.Common/issues/184).As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.md
**/*.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
MD013rule 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
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
🧠 Learnings (1)
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Applied to files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...issues/177). - Compare-DscParameterState - Fixed a bug comparing single-valued arra...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.DscResource.Common (Build Package Module)
gaelcolas
approved these changes
Nov 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request (PR) description
Compare-DscParameterState: Fixed a bug comparing single-valued arrays in the desired state when TurnOffTypeChecking is used
This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
DSC Community Testing Guidelines.
This change is