Enhance performance and documentation for DSC resource integration tests #2141
Conversation
- Improved performance in `.build/Test-ShouldRunDscResourceIntegrationTests.ps1` by adding early optimization to check for changes under the source folder before running expensive analysis. - Updated public command discovery to execute only when source changes are detected. - Added flow diagram and optimized analysis workflow description in `.build/README.md`. - Included a new requirement in DSC community style guidelines to follow existing code patterns.
WalkthroughUpdates document and implement an ordered change-analysis workflow for Test-ShouldRunDscResourceIntegrationTests.ps1, adding an early skip when no source/ changes are detected and deferring public-command discovery. README and style guidelines are updated accordingly, and CHANGELOG records these build/test documentation and workflow modifications. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI Pipeline
participant Script as Test-ShouldRunDscResourceIntegrationTests.ps1
participant Git as Git Diff
participant Analyzer as Discovery/Analysis
CI->>Script: Invoke with BaseBranch/CurrentBranch[/UseMergeBase]
Script->>Git: Get changed files
Script->>Script: Check for changes under source/
alt No changes under source/
Script-->>CI: return false (skip integration tests)
else Changes under source/
Script->>Analyzer: Discover public commands used by DSC resources
Script->>Analyzer: Check DSCResources and Classes
Analyzer-->>Script: Affected public commands/private functions
Script->>Analyzer: Check integration tests under tests/Integration/Resources/
Script-->>CI: return true/false (run/skip)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.build/Test-ShouldRunDscResourceIntegrationTests.ps1 (5)
488-493: Parameterize Public path and avoid false negatives with custom -SourcePathSame issue: hard-coded '^source/Public/'. Use $sourcePrefix.
Apply this diff:
-# Check if any public commands used by DSC resources are changed -$changedPublicCommands = $changedFiles | Where-Object -FilterScript { $_ -match '^source/Public/(.+)\.ps1$' } | - ForEach-Object -Process { [System.IO.Path]::GetFileNameWithoutExtension((Split-Path -Path $_ -Leaf)) } +# Check if any public commands used by DSC resources are changed +$changedPublicCommands = $changedFiles | + Where-Object -FilterScript { $_ -match ($sourcePrefix + 'Public/(.+)\.ps1$') } | + ForEach-Object -Process { [System.IO.Path]::GetFileNameWithoutExtension((Split-Path -Path $_ -Leaf)) }
503-506: Parameterize Private path with -SourcePathAlso hard-coded. Use $sourcePrefix to support custom layouts.
Apply this diff:
-# Check if any private functions used by the affected public commands or class-based DSC resources are changed -$changedPrivateFunctions = $changedFiles | Where-Object -FilterScript { $_ -match '^source/Private/(.+)\.ps1$' } | - ForEach-Object -Process { [System.IO.Path]::GetFileNameWithoutExtension((Split-Path -Path $_ -Leaf)) } +# Check if any private functions used by the affected public commands or class-based DSC resources are changed +$changedPrivateFunctions = $changedFiles | + Where-Object -FilterScript { $_ -match ($sourcePrefix + 'Private/(.+)\.ps1$') } | + ForEach-Object -Process { [System.IO.Path]::GetFileNameWithoutExtension((Split-Path -Path $_ -Leaf)) }
109-114: Escape command names in regex; hyphens break \b word boundariesPublic command names (Verb-Noun) contain hyphens; using "\b$commandName\b" can miss matches or produce false positives. Escape the name and use a boundary that treats hyphen as part of the token.
Apply this diff:
-# Look for command usage patterns: commandName, & commandName, or | commandName -if ($content -match "\b$commandName\b") +# Look for command usage; escape name and treat hyphen as part of token +$escaped = [regex]::Escape($commandName) +if ($content -match "(?<![\w-])$escaped(?![\w-])")Also applies to: 131-136
293-299: Avoid exceptions when Private doesn't exist; escape function names in regex
- Add -ErrorAction SilentlyContinue to Get-ChildItem for Private functions.
- Escape function names and use hyphen-aware token boundaries.
Apply this diff:
-$privateFunctionFiles = Get-ChildItem -Path (Join-Path -Path $SourcePath -ChildPath "Private") -Filter "*.ps1" | Select-Object -ExpandProperty BaseName +$privateFunctionFiles = Get-ChildItem -Path (Join-Path -Path $SourcePath -ChildPath "Private") -Filter "*.ps1" -ErrorAction SilentlyContinue | Select-Object -ExpandProperty BaseName foreach ($privateFunction in $privateFunctionFiles) { - if ($content -match "\b$privateFunction\b") + $escaped = [regex]::Escape($privateFunction) + if ($content -match "(?<![\w-])$escaped(?![\w-])") { $privateFunctions += $privateFunction } }
476-486: Parameterize DSC resource/class path checks using $SourcePathThe script currently hard-codes “^source/…” when detecting changed DSC resources or classes, but it already supports a customizable
-SourcePath. Let’s use that variable so custom source roots are honored.File: .build/Test-ShouldRunDscResourceIntegrationTests.ps1
Lines: 476–486- # Check if any DSC resources are directly changed - $changedDscResources = $changedFiles | Where-Object -FilterScript { $_ -match '^source/DSCResources/' -or $_ -match '^source/Classes/' } + # Check if any DSC resources or classes are directly changed + $changedDscResources = $changedFiles | Where-Object -FilterScript { + $_ -match "^$SourcePath/DSCResources/" -or $_ -match "^$SourcePath/Classes/" + }This ensures that if
-SourcePathis overridden (e.g. not “source”), the integration-test logic still detects resource or class changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.build/README.md(2 hunks).build/Test-ShouldRunDscResourceIntegrationTests.ps1(2 hunks).github/instructions/dsc-community-style-guidelines.instructions.md(1 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Always update the Unreleased section in CHANGELOG.md using Keep a Changelog format with concrete release notes for changes (new/modified commands, functions, resources)
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe main changes as concise release notes
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Markdown: wrap lines at word boundaries when exceeding 80 characters; use 2 spaces for indentation
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Surround fenced code blocks with blank lines
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
Files:
CHANGELOG.md
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.md
🧠 Learnings (12)
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named <Command>.Integration.Tests.ps1
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Classes/*.ps1 : Derived DSC resource classes must override Get, Test, Set, GetCurrentState, Modify, AssertProperties, NormalizeProperties following the provided pattern
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Public/*.ps1 : Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Unit tests for public commands must be placed in tests/Unit/Public and named <Command>.Tests.ps1
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Public command unit tests must include parameter set validation using the provided template/pattern
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Public command unit tests must include tests to validate parameter properties (e.g., Mandatory, ValueFromPipeline)
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Private/*.ps1 : Private functions must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Classes/*.Tests.ps1 : Unit tests for class-based resources must be placed in tests/Unit/Classes and named <Resource>.Tests.ps1
Applied to files:
.github/instructions/dsc-community-style-guidelines.instructions.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/{Public,Private,Classes}/*.ps1 : Use PascalCase for function names and parameters in public commands, private functions, and class-based resources
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Public/*.ps1 : Each public PowerShell command must be in its own script file named exactly as the command and located in source/Public
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
🪛 LanguageTool
.github/instructions/dsc-community-style-guidelines.instructions.md
[grammar] ~26-~26: There might be a mistake here.
Context: ...w guidelines over existing code patterns - Always update CHANGELOG.md Unreleased se...
(QB_NEW_EN_OTHER)
.build/README.md
[grammar] ~19-~19: Use correct spacing
Context: ...s by checking for changes in this order: 1. Early Source Check: First checks if an...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~21-~21: There might be a mistake here.
Context: ...f any files under source/ have changed - If no source changes, skips integration ...
(QB_NEW_EN_OTHER)
[grammar] ~22-~22: There might be a mistake here.
Context: ...ges, skips integration tests immediately 1. DSC Resources: Files under `source/DSC...
(QB_NEW_EN_OTHER)
[grammar] ~32-~32: Use correct spacing
Context: ...ntegration/Resources/` ### Flow Diagram The following diagram illustrates the de...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~34-~34: Use correct spacing
Context: ...strates the decision flow of the script: mermaid flowchart TD Start([Start Script]) --> Init[Initialize Parameters<br/>BaseBranch, CurrentBranch, UseMergeBase] Init --> GetChanges[Get Changed Files<br/>git diff between branches] GetChanges --> HasChanges{Any Changed<br/>Files?} HasChanges -->|No| RunTrue[Return TRUE<br/>Run integration tests] HasChanges -->|Yes| CheckSource{Any Changes<br/>Under source/?} CheckSource -->|No| Skip[Return FALSE<br/>Skip integration tests] CheckSource -->|Yes| Discover[Discover Public Commands<br/>Used by DSC Resources] Discover --> CheckDscRes{DSC Resources or<br/>Classes Changed?<br/>source/DSCResources/<br/>source/Classes/} CheckDscRes -->|Yes| RunTrue CheckDscRes -->|No| CheckPublic{Public Commands<br/>Used by DSC<br/>Resources Changed?<br/>source/Public/} CheckPublic -->|Yes| RunTrue CheckPublic -->|No| CheckPrivate{Private Functions<br/>Used by DSC-related<br/>Commands Changed?<br/>source/Private/} CheckPrivate -->|Yes| RunTrue CheckPrivate -->|No| CheckTests{Integration Test<br/>Files Changed?<br/>tests/Integration/Resources/} CheckTests -->|Yes| RunTrue CheckTests -->|No| Skip RunTrue --> End([End]) Skip --> End style Start fill:#e1f5fe style End fill:#e8f5e8 style RunTrue fill:#fff3e0 style Skip fill:#f3e5f5 ### Parameters | Parameter | Type | Default...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
.github/instructions/dsc-community-style-guidelines.instructions.md
26-26: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🔇 Additional comments (2)
.build/Test-ShouldRunDscResourceIntegrationTests.ps1 (2)
383-388: Doc update reads well and matches the new flow (early source check)The expanded DESCRIPTION correctly explains the early optimization and ordered analysis.
471-475: Defer discovery only when needed: LGTMPublic command discovery is correctly deferred until after the early skip. No further changes needed here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2141 +/- ##
====================================
Coverage 94% 94%
====================================
Files 110 110
Lines 8171 8171
====================================
Hits 7732 7732
Misses 439 439
🚀 New features to boost your workflow:
|
1 participant
Pull Request (PR) description
.build/Test-ShouldRunDscResourceIntegrationTests.ps1by adding early optimization to check for changes under the source folder before running expensive analysis..build/README.md.This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is