Skip to content

Test2048 - #2065

Open
Aditya Patwardhan (adityapatwardhan) wants to merge 3 commits into
masterfrom
test2048
Open

Aditya Patwardhan (adityapatwardhan) wants to merge 3 commits into
masterfrom
test2048

Conversation

@adityapatwardhan

Copy link
Copy Markdown
Member

PR Summary

PR Context

PR Checklist

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@adityapatwardhan

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Wrong-scope deletion, inconsistent version matching, and failures for already-absent repositories remain unresolved.

3 open findings
What changed in this PR

This PR unifies DSC resource-state planning in PSResourceGet and expands regression coverage.

Changes:

  • Shares action planning across get, test, set, and what-if operations.
  • Updates scope/version handling and repository error reporting.
  • Adds tests for desired-state checks, repeated operations, and resource discovery.
File Description
test/​DscResource/​PSResourceGetDSCResource.Tests.ps1 Adds helpers and DSC regression tests.
src/​dsc/​psresourceget.ps1 Centralizes action planning and updates operation handling.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/dsc/psresourceget.ps1

$allPSResources = @()
$name = $desiredResource.name
$matchingResources = @($installedResources | Where-Object { $_.name -eq $name })
Comment thread src/dsc/psresourceget.ps1
# Only remove the requested version (range) when there is one, otherwise all versions are removed
$versionParam = @{}
if ($_.desired.version) {
$versionParam['Version'] = $_.desired.version
Comment thread src/dsc/psresourceget.ps1
if ($inputObj._exist -eq $false) {
Write-Trace -message "Repository $($inputObj.Name) exists and _exist is false. Deleting it." -level debug
Unregister-PSResourceRepository -Name $inputObj.Name
Unregister-PSResourceRepository -Name $inputObj.Name -ErrorAction Stop
@adityapatwardhan

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@adityapatwardhan

Copy link
Copy Markdown
Member Author

Copilot - review PR again

Copilot AI commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Copilot - review PR again

Re-reviewed the last three commits. The three earlier findings are fixed, but two issues remain:

  • P2 — Unscoped absence does not converge in one set (/home/runner/work/PSResourceGet/PSResourceGet/src/dsc/psresourceget.ps1:704–710): when a module is installed in both scopes, _exist: false without a scope schedules removal from only one. Set succeeds with the other copy still present, while what-if projects absence. Schedule removal from every matching scope.
  • P2 — Equivalent absence constraints can fail after removal (/home/runner/work/PSResourceGet/PSResourceGet/src/dsc/psresourceget.ps1:708–710): entries for 1.0.0 and [1.0.0] create separate uninstall actions. With only that version installed, the second uninstall raises ResourceNotInstalled, and -ErrorAction Stop fails set. Deduplicate concrete uninstall targets or recheck matches before execution.

Both changed PowerShell files parse successfully, and the whitespace check passed. DSC integration tests could not run locally because DSC is unavailable. Azure Pipelines reports macOS and Windows PowerShell failures, but its log endpoint was blocked here, so their causes remain unverified. No files changed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants