fix(dsc): make PSResourceList test, set and what-if agree on desired state - #2048
Open
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
Open
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved issues affect scope-aware convergence, install deduplication, and test isolation.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Unifies PSResourceList desired-state evaluation across test, set, and what-if.
Changes:
- Centralizes resource action pairing.
- Handles versions, scopes,
_exist, and error tracing. - Adds DSC regression coverage.
| File | Description |
|---|---|
test/DscResource/PSResourceGetDSCResource.Tests.ps1 |
Adds regression and scope-related tests. |
src/dsc/psresourceget.ps1 |
Implements shared desired-state actions and scope-aware operations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+361
to
364
| else { | ||
| # No version constraint: any installed version means the resource exists. | ||
| # Only record the first match so that one input resource maps to one current resource. | ||
| Write-Trace -message "No version constraint for input: $($inputResource.Name). Treating installed version $($matchingResources[0].Version) as a match." -level debug | ||
| $preferred = $matchingResources | Select-Object -First 1 |
| # Install if resource should exist but doesn't, or exists but not in desired state | ||
| elseif ($resourceAction.action -eq 'Install') { | ||
| Write-Trace -message "Resource $name needs to be installed." -level debug | ||
| $key = $name.ToLowerInvariant() + '-' + $versionStr.ToLowerInvariant() |
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
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.

PR Summary
test,setandwhat-ifforpsresourcelisteach computed desired state on their own and disagreed. They now share one pairing step (GetPSResourceListActions) that matches every desired resource to its current resource by index and decidesNone,InstallorUninstall.PR Context
This PR fixes the following points:
testonly checked the first resource.A resource without
versionnever converged.setcompared every desired resource against every current one.All resources were installed or uninstalled with the last resource's scope.
_existis compared before version and repository, and those only apply when specified.Uninstall removes the resource from the scope it is installed in.
Before,
Uninstall-PSResourcereceived the desired scope, which defaults toCurrentUserwhen omitted. A resource installed inAllUserswas therefore never removed andsetnever converged.Note
scopeis still not a filter.GetPSResourceListmatches the current resource on name and version across both scopes, so an explicitscope: CurrentUserwith_exist: falsealso removes anAllUserscopy. I'll create another issue for it.Errors trace at
errorlevel, so the real message is visible without--trace-level debug.This PR also includes a couple of behavioral changes:
_exist: falsewith aversionremoves only that version (range); withoutversionall versions are removed.Would install, matching whatsetdoes.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.