Enhance DSC compilation error handling and performance#240
Enhance DSC compilation error handling and performance#240raandree merged 8 commits intodsccommunity:mainfrom
Conversation
- Improved error reporting for DSC compilation failures, ensuring root causes are surfaced. - Updated test case discovery to include all DSC resources, even those that fail to load. - Introduced helper scripts to compile DSC configurations in a single PowerShell process, reducing overhead. - Suppressed non-critical errors during DSC resource initialization to streamline output.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMoves DSC compilation to an optional batch precompile step (Windows PowerShell 5.1) when tests run under PowerShell 7+, adds folder-based DSC discovery, robust error capture/reporting for compilation, and updates test scripts, CI settings, changelog, and module version pins. Changes
Sequence DiagramsequenceDiagram
actor TestRunner as Test Runner
participant Discover as Discovery (PS7+)
participant Batch as Batch Process (WinPS 5.1)
participant Compiler as Compiler/Config Script
participant Results as Results JSON
participant TestExec as Test Execution
TestRunner->>Discover: Start tests (detect PS7+)
Discover->>Discover: Enumerate DSC resource folders
Discover->>Batch: Spawn WinPS 5.1 process with resource list
activate Batch
Batch->>Batch: Initialize Datum and DSC metadata once
loop For each DSC resource
Batch->>Compiler: Compile resource configuration
Compiler->>Compiler: Import module, build config, capture errors
Compiler-->>Batch: Return success/failure details
end
Batch->>Results: Write aggregated compile results
deactivate Batch
Discover->>Results: Read aggregated results
Discover->>TestExec: Provide per-resource compile outcomes
TestExec->>TestExec: Validate/Assert MOF generation using results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
tests/Unit/DSCResources/DscResources.Tests.ps1 (1)
127-143:$stderrTaskis started but its result is never consumed.
ReadToEndAsync()is correctly used to drain stderr asynchronously and prevent deadlocks (line 131-132). However, theTask<string>result is never awaited or read afterWaitForExit(). If the batch script writes diagnostic info to stderr, it's silently discarded. Consider logging it:💡 Optional: log stderr output
$proc.WaitForExit() $script:batchExitCode = $proc.ExitCode + + $stderrOutput = $stderrTask.GetAwaiter().GetResult() + if ($stderrOutput) + { + Write-Warning "Batch compilation stderr: $stderrOutput" + } + $proc.Dispose()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/DSCResources/DscResources.Tests.ps1` around lines 127 - 143, The stderr Task started into $stderrTask (via StandardError.ReadToEndAsync()) is never awaited or its result consumed, so any stderr from the launched process is lost; after WaitForExit() (and before disposing $proc) await or synchronously retrieve $stderrTask.Result and write it to the test output (e.g. via [Console]::Error.WriteLine or the same logger used for stdout) to ensure stderr is captured and logged; keep $proc disposal and exit-code handling unchanged.CHANGELOG.md (1)
8-37: Changelog entries exceed the ≤2 items per change type guideline and are overly verbose.The
### Fixedsection has 3 top-level items, each with multi-line sub-items that read more like implementation notes than user-facing changelog entries. Consider condensing to ≤2 brief entries, e.g.:
- DSC compilation errors now surface the actual cause instead of being silently swallowed.
- Performance: all DSC resources compile in a single powershell.exe process, removing per-resource startup overhead.
As per coding guidelines: "Describe notable changes briefly, ≤2 items per change type".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 8 - 37, Condense the "### Fixed" changelog into at most two brief, user-facing bullet points by merging the existing three detailed items into (1) an entry that states "DSC compilation errors now surface actual causes instead of being silently swallowed" and (2) an entry that states "Performance: compile all DSC resources in a single powershell.exe to remove per-resource startup overhead"; remove implementation details about DscResources.Tests.ps1, CompileDscConfiguration.ps1, Get-DscResource, Initialize-DscResourceMetaInfo, and PowerShellGet duplication from the visible changelog (keep those as internal/commit notes if needed).tests/Unit/DSCResources/CompileDscConfigurations.ps1 (1)
185-194: Catch block re-collects$Errorincluding the re-thrown exception from the inner check.When a
throwfrom lines 171/175/177 is caught here,$Errornow contains both the original config errors and the throw itself. The aggregated message at line 190 will therefore duplicate the already-formatted message from line 171 alongside the raw config errors. This is noisy but not incorrect — the caller reads$result.Errorfrom JSON, so redundancy is acceptable for diagnostics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/DSCResources/CompileDscConfigurations.ps1` around lines 185 - 194, The catch block collects $Error into $allErrors and builds $result.Error, which causes duplication when a previous throw already formatted the message; modify the catch to filter out the re-thrown/formatted message by removing entries that equal or contain the already-set $result.Error (or the specific formatted prefix used when throwing) before joining $allMessages (i.e., after building $allMessages from $allErrors, apply a Where-Object filter to exclude messages matching $result.Error or the known thrown text), ensuring $result.Error contains only the original config errors rather than duplicating the re-thrown message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 18: Fix the typo in the changelog by inserting a space between
"definition)in" making it "definition) in"; update the text fragment
"definition)in its own try/catch and collected all errors from $Error in
chronological" to read "definition) in its own try/catch and collected all
errors from $Error in chronological".
In `@tests/Unit/DSCResources/CompileDscConfiguration.ps1`:
- Around line 58-65: The PSModulePath is not being filtered for PowerShell 7
entries in CompileDscConfiguration.ps1, which can cause WinPS 5.1 to fail to
load core cmdlets; update the script that builds $env:PSModulePath (the block
that concatenates $ModulePath, $RequiredModulesPath and $env:PSModulePath) to
remove any entries matching the PowerShell 7 pattern (e.g. '*\PowerShell\7\*')
like the filtering in CompileDscConfigurations.ps1, and also update the
SYNOPSIS/description at the top of CompileDscConfiguration.ps1 to accurately
state that this script is not currently invoked by the Pester tests.
In `@tests/Unit/DSCResources/CompileDscConfigurations.ps1`:
- Around line 1-37: The comment-based help is missing documentation for the
ResultPath parameter; add a .PARAMETER entry describing -ResultPath (what it is,
expected type/path, and what the script writes there) near the other .PARAMETER
blocks so the help output and Get-Help include ResultPath; reference the
parameter name "-ResultPath" and ensure the description matches its usage in the
script where ResultPath is used to write the JSON result file.
In `@tests/Unit/DSCResources/DscResources.Tests.ps1`:
- Around line 88-95: The test re-declares $skipped, duplicating the earlier
$skippedDscResources and risking divergence; update the block that builds
$resourceNames (the Get-ChildItem ... Where-Object pipeline used for
$resourceNames and $namesCsv) to remove the new $skipped declaration and instead
use the existing $skippedDscResources variable in the Where-Object filter so
both places agree on which resources are skipped.
- Around line 7-14: Remove the unused $dscResources and its surrounding
try/catch (the guarded Get-DscResource block) since $dscResources is never
referenced; replace the plain Get-DscResource call that sets $compositeResources
with a guarded version that uses ErrorAction SilentlyContinue/2>$null and
catches errors to assign $compositeResources = @() in the catch block; finally,
consolidate the duplicate skipped-lists by defining $skippedDscResources once
and reusing that single definition instead of having both $skippedDscResources
and a separate $skipped list (update references to use the single symbol).
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 8-37: Condense the "### Fixed" changelog into at most two brief,
user-facing bullet points by merging the existing three detailed items into (1)
an entry that states "DSC compilation errors now surface actual causes instead
of being silently swallowed" and (2) an entry that states "Performance: compile
all DSC resources in a single powershell.exe to remove per-resource startup
overhead"; remove implementation details about DscResources.Tests.ps1,
CompileDscConfiguration.ps1, Get-DscResource, Initialize-DscResourceMetaInfo,
and PowerShellGet duplication from the visible changelog (keep those as
internal/commit notes if needed).
In `@tests/Unit/DSCResources/CompileDscConfigurations.ps1`:
- Around line 185-194: The catch block collects $Error into $allErrors and
builds $result.Error, which causes duplication when a previous throw already
formatted the message; modify the catch to filter out the re-thrown/formatted
message by removing entries that equal or contain the already-set $result.Error
(or the specific formatted prefix used when throwing) before joining
$allMessages (i.e., after building $allMessages from $allErrors, apply a
Where-Object filter to exclude messages matching $result.Error or the known
thrown text), ensuring $result.Error contains only the original config errors
rather than duplicating the re-thrown message.
In `@tests/Unit/DSCResources/DscResources.Tests.ps1`:
- Around line 127-143: The stderr Task started into $stderrTask (via
StandardError.ReadToEndAsync()) is never awaited or its result consumed, so any
stderr from the launched process is lost; after WaitForExit() (and before
disposing $proc) await or synchronously retrieve $stderrTask.Result and write it
to the test output (e.g. via [Console]::Error.WriteLine or the same logger used
for stdout) to ensure stderr is captured and logged; keep $proc disposal and
exit-code handling unchanged.
| gets a test case even if it fails to load, and the actual compilation step in | ||
| PS 5.1 surfaces the real error. | ||
| - `CompileDscConfiguration.ps1` — Wrapped the Invoke-Expression (configuration | ||
| definition)in its own try/catch and collected all errors from $Error in chronological |
There was a problem hiding this comment.
Typo: missing space.
definition)in → definition) in
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 18, Fix the typo in the changelog by inserting a space
between "definition)in" making it "definition) in"; update the text fragment
"definition)in its own try/catch and collected all errors from $Error in
chronological" to read "definition) in its own try/catch and collected all
errors from $Error in chronological".
| try | ||
| { | ||
| # Set up PSModulePath to find all required modules | ||
| $env:PSModulePath = @( | ||
| $ModulePath | ||
| $RequiredModulesPath | ||
| $env:PSModulePath | ||
| ) -join ';' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for references to CompileDscConfiguration.ps1 (singular)
rg -n 'CompileDscConfiguration\.ps1' --type psRepository: dsccommunity/CommonTasks
Length of output: 50
🏁 Script executed:
# Also check if it's referenced in any other file types
rg -n 'CompileDscConfiguration\.ps1' -lRepository: dsccommunity/CommonTasks
Length of output: 80
🏁 Script executed:
# Search for references to CompileDscConfigurations.ps1 (plural) to compare
rg -n 'CompileDscConfigurations\.ps1' --type ps -A 2 -B 2Repository: dsccommunity/CommonTasks
Length of output: 569
🏁 Script executed:
# Check the content of CompileDscConfiguration.ps1 to see the PSModulePath handling
head -80 tests/Unit/DSCResources/CompileDscConfiguration.ps1 | tail -30Repository: dsccommunity/CommonTasks
Length of output: 912
🏁 Script executed:
# Check if CompileDscConfiguration.ps1 appears in CHANGELOG.md
rg -n 'CompileDscConfiguration' CHANGELOG.md -A 2 -B 2Repository: dsccommunity/CommonTasks
Length of output: 709
🏁 Script executed:
# List the DSCResources directory to see what helper scripts exist
ls -la tests/Unit/DSCResources/*.ps1 | grep -i compileRepository: dsccommunity/CommonTasks
Length of output: 272
🏁 Script executed:
# Check the SYNOPSIS of CompileDscConfiguration.ps1 to understand its intended use
head -30 tests/Unit/DSCResources/CompileDscConfiguration.ps1Repository: dsccommunity/CommonTasks
Length of output: 1023
🏁 Script executed:
# Compare the PSModulePath handling between both scripts
echo "=== CompileDscConfiguration.ps1 (singular) ===" && \
sed -n '58,70p' tests/Unit/DSCResources/CompileDscConfiguration.ps1 && \
echo "" && \
echo "=== CompileDscConfigurations.ps1 (plural) ===" && \
sed -n '74,85p' tests/Unit/DSCResources/CompileDscConfigurations.ps1Repository: dsccommunity/CommonTasks
Length of output: 1077
🏁 Script executed:
# Check if there's any reference to CompileDscConfiguration in test files or configuration
rg -i 'compiledsc' tests/ -lRepository: dsccommunity/CommonTasks
Length of output: 114
🏁 Script executed:
# Examine the full context of both PSModulePath assignments
echo "=== Singular version context ===" && \
sed -n '55,75p' tests/Unit/DSCResources/CompileDscConfiguration.ps1 && \
echo "" && echo "=== Plural version context ===" && \
sed -n '70,90p' tests/Unit/DSCResources/CompileDscConfigurations.ps1Repository: dsccommunity/CommonTasks
Length of output: 1596
🏁 Script executed:
# Check for any dynamic invocation of the singular script
rg -i 'CompileDscConfiguration' tests/ --type psRepository: dsccommunity/CommonTasks
Length of output: 213
🏁 Script executed:
# Check if there's any parameter passing or script block invocation
rg -i '\$CompileDsc|&.*CompileDsc|Invoke.*CompileDsc' tests/ --type psRepository: dsccommunity/CommonTasks
Length of output: 50
🏁 Script executed:
# Verify the actual usage in DscResources.Tests.ps1 around line 97
sed -n '85,110p' tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 1719
🏁 Script executed:
# Check if CompileDscConfiguration.ps1 is mentioned in any comments or documentation
rg -i 'singula|per.resource|per-resource' tests/Unit/DSCResources/ --type ps -A 2 -B 2Repository: dsccommunity/CommonTasks
Length of output: 675
🏁 Script executed:
# Verify the full CHANGELOG entry to understand the intended use
grep -A 10 "CompileDscConfiguration.ps1" CHANGELOG.mdRepository: dsccommunity/CommonTasks
Length of output: 926
🏁 Script executed:
# Check the git history or comments around CompileDscConfiguration.ps1 to understand its status
head -40 tests/Unit/DSCResources/CompileDscConfiguration.ps1Repository: dsccommunity/CommonTasks
Length of output: 1208
🏁 Script executed:
# Verify if there are any TODOs, FIXMEs, or notes about future integration
rg -i 'todo|fixme|future|planned' tests/Unit/DSCResources/CompileDscConfiguration.ps1Repository: dsccommunity/CommonTasks
Length of output: 50
🏁 Script executed:
# Double-check by searching for any dynamic script invocation patterns
rg -i 'scriptblock|invoke-command|start-process' tests/Unit/DSCResources/ --type ps | grep -i compileRepository: dsccommunity/CommonTasks
Length of output: 50
🏁 Script executed:
# Check git log to see when these scripts were added and any context
git log --oneline -- tests/Unit/DSCResources/CompileDscConfiguration.ps1 | head -5Repository: dsccommunity/CommonTasks
Length of output: 86
🏁 Script executed:
# Final verification: search the entire repo for any reference to CompileDscConfiguration (not CompileDscConfigurations)
find . -name "*.ps1" -o -name "*.psm1" -o -name "*.md" | xargs grep -l "CompileDscConfiguration" | grep -v ".git"Repository: dsccommunity/CommonTasks
Length of output: 131
🏁 Script executed:
# Check if there's any test parameterization or matrix that might invoke the singular script
grep -r "CompileDsc" tests/ --include="*.ps1" | head -20Repository: dsccommunity/CommonTasks
Length of output: 213
PSModulePath filtering missing—add PS7 module exclusion for consistency and correctness.
CompileDscConfiguration.ps1 lacks the PowerShell 7 module path filtering present in CompileDscConfigurations.ps1 (lines 74–79). Although this script is not currently invoked by tests, if it were used, the inherited $env:PSModulePath could contain *\PowerShell\7\* entries that would prevent WinPS 5.1 from loading core cmdlets like ConvertTo-SecureString.
Additionally, the SYNOPSIS states this script is "called from the Pester tests," but it is not currently referenced anywhere in the test suite—this documentation should be clarified.
🔧 Proposed fix: align with the batch script's filtering
# Set up PSModulePath to find all required modules
+ # Filter out PowerShell 7 module paths – they contain modules that are
+ # .NET Core-only and cannot be loaded by Windows PowerShell 5.1.
+ $filteredParentPath = ($env:PSModulePath -split ';' |
+ Where-Object { $_ -and $_ -notlike '*\PowerShell\7\*' }) -join ';'
+
$env:PSModulePath = @(
$ModulePath
$RequiredModulesPath
- $env:PSModulePath
+ $filteredParentPath
) -join ';'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Unit/DSCResources/CompileDscConfiguration.ps1` around lines 58 - 65,
The PSModulePath is not being filtered for PowerShell 7 entries in
CompileDscConfiguration.ps1, which can cause WinPS 5.1 to fail to load core
cmdlets; update the script that builds $env:PSModulePath (the block that
concatenates $ModulePath, $RequiredModulesPath and $env:PSModulePath) to remove
any entries matching the PowerShell 7 pattern (e.g. '*\PowerShell\7\*') like the
filtering in CompileDscConfigurations.ps1, and also update the
SYNOPSIS/description at the top of CompileDscConfiguration.ps1 to accurately
state that this script is not currently invoked by the Pester tests.
| <# | ||
| .SYNOPSIS | ||
| Helper script to compile multiple DSC configurations in a single | ||
| Windows PowerShell 5.1 process. | ||
|
|
||
| .DESCRIPTION | ||
| This script performs the expensive module imports and DSC resource | ||
| metadata initialisation only once, then iterates over every requested | ||
| DSC resource and compiles its MOF file. The result is a JSON file | ||
| written to the path specified by -ResultPath: | ||
|
|
||
| [ | ||
| { "Name": "AddsDomain", "Success": true, "Error": null }, | ||
| { "Name": "AddsDomainController", "Success": false, "Error": "..." } | ||
| ] | ||
|
|
||
| Console output (Write-Host) streams in real-time so the caller can | ||
| see progress as each resource is compiled. | ||
|
|
||
| .PARAMETER DscResourceNames | ||
| Comma-separated list of DSC composite resource names to compile. | ||
|
|
||
| .PARAMETER ModuleName | ||
| The name of the module under test (e.g. CommonTasks). | ||
|
|
||
| .PARAMETER OutputPath | ||
| The directory where the MOF files should be generated. | ||
|
|
||
| .PARAMETER AssetsPath | ||
| The path to the test Assets directory containing Datum.yml and Config data. | ||
|
|
||
| .PARAMETER RequiredModulesPath | ||
| The path to the RequiredModules directory. | ||
|
|
||
| .PARAMETER ModulePath | ||
| The path to the built module output directory. | ||
| #> |
There was a problem hiding this comment.
Missing -ResultPath parameter documentation in comment-based help.
The ResultPath parameter (line 64-66) is not documented in the .PARAMETER section of the comment-based help.
📝 Proposed fix
.PARAMETER ModulePath
The path to the built module output directory.
+
+ .PARAMETER ResultPath
+ The path to the JSON file where compilation results will be written.
#>🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'CompileDscConfigurations.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Unit/DSCResources/CompileDscConfigurations.ps1` around lines 1 - 37,
The comment-based help is missing documentation for the ResultPath parameter;
add a .PARAMETER entry describing -ResultPath (what it is, expected type/path,
and what the script writes there) near the other .PARAMETER blocks so the help
output and Get-Help include ResultPath; reference the parameter name
"-ResultPath" and ensure the description matches its usage in the script where
ResultPath is used to write the JSON result file.
| try | ||
| { | ||
| $dscResources = Get-DscResource -Module $moduleUnderTest.Name -ErrorAction SilentlyContinue 2>$null | ||
| } | ||
| catch | ||
| { | ||
| # Non-critical – resource discovery issues surface during compilation below. | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file
fd -t f "DscResources.Tests.ps1" tests/Unit/Repository: dsccommunity/CommonTasks
Length of output: 114
🏁 Script executed:
# Read the full file to understand context
wc -l tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 118
🏁 Script executed:
# Read the specific sections mentioned in the review
cat -n tests/Unit/DSCResources/DscResources.Tests.ps1 | sed -n '1,60p'Repository: dsccommunity/CommonTasks
Length of output: 2921
🏁 Script executed:
# Check for all references to $dscResources variable
rg -n '\$dscResources' tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 177
🏁 Script executed:
# Check for references to $stderrTask
rg -n '\$stderrTask' tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 134
🏁 Script executed:
# Check the skipped resources lists at different locations
rg -n 'skipped|PowerShellRepositories|RemoteDesktopCollections|RemoteDesktopDeployment' tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 1241
Remove dead code: $dscResources variable and guarded Get-DscResource call.
Lines 7–14 assign $dscResources but never reference it. Test cases are built from $dscResourceFolders (line 44), making this entire try/catch block unused. Additionally, the second call on line 55 ($compositeResources = Get-DscResource …) lacks error suppression, inconsistent with the guarded approach on line 9.
Delete lines 7–14 and add error handling to line 55:
try
{
$compositeResources = Get-DscResource -Module $moduleUnderTest.Name -ErrorAction SilentlyContinue 2>$null
}
catch
{
# Non-critical – resource discovery issues surface during compilation below.
$compositeResources = @()
}Also consolidate the duplicate $skippedDscResources list (line 18) and $skipped (line 91)—define once and reuse.
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] 11-14: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
(PSAvoidUsingEmptyCatchBlock)
[warning] 9-9: The variable 'dscResources' is assigned but never used.
(PSUseDeclaredVarsMoreThanAssignments)
[warning] Missing BOM encoding for non-ASCII encoded file 'DscResources.Tests.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Unit/DSCResources/DscResources.Tests.ps1` around lines 7 - 14, Remove
the unused $dscResources and its surrounding try/catch (the guarded
Get-DscResource block) since $dscResources is never referenced; replace the
plain Get-DscResource call that sets $compositeResources with a guarded version
that uses ErrorAction SilentlyContinue/2>$null and catches errors to assign
$compositeResources = @() in the catch block; finally, consolidate the duplicate
skipped-lists by defining $skippedDscResources once and reusing that single
definition instead of having both $skippedDscResources and a separate $skipped
list (update references to use the single symbol).
| # in a single Windows PowerShell 5.1 process. This avoids the | ||
| # expensive per-resource overhead of module imports and | ||
| # Initialize-DscResourceMetaInfo. | ||
| $skipped = @('PowerShellRepositories', 'RemoteDesktopCollections', 'RemoteDesktopDeployment') | ||
| $resourceNames = Get-ChildItem -Path "$($moduleUnderTest.ModuleBase)\DSCResources\*" -Directory | | ||
| Where-Object { $_.BaseName -notin $skipped } | | ||
| ForEach-Object { $_.BaseName } | ||
| $namesCsv = $resourceNames -join ',' |
There was a problem hiding this comment.
Duplicate skipped-resources list — divergence risk.
Line 91 re-declares a $skipped array identical to $skippedDscResources on line 18. If someone adds a resource to one list but not the other, the batch compilation and test-case construction will disagree on what to skip.
🔧 Proposed fix: reuse the existing variable
- $skipped = @('PowerShellRepositories', 'RemoteDesktopCollections', 'RemoteDesktopDeployment')
$resourceNames = Get-ChildItem -Path "$($moduleUnderTest.ModuleBase)\DSCResources\*" -Directory |
- Where-Object { $_.BaseName -notin $skipped } |
+ Where-Object { $_.BaseName -notin $skippedDscResources } |
ForEach-Object { $_.BaseName }🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'DscResources.Tests.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Unit/DSCResources/DscResources.Tests.ps1` around lines 88 - 95, The
test re-declares $skipped, duplicating the earlier $skippedDscResources and
risking divergence; update the block that builds $resourceNames (the
Get-ChildItem ... Where-Object pipeline used for $resourceNames and $namesCsv)
to remove the new $skipped declaration and instead use the existing
$skippedDscResources variable in the Where-Object filter so both places agree on
which resources are skipped.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
build.yaml (1)
46-46: Consider removing the commented-outFixEncodingstep entirely.The PR intent (per commit message and title) is to permanently remove this task. Leaving a commented-out step risks it being accidentally re-enabled or causing confusion about whether it should be restored. Module output encoding is already governed by
Encoding: UTF8at line 8.♻️ Proposed cleanup
- Build_Module_ModuleBuilder - Build_NestedModules_ModuleBuilder - Create_Changelog_Release_Output - `#-` FixEncoding🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.yaml` at line 46, Remove the commented-out FixEncoding step from build.yaml (the line containing "#- FixEncoding") so the file no longer contains the disabled task; the Encoding: UTF8 setting already governs output encoding, so simply delete the commented line to avoid confusion or accidental re-enablement and commit the cleaned build.yaml.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@build.yaml`:
- Line 46: Remove the commented-out FixEncoding step from build.yaml (the line
containing "#- FixEncoding") so the file no longer contains the disabled task;
the Encoding: UTF8 setting already governs output encoding, so simply delete the
commented line to avoid confusion or accidental re-enablement and commit the
cleaned build.yaml.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
RequiredModules.psd1 (1)
21-21: Consider pinningDscBuildHelpersto a known-good versionFloating to
'latest'withAllowPreRelease = $truemeans any new prerelease pushed to PSGallery will silently roll in. The newCompileDscConfigurations.ps1script (added in this PR) directly usesDscBuildHelpersAPIs; a breaking prerelease could silently break CI. All other build tools (InvokeBuild,Pester, etc.) also use'latest', so this is consistent with the project pattern — but given DscBuildHelpers is central to the new compilation flow, a pinned version (or at minimum a minimum-version constraint) would improve reproducibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RequiredModules.psd1` at line 21, Replace the floating 'latest' entry for DscBuildHelpers in RequiredModules.psd1 with a pinned or minimum-version constraint to prevent automatic prerelease roll-ins; specifically change the DscBuildHelpers entry (referenced by the symbol DscBuildHelpers and consumed by CompileDscConfigurations.ps1) from 'latest' to either a specific known-good version string or a minimum-version spec (e.g., a RequiredVersion or MinimumVersion value) and ensure AllowPreRelease handling is adjusted if you still want prerelease behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@RequiredModules.psd1`:
- Line 27: The PSDesiredStateConfiguration entry in RequiredModules.psd1 is set
to 'latest' which can resolve to v3.x prereleases incompatible with Windows
PowerShell 5.1; update the RequiredModules.psd1 entry for
PSDesiredStateConfiguration to either remove the entry (so the inbox v1.1 is
used) or pin it to a PS5.1-compatible version (e.g., '1.1.0.0'), and ensure any
PSDependOptions that allow prerelease are not causing installs of v3.x; look for
the PSDesiredStateConfiguration key in RequiredModules.psd1 and related
PSDependOptions/CompileDscConfigurations.ps1 usage to apply the change.
---
Nitpick comments:
In `@RequiredModules.psd1`:
- Line 21: Replace the floating 'latest' entry for DscBuildHelpers in
RequiredModules.psd1 with a pinned or minimum-version constraint to prevent
automatic prerelease roll-ins; specifically change the DscBuildHelpers entry
(referenced by the symbol DscBuildHelpers and consumed by
CompileDscConfigurations.ps1) from 'latest' to either a specific known-good
version string or a minimum-version spec (e.g., a RequiredVersion or
MinimumVersion value) and ensure AllowPreRelease handling is adjusted if you
still want prerelease behavior.
| 'DscResource.DocGenerator' = 'latest' | ||
| PowerShellForGitHub = 'latest' | ||
| #PSDesiredStateConfiguration = 'latest' | ||
| PSDesiredStateConfiguration = 'latest' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full RequiredModules.psd1 file to understand context
cat RequiredModules.psd1 | head -80Repository: dsccommunity/CommonTasks
Length of output: 2794
🏁 Script executed:
# Search for any PowerShell 5.1 or powershell.exe subprocess references
rg -n "powershell\.exe|pwsh.*false|PowerShell.*5|WinPS" --type=ps1 --type=psd1 -iRepository: dsccommunity/CommonTasks
Length of output: 95
🏁 Script executed:
# Check for any build or compilation scripts that might use PS5.1
fd -e ps1 -e psd1 "build|compile|dsc" | head -20Repository: dsccommunity/CommonTasks
Length of output: 575
🏁 Script executed:
# Search for PSDesiredStateConfiguration usage and configuration
rg -n "PSDesiredStateConfiguration" --type=ps1 --type=psd1Repository: dsccommunity/CommonTasks
Length of output: 95
🏁 Script executed:
# Look at the build.ps1 file to see if there's PS5.1 subprocess handling
head -100 build.ps1Repository: dsccommunity/CommonTasks
Length of output: 3560
🏁 Script executed:
# Search for any references to powershell.exe or Windows PowerShell in build scripts
grep -r "powershell.exe\|pwsh.*false\|WindowsPowerShell" . --include="*.ps1" --include="*.yml" --include="*.yaml" 2>/dev/null | head -20Repository: dsccommunity/CommonTasks
Length of output: 1414
🏁 Script executed:
# Check the git diff or any indication of what changed in RequiredModules.psd1
git log --oneline -n 20Repository: dsccommunity/CommonTasks
Length of output: 149
🏁 Script executed:
# Search more broadly for PS5.1 or WinPS references
grep -r "PSVersion.*5\|PowerShell.*5\|WinPS" . --include="*.ps1" --include="*.psd1" 2>/dev/null | head -20Repository: dsccommunity/CommonTasks
Length of output: 1712
🏁 Script executed:
# Check the specific line 27 in RequiredModules.psd1 and surrounding context
sed -n '20,35p' RequiredModules.psd1Repository: dsccommunity/CommonTasks
Length of output: 710
🏁 Script executed:
# Look at the actual DSC compilation test to see how PSDesiredStateConfiguration is handled
cat tests/Unit/DSCResources/CompileDscConfiguration.ps1Repository: dsccommunity/CommonTasks
Length of output: 5251
🏁 Script executed:
# Check how CompileDscConfigurations.ps1 handles the subprocess
cat tests/Unit/DSCResources/CompileDscConfigurations.ps1Repository: dsccommunity/CommonTasks
Length of output: 6196
🏁 Script executed:
# See what DscResources.Tests.ps1 reveals about the WinPS subprocess and module loading
sed -n '1,100p' tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 4395
🏁 Script executed:
# Check if there are any safeguards or module path manipulation for PSDesiredStateConfiguration
grep -n "PSModulePath\|PSDesiredStateConfiguration" tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 224
🌐 Web query:
PSDesiredStateConfiguration PowerShell 7 v2.0 v3.0 MOF support compatibility
💡 Result:
-
PowerShell DSC /
PSDesiredStateConfiguration“v2.x” (module 2.0.5–2.0.7) is the PowerShell 7-era DSC module (separated from the PowerShell install starting in PowerShell 7.2). (learn.microsoft.com)- It still exposes the PowerShell DSC surface area (for example
Configuration,Get-DscResource,Invoke-DscResource). (powershellgallery.com) - MOF is still part of the PowerShell DSC world (resource schemas are still MOF-based; classic “compile a configuration” workflows are still tied to the Windows LCM side of DSC, which is not what DSC v3 uses). (powershellgallery.com)
- It still exposes the PowerShell DSC surface area (for example
-
PSDesiredStateConfiguration“v3.0.0-beta1” (PowerShell module) is not “DSC v3” (the Rustdsctool). It’s a new major version of the PowerShell module and still exportsConfiguration(and adds things likeConvertTo-DscJsonSchema). (powershellgallery.com) -
Microsoft DSC “v3.x” (the
dsc/dsc.exeCLI):- Does not support MOF configuration documents (MOF configs are incompatible). (devblogs.microsoft.com)
- Uses JSON/YAML configuration documents instead. (devblogs.microsoft.com)
- Tries to keep resource compatibility (you can still use existing PSDSC resources), but configuration-document compatibility (MOF) is not provided. (devblogs.microsoft.com)
Practical compatibility takeaway: if your automation depends on MOF configuration files / LCM push-pull, you’re in the PowerShell DSC (PSDesiredStateConfiguration) track, not DSC v3; moving to DSC v3 requires converting MOF configs to JSON/YAML documents (even if you continue using the same underlying resources). (devblogs.microsoft.com)
Citations:
- 1: https://learn.microsoft.com/en-us/azure/governance/machine-configuration/whats-new/psdsc-in-machine-configuration?utm_source=openai
- 2: https://www.powershellgallery.com/packages/PSDesiredStateConfiguration/2.0.7?utm_source=openai
- 3: https://www.powershellgallery.com/packages/PSDesiredStateConfiguration/2.0.7?utm_source=openai
- 4: https://www.powershellgallery.com/packages/PSDesiredStateConfiguration/3.0.0-beta1/Content/PSDesiredStateConfiguration.psd1
- 5: https://devblogs.microsoft.com/powershell/announcing-dsc-v3/?utm_source=openai
- 6: https://devblogs.microsoft.com/powershell/announcing-dsc-v3/?utm_source=openai
- 7: https://devblogs.microsoft.com/powershell/announcing-dsc-v3/?utm_source=openai
- 8: https://devblogs.microsoft.com/powershell/announcing-dsc-v3/?utm_source=openai
🏁 Script executed:
# Check the latest PSDesiredStateConfiguration on PSGallery to see actual version info
web_search "PSDesiredStateConfiguration 2.0.7 PowerShell 5.1 compatibility requirements"Repository: dsccommunity/CommonTasks
Length of output: 118
🏁 Script executed:
# Let's check what modules are actually MOF-based in the RequiredModules.psd1
# by examining the module manifests or documentation
for mod in xPSDesiredStateConfiguration ComputerManagementDsc NetworkingDsc; do
echo "=== Checking $mod ==="
# Try to find if these are MOF or class-based
grep -r "ResourceType.*Mof\|class.*Resource" . --include="*.psm1" --include="*.psd1" | grep -i "$mod" | head -5
doneRepository: dsccommunity/CommonTasks
Length of output: 183
🏁 Script executed:
# Verify the current state: is PSDesiredStateConfiguration commented out or active in git?
git show HEAD:RequiredModules.psd1 | grep -n "PSDesiredStateConfiguration" -A 1 -B 1Repository: dsccommunity/CommonTasks
Length of output: 280
🏁 Script executed:
# Check if there's any documentation or comments about why PSDesiredStateConfiguration
# was previously commented out
git log --all --oneline --grep="PSDesiredStateConfiguration" | head -10Repository: dsccommunity/CommonTasks
Length of output: 543
🏁 Script executed:
# Look at the actual error handling in CompileDscConfigurations when modules fail to load
grep -n "Error\|Exception\|Import-Module" tests/Unit/DSCResources/CompileDscConfigurations.ps1 | head -20Repository: dsccommunity/CommonTasks
Length of output: 1441
🏁 Script executed:
# Check if xPSDesiredStateConfiguration is actually MOF-based by looking at what it contains
# when installed via PSGallery or how it's referenced in the tests
find . -name "*.psm1" -o -name "*.ps1" | xargs grep -l "xPSDesiredStateConfiguration" | head -5Repository: dsccommunity/CommonTasks
Length of output: 419
🏁 Script executed:
# Let's look at what the current tests actually import and use
grep -n "Import-DscResource\|Import-Module.*PSDesiredStateConfiguration" tests/Unit/DSCResources/DscResources.Tests.ps1Repository: dsccommunity/CommonTasks
Length of output: 233
🏁 Script executed:
# Check if PSDesiredStateConfiguration is strictly necessary or just suppressing warnings
grep -rn "PSDesiredStateConfiguration" source/DSCResources/ --include="*.ps1" --include="*.psm1" | head -10Repository: dsccommunity/CommonTasks
Length of output: 1439
🏁 Script executed:
# See if there's any documentation about PSDesiredStateConfiguration being essential
grep -rn "PSDesiredStateConfiguration" . --include="*.md" --include="*.txt" | grep -i "require\|version\|compat"Repository: dsccommunity/CommonTasks
Length of output: 50
🌐 Web query:
PSDesiredStateConfiguration v2.0.7 Windows PowerShell 5.1 MOF support
💡 Result:
PSDesiredStateConfiguration v2.0.7 does not support Windows PowerShell 5.1.
- The PowerShell Gallery package for PSDesiredStateConfiguration 2.0.7 lists Minimum PowerShell version: 6.1, which excludes Windows PowerShell 5.1. [1]
- Microsoft’s current guidance for installing/using PSDesiredStateConfiguration 2.0.7 is in PowerShell 7 (not Windows PowerShell 5.1). [2]
- Related Microsoft docs that reference “DSC v2 support” also require PowerShell 7.2+ with PSDesiredStateConfiguration 2.0.7+. [3]
Implication for MOF / classic DSC on Windows PowerShell 5.1:
- If by “MOF support” you mean the classic workflow (authoring
configuration {}blocks to compile MOF and applying them via the Windows LCM /Start-DscConfiguration), that’s the in-box DSC in Windows PowerShell 5.1, not PSDesiredStateConfiguration 2.0.7. (v2.x is intended for PowerShell 6+/7 scenarios and isn’t something you can run under 5.1.) [1][2]
Sources
[1] PowerShell Gallery – PSDesiredStateConfiguration 2.0.7 (minimum PowerShell 6.1)
[2] Microsoft Learn – install PSDesiredStateConfiguration 2.0.7 in PowerShell 7
[3] Microsoft Learn (PowerToys DSC) – prerequisites: PowerShell 7.2+ and PSDesiredStateConfiguration 2.0.7+
🌐 Web query:
PSDesiredStateConfiguration 3.0.0-beta1 PowerShell 5.1 compatibility
💡 Result:
PSDesiredStateConfiguration 3.0.0-beta1 is not compatible with Windows PowerShell 5.1.
- The 3.0.0-beta1 module manifest declares
CompatiblePSEditions = @('Core')andPowerShellVersion = '7.2', meaning it only supports PowerShell (Core) 7.2+, not Windows PowerShell 5.1. [1] - On Windows PowerShell 5.1, the DSC module you get is the in-box
PSDesiredStateConfigurationv1.1, which is only available in Windows PowerShell (not PowerShell 7). [2] - If you need DSC from the Gallery on PowerShell 7, Microsoft Learn examples use
PSDesiredStateConfiguration2.0.7 (stable) for Windows/PowerShell 7, and use 3.0.0-beta1 in PowerShell 7 (example shown for Linux, but the key point is it’s PowerShell 7+). [3]
What to do
- Stay on Windows PowerShell 5.1: use the built-in
PSDesiredStateConfiguration(v1.1). - Want the newer Gallery module: install/run it under PowerShell 7.2+ (not 5.1).
Sources: [1] [2] [3]
🏁 Script executed:
# Check if any of the composite resources strictly require PSDesiredStateConfiguration to be in RequiredModules
# rather than just the in-box v1.1
grep -rn "Import-DscResource.*PSDesiredStateConfiguration" source/DSCResources/ | wc -lRepository: dsccommunity/CommonTasks
Length of output: 71
🏁 Script executed:
# See if there's documentation about the requirement
grep -rn "PSDesiredStateConfiguration" . --include="README*" --include="CHANGELOG*"Repository: dsccommunity/CommonTasks
Length of output: 189
PSDesiredStateConfiguration = 'latest' will break PS5.1 DSC compilation
PSDesiredStateConfiguration v2.0.7 requires PowerShell 6.1 minimum, and v3.0.0-beta1 requires PowerShell 7.2+ (with CompatiblePSEditions = @('Core')). Neither runs under Windows PowerShell 5.1. With AllowPreRelease = $true in PSDependOptions, 'latest' will resolve to the incompatible v3.0.0-beta1 on PSGallery.
The test code (CompileDscConfigurations.ps1) explicitly runs DSC compilation in a PS5.1 subprocess and prepends $RequiredModulesPath to $env:PSModulePath. If PSDesiredStateConfiguration v3.0.0-beta1 is installed there, PS5.1 cannot load it, causing all DSC compilation to fail. The built-in PSDesiredStateConfiguration v1.1 in PS5.1 is sufficient for all 138 composite resources that import it.
Either pin to a PS5.1-compatible version or omit this entry entirely and rely on the inbox module:
- PSDesiredStateConfiguration = 'latest'
+ # PSDesiredStateConfiguration = '1.1' # PS5.1 has this built-in; omit to use it📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PSDesiredStateConfiguration = 'latest' | |
| # PSDesiredStateConfiguration = '1.1' # PS5.1 has this built-in; omit to use it |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RequiredModules.psd1` at line 27, The PSDesiredStateConfiguration entry in
RequiredModules.psd1 is set to 'latest' which can resolve to v3.x prereleases
incompatible with Windows PowerShell 5.1; update the RequiredModules.psd1 entry
for PSDesiredStateConfiguration to either remove the entry (so the inbox v1.1 is
used) or pin it to a PS5.1-compatible version (e.g., '1.1.0.0'), and ensure any
PSDependOptions that allow prerelease are not causing installs of v3.x; look for
the PSDesiredStateConfiguration key in RequiredModules.psd1 and related
PSDependOptions/CompileDscConfigurations.ps1 usage to apply the change.
1 participant
Pull Request
Pull Request (PR) description
Fixed
broken resources without reporting why, so the Pester tests never ran for
them — the only symptom was an unhelpful "Expected 1, but got 2" count mismatch.
DscResources.Tests.ps1— Changed test case discovery to enumerate DSCResourcesfolders instead of relying on Get-DscResource output. This ensures every resource
gets a test case even if it fails to load, and the actual compilation step in
PS 5.1 surfaces the real error.
CompileDscConfiguration.ps1— Wrapped the Invoke-Expression (configurationdefinition)in its own try/catch and collected all errors from $Error in chronological
order. If the configuration function isn't created, the full error chain is thrown —
root cause first. This generically reports any error (invalid properties, missing
modules, syntax errors, etc.) without needing error-type-specific detection.
process - instead of spawning a separate process per resource. The expensive
one-time initialization (module imports, New-DatumStructure,
Initialize-DscResourceMetaInfo) now runs only once. With more resources, the savings
scale linearly — each additional resource adds only its compilation time rather than
~45s of setup overhead.
C:\Program Files\WindowsPowerShell\Modules. When DSC scans all paths on PSModulePath,
it finds duplicate MOF schema definitions (MSFT_PSModule, MSFT_PSRepository) and emits
Write-Error for each — ugly but non-fatal.
with -ErrorAction SilentlyContinue 2>$null to suppress both terminating exceptions
(e.g. empty PSModulePath entries) and non-terminating Write-Error output from the
duplicate CIM class detections. This was done in both DscResources.Tests.ps1 and
CompileDscConfigurations.ps1.
Task list
build.ps1 -ResolveDependency).and comment-based help.
This change is