Update tests to Pester 5 #86
Merged
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
WalkthroughRemoved legacy Pester4 unit tests, added Pester5 test suites and WSUS stubs/docs, updated analyzer and build pipeline, added localization strings, and changed post-Set error signaling in two DSC resources (switched to New-InvalidResultException usage / removed passing ErrorRecord). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant ApprovalRule as MSFT_UpdateServicesApprovalRule
Caller->>ApprovalRule: Set-TargetResource(params)
activate ApprovalRule
ApprovalRule->>ApprovalRule: Apply configuration
ApprovalRule->>ApprovalRule: Test-TargetResource()
alt Test returns false
note right of ApprovalRule #FFEB99: Calls New-InvalidResultException\n(ErrorRecord not supplied)
ApprovalRule-->>Caller: Non-terminating error object
else Test returns true
ApprovalRule-->>Caller: Success
end
deactivate ApprovalRule
sequenceDiagram
autonumber
participant Caller
participant Cleanup as MSFT_UpdateServicesCleanup
Caller->>Cleanup: Set-TargetResource(params)
activate Cleanup
Cleanup->>Cleanup: Apply configuration
Cleanup->>Cleanup: Test-TargetResource()
alt Test returns false
note right of Cleanup #FFEB99: New-InvalidResultException created\n(replaces previous terminating throw)
Cleanup-->>Caller: Non-terminating error object
else Test returns true
Cleanup-->>Caller: Success
end
deactivate Cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
|
@Borgquite, don't go too mad on the updating the other tests. Once #85 is merged, this will fix your issues you are seeing. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Tests/Helpers/ImitateUpdateServicesModule.psm1(0 hunks)Tests/Unit/MSFT_UpdateServicesApprovalRule.tests.ps1(0 hunks)Tests/Unit/MSFT_UpdateServicesCleanup.tests.ps1(0 hunks)Tests/Unit/MSFT_UpdateServicesServer.tests.ps1(0 hunks)tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesServer.Tests.ps1(1 hunks)tests/Unit/Stubs/README.md(1 hunks)tests/Unit/Stubs/UpdateServices.stubs.psm1(1 hunks)
💤 Files with no reviewable changes (4)
- Tests/Unit/MSFT_UpdateServicesApprovalRule.tests.ps1
- Tests/Unit/MSFT_UpdateServicesCleanup.tests.ps1
- Tests/Unit/MSFT_UpdateServicesServer.tests.ps1
- Tests/Helpers/ImitateUpdateServicesModule.psm1
🧰 Additional context used
📓 Path-based instructions (5)
**
⚙️ 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/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/Stubs/README.mdtests/Unit/Stubs/UpdateServices.stubs.psm1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/Stubs/UpdateServices.stubs.psm1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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:
tests/Unit/Stubs/README.md
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/Resources/*.[iI]ntegration.[tT]ests.ps1 : Place resource integration tests at tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Every MOF DSC resource module must define the functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1
📚 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: In unit tests, add $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Private functions and class resources: always use InModuleScope
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: Unit tests must load SMO stub types from SMO.cs (e.g., Add-Type -Path "$PSScriptRoot/../Stubs/SMO.cs")
Applied to files:
tests/Unit/Stubs/README.md
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: Applies to tests/Stubs/SMO.cs : 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/Stubs/README.md
📚 Learning: 2025-09-23T10:20:59.832Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-09-23T10:20:59.832Z
Learning: Applies to tests/Stubs/SMO.cs : Unit tests that cover classes/commands using SMO types must have those SMO types stubbed in tests/Stubs/SMO.cs
Applied to files:
tests/Unit/Stubs/README.md
📚 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 : Set-TargetResource must not return anything (void)
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 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 : Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.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.UpdateServicesDsc (Build Package Module)
|
@CodeRabbit, waiting for #85 to be merged then each of the remaining unit tests will be converted. Do not review again until requested to. |
|
@dan-hughes, understood! I'll pause automatic reviews for this PR. Once PR #85 is merged and you've converted the remaining unit tests, feel free to mention me when you'd like me to review the changes. 🧠 Learnings used✅ Actions performedReviews paused. |
2 similar comments
|
@dan-hughes, understood! I'll pause automatic reviews for this PR. Once PR #85 is merged and you've converted the remaining unit tests, feel free to mention me when you'd like me to review the changes. 🧠 Learnings used✅ Actions performedReviews paused. |
|
@dan-hughes, understood! I'll pause automatic reviews for this PR. Once PR #85 is merged and you've converted the remaining unit tests, feel free to mention me when you'd like me to review the changes. 🧠 Learnings used✅ Actions performedReviews paused. |
OK - if you're going to do this, that's great. I've got a window at work where I can work on this module but it won't last too long and I'd like to make the most of it :) |
It's at the front of my todo list. It's the UpdateServer tests which will take a little longer to do. |
|
P.S. @dan-hughes As you work through the UpdateServicesServer tests you'll want to fix this one - hat tip to CodeRabbit for picking it up: |
Thanks. A |
9 tasks
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (1)
5-44: Align the setup block with the exact DSCCommunity template (Already flagged previously)The setup diverges from the required template (Initialize-TestEnvironment, variable names). Please use the prescribed block verbatim.
🧹 Nitpick comments (7)
source/Modules/PDT/en-US/PDT.strings.psd1 (1)
1-10: Add string IDs to all localized strings for traceability.Based on learnings, all localization strings should include IDs in the form
(PREFIX####)where PREFIX is typically initials from the module/function name and numbers are sequential from 0001. This improves traceability in logs and maintains consistency with DSC Community standards.Apply this diff to add string IDs:
ConvertFrom-StringData @' - FileNotFound = File not found in the environment path. - PathVariableEmpty = The $Env:Path variable is empty. - AbsolutePathOrFileName = Absolute path or file name expected. - InvalidArgument = Invalid argument: '{0}' with value: '{1}'. - ProcessFailedToStartError = The process '{0}' with arguments '{1}' failed to start within the specified timeout. - ProcessStarted = Process matching path '{0}' started in process ID {1}. - ProcessAlreadyStarted = Process matching path '{0}' already started in process ID {1}. - ProcessFailedToStopError = The process '{0}' with arguments '{1}' failed to stop within the specified timeout. + FileNotFound = File not found in the environment path. (PDT0001) + PathVariableEmpty = The $Env:Path variable is empty. (PDT0002) + AbsolutePathOrFileName = Absolute path or file name expected. (PDT0003) + InvalidArgument = Invalid argument: '{0}' with value: '{1}'. (PDT0004) + ProcessFailedToStartError = The process '{0}' with arguments '{1}' failed to start within the specified timeout. (PDT0005) + ProcessStarted = Process matching path '{0}' started in process ID {1}. (PDT0006) + ProcessAlreadyStarted = Process matching path '{0}' already started in process ID {1}. (PDT0007) + ProcessFailedToStopError = The process '{0}' with arguments '{1}' failed to stop within the specified timeout. (PDT0008) '@build.yaml (1)
55-59: Merge stage is orphaned (never invoked)BuildWorkflow '.' only runs build and test; merge is not referenced. If intended to run, add it to '.' (or appropriate stage).
Example wiring:
'.': - - build - - test + - build + - test + - mergetests/Unit/MSFT_UpdateServicesServer.Tests.ps1 (5)
1-4: Align the setup preamble with DSC Community template (add Justification, keep suppressions minimal).Add Justification to the suppressions to match the standard block and improve analyzer clarity.
As per coding guidelines
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] -[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '')] +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester syntax confuses Script Analyzer.')] +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '', Justification = 'Allowed in tests to construct PSCredential fixtures.')] param ()
11-16: Use the exact BeforeDiscovery fallback and error message from the template.Standardize the build fallback path and message for consistency with other tests.
As per coding guidelines
- & "$PSScriptRoot/../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null + & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null ... - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'Also applies to: 24-25
87-116: Remove commented-out StrictMode and “ISSUE” notes; either enable StrictMode or prep needed vars.Commented-out code violates the guidelines. If resource variables (e.g., $ProxyCredentialUserName) need initialization, set them in InModuleScope before enabling StrictMode; otherwise remove these comments.
As per coding guidelines
Example minimal cleanup within each It (illustrative):
- #ISSUE: $ProxyCredentialUserName is not initialized - # Set-StrictMode -Version 1.0 + # Initialize module variables if needed, then enable strict mode. + # $script:ProxyCredentialUserName = $null + Set-StrictMode -Version 1.0Also applies to: 124-153, 181-210, 218-247, 274-286
310-316: Avoid cross-It dependency on $script:result; compute inside the data-driven It.This removes order-dependence and keeps results+assertions in the same It block.
As per coding guidelines
- InModuleScope -Parameters $_ -ScriptBlock { - Set-StrictMode -Version 1.0 - - $script:result.Products | Should -Contain $name - } + InModuleScope -Parameters $_ -ScriptBlock { + Set-StrictMode -Version 1.0 + $result = Get-TargetResource -Ensure 'Present' + $result.Products | Should -Contain $Name + }
28-45: File location and setup consistency.Place this resource test under tests/Unit/Classes and keep the template block unchanged (including ../../../ path). This aligns with repo conventions and makes the setup block reusable across files.
As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.vscode/analyzersettings.psd1(1 hunks)CHANGELOG.md(1 hunks)build.yaml(1 hunks)source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1(1 hunks)source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1(1 hunks)source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1(1 hunks)source/Modules/PDT/en-US/PDT.strings.psd1(1 hunks)tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesServer.Tests.ps1(1 hunks)tests/Unit/TestHelpers/CommonTestHelper.psm1(10 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (5)
**
⚙️ 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:
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1source/Modules/PDT/en-US/PDT.strings.psd1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/TestHelpers/CommonTestHelper.psm1build.yaml
{**/*.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/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1source/Modules/PDT/en-US/PDT.strings.psd1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/TestHelpers/CommonTestHelper.psm1
source/DSCResources/**/*.psm1
⚙️ CodeRabbit configuration file
source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources GuidelinesRequired Functions
- Every DSC resource must define:
Get-TargetResource,Set-TargetResource,Test-TargetResource- Export using
*-TargetResourcepatternFunction Return Types
Get-TargetResource: Must return hashtable with all resource propertiesTest-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 valuesGet-TargetResource: Remove non-mandatory parameters that are never usedSet-TargetResourceandTest-TargetResource: Must have identical parametersSet-TargetResourceandTest-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help commentRequired Elements
- Each function must include
Write-Verboseat 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-LocalizedDataat module topError Handling
- Do not use
throwfor terminating errors- Use
try/catchblocks to handle exceptions- Throw localized exceptions using the appropriate
New-*Exceptioncmdlet:
New‑InvalidDataExceptionNew-ArgumentExceptionNew-InvalidOperationException- [
New-ObjectNotFoundException](https:/...
Files:
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1
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/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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:
source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-28T14:00:12.946Z
Learnt from: dan-hughes
PR: dsccommunity/DhcpServerDsc#96
File: tests/Unit/DhcpServerDsc.OptionValueHelper/Public/Test-TargetResourceHelper.Tests.ps1:26-29
Timestamp: 2025-09-28T14:00:12.946Z
Learning: When DscResource.Test module dependency is not found in BeforeDiscovery blocks, the error message should instruct users to run ".\build.ps1 -ResolveDependency -Tasks build" (not "noop") because the build task is required to resolve dependencies and prepare for testing.
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store command/function localization in source/en-US/{MyModuleName}.strings.psd1
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Define strings using ConvertFrom-StringData with entries like `KeyName = Message with {0} placeholder. (PREFIX0001)`
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store class localization in source/en-US/{ResourceClassName}.strings.psd1
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 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 : Import localized strings at the top of the module using Get-LocalizedData
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 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/**/en-US/DSC_*.strings.psd1 : Each resource directory must contain an en-US localization folder with a strings file named DSC_<ResourceName>.strings.psd1
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : Use localized string keys instead of hardcoded strings in script output/messages
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Include string IDs in the form (PREFIX####), where PREFIX is initials from the class/function name and numbers are sequential from 0001
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 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 : Use localized strings for all messages (e.g., Write-Verbose, Write-Error)
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-08-10T15:11:52.897Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryDsc#741
File: azure-pipelines.yml:104-104
Timestamp: 2025-08-10T15:11:52.897Z
Learning: In the ActiveDirectoryDsc project, HQRM (High Quality Resource Module) tests must run in PowerShell 5 (Windows PowerShell) using `pwsh: false` in the Azure Pipelines configuration, while unit tests can run in PowerShell 7 (PowerShell Core) with `pwsh: true`. This differentiation is intentional due to HQRM's specific requirements and dependencies on Windows PowerShell.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource must not return anything (void)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.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.UpdateServicesDsc (Test Unit)
- GitHub Check: dsccommunity.UpdateServicesDsc (Test HQRM)
- GitHub Check: dsccommunity.UpdateServicesDsc (Build Package Module)
🔇 Additional comments (11)
.vscode/analyzersettings.psd1 (2)
4-68: LGTM! Well-organized rule expansion.The expanded IncludeRules section significantly improves code quality enforcement by including comprehensive rule sets from multiple sources. The organization by source (ScriptAnalyzer, DscResource.AnalyzerRules, Indented.ScriptAnalyzerRules) with clear comments enhances maintainability.
80-111: LGTM! Comprehensive rule configuration.The detailed rule configurations enforce consistent formatting across the codebase:
- PSUseConsistentWhitespace: Comprehensive whitespace checks (with intentional exclusions for operator and parameter spacing)
- PSPlaceOpenBrace/PSPlaceCloseBrace: Enforces DSC Community's NewLine brace style
- PSAlignAssignmentStatement: Ensures consistent alignment in hashtables
This configuration aligns well with DSC Community style guidelines and supports the PR's goal of modernizing the codebase.
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1 (2)
295-295: Correct fix: remove-ErrorRecordparameter outside catch block.The removal of
-ErrorRecord $_is correct. This code path is not inside a catch block, so$_has no meaningful exception context to pass. The previous code was incorrect.
263-265: No instances of-ErrorRecord $_outside catch blocks. Verified occurrences at lines 93, 289, and 310 are all withincatch;$_is populated correctly.tests/Unit/TestHelpers/CommonTestHelper.psm1 (1)
3-155: Test helper stubs look goodObjects and script methods are well-shaped for WSUS test scenarios; no issues spotted.
Also applies to: 157-238
source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1 (1)
14-15: Localized key addition LGTMTestFailedAfterSet string aligns with resource usage.
build.yaml (1)
55-58: Verify task names exist in imported task packs EnsurePester_If_Code_Coverage_Under_ThresholdandMerge_CodeCoverage_Filesare actually defined in the imported .ib.tasks/Task. files (Sampler and DscResource.Test modules); adjust workflow entries or task definitions if they’re missing.tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (3)
444-444: Verify mock consistency for Register-ScheduledTask.On line 444, the mock for
Register-ScheduledTaskincludes-RemoveParameterValidationand-RemoveParameterTypeparameters, but the mock on line 477 (in the 'When the resource is absent' context) does not include these parameters. Ensure this difference is intentional, or apply consistent mock configuration across both contexts.
1-506: Active Pester 5 test suite successfully implemented.The file now contains executable Pester v5 tests following the project's standard setup, which resolves the previous concern about commented-out legacy code. The test structure correctly covers Get-TargetResource, Test-TargetResource, and Set-TargetResource with proper use of InModuleScope, BeforeAll/AfterAll blocks, and data-driven tests.
Based on learnings
23-23: Update error message to match the task being run.The error message mentions running
buildbut the actual command on line 14 runs thenooptask.Apply this diff:
- throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'⛔ Skipped due to learnings
Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0 Timestamp: 2025-09-16T16:34:44.689Z Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested moduleLearnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0 Timestamp: 2025-09-16T16:32:58.363Z Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction StopLearnt from: dan-hughes PR: dsccommunity/DhcpServerDsc#96 File: tests/Unit/DhcpServerDsc.OptionValueHelper/Public/Test-TargetResourceHelper.Tests.ps1:26-29 Timestamp: 2025-09-28T14:00:12.946Z Learning: When DscResource.Test module dependency is not found in BeforeDiscovery blocks, the error message should instruct users to run ".\build.ps1 -ResolveDependency -Tasks build" (not "noop") because the build task is required to resolve dependencies and prepare for testing.Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0 Timestamp: 2025-09-16T16:32:58.363Z Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Catch [System.IO.FileNotFoundException] during setup and throw: "DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first."Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0 Timestamp: 2025-09-12T13:21:31.054Z Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cyclesLearnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0 Timestamp: 2025-09-16T16:35:31.909Z Learning: Build the project before running tests using ./build.ps1 -Tasks buildtests/Unit/MSFT_UpdateServicesServer.Tests.ps1 (1)
62-736: Overall: solid Pester 5 conversion and coverage.Good use of Mock/Should -Invoke, data-driven tests, and test helpers. The Get/Test/Set scenarios and wildcard handling are covered well.
Please confirm:
- Tests pass with Invoke-Pester in a fresh pwsh session after the preamble tweaks.
- No remaining reliance on values set in other Its after the data-driven change.
source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1 (1)
262-266: Do not pass -ErrorRecord $_ outside a catchHere $_ is not defined; passing it may yield misleading errors. Drop -ErrorRecord (or wrap in try/catch and pass the caught record).
Apply:
- New-InvalidOperationException -Message ( - $script:localizedData.RuleFailedToCreate -f $Name - ) -ErrorRecord $_ + New-InvalidOperationException -Message ( + $script:localizedData.RuleFailedToCreate -f $Name + )As per coding guidelines
♻️ Duplicate comments (1)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (1)
5-44: Align the Pester setup block with the DSC Community templateCurrent setup diverges: non-templated Initialize-TestEnvironment reliance, custom variables ($script:dscModuleName/ResourceName), and a non-canonical build.ps1 path. Use the exact template: set $script:moduleName, Import-Module explicitly, and set PSDefaultParameterValues per template.
-# Suppressing this rule because Script Analyzer does not understand Pester's syntax. -[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] -param () - -BeforeDiscovery { +[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 has been resolved, so if this module is not available, run 'noop' task. + # 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 + & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies has not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } } BeforeAll { - $script:dscModuleName = 'UpdateServicesDsc' - $script:dscResourceName = 'MSFT_UpdateServicesApprovalRule' - - $script:testEnvironment = Initialize-TestEnvironment ` - -DSCModuleName $script:dscModuleName ` - -DSCResourceName $script:dscResourceName ` - -ResourceType 'Mof' ` - -TestType 'Unit' + $script:moduleName = 'UpdateServicesDsc' + Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' # Load stub cmdlets and classes. Import-Module (Join-Path -Path $PSScriptRoot -ChildPath 'Stubs\UpdateServices.stubs.psm1') Import-Module (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers\CommonTestHelper.psm1') - $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:dscResourceName - $PSDefaultParameterValues['Mock:ModuleName'] = $script:dscResourceName - $PSDefaultParameterValues['Should:ModuleName'] = $script:dscResourceName + $PSDefaultParameterValues['InModuleScope:ModuleName'] = 'MSFT_UpdateServicesApprovalRule' + $PSDefaultParameterValues['Mock:ModuleName'] = 'MSFT_UpdateServicesApprovalRule' + $PSDefaultParameterValues['Should:ModuleName'] = 'MSFT_UpdateServicesApprovalRule' } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleName') $PSDefaultParameterValues.Remove('Should:ModuleName') - - Restore-TestEnvironment -TestEnvironment $script:testEnvironment - - # Unload stub module + # Unload stub modules Remove-Module -Name UpdateServices.stubs -Force Remove-Module -Name CommonTestHelper -Force - # Unload the module being tested so that It doesn't impact any other tests. - Get-Module -Name $script:dscResourceName -All | Remove-Module -Force + # Unload the module being tested so that it doesn't impact any other tests. + Get-Module -Name $script:moduleName -All | Remove-Module -Force }As per coding guidelines
🧹 Nitpick comments (10)
build.yaml (1)
57-58: 'merge' workflow is not wired into the default pipelineYou added a merge stage with Merge_CodeCoverage_Files, but '.' currently runs only build and test. This merge task will not execute unless:
- You explicitly call BuildWorkflow 'merge' from CI, or
- You add '- merge' under '.' so it runs after tests.
Recommend wiring it into '.' if it should always run after unit tests; otherwise, confirm CI calls it explicitly.
If you want it to run after tests by default, add it under '.':
BuildWorkflow: '.': - build - test + - mergesource/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1 (2)
274-277: Avoid dereferencing $null in verbose messageIn this else-branch $ApprovalRule is $null; use $Name instead to avoid $null.Name.
- Write-Verbose -Message ($script:localizedData.RuleDoNotExist -f $ApprovalRule.Name) + Write-Verbose -Message ($script:localizedData.RuleDoNotExist -f $Name)
25-27: Import-Module should stop on failureAdd -ErrorAction Stop to fail fast and surface a proper terminating error.
-Import-Module -Name $script:resourceHelperModulePath +Import-Module -Name $script:resourceHelperModulePath -ErrorAction StopEnsure this aligns with other resources (Cleanup already uses -ErrorAction Stop). As per coding guidelines
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1 (1)
87-96: Re-enable StrictMode by initializing required variablesSeveral It blocks comment out Set-StrictMode due to uninitialized variables (e.g., proxy creds). Better initialize those variables in the test scope so StrictMode can remain enabled to catch regressions.
Initialize placeholders (e.g., $ProxyCredentialUserName = $null) in the relevant InModuleScope blocks or a BeforeEach to keep Set-StrictMode -Version 1.0 active.
Also applies to: 125-150, 181-207, 218-247
.vscode/analyzersettings.psd1 (3)
81-91: Enable operator whitespace checks to enforce style guideYour style guide requires one space around operators. PSUseConsistentWhitespace has CheckOperator = $false, which weakens enforcement.
Apply this diff:
PSUseConsistentWhitespace = @{ Enable = $true CheckOpenBrace = $true CheckInnerBrace = $true CheckOpenParen = $true - CheckOperator = $false + CheckOperator = $true CheckSeparator = $true CheckPipe = $true CheckPipeForRedundantWhitespace = $true CheckParameter = $false }As per coding guidelines
44-47: Avoid overlapping alignment rules (PSSA vs Indented) to reduce duplicate/conflicting diagnosticsYou include Indented’s AlignAssignmentStatement and also configure PSSA’s PSAlignAssignmentStatement. Pick one for consistency; keeping both often yields duplicate messages.
Option A (keep PSSA, drop Indented):
'PSPlaceCloseBrace' - 'AlignAssignmentStatement' 'AvoidUsingDoubleQuotesForConstantString'Option B (keep Indented, drop PSSA config):
- PSAlignAssignmentStatement = @{ - Enable = $true - CheckHashtable = $true - }As per coding guidelines
Also applies to: 107-110
70-77: Typos in comment blockMinor nit: “ecluded” → “excluded”; also “reported by PSScriptAnalyzer, so they cannot be excluded.” improves clarity.
- The following types are not rules but parse errors reported by PSScriptAnalyzer - so they cannot be ecluded. They need to be filtered out from the result of + The following types are not rules but parse errors reported by PSScriptAnalyzer, + so they cannot be excluded. They need to be filtered out from the result of Invoke-ScriptAnalyzer.tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (3)
1-3: Add Justification to SuppressMessage for analyzer complianceTemplate requires the Justification argument on the attribute. Keep the comment if you want, but add the property.
-# Suppressing this rule because Script Analyzer does not understand Pester's syntax. -[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +# Suppressing this rule because Script Analyzer does not understand Pester's syntax. +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param ()Based on learnings
14-15: Align fallback invocation and guidance; prefer Join-Path for portability
- The code runs build.ps1 with -Tasks 'noop' but the error message says -Tasks build. Align the message.
- Use Join-Path for the fallback script path to avoid path separator issues.
- # 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 + # Redirect all streams to $null, except the error stream (stream 2) + & (Join-Path -Path $PSScriptRoot -ChildPath '..\..\build.ps1') -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null ... - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'As per coding guidelines
Also applies to: 23-24
51-59: Optional: Avoid double unloading; Restore-TestEnvironment already cleans upRestore-TestEnvironment usually unloads the tested module. Extra removal of the resource module is typically redundant.
Restore-TestEnvironment -TestEnvironment $script:testEnvironment # Unload stub module Remove-Module -Name UpdateServices.stubs -Force Remove-Module -Name CommonTestHelper -Force - # Unload the module being tested so that It doesn't impact any other tests. - Get-Module -Name $script:dscResourceName -All | Remove-Module -Force
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.vscode/analyzersettings.psd1(1 hunks)CHANGELOG.md(1 hunks)build.yaml(1 hunks)source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1(1 hunks)source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1(1 hunks)source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1(1 hunks)source/Modules/PDT/en-US/PDT.strings.psd1(1 hunks)tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesServer.Tests.ps1(1 hunks)tests/Unit/TestHelpers/CommonTestHelper.psm1(10 hunks)
✅ Files skipped from review due to trivial changes (1)
- source/DSCResources/MSFT_UpdateServicesCleanup/en-US/MSFT_UpdateServicesCleanup.strings.psd1
🧰 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:
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1source/Modules/PDT/en-US/PDT.strings.psd1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1build.yamlCHANGELOG.mdtests/Unit/TestHelpers/CommonTestHelper.psm1
{**/*.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/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1source/Modules/PDT/en-US/PDT.strings.psd1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/TestHelpers/CommonTestHelper.psm1
source/DSCResources/**/*.psm1
⚙️ CodeRabbit configuration file
source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources GuidelinesRequired Functions
- Every DSC resource must define:
Get-TargetResource,Set-TargetResource,Test-TargetResource- Export using
*-TargetResourcepatternFunction Return Types
Get-TargetResource: Must return hashtable with all resource propertiesTest-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 valuesGet-TargetResource: Remove non-mandatory parameters that are never usedSet-TargetResourceandTest-TargetResource: Must have identical parametersSet-TargetResourceandTest-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help commentRequired Elements
- Each function must include
Write-Verboseat 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-LocalizedDataat module topError Handling
- Do not use
throwfor terminating errors- Use
try/catchblocks to handle exceptions- Throw localized exceptions using the appropriate
New-*Exceptioncmdlet:
New‑InvalidDataExceptionNew-ArgumentExceptionNew-InvalidOperationException- [
New-ObjectNotFoundException](https:/...
Files:
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1
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/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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
🧠 Learnings (15)
📓 Common learnings
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/MSFT_UpdateServicesServer.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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:
source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Define strings using ConvertFrom-StringData with entries like `KeyName = Message with {0} placeholder. (PREFIX0001)`
Applied to files:
source/Modules/PDT/en-US/PDT.strings.psd1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-08-10T15:11:52.897Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryDsc#741
File: azure-pipelines.yml:104-104
Timestamp: 2025-08-10T15:11:52.897Z
Learning: In the ActiveDirectoryDsc project, HQRM (High Quality Resource Module) tests must run in PowerShell 5 (Windows PowerShell) using `pwsh: false` in the Azure Pipelines configuration, while unit tests can run in PowerShell 7 (PowerShell Core) with `pwsh: true`. This differentiation is intentional due to HQRM's specific requirements and dependencies on Windows PowerShell.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource must not return anything (void)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.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.UpdateServicesDsc (Test Unit)
- GitHub Check: dsccommunity.UpdateServicesDsc (Test HQRM)
- GitHub Check: dsccommunity.UpdateServicesDsc (Build Package Module)
🔇 Additional comments (7)
build.yaml (1)
55-56: Confirm exact Invoke-Build task name spellingThe capitalization change to Pester_If_Code_Coverage_Under_Threshold looks intentional, but Invoke-Build tasks must match the defined task name in the imported task files. Please confirm the task in Sampler/DscResource.Test uses this exact casing/spelling to avoid a “task not found” at runtime.
source/DSCResources/MSFT_UpdateServicesApprovalRule/MSFT_UpdateServicesApprovalRule.psm1 (1)
294-296: Correct switch to New-InvalidResultException without -ErrorRecordGood fix: after Test-TargetResource fails there’s no active exception, so omitting -ErrorRecord is correct.
Confirm the localized key TestFailedAfterSet exists for this resource (strings file). Based on learnings
CHANGELOG.md (1)
28-31: Changelog updates look goodItems clearly note Pester 5 migration and related infra changes. No action needed.
source/DSCResources/MSFT_UpdateServicesCleanup/MSFT_UpdateServicesCleanup.psm1 (1)
230-233: Correct usage of New-InvalidResultException after Test-TargetResource failureAppropriate pattern; no -ErrorRecord since no active exception. Matches MOF resource guidelines.
tests/Unit/TestHelpers/CommonTestHelper.psm1 (1)
1-238: Stubs look consistent and usefulGood use of PSCustomObject/ScriptMethod to emulate WSUS API surface; aligns with tests’ expectations.
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (1)
127-131: Incorrect mock name: Get-WSUSServer vs Get-WsusServerThe resource calls Get-WsusServer. Mocking Get-WSUSServer won’t intercept calls and will break Should -Invoke.
- BeforeAll { - Mock -CommandName Get-WSUSServer - } + BeforeAll { + Mock -CommandName Get-WsusServer + }Also applies to: 143-145
⛔ Skipped due to learnings
Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0 Timestamp: 2025-09-12T13:21:31.054Z Learning: Applies to **/*.[Tt]ests.ps1 : Never use Assert-MockCalled; use Should -Invoke insteadLearnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0 Timestamp: 2025-09-12T13:21:31.054Z Learning: Applies to **/*.[Tt]ests.ps1 : Mock variables must be prefixed with 'mock'Learnt from: CR PR: dsccommunity/SqlServerDsc#0 File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0 Timestamp: 2025-09-12T13:21:31.054Z Learning: Applies to **/*.[Tt]ests.ps1 : Perform mocking in BeforeAll (use BeforeEach only when required)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (1)
61-506: Solid Pester 5 migration and coverageGood use of InModuleScope, -ForEach on Context, modern Should -Invoke assertions, and $TestDrive independence. Suites cover Present/Absent, drift, and error paths cleanly.
Please confirm the suite runs on both Windows PowerShell 5.1 and PowerShell 7 in CI as per project conventions.
This was referenced Oct 10, 2025
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (1)
446-449: Fix parameter binding: use Classifications (plural).Set-TargetResource expects -Classifications [string[]]. Using -Classification will fail parameter binding.
Apply:
- Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid')Repeat for all shown occurrences in these blocks. Based on learnings
Also applies to: 495-501, 524-531, 577-584, 626-633, 660-667
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (1)
218-227: Remove assignment of Should assertions.Don’t assign Should; either assert directly or capture the cmd output first.
Apply:
- $result = Test-TargetResource @testParams | Should -BeTrue + Test-TargetResource @testParams | Should -BeTrue- $result = Test-TargetResource @testParams | Should -BeFalse + Test-TargetResource @testParams | Should -BeFalse- $result = Test-TargetResource @testParams | Should -BeFalse + Test-TargetResource @testParams | Should -BeFalseAlso applies to: 244-261, 284-293
🧹 Nitpick comments (4)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (2)
1-3: Align setup header to the standard template (Justification + noop).
- Add Justification to SuppressMessage.
- In the catch message, use -Tasks 'noop' per guidelines.
Apply:
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]- throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'Based on learnings
Also applies to: 5-25
127-129: Normalize mock command casing for consistency.Use the same casing as elsewhere in the file.
- Mock -CommandName Get-WSUSServer + Mock -CommandName Get-WsusServer.vscode/analyzersettings.psd1 (1)
41-49: Correct rule IDs to canonical PSScriptAnalyzer names.Several entries lack the PS- prefix and may not resolve.
Apply:
- 'PSUseConsistentWhitespace' - 'UseCorrectCasing' + 'PSUseConsistentWhitespace' + 'PSUseCorrectCasing' 'PSPlaceOpenBrace' 'PSPlaceCloseBrace' - 'AlignAssignmentStatement' - 'AvoidUsingDoubleQuotesForConstantString' - 'UseShouldProcessForStateChangingFunctions' + 'PSAlignAssignmentStatement' + 'PSAvoidUsingDoubleQuotesForConstantString' + 'PSUseShouldProcessForStateChangingFunctions'Ensure IncludeRules matches the IDs configured under Rules (e.g., PSAlignAssignmentStatement).
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (1)
1-3: Align setup header to the standard template (Justification + noop).
- Add Justification to SuppressMessage.
- In the catch message, use -Tasks 'noop'.
Apply:
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]- throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'Based on learnings
Also applies to: 5-25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.vscode/analyzersettings.psd1(1 hunks)tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1(1 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/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.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/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-10T14:01:42.665Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.665Z
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/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-08-10T15:11:52.897Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryDsc#741
File: azure-pipelines.yml:104-104
Timestamp: 2025-08-10T15:11:52.897Z
Learning: In the ActiveDirectoryDsc project, HQRM (High Quality Resource Module) tests must run in PowerShell 5 (Windows PowerShell) using `pwsh: false` in the Azure Pipelines configuration, while unit tests can run in PowerShell 7 (PowerShell Core) with `pwsh: true`. This differentiation is intentional due to HQRM's specific requirements and dependencies on Windows PowerShell.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use Assert-MockCalled; use Should -Invoke instead
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Keep results and assertions in the same It block
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Avoid using ExpectedMessage with Should -Throw assertions
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-25T16:38:08.867Z
Learnt from: johlju
PR: dsccommunity/ActiveDirectoryDsc#741
File: tests/Integration/MSFT_ADReadOnlyDomainControllerAccount.Integration.Tests.ps1:102-104
Timestamp: 2025-09-25T16:38:08.867Z
Learning: The PowerShell guideline "Use -BeTrue/-BeFalse; never use -Be $true/-Be $false" applies only when the actual return value is a boolean. When a cmdlet returns string values like 'True' or 'False', Should -Be 'True' is the appropriate assertion.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource must not return anything (void)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
🔇 Additional comments (1)
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (1)
105-112: Strict mode temporarily disabled in Absent scenarios.Acknowledged as tracked work; no change requested here. Ensure the follow-up issue remains open.
If helpful, confirm the tracking issue is still open and linked in the PR description.
Also applies to: 143-150
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (2)
226-226: Do not assign the result of Should assertionsAssigning the pipeline with Should to a variable is incorrect; call the assertion standalone.
- $result = Test-TargetResource @testParams | Should -BeTrue + Test-TargetResource @testParams | Should -BeTrue @@ - $result = Test-TargetResource @testParams | Should -BeFalse + Test-TargetResource @testParams | Should -BeFalse @@ - $result = Test-TargetResource @testParams | Should -BeFalse + Test-TargetResource @testParams | Should -BeFalseAlso applies to: 260-260, 292-292
107-108: Strict mode is commented out in these contextsKnown strict mode issues are tracked (Issue #89). Leave commented for now or fix underlying resource init so Set-StrictMode can be enabled.
Also applies to: 145-146
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (1)
446-449: Fix parameter name: use Classifications (plural) to match resource contractSet-TargetResource expects -Classifications [string[]]. Using -Classification will cause parameter binding errors and failing tests.
@@ - Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid') @@ - Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid') @@ - Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid') @@ - Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid') @@ - Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid') @@ - Classification = @('00000000-0000-0000-0000-0000testguid') + Classifications = @('00000000-0000-0000-0000-0000testguid')Based on learnings
Also applies to: 495-502, 524-531, 577-584, 627-633, 661-667
🧹 Nitpick comments (3)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (2)
2-2: Standardize the setup block to match the template
- Add the Justification argument to SuppressMessage.
- Align the thrown guidance to use -Tasks 'noop'.
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] @@ - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'Based on learnings
Also applies to: 23-24
128-129: Use a consistent command name for the mock and verificationUse the same casing as elsewhere to avoid confusion and ensure Should -Invoke finds the mock.
- Mock -CommandName Get-WSUSServer + Mock -CommandName Get-WsusServertests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (1)
2-2: Standardize the setup block to match the template
- Add the Justification argument to SuppressMessage.
- Align the thrown guidance to use -Tasks 'noop'.
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] @@ - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'Based on learnings
Also applies to: 23-24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.vscode/analyzersettings.psd1(1 hunks)tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1(1 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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-08-10T15:11:52.897Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryDsc#741
File: azure-pipelines.yml:104-104
Timestamp: 2025-08-10T15:11:52.897Z
Learning: In the ActiveDirectoryDsc project, HQRM (High Quality Resource Module) tests must run in PowerShell 5 (Windows PowerShell) using `pwsh: false` in the Azure Pipelines configuration, while unit tests can run in PowerShell 7 (PowerShell Core) with `pwsh: true`. This differentiation is intentional due to HQRM's specific requirements and dependencies on Windows PowerShell.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use Assert-MockCalled; use Should -Invoke instead
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Keep results and assertions in the same It block
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Avoid using ExpectedMessage with Should -Throw assertions
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-25T16:38:08.867Z
Learnt from: johlju
PR: dsccommunity/ActiveDirectoryDsc#741
File: tests/Integration/MSFT_ADReadOnlyDomainControllerAccount.Integration.Tests.ps1:102-104
Timestamp: 2025-09-25T16:38:08.867Z
Learning: The PowerShell guideline "Use -BeTrue/-BeFalse; never use -Be $true/-Be $false" applies only when the actual return value is a boolean. When a cmdlet returns string values like 'True' or 'False', Should -Be 'True' is the appropriate assertion.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-10T14:01:42.665Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.665Z
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/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource must not return anything (void)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.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.UpdateServicesDsc (Test HQRM)
- GitHub Check: dsccommunity.UpdateServicesDsc (Test Unit)
- GitHub Check: dsccommunity.UpdateServicesDsc (Build Package Module)
🔇 Additional comments (3)
.vscode/analyzersettings.psd1 (3)
70-78: LGTM!The comment clearly explains why TypeNotFound and RequiresModuleInvalid parse errors cannot be excluded through ExcludeRules and must be filtered from Invoke-ScriptAnalyzer results. The typo flagged in the previous review ("ecluded") has been correctly fixed to "excluded" on line 72.
80-111: LGTM!The Rules section provides detailed configuration for formatting rules. The settings are reasonable and align with the DSC Community style guidelines:
- PSUseConsistentWhitespace: CheckOperator and CheckParameter set to $false likely to reduce noise
- PSPlaceOpenBrace: OnSameLine = $false enforces Allman-style bracing (open brace on new line)
- PSPlaceCloseBrace and PSAlignAssignmentStatement: Sensible defaults for consistency
4-68: No rule names changes needed. The mix of PS-prefixed and unprefixed rule names in both IncludeRules and Rules matches the official PSScriptAnalyzer documentation; no edits required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1 (4)
1-3: Add Justification to the suppression attribute (match the standard header).Use the exact header to align with DSC Community test template.
-# Suppressing this rule because Script Analyzer does not understand Pester's syntax. -[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +# Suppressing this rule because Script Analyzer does not understand Pester's syntax. +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]As per coding guidelines
5-25: Tweak wording in setup: grammar and guidance string.
- “Assumes dependencies has been resolved” → “have been”
- Throw message should reference the noop bootstrap, not build.
- # Assumes dependencies has been resolved, so if this module is not available, run 'noop' task. + # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. ... - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'As per coding guidelines
126-133: Normalize mocked command name casing for consistency.You mock Get-WSUSServer but assert invocation for Get-WsusServer. PowerShell is case-insensitive, but keeping names consistent reduces confusion and avoids mismatches in future refactors.
- BeforeAll { - Mock -CommandName Get-WSUSServer - } + BeforeAll { + Mock -CommandName Get-WsusServer + }
441-457: Fix misleading Context description (no Synchronize parameter used).The test doesn’t pass Synchronize; it verifies that a null server doesn’t throw. Reword for clarity.
- Context 'When property ''Synchronize'' is $false the resource should not throw' { + Context 'When Get-WsusServer returns $null it should not throw' {tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1 (2)
1-3: Add Justification to the suppression attribute (match the standard header).Adopt the exact suppression header used across DSC Community tests.
-# Suppressing this rule because Script Analyzer does not understand Pester's syntax. -[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')] +# Suppressing this rule because Script Analyzer does not understand Pester syntax. +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]As per coding guidelines
5-25: Update setup wording: grammar and noop guidance.Align the comments and error message with the standard template.
- # Assumes dependencies has been resolved, so if this module is not available, run 'noop' task. + # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. ... - throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' + throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.'As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1(1 hunks)tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1(1 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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.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/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Add unit tests for all commands, functions, and resources
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-16T16:35:31.909Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-09-16T16:35:31.909Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Add integration tests for all public commands (and resources)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use Pester v5 syntax only
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Avoid unnecessary remove/recreate cycles
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : All public commands and class-based resources must have integration tests
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 Learning: 2025-08-10T15:11:52.897Z
Learnt from: dan-hughes
PR: dsccommunity/ActiveDirectoryDsc#741
File: azure-pipelines.yml:104-104
Timestamp: 2025-08-10T15:11:52.897Z
Learning: In the ActiveDirectoryDsc project, HQRM (High Quality Resource Module) tests must run in PowerShell 5 (Windows PowerShell) using `pwsh: false` in the Azure Pipelines configuration, while unit tests can run in PowerShell 7 (PowerShell Core) with `pwsh: true`. This differentiation is intentional due to HQRM's specific requirements and dependencies on Windows PowerShell.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Never use Assert-MockCalled; use Should -Invoke instead
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Keep results and assertions in the same It block
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-12T13:21:31.054Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-09-12T13:21:31.054Z
Learning: Applies to **/*.[Tt]ests.ps1 : Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : Avoid using ExpectedMessage with Should -Throw assertions
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-25T16:38:08.867Z
Learnt from: johlju
PR: dsccommunity/ActiveDirectoryDsc#741
File: tests/Integration/MSFT_ADReadOnlyDomainControllerAccount.Integration.Tests.ps1:102-104
Timestamp: 2025-09-25T16:38:08.867Z
Learning: The PowerShell guideline "Use -BeTrue/-BeFalse; never use -Be $true/-Be $false" applies only when the actual return value is a boolean. When a cmdlet returns string values like 'True' or 'False', Should -Be 'True' is the appropriate assertion.
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 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 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-09-16T16:32:58.363Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:32:58.363Z
Learning: Applies to tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1 : In BeforeDiscovery, ensure DscResource.Test is available; if not loaded or not available, run build.ps1 -Tasks 'noop' (suppressing non-error streams) and Import-Module 'DscResource.Test' -Force -ErrorAction Stop
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1tests/Unit/MSFT_UpdateServicesApprovalRule.Tests.ps1
📚 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 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
tests/Unit/MSFT_UpdateServicesCleanup.Tests.ps1
📚 Learning: 2025-10-10T14:01:42.665Z
Learnt from: dan-hughes
PR: dsccommunity/UpdateServicesDsc#86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.665Z
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/MSFT_UpdateServicesCleanup.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.UpdateServicesDsc (Test Unit)
- GitHub Check: dsccommunity.UpdateServicesDsc (Test HQRM)
- GitHub Check: dsccommunity.UpdateServicesDsc (Build Package Module)
johlju
approved these changes
Oct 10, 2025
There was a problem hiding this comment.
@johlju reviewed 11 of 14 files at r1, 8 of 11 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @dan-hughes)
|
@dan-hughes awesome work as usual! 🙂 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is