Run tests for PR #1963

[alerickson]

PR Summary

PR Context

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Documentation needed
    • Issue filed:
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and enumerated concerns in the summary.

[azure-pipelines]

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

[alerickson]

/azp run

[azure-pipelines]

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

[Copilot]

🟡 Changes recommended

Critical and moderate issues remain in RID/TFM filtering, dependency parsing, archive handling, tests, and CI authentication.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds platform-aware RID/TFM package installation, dependency selection, asset merging, tests, and CI authentication handling.

Changes:

• Adds RID and TFM detection, filtering, overrides, and completion.
• Adds dependency-group parsing and platform-aware installation merging.
• Adds platform-filtering tests and CI token handling.

File summaries

File	Review summary
test/PlatformFilteringTests/RuntimePackageHelper.Tests.ps1	Tests runtime package path filtering.
test/PlatformFilteringTests/RuntimeIdentifierHelper.Tests.ps1	Tests RID detection and compatibility.
test/PlatformFilteringTests/PlatformAwareInstall.Tests.ps1	Moderate (3 votes): include the detected musl RID in fixtures. Nit (2 votes): ensure the dependency-only fixture exercises Nuspec parsing. Nit (3 votes): require dependency assertions. Nit (3 votes): assert library directories exist before enumeration.
src/code/Utils.cs	Adds recursive directory merging.
src/code/RuntimePackageHelper.cs	Critical (2 votes): recognize supported base RIDs. Moderate (1 vote): enforce RID folder boundaries when filtering.
src/code/RuntimeIdentifierHelper.cs	Critical (2 votes): prevent musl/glibc incompatibility. Moderate (1 vote): add generic Linux fallbacks.
src/code/PSResourceInfo.cs	Critical (1 vote): handle empty dependency groups without throwing, including the occurrence at line 1711.
src/code/InternalHooks.cs	Exposes helpers for tests.
src/code/InstallPSResource.cs	Moderate (1 vote): propagate the requested target framework through dependency selection.
src/code/InstallHelper.cs	Moderate (3 votes): support runtimes/{rid}/lib/{tfm} assets. Moderate (1 vote): reject or report unsupported explicit TFMs. Moderate (1 vote): retain valid zero-byte files.
src/code/ArgumentCompleter.cs	Adds RID and TFM completion.
.ci/test.yml	Moderate (1 vote): use the environment token when constructing the authorization header.

Review details

Suppressed comments (12)

.ci/test.yml:115

• When a token is present, this branch leaves the literal ****** Authorization value in the hashtable because it never assigns $env:GITHUB_TOKEN. Internal CI therefore sends an invalid credential, while only fork builds take the unauthenticated path; construct the Authorization header from the environment token when it is valid and omit it otherwise.

      if ([string]::IsNullOrWhiteSpace($env:GITHUB_TOKEN) -or
          $env:GITHUB_TOKEN -match '^\$\([^)]+\)$') {
          $null = $headers.Remove("Authorization")
      }

src/code/InstallHelper.cs:1323

• An unsupported explicit TargetFramework is silently replaced with host auto-detection here. This means a typo can install the host's TFM instead of honoring the explicit parameter (despite the public contract saying it selects the requested TFM); reject invalid input or emit an explicit error rather than quietly ignoring it.

                        bestLibFramework = NuGetFramework.ParseFolder(_targetFramework);
                        if (bestLibFramework == null || bestLibFramework.IsUnsupported)
                        {
                            _cmdletPassedIn.WriteDebug($"Could not parse specified TargetFramework '{_targetFramework}', falling back to auto-detection.");
                            bestLibFramework = GetBestLibFramework(archive);

src/code/InstallHelper.cs:1342

• Filtering by CompressedLength > 0 drops valid zero-byte package files, because an empty file can have a compressed length of zero just like a directory entry. Such files should still be installed; skip directory entries by checking entry.Name (or the trailing separator) instead.

                    foreach (ZipArchiveEntry entry in archive.Entries.Where(entry => entry.CompressedLength > 0))

src/code/InstallPSResource.cs:624

• TargetFramework is forwarded to extraction here, but dependency resolution runs earlier in FindHelper using PSResourceInfo dependencies selected for the host runtime. An install such as -TargetFramework net472 can therefore select the net8 dependency group while extracting net472 assemblies. Propagate the requested framework through dependency-group selection (or make this limitation explicit in the parameter contract).

                runtimeIdentifier: RuntimeIdentifier,
                targetFramework: TargetFramework);

src/code/PSResourceInfo.cs:1714

• The metadata passed here is not populated by NuspecReader: the local package paths flatten each XML child into a string under dependencies and never create dependencyGroups or PackageDependencyGroup objects. Consequently this cast is null and all .nuspec dependencies are silently returned as empty. Update the loader/parser to consume the actual XML shape while preserving dependency-group TFMs.

            var dependencyGroups = pkgMetadata["dependencyGroups"] as List<PackageDependencyGroup>;
            if (dependencyGroups == null || dependencyGroups.Count == 0)
            {
                return new Dependency[] { };

src/code/RuntimeIdentifierHelper.cs:350

• The generic linux RID also gets no unix/any fallbacks because all additions are inside if (arch != null). Therefore an explicit target RID of linux rejects runtimes/any/... assets, despite any being a valid fallback; add the generic Linux chain outside the architecture conditional.

                string arch = ExtractArchitecture(primaryRid);
                if (arch != null)
                {

src/code/RuntimePackageHelper.cs:64

• Using StartsWith(prefix) without a RID boundary classifies ordinary root folders such as windows-x64 or linuxfoo-x64 as runtime folders. Their contents are then silently removed on nonmatching platforms even though these are not valid .NET RID names; require a separator/version boundary or validate against the RID graph before filtering root-level folders.

                if (folderName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))

test/PlatformFilteringTests/PlatformAwareInstall.Tests.ps1:250

• This conditional allows the Windows PowerShell TFM test to pass without checking anything when the expected lib directory is absent. The fixture always creates lib assets, so assert its existence before checking for net472.

            if (Test-Path $libDir) {
                $installedTfmFolders = @((Get-ChildItem $libDir -Directory).Name)

test/PlatformFilteringTests/PlatformAwareInstall.Tests.ps1:262

• This conditional allows the PowerShell 7+ TFM test to pass without checking anything when the expected lib directory is absent. The fixture always creates lib assets, so assert its existence before checking that net472 was excluded.

            if (Test-Path $libDir) {
                $installedTfmFolders = @((Get-ChildItem $libDir -Directory).Name)

test/PlatformFilteringTests/PlatformAwareInstall.Tests.ps1:337

• This conditional allows the explicit net6.0 test to pass without checking anything when the expected lib directory is absent. The fixture always creates lib assets, so assert its existence before checking the selected folder.

            if (Test-Path $libDir) {
                $installedTfmFolders = @((Get-ChildItem $libDir -Directory).Name)

test/PlatformFilteringTests/PlatformAwareInstall.Tests.ps1:352

• This conditional allows the explicit net472 test to pass without checking anything when the expected lib directory is absent. The fixture always creates lib assets, so assert its existence before checking the selected folder.

            if (Test-Path $libDir) {
                $installedTfmFolders = @((Get-ChildItem $libDir -Directory).Name)

test/PlatformFilteringTests/PlatformAwareInstall.Tests.ps1:368

• This conditional allows the combined -RuntimeIdentifier/-TargetFramework test to pass without checking anything when the expected lib directory is absent. The fixture always creates lib assets, so assert its existence before checking the selected folder.

            if (Test-Path $libDir) {
                $installedTfmFolders = @((Get-ChildItem $libDir -Directory).Name)

• Files reviewed: 12/12 changed files
• Comments generated: 8
• 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]

/azp run

[azure-pipelines]

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