Skip to content

Running tests for PR #1912 - #2027

Open
alerickson wants to merge 11 commits into
masterfrom
PSContentPath
Open

Running tests for PR #1912#2027
alerickson wants to merge 11 commits into
masterfrom
PSContentPath

Conversation

@alerickson

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.

@alerickson

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.

🔵 Needs a closer look

There is a confirmed null-dereference in GetPathsFromEnvVarAndScope when scope is null, which can cause runtime failure.

Pull request overview

This PR updates PSResourceGet’s user-scope install/search path resolution to prefer PowerShell’s $PSUserContentPath (with legacy fallbacks) and adds end-to-end coverage to validate the resolved install locations.

Changes:

  • Update Utils.GetStandardPlatformPaths to resolve the current-user base path via $PSUserContentPath, falling back to legacy paths when unavailable.
  • Add internal test hooks to capture the last-resolved user content path and its source.
  • Add Pester end-to-end tests validating CurrentUser vs AllUsers installation locations and the $PSUserContentPath/legacy selection behavior.
File summaries
File Description
test/PSContentPath.Tests.ps1 Adds end-to-end Pester coverage around user content path selection and install locations.
src/code/Utils.cs Switches current-user base directory resolution to $PSUserContentPath with legacy fallback and verbose tracing.
src/code/InternalHooks.cs Adds hook fields + helper methods for testing the resolved content path/source.
Review details

Suppressed comments (1)

src/code/Utils.cs:1116

  • GetPathsFromEnvVarAndScope claims to support scope being null (see the scope is null check), but it still dereferences scope.Value when adding AllUsers paths. If scope is actually null, this will throw a NullReferenceException and also prevents returning the AllUsers paths when scope is omitted.
            List<string> resourcePaths = new List<string>();
            if (scope is null || scope.Value is ScopeType.CurrentUser)
            {
                resourcePaths.Add(Path.Combine(psUserContentPath, "Modules"));
                resourcePaths.Add(Path.Combine(psUserContentPath, "Scripts"));
            }

            if (scope.Value is ScopeType.AllUsers)
            {
                resourcePaths.Add(Path.Combine(programFilesPath, "Modules"));
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

@alerickson

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

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

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.

3 participants