Adds Test for correct ResourceIds#245
Adds Test for correct ResourceIds#245ChristophHannappel wants to merge 2 commits intodsccommunity:mainfrom
Conversation
WalkthroughA new test case is added to validate that DSC resource MOF files contain ResourceID values ending with a specific naming pattern Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 2
🧹 Nitpick comments (1)
tests/Unit/DSCResources/DscResources.Tests.ps1 (1)
225-225: Rename thisItblock to start withShould.The new test name breaks the repo's Pester naming rule.
Should have ResourceIDs ending with ...would match the existing convention.As per coding guidelines, "It descriptions start with 'Should', must not contain 'when'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/DSCResources/DscResources.Tests.ps1` at line 225, Rename the Pester It block description so it begins with "Should" to follow the repo naming rule: change the It description "'<DscResourceName>' has ResourceIDs ending with [<DscResourceName>]<DscResourceName>" to something like "Should have ResourceIDs ending with [<DscResourceName>]<DscResourceName]" while keeping the same test invocation (It ... -TestCases $testCases) so no other logic changes are needed.
🤖 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`:
- Around line 10-11: The changelog entry contains a malformed example suffix
"[<ResourceName>]<Resourcename" that doesn't match the test's asserted pattern
for ResourceId; update the text so the example uses the correct casing and
bracketed pattern that the test validates (e.g., use
"[<ResourceName>]<ResourceName>" or the exact pattern your test asserts), and
ensure the entry references the ResourceId, ResourceName, DependsOn and
DSCWorkshop Framework consistently so the documented example matches the
implemented assertion.
In `@tests/Unit/DSCResources/DscResources.Tests.ps1`:
- Around line 231-234: Add an explicit assertion that Select-String found
matches before iterating: after the $selectString = Select-String -InputObject
$mofContent -Pattern 'ResourceID = ".+' call and before the foreach ($value in
$selectString.Matches ...) loop, assert that $selectString.Matches.Count |
Should -BeGreaterThan 0 (or equivalent) so the test fails when no ResourceID
lines are present; reference the $selectString variable, the Select-String
invocation, and the foreach loop when making the change.
---
Nitpick comments:
In `@tests/Unit/DSCResources/DscResources.Tests.ps1`:
- Line 225: Rename the Pester It block description so it begins with "Should" to
follow the repo naming rule: change the It description "'<DscResourceName>' has
ResourceIDs ending with [<DscResourceName>]<DscResourceName>" to something like
"Should have ResourceIDs ending with [<DscResourceName>]<DscResourceName]" while
keeping the same test invocation (It ... -TestCases $testCases) so no other
logic changes are needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65f7df8e-2604-430b-bfcb-b621c9f915d9
📒 Files selected for processing (2)
CHANGELOG.mdtests/Unit/DSCResources/DscResources.Tests.ps1
| - Added a Test | ||
| Checks if the ResourceId ends with the [<ResourceName>]<Resourcename to prevent `DependsOn` Issues with the DSCWorkshop Framework. |
There was a problem hiding this comment.
Fix the documented ResourceID suffix example.
[<ResourceName>]<Resourcename is malformed and does not match the pattern the new test is asserting, so the changelog entry is misleading as written.
Suggested wording
-- Added a Test
- Checks if the ResourceId ends with the [<ResourceName>]<Resourcename to prevent `DependsOn` Issues with the DSCWorkshop Framework.
+- Added a test that verifies composite `ResourceID` values end with `::[<ResourceName>]<ResourceName>` to prevent `DependsOn` issues in the DSCWorkshop framework.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 10 - 11, The changelog entry contains a malformed
example suffix "[<ResourceName>]<Resourcename" that doesn't match the test's
asserted pattern for ResourceId; update the text so the example uses the correct
casing and bracketed pattern that the test validates (e.g., use
"[<ResourceName>]<ResourceName>" or the exact pattern your test asserts), and
ensure the entry references the ResourceId, ResourceName, DependsOn and
DSCWorkshop Framework consistently so the documented example matches the
implemented assertion.
| $selectString = Select-String -InputObject $mofContent -Pattern 'ResourceID = ".+' -AllMatches | ||
| foreach ($value in $selectString.Matches | Select-Object -ExpandProperty Value) | ||
| { | ||
| $value | Should -Match -RegularExpression "(ResourceID = `".*)(:{2}\[$DscResourceName\]$DscResourceName`";)" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Pester v5, is Should -Match -RegularExpression ""valid syntax, or must the regex be passed positionally asShould -Match ""?
💡 Result:
Yes—Should -Match -RegularExpression "<pattern>" is valid Pester v5 syntax; -RegularExpression is a named parameter for the -Match assertion. The regex can also be passed positionally as Should -Match "<pattern>" (equivalent). [1], [2]
🏁 Script executed:
#!/bin/bash
# Test Select-String behavior when no matches are found
pwsh -NoProfile -Command @'
# Simulate content with no matching ResourceID pattern
$content = "Some other content without ResourceID"
$selectString = Select-String -InputObject $content -Pattern 'ResourceID = ".+' -AllMatches
Write-Host "selectString result: $selectString"
Write-Host "selectString is null: $($selectString -eq $null)"
if ($selectString) {
$matches = $selectString.Matches | Select-Object -ExpandProperty Value
Write-Host "matches: $matches"
Write-Host "matches count: $($matches.Count)"
$runCount = 0
foreach ($value in $matches) {
$runCount++
}
Write-Host "foreach executed $runCount times"
} else {
Write-Host "selectString is falsy, foreach would not run"
}
Write-Host ""
Write-Host "Testing with actual matches:"
# Simulate content WITH matching ResourceID pattern
$content2 = 'ResourceID = "something"'
$selectString2 = Select-String -InputObject $content2 -Pattern 'ResourceID = ".+' -AllMatches
Write-Host "selectString2 result: $selectString2"
Write-Host "selectString2 matches count: $($selectString2.Matches.Count)"
foreach ($value in $selectString2.Matches | Select-Object -ExpandProperty Value) {
Write-Host "Found: $value"
}
'@Repository: dsccommunity/CommonTasks
Length of output: 237
🏁 Script executed:
#!/bin/bash
pwsh -NoProfile << 'EOF'
# Test Select-String behavior when no matches are found
$content = "Some other content without ResourceID"
$selectString = Select-String -InputObject $content -Pattern 'ResourceID = ".+' -AllMatches
Write-Host "selectString result: $selectString"
Write-Host "selectString is null: $($null -eq $selectString)"
if ($selectString) {
$matches = $selectString.Matches | Select-Object -ExpandProperty Value
Write-Host "matches: $matches"
Write-Host "matches count: $($matches.Count)"
} else {
Write-Host "selectString is null/falsy, foreach would not execute"
}
Write-Host ""
Write-Host "Testing with actual matches:"
# Simulate content WITH matching ResourceID pattern
$content2 = 'ResourceID = "something"'
$selectString2 = Select-String -InputObject $content2 -Pattern 'ResourceID = ".+' -AllMatches
Write-Host "selectString2 result: $selectString2"
Write-Host "selectString2 matches count: $($selectString2.Matches.Count)"
foreach ($value in $selectString2.Matches | Select-Object -ExpandProperty Value) {
Write-Host "Found: $value"
}
EOFRepository: dsccommunity/CommonTasks
Length of output: 1726
Add a check to ensure at least one ResourceID match exists before the foreach loop.
The foreach loop will not execute if Select-String returns $null (no matches found), causing the test to pass without validating any ResourceID lines. Add $selectString.Matches.Count | Should -BeGreaterThan 0 or equivalent after the Select-String call and before the loop.
Note: The -RegularExpression parameter is valid Pester v5 syntax and does not need to be removed.
🧰 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 231 - 234, Add
an explicit assertion that Select-String found matches before iterating: after
the $selectString = Select-String -InputObject $mofContent -Pattern 'ResourceID
= ".+' call and before the foreach ($value in $selectString.Matches ...) loop,
assert that $selectString.Matches.Count | Should -BeGreaterThan 0 (or
equivalent) so the test fails when no ResourceID lines are present; reference
the $selectString variable, the Select-String invocation, and the foreach loop
when making the change.
1 participant
Pull Request
Pull Request (PR) description
Adds a Test, of the ResourceIDs of the the Composite DSC Ressources end with a [ResourceName]ResourceName.
This is necessary for the DSCWorkshop Framework to use it at the DependsOn Level and helps avoiding an error where the resource can not find its dependency.
Added
Task list
build.ps1 -ResolveDependency).and comment-based help.
This change is