Tests

Merge pull request #177 from juemerson-at-purestorage/feat/ci-static-… #184

Workflow file for this run

.github/workflows/cross-platform-tests.yml at c498a87

	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

Navigation Menu

Merge pull request #177 from juemerson-at-purestorage/feat/ci-static-… #184

Workflow file

Merge pull request #177 from juemerson-at-purestorage/feat/ci-static-… #184

Uh oh!

Workflow file for this run