cross-platform-tests.yml

name: Tests

# Runs the Pester suite across every OS/PowerShell-edition combination the module
# claims to support (PureStorageFlashBladePowerShell.psd1: PowerShellVersion = '5.1'),
# so platform-specific bugs (e.g. a .NET marshaling difference between Windows and
# Linux/macOS) are caught on every push/PR instead of only surfacing much later on
# whichever CI job happens to run on a non-Windows runner.
#
# Two separate jobs rather than one shell-matrixed job: the `shell:` key on a step does
# NOT have access to the `matrix` context (unlike `run:`, `env:`, or a job's own `name:`/
# `runs-on:`) -- confirmed via `gh workflow run`: "Unrecognized named-value: 'matrix'.
# Located at position 1 within expression: matrix.shell". In a workflow, `shell:` must be
# a literal. (Inside a *composite action* `shell:` does accept `${{ inputs.* }}` -- but
# still not `matrix`/`env` -- which is how .github/actions/install-test-modules serves
# both editions from one implementation.)
#
# workflow_call: publish-to-gallery.yml calls this workflow as its test gate, so a
# release only ships after passing on all 4 OS/PowerShell-edition combinations below --
# not just whichever single platform a standalone Pester step would happen to run on.

# `push` is filtered to main; `pull_request` is not. An unfiltered `push` matched every branch
# commit, and a commit on a branch with an open PR ALSO matches `pull_request` -- so one push
# ran the whole matrix twice, 10 jobs for one commit, with Windows pwsh alone near 17 minutes
# on each side.
#
# Branch commits keep their coverage: `pull_request` fires on every push to a branch with an
# open PR (the `synchronize` event). The only case that loses a run is a branch commit pushed
# BEFORE its PR exists -- and a `pull_request` run is the better of the two anyway, because it
# tests the MERGE commit rather than the branch tip, i.e. the tree that would actually ship.
on:
  push:
    branches: [main]
  pull_request:
  workflow_dispatch: {}
  workflow_call: {}

# Read-only: this workflow checks out, restores/saves the spec cache, moves artifacts between
# jobs and runs Pester. It never writes to the repository. Declared rather than inherited
# because without a permissions block the GITHUB_TOKEN gets whatever the repo's
# default_workflow_permissions setting happens to be -- a setting an admin can flip to write
# with no change to any file here. Cache and artifact actions are unaffected either way: they
# authenticate with ACTIONS_RUNTIME_TOKEN, not this token.
permissions:
  contents: read

jobs:
  # Issue #63: tools/specs/ is a ~50MB cache of raw OpenAPI specs, gitignored because it is a
  # build input rather than source, so on a bare runner it does not exist. Every tooling test
  # gated on its presence skipped gracefully while the job reported success -- roughly 23% of
  # the suite, including the absolute-path regression guards from PR #62, invisible in the run
  # summary. This job materialises the cache once per run and hands it to every test leg.
  #
  # The cache key prefix is deliberately shared with update-api-capability-map.yml, so a warm
  # cache written by either workflow serves both. Update-PfbApiSpecs.ps1 skips any version
  # already on disk, so a cache hit means only newly-published versions get fetched instead of
  # the full ~29-version history.
  #
  # An artifact rather than a per-leg cache restore: it fetches from the published spec index
  # at most once per run instead of up to four times, and it is immune to cache key/branch
  # scoping differences across the matrix. 29 JSON files, ~50MB raw, which compress well.
  prepare-specs:
    name: Prepare API spec cache
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

      - name: Restore cached spec files
        uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
        with:
          path: tools/specs
          key: pfb-specs-${{ github.run_id }}
          restore-keys: |
            pfb-specs-

      - name: Fetch any missing REST API spec versions
        shell: pwsh
        run: ./tools/Update-PfbApiSpecs.ps1

      # The whole point of this job. A silent no-op here would put us straight back to the
      # green-but-empty runs issue #63 is about, so an empty result is a hard failure.
      - name: Assert the spec set is non-empty
        shell: pwsh
        run: ./scripts/Assert-PfbSpecCache.ps1

      - name: Save spec cache
        uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
        if: always()
        with:
          path: tools/specs
          key: pfb-specs-${{ github.run_id }}

      - name: Publish specs to the test jobs
        uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
        with:
          name: pfb-specs
          path: tools/specs
          retention-days: 1

  test-pwsh:
    name: Test (${{ matrix.os }}, pwsh)
    runs-on: ${{ matrix.os }}
    needs: prepare-specs
    strategy:
      fail-fast: false
      matrix:
        os: [ubuntu-latest, windows-latest, macos-latest]
    steps:
      - name: Checkout
        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

      # Issue #63: without this, every tools/specs-gated test skips and the job still passes.
      - name: Download API spec cache
        uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0
        with:
          name: pfb-specs
          path: tools/specs

      # Pinned + cached; see .github/actions/install-test-modules/action.yml for why
      # (including why Posh-SSH is needed at all).
      - name: Install test dependencies
        uses: ./.github/actions/install-test-modules
        with:
          shell: pwsh

      # Suite invocation + the issue-#63 coverage gate live in scripts/Invoke-PfbCiPester.ps1
      # rather than inline here: this same block was copy-pasted in three places across two
      # workflows, and all three needed the same change. A script is also lintable, diffable
      # and runnable outside Actions, which YAML-embedded PowerShell is not.
      - name: Run Pester tests
        shell: pwsh
        run: ./scripts/Invoke-PfbCiPester.ps1 -Edition pwsh7

  # T8 + T11 -- PSScriptAnalyzer.
  #
  # WHY A CI JOB AND NOT A HOOK. The PostToolUse parse-check hook is per-edit and only ever
  # sees the file just written. This rule set is repo-wide and low-frequency: what it catches
  # is "someone added a 5.1-incompatible construct anywhere", which no per-file check can see.
  #
  # WHY ubuntu-latest AND ONE LEG. The analyzer parses; it does not execute the module, so its
  # findings do not vary by host OS or PowerShell edition -- verified by running the identical
  # sweep on Windows pwsh 7 and on Ubuntu 26.04 / pwsh 7.6.3 and getting the same total (276
  # at the time of that check, 149 once the dead-variable cleanup landed), the same zeros on
  # every guard, and the same Private/ controls at 12 and 2. Running it
  # across the existing 4-leg matrix would quadruple the cost for four identical results.
  #
  # The COMPATIBILITY rules are the apparent exception and are not: they target 5.1 and 7.0 by
  # configuration, from static profiles that ship with the analyzer on every platform, so this
  # job reports on 5.1 from a Linux runner with no 5.1 in sight. That the two Windows profiles
  # resolve on a Linux install was checked, not assumed -- if they did not, the rule would
  # evaluate nothing and hand the Public/ gate a permanent free pass.
  #
  # WHY IT DOES NOT NEED prepare-specs. Nothing here reads tools/specs/, so the job does not
  # download the spec artifact and does not depend on that job. It starts immediately and
  # finishes while the test matrix is still running.
  analyze:
    name: PSScriptAnalyzer
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

      # Pinned and cached for the same reason Pester and Posh-SSH are pinned in
      # .github/actions/install-test-modules: an unpinned MinimumVersion is how this
      # repo silently moved from Pester 5 to Pester 6 with nobody deciding to, and a
      # transient PSGallery blip has already failed a run on a SHA that passed 47s
      # later. A new analyzer version can add rules or change counts, which would
      # present as an unexplained CI failure on an unrelated PR.
      #
      # Deliberately NOT added as a third module to install-test-modules: that action
      # runs in all four test legs, which would download the analyzer four times per
      # run for a job that needs it once.
      - name: Restore cached PSScriptAnalyzer
        id: pssa-cache
        uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
        with:
          path: .psmodules
          key: pssa-${{ runner.os }}-1.25.0

      - name: Save PSScriptAnalyzer
        if: steps.pssa-cache.outputs.cache-hit != 'true'
        shell: pwsh
        run: |
          New-Item -ItemType Directory -Force -Path .psmodules | Out-Null
          Save-Module PSScriptAnalyzer -RequiredVersion 1.25.0 -Path .psmodules -Force

      - name: Run PSScriptAnalyzer
        shell: pwsh
        run: |
          $ErrorActionPreference = 'Stop'
          $env:PSModulePath = (Resolve-Path .psmodules).Path + [IO.Path]::PathSeparator + $env:PSModulePath
          Import-Module PSScriptAnalyzer -RequiredVersion 1.25.0
          $settings = './PSScriptAnalyzerSettings.psd1'

          # ---------------------------------------------------------------
          # CONTROL FIRST. Every assertion below is "this count is zero", and an
          # analyzer that returns nothing satisfies all of them. That is not
          # hypothetical: Invoke-ScriptAnalyzer honours -WhatIf and returns an empty
          # set, and a mis-specified rule name or an unparseable settings file can
          # produce the same silence. So prove the tool reports a finding it must
          # report before believing any zero it gives us.
          # ---------------------------------------------------------------
          $control = @(Invoke-ScriptAnalyzer -ScriptDefinition 'function Test-Probe { $x = 1 }' `
              -IncludeRule PSUseDeclaredVarsMoreThanAssignments)
          if ($control.Count -lt 1) {
              throw 'CONTROL FAILED: the analyzer found nothing in a snippet that definitely has a finding. Every zero in this run is vacuous.'
          }
          Write-Host "control: analyzer live ($($control.Count) finding on the probe)"

          # Paths. The repo ROOT is included as an explicit FILE list, not as a
          # directory: the module .psd1/.psm1 live there, and a directory sweep of the
          # five source folders never analyses them -- which silently disables four
          # manifest/module rules including the PSGallery preset's own
          # PSMissingModuleManifestField. As files it also cannot recurse and
          # double-count. build/ is gitignored generated output and stays out.
          $paths = @('Public', 'Private', 'Tests', 'tools', 'scripts') |
              Where-Object { Test-Path $_ } |
              ForEach-Object { [pscustomobject]@{ Path = $_; Recurse = $true } }
          $paths += Get-ChildItem -File |
              Where-Object Extension -in '.ps1', '.psm1', '.psd1' |
              ForEach-Object { [pscustomobject]@{ Path = $_.FullName; Recurse = $false } }

          # -Path takes a single string. Passing an array fails with a type-conversion
          # error and leaves the variable empty, so a naive .Count then reports a
          # confident "0 findings". Hence the loop.
          $all = foreach ($p in $paths) {
              $splat = @{ Path = $p.Path; Settings = $settings }
              if ($p.Recurse) { $splat.Recurse = $true }
              Invoke-ScriptAnalyzer @splat
          }
          $all = @($all)

          # Severity round-trips as UInt32 in some paths and compares as neither
          # reliably; cast to string before comparing, always.
          $errors = @($all | Where-Object { [string]$_.Severity -eq 'Error' })
          $warnings = @($all | Where-Object { [string]$_.Severity -eq 'Warning' })
          Write-Host "total $($all.Count): $($errors.Count) Error, $($warnings.Count) Warning"

          $failures = [System.Collections.Generic.List[string]]::new()

          # --- Guard 1: no Errors, ever. Reached by T9; this holds the line.
          if ($errors.Count) {
              $failures.Add("$($errors.Count) Error-severity finding(s)")
              $errors | ForEach-Object { Write-Host "::error file=$($_.ScriptPath),line=$($_.Line)::$($_.RuleName): $($_.Message)" }
          }

          # --- Guards 2-5: rules at a genuine repo-wide zero. Each is a REGRESSION
          # guard: it is not cleaning anything up, it is refusing to let the first one
          # in. PSUseCompatibleSyntax is the load-bearing one -- it enforces the 5.1
          # mandate with the real parser rather than the hook's regexes.
          #
          # PSUseDeclaredVarsMoreThanAssignments is here only because the 127 dead
          # $manifest assignments were deleted first. That is the whole point of having
          # deleted them: at 127 the rule reported nothing but known boilerplate, so a
          # genuinely dead variable in a new test file was finding 128 of 127 and
          # invisible, and no gate could ever be written. It is also the rule the
          # control probe above uses, so its liveness is proven on every run.
          foreach ($rule in 'PSUseCompatibleSyntax', 'PSAvoidAssignmentToAutomaticVariable', 'PSUseBOMForUnicodeEncodedFile', 'PSUseApprovedVerbs', 'PSUseDeclaredVarsMoreThanAssignments') {
              $hits = @($all | Where-Object RuleName -eq $rule)
              if ($hits.Count) {
                  $failures.Add("$rule regressed: $($hits.Count) finding(s), expected 0")
                  $hits | ForEach-Object { Write-Host "::error file=$($_.ScriptPath),line=$($_.Line)::$($_.RuleName): $($_.Message)" }
              }
          }

          # --- Guards 6-7 (T11): two rules that are clean in Public/ ONLY, so they are
          # requested explicitly and scoped there.
          #
          # THE RuleName FILTER IS MANDATORY, NOT DEFENSIVE. A caller's -IncludeRule is
          # UNION'd with the settings file's IncludeRules -- measured; it does not
          # replace it. Unfiltered, this scan returns 22 records over Public/ and none
          # of them belong to the rule being gated, so the gate would fail on unrelated
          # preset findings while reporting the wrong cause.
          foreach ($rule in 'PSProvideCommentHelp', 'PSUseCompatibleCommands') {
              $scoped = @(Invoke-ScriptAnalyzer -Path 'Public' -Recurse -Settings $settings -IncludeRule $rule |
                  Where-Object RuleName -eq $rule)

              # Per-rule control: the same rule must be NONZERO in Private/. Without it
              # a zero cannot be distinguished from an inert rule -- and for
              # PSProvideCommentHelp that is the LIKELY failure, because its default
              # ExportedOnly = $true silences it completely in this codebase (one
              # function per dot-sourced file, exports declared in the manifest). The
              # settings file sets $false; if that config is ever dropped, this control
              # fails instead of the gate silently passing forever.
              $control2 = @(Invoke-ScriptAnalyzer -Path 'Private' -Recurse -Settings $settings -IncludeRule $rule |
                  Where-Object RuleName -eq $rule)
              if ($control2.Count -eq 0) {
                  $failures.Add("CONTROL FAILED for ${rule}: 0 findings in Private/ too, so the Public/ zero proves nothing")
              }

              if ($scoped.Count) {
                  # ${rule}, not $rule -- "$rule:" parses the colon as a SCOPE qualifier
                  # (as in $script:x) and is a syntax error, not a runtime surprise.
                  $failures.Add("${rule}: $($scoped.Count) finding(s) in Public/, expected 0")
                  $scoped | ForEach-Object { Write-Host "::error file=$($_.ScriptPath),line=$($_.Line)::$($_.RuleName): $($_.Message)" }
              } else {
                  Write-Host "gate $rule (Public/): 0, control Private/: $($control2.Count)"
              }
          }

          # Warnings are reported, not gated. 149 stand today and the plan does not
          # clear them; a threshold would either be met trivially or block every PR.
          # Revisit only with a number someone has committed to driving down.
          Write-Host "::notice::PSScriptAnalyzer: $($warnings.Count) Warning-severity findings (not gated)"

          if ($failures.Count) {
              Write-Host ''
              $failures | ForEach-Object { Write-Host "FAIL: $_" }
              throw "PSScriptAnalyzer gate failed: $($failures.Count) condition(s)."
          }
          Write-Host 'PSScriptAnalyzer gate passed.'

  # =====================================================================
  # NOT gated, and why -- each of these would fail today:
  #
  #   PSUseCompatibleCommands (repo-wide)    21,889 -- 21,870 of them in Tests/, because
  #                                          the rule knows only built-in commands and so
  #                                          reports every Pester `Should` parameter.
  #                                          Gated on Public/ only, above.
  #   PSAvoidGlobalVars                      61, a deliberate Pester cross-mock-scope
  #                                          pattern. In the PSGallery preset, so it is
  #                                          kept in the settings file and not gated.
  #   PSUseSingularNouns                     44, deliberately kept in the settings file
  #                                          and deliberately not gated.
  #   PSAvoidUsingEmptyCatchBlock            11, deferred to dmann000/fb-powershell#117,
  #                                          which already adjudicated all three shipped
  #                                          sites. Reported, never gated -- so this job
  #                                          is green with them outstanding, and fixing
  #                                          one would need a live FlashBlade run because
  #                                          every working fix adds an executable line to
  #                                          shipped code. A comment does NOT clear this
  #                                          rule.
  #
  # Warning count today: 149. Measured under the committed settings file, not carried
  # forward -- every earlier figure here went stale within days: 302, then 276 once
  # T1/T2 and T3 took the BOM 24 and the automatic-variable 1 to zero, then 149 once the
  # 127 dead $manifest assignments went. Re-measure rather than trusting this line.
  # =====================================================================

  test-windows-powershell-5-1:
    name: Test (windows-latest, Windows PowerShell 5.1)
    runs-on: windows-latest
    needs: prepare-specs
    steps:
      - name: Checkout
        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

      # Parses everything this leg parses (Public/, Private/, the root module, Tests/,
      # scripts/) with the 5.1 parser, and checks shipped code for 5.1 runtime and
      # edition-divergence traps. A parse error fails here with its file and line; runtime
      # and divergence findings are warnings. Run directly, not through a nested
      # `powershell.exe` child: this step's shell already IS Windows PowerShell 5.1.
      # The script exits 2 when it cannot run, but this shell reports any non-zero exit as
      # 1, so the step fails either way and the log says which it was.
      - name: Check Windows PowerShell 5.1 compatibility
        shell: powershell
        run: ./tools/Test-PfbPs51Compat.ps1 -All

      # Wired here too, for symmetry with the pwsh job. The tools/specs-gated Describes are
      # additionally PS7-gated so they still skip on this leg, but a future spec-dependent
      # test that does NOT carry the PS7 guard then works with no workflow change.
      - name: Download API spec cache
        uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0
        with:
          name: pfb-specs
          path: tools/specs

      # Same pinned modules and same cache entry as the pwsh job on this OS -- the saved
      # files are edition-independent (Pester 6.0.1 declares PowerShellVersion = '5.1').
      # `shell:` is passed explicitly because a composite action has no default shell and
      # cannot read `matrix`.
      - name: Install test dependencies
        uses: ./.github/actions/install-test-modules
        with:
          shell: powershell

      # `shell:` stays per-leg and must remain a literal (see the header note about the
      # matrix context) -- it selects the interpreter. -Edition selects only which
      # Tests/coverage-baseline.psd1 block applies, since the two editions legitimately
      # differ by ~200 skips.
      - name: Run Pester tests
        shell: powershell
        run: ./scripts/Invoke-PfbCiPester.ps1 -Edition winps51