diff --git a/.github/workflows/CI.yaml b/.github/workflows/CI.yaml index ac08765..e308cc0 100644 --- a/.github/workflows/CI.yaml +++ b/.github/workflows/CI.yaml @@ -15,6 +15,9 @@ jobs: lint: name: PSScriptAnalyzer Lint runs-on: ubuntu-latest + # Guard against a hung step burning the 6-hour default. Lint normally + # finishes in well under a minute. + timeout-minutes: 10 steps: - uses: actions/checkout@v7 @@ -33,27 +36,37 @@ jobs: echo "is_template=false" >> "$GITHUB_OUTPUT" fi - - name: Cache PowerShell modules - if: steps.template_guard.outputs.is_template == 'false' - id: cache-lint-modules - uses: actions/cache@v6 - with: - path: ~/.local/share/powershell/Modules - key: ${{ runner.os }}-psmodules-lint-${{ hashFiles('build.depend.psd1') }} - restore-keys: | - ${{ runner.os }}-psmodules-lint- - + # No module cache here on purpose -- see the note in the unit-tests job. + # The cache this replaced held 209 bytes: PSScriptAnalyzer ships on the + # runner image, so it was never installed into the cached path and the + # cache only ever restored an empty directory. Install only when the + # image does not already provide it, rather than unconditionally -- the + # module is ~339 MB on disk and re-downloading it every run is far more + # expensive than the cache ever saved. - name: Install PSScriptAnalyzer - if: steps.template_guard.outputs.is_template == 'false' && steps.cache-lint-modules.outputs.cache-hit != 'true' + if: steps.template_guard.outputs.is_template == 'false' shell: pwsh run: | + # Pin to the version build.depend.psd1 declares, so lint results here match + # a local ./build.ps1 -Task Analyze. Accepting whatever the runner image + # happens to ship means the two can disagree silently. + $required = (Import-PowerShellDataFile -Path build.depend.psd1).PSScriptAnalyzer.Version + $installed = Get-Module -Name PSScriptAnalyzer -ListAvailable | + Where-Object { $_.Version -eq $required } + if ($installed) { + Write-Host "PSScriptAnalyzer $required already available; skipping install." + return + } + Set-PSRepository -Name PSGallery -InstallationPolicy Trusted - Install-Module -Name PSScriptAnalyzer -Force -Scope CurrentUser + Install-Module -Name PSScriptAnalyzer -RequiredVersion $required -Force -Scope CurrentUser - name: Run PSScriptAnalyzer if: steps.template_guard.outputs.is_template == 'false' shell: pwsh run: | + $required = (Import-PowerShellDataFile -Path build.depend.psd1).PSScriptAnalyzer.Version + Import-Module -Name PSScriptAnalyzer -RequiredVersion $required -Force -ErrorAction Stop $results = Invoke-ScriptAnalyzer -Path ./{{ModuleName}} -Recurse -Settings PSGallery -ReportSummary $errors = $results | Where-Object { $_.Severity -eq 'Error' } @@ -73,6 +86,13 @@ jobs: unit-tests: name: Unit Tests (${{ matrix.os }}) runs-on: ${{ matrix.os }} + # Guard against a hung step burning the 6-hour default. A cold run -- + # installing every dependency from the gallery -- completes in about + # 90 seconds. The specific hazard this caught: BuildHelpers' Invoke-Git + # calls WaitForExit() before draining stdout, so on Windows a HEAD commit + # message larger than the 4096-byte pipe buffer deadlocks the build with + # no output at all. Keep merge-commit messages short. + timeout-minutes: 15 strategy: fail-fast: false matrix: @@ -99,17 +119,13 @@ jobs: echo "is_template=false" >> "$GITHUB_OUTPUT" fi - - name: Cache PowerShell modules - if: steps.template_guard.outputs.is_template == 'false' - uses: actions/cache@v6 - with: - path: | - ~/Documents/PowerShell/Modules - ~/.local/share/powershell/Modules - key: ${{ runner.os }}-psmodules-${{ hashFiles('build.depend.psd1') }} - restore-keys: | - ${{ runner.os }}-psmodules- - + # No module cache here on purpose. Measured on a derived repo, same tree: + # ubuntu 27s warm / 27s cold, macOS 16s / 26s, Windows 28s / 50s. The + # three jobs run in parallel, so the cache bought at most ~22s of + # wall-clock. Against that it made pull-request runs warm and main runs + # cold, so a PR could pass on a code path main never exercised. + # build.ps1 -Bootstrap gates installation on Invoke-PSDepend -Test, so a + # runner that already satisfies the dependency file does no install work. - name: Build and Test if: steps.template_guard.outputs.is_template == 'false' shell: pwsh diff --git a/build.depend.psd1 b/build.depend.psd1 index a6acd25..eb2b758 100644 --- a/build.depend.psd1 +++ b/build.depend.psd1 @@ -8,7 +8,7 @@ } } 'Pester' = @{ - Version = '6.0.1' + Version = 'latest' Parameters = @{ SkipPublisherCheck = $true } diff --git a/build.ps1 b/build.ps1 index c76be82..a963cf0 100644 --- a/build.ps1 +++ b/build.ps1 @@ -67,52 +67,115 @@ $dependencyFilePath = Join-Path -Path $PSScriptRoot -ChildPath $dependencyFilePa if ($Bootstrap) { $null = PackageManagement\Get-PackageProvider -Name 'NuGet' -ForceBootstrap if ((Test-Path -Path $dependencyFilePath)) { - # Ensure PSGallery is registered and trusted - if (-not (Get-PSRepository -Name 'PSGallery' -ErrorAction 'SilentlyContinue')) { - Register-PSRepository -Default + # Ensure PSGallery is registered and trusted. + # + # Register-PSRepository -Default shells out to nuget.exe, which on some Windows + # runner images fails with: + # + # NuGet.Commands.CommandException: Missing option value for: '-source' + # + # That leaves PSGallery unregistered, and the Set-PSRepository call that used to + # follow it unconditionally then died with "No repository with the name + # 'PSGallery' was found", masking the real cause. Fall back to registering the + # gallery explicitly by URL, and only configure it once it actually exists. + $psGallery = Get-PSRepository -Name 'PSGallery' -ErrorAction 'SilentlyContinue' + if (-not $psGallery) { + try { + Register-PSRepository -Default -ErrorAction 'Stop' + } + catch { + Write-Verbose "Register-PSRepository -Default failed ($($_.Exception.Message)); registering PSGallery explicitly." -Verbose + } + + $psGallery = Get-PSRepository -Name 'PSGallery' -ErrorAction 'SilentlyContinue' + if (-not $psGallery) { + $registerParameters = @{ + Name = 'PSGallery' + SourceLocation = 'https://www.powershellgallery.com/api/v2' + InstallationPolicy = 'Trusted' + ErrorAction = 'Stop' + } + Register-PSRepository @registerParameters + $psGallery = Get-PSRepository -Name 'PSGallery' -ErrorAction 'SilentlyContinue' + } + } + + if (-not $psGallery) { + throw 'Could not register the PSGallery repository; build dependencies cannot be installed.' + } + + if ($psGallery.InstallationPolicy -ne 'Trusted') { + Set-PSRepository -Name 'PSGallery' -InstallationPolicy 'Trusted' } - Set-PSRepository -Name 'PSGallery' -InstallationPolicy 'Trusted' if (-not (Get-Module -Name 'PSDepend' -ListAvailable)) { Install-Module -Name 'PSDepend' -Scope 'CurrentUser' -Repository 'PSGallery' -Force } Import-Module -Name 'PSDepend' -Verbose:$false - # Try to import existing modules first to avoid installation locks - # Only install if import fails (missing modules or wrong versions) $psDependParameters = @{ Path = $PSScriptRoot Recurse = $False WarningAction = 'SilentlyContinue' - Import = $True Force = $True ErrorAction = 'Stop' } - $importSucceeded = $false + # Install before importing, never the other way round. + # + # This used to attempt an import first and only install if that failed, to avoid + # installation locks. That is safe on a warm cache, where the requested versions + # are already present, but wrong on a cold one: the import pass loads whatever + # version happens to be on the machine already -- typically an older Pester from + # the runner image -- and Pester ships a binary Pester.dll that cannot then be + # replaced in-process by the version the subsequent install brings in: + # + # An incompatible version of the Pester.dll assembly is already loaded. + # The loaded dll version is 5.9.0.0, but at least version 6.1.0 is required + # + # -Test reports whether each dependency is already satisfied without importing + # anything, so it is safe to run before anything is loaded (measured at ~6s here). + # Gate the install on it so a satisfied machine does no install work at all -- + # -Install carries -Force, which re-resolves and re-downloads every dependency. + $dependenciesSatisfied = $false try { - Invoke-PSDepend @psDependParameters - $importSucceeded = $true - Write-Verbose 'Successfully imported existing modules.' -Verbose + $testResults = @(Invoke-PSDepend @psDependParameters -Test -Quiet) + $dependenciesSatisfied = $testResults.Count -gt 0 -and $testResults -notcontains $false } catch { - Write-Verbose "Could not import all required modules: $_" -Verbose - Write-Verbose 'Attempting to install missing or outdated dependencies...' -Verbose + Write-Verbose "Could not determine dependency status: $_" -Verbose } - # If import failed, install the dependencies - if (-not $importSucceeded) { + if (-not $dependenciesSatisfied) { + Write-Verbose 'Installing missing or outdated dependencies...' -Verbose try { Invoke-PSDepend @psDependParameters -Install } catch { - Write-Error "Failed to install and import required dependencies: $_" - Write-Error 'This may be due to locked module files. Please restart the build environment or clear module locks.' + # Compose one message and throw it, rather than emitting several + # Write-Error calls: $ErrorActionPreference is 'Stop' in this script, so + # the first Write-Error terminates and every diagnostic after it -- the + # lock hint, the inner exception -- is silently dropped. + $installError = "Failed to install required dependencies: $($_.Exception.Message)" if ($_.Exception.InnerException) { - Write-Error "Inner exception: $($_.Exception.InnerException.Message)" + $installError += " Inner exception: $($_.Exception.InnerException.Message)" } - throw + $installError += ' This may be due to locked module files; restart the build environment or clear module locks.' + throw $installError + } + } + + try { + Invoke-PSDepend @psDependParameters -Import + Write-Verbose 'Successfully imported required modules.' -Verbose + } + catch { + # Single composed throw -- see the note in the install catch above. + $importError = "Failed to import required dependencies: $($_.Exception.Message)" + if ($_.Exception.InnerException) { + $importError += " Inner exception: $($_.Exception.InnerException.Message)" } + throw $importError } } else { diff --git a/build.psake.ps1 b/build.psake.ps1 index 1f2b47b..fcf46c9 100644 --- a/build.psake.ps1 +++ b/build.psake.ps1 @@ -49,8 +49,7 @@ Task -Name 'Init_Integration' -Description 'Load integration test environment va # PowerShell Gallery release-notes panel shows the curated, user-facing notes (the same # content used for the GitHub release) instead of just a link. Depends on Build so the # staged manifest in ModuleOutDir exists; runs before Publish (see $PSBPublishDependency -# below). Non-fatal if the changelog can't be read or has no entry for the version being -# published, so a release is never blocked. +# below). Non-fatal at every step so a release is never blocked. Task -Name 'UpdateReleaseNotes' -Depends 'Build' -Description 'Set built manifest ReleaseNotes from the matching CHANGELOG.md entry' { $changelogPath = Join-Path -Path $PSScriptRoot -ChildPath 'CHANGELOG.md' if (-not (Test-Path -Path $changelogPath)) { @@ -101,5 +100,185 @@ Task -Name 'UpdateReleaseNotes' -Depends 'Build' -Description 'Set built manifes # defaults to depending only on 'Test'). $PSBPublishDependency = @('Test', 'UpdateReleaseNotes') -# Note: -Depends replaces PowerShellBuild's default dependencies, so we must include Pester and Analyze explicitly -Task -Name 'Test' -FromModule 'PowerShellBuild' -MinimumVersion '0.7.3' -Depends 'Init_Integration', 'Pester', 'Analyze' +# Custom Pester task, used instead of PowerShellBuild's built-in 'Pester' task. +# +# Two separate reasons it exists. +# +# 1. Version agreement. PowerShellBuild's Test-PSBuildPester runs +# `Import-Module Pester -MinimumVersion 5.0.0`, which resolves to the *highest* +# installed version. Pester 6 also re-resolves Describe by autoloading during +# its per-file discovery, and autoload likewise picks the highest installed +# version -- so an exact pin is never actually honoured. Whenever the runner +# image ships something newer than the pin, the two collide: +# +# An incompatible version of the Pester.dll assembly is already loaded. +# +# Pester 6.0.1 arrived 2026-07-18 and 6.1.0 on 2026-08-11, breaking CI both +# times with no commit to blame. build.depend.psd1 therefore uses +# Version = 'latest' and this task imports the highest installed version, so +# PSDepend, this task and Pester's autoload all agree and cannot collide. +# Do not narrow either back to an exact version without changing the other. +# +# 2. Failed containers. PowerShellBuild's gate throws only on FailedCount, which +# cannot see a test file that died during discovery -- it generates no tests +# at all, so zero failures reads as success. See the gate below. +$unitTestPreReqs = { + # A psake PreCondition returning $false *skips* the task and lets the build succeed. + # That is the right behaviour for testing being deliberately switched off, and exactly + # the wrong behaviour for a missing test directory -- 'Test' would pass having run + # nothing at all. So only Test.Enabled may skip; anything else throws. + if (-not $PSBPreference.Test.Enabled) { + Write-Warning 'Pester testing is not enabled; skipping UnitTest.' + return $false + } + + if (-not (Test-Path -Path $PSBPreference.Test.RootDir)) { + throw "Test directory [$($PSBPreference.Test.RootDir)] not found, but testing is enabled. Refusing to report success without running tests." + } + + return $true +} + +# Depends on 'Build' because $PSBPreference.Build.ModuleOutDir is only populated once +# PowerShellBuild's Build task has run and staged the module. +Task -Name 'UnitTest' -Depends 'Build' -PreCondition $unitTestPreReqs -Description 'Execute Pester tests, failing on failed containers as well as failed tests' { + # build.depend.psd1 is the single source of truth for the Pester version. + $dependencyFile = Join-Path -Path $PSScriptRoot -ChildPath 'build.depend.psd1' + $pesterVersion = (Import-PowerShellDataFile -Path $dependencyFile).Pester.Version + + if ($pesterVersion -and $pesterVersion -ne 'latest') { + Import-Module -Name 'Pester' -RequiredVersion $pesterVersion -Force -ErrorAction 'Stop' + } + else { + # With 'latest', import the newest installed version. That is also what Pester's + # own autoloading resolves to when it re-resolves Describe during per-file + # discovery -- keeping the two in agreement is precisely what avoids the + # assembly collision, so do not narrow this to a specific version. + $newestPester = Get-Module -Name 'Pester' -ListAvailable | + Sort-Object -Property 'Version' -Descending | + Select-Object -First 1 + if (-not $newestPester) { + throw 'Pester is not installed.' + } + + # Import by path, not by -Name $newestPester. A PSModuleInfo stringifies to + # its Name, so -Name would import 'Pester' by name and resolve the version + # itself -- and if an incompatible Pester is already loaded that re-raises + # the very collision this task exists to prevent: + # + # An incompatible version of the Pester.dll assembly is already loaded. + # + # Verified against 5.7.1 preloaded: -Name left the session on 5.7.1. + # Unload first so the selected version is the only one in play. + Get-Module -Name 'Pester' | Remove-Module -Force -ErrorAction 'SilentlyContinue' + Import-Module -Name $newestPester.Path -Force -ErrorAction 'Stop' + } + Write-Verbose "Using Pester $((Get-Module -Name 'Pester').Version)" -Verbose + + # Remove any previously imported project module and import from the output dir + $moduleManifest = Join-Path -Path $PSBPreference.Build.ModuleOutDir -ChildPath "$($PSBPreference.General.ModuleName).psd1" + Get-Module -Name $PSBPreference.General.ModuleName | Remove-Module -Force -ErrorAction 'SilentlyContinue' + # -ErrorAction Stop so a non-terminating import error fails here rather than letting + # the run continue into Pester against a module that was never loaded. + Import-Module -Name $moduleManifest -Force -ErrorAction 'Stop' + + Push-Location -LiteralPath $PSBPreference.Test.RootDir + + try { + $configuration = [PesterConfiguration]::Default + $configuration.Output.Verbosity = 'Detailed' + $configuration.Run.PassThru = $true + $configuration.Run.Path = $PSBPreference.Test.RootDir + $configuration.TestResult.Enabled = -not [string]::IsNullOrEmpty($PSBPreference.Test.OutputFile) + $configuration.TestResult.OutputPath = $PSBPreference.Test.OutputFile + $configuration.TestResult.OutputFormat = $PSBPreference.Test.OutputFormat + + if ($PSBPreference.Test.CodeCoverage.Enabled) { + $configuration.CodeCoverage.Enabled = $true + # Pester 6 defaults CoveragePercentTarget to 75; this project sets the + # threshold to 0 and enforces coverage via Codecov instead. Carry the + # configured value across or the default silently reintroduces a gate. + # + # The two use different units: PowerShellBuild's Threshold is a fraction + # ("Threshold required to pass code coverage test (.90 = 90%)"), while + # Pester's CoveragePercentTarget is a percentage. Assigning one to the + # other unconverted is a no-op at 0, but would turn a later 0.90 into + # 0.9% and quietly disable the gate. + $configuration.CodeCoverage.CoveragePercentTarget = [double]$PSBPreference.Test.CodeCoverage.Threshold * 100 + if ($PSBPreference.Test.CodeCoverage.Files.Count -gt 0) { + $configuration.CodeCoverage.Path = $PSBPreference.Test.CodeCoverage.Files + } + $configuration.CodeCoverage.OutputPath = $PSBPreference.Test.CodeCoverage.OutputFile + $configuration.CodeCoverage.OutputFormat = $PSBPreference.Test.CodeCoverage.OutputFileFormat + } + + $testResult = Invoke-Pester -Configuration $configuration + + # FailedCount alone is not enough. When a file fails during discovery -- for + # example an empty -ForEach under Pester 6 -- Pester fails the whole container + # and it generates no tests at all: zero passed, zero failed. Gating only on + # FailedCount reports success while that file never ran, which is how ~777 + # tests sat silently disabled in PlexAutomationToolkit. + # Use FailedContainersCount, not `Containers | Where-Object { -not $_.Passed }`. + # A container that dies during discovery still reports Passed = $true on the + # container object, so filtering on it silently matches nothing -- reproducing + # the exact bug this check exists to catch. FailedContainersCount is the + # property Pester actually maintains. + if ($testResult.FailedContainersCount -gt 0) { + $testResult.FailedContainers | ForEach-Object { Write-Warning "Container failed: $($_.Item)" } + throw "$($testResult.FailedContainersCount) test file(s) failed to run. See 'Container failed' above." + } + + # Setup/teardown failures are counted separately again. A BeforeAll that throws + # can leave FailedCount at 0, and a failing AfterAll leaves both FailedCount and + # FailedContainersCount at 0 while the run still reports passing tests -- verified + # against Pester 6.1.0: + # failing AfterAll -> Failed 0, FailedContainers 0, FailedBlocks 1, Passed 1 + if ($testResult.FailedBlocksCount -gt 0) { + $testResult.FailedBlocks | ForEach-Object { Write-Warning "Block failed: $($_.Path -join ' > ')" } + throw "$($testResult.FailedBlocksCount) setup/teardown block(s) failed. See 'Block failed' above." + } + + if ($testResult.FailedCount -gt 0) { + throw 'One or more Pester tests failed' + } + + # A run that executed nothing is not a passing run. Every gate above counts + # failures, and a run with no executed tests produces zero of all of them. + # + # Two distinct ways to get there, and TotalCount alone only catches the first: + # 1. Nothing discovered -- a bad Run.Path, or a tests directory that stopped + # matching *.Tests.ps1. TotalCount is 0. + # 2. Everything discovered but nothing run -- an over-eager filter. Measured + # against Pester 6.1.0 with a filter matching no test name: + # Passed 0 | Failed 0 | Skipped 0 | NotRun 120 | TotalCount 120 + # TotalCount is non-zero, every failure count is 0, and the build passed. + # + # Test.Enabled is the deliberate opt-out and is handled in the PreCondition; + # reaching here having run nothing is a fault either way. + # Count tests that actually produced a result. Measured against Pester 6.1.0, + # three ways to reach "nothing ran" that every failure count reads as success: + # + # empty test directory -> Total 0, Passed 0, Failed 0, Skipped 0, NotRun 0 + # filter matching no test -> Total 120, Passed 0, Failed 0, Skipped 0, NotRun 120 + # every test -Skip -> Total 3, Passed 0, Failed 0, Skipped 3, NotRun 0 + # + # TotalCount minus NotRunCount misses the third, and so does filtering on the + # per-test .Executed property -- skipped tests report Executed = $true. Only + # passed-plus-failed distinguishes a suite that ran from one that did not. + # Casts are deliberate: with nothing discovered these come back null. + $ranCount = [int]$testResult.PassedCount + [int]$testResult.FailedCount + if ($ranCount -le 0) { + $counts = "discovered $([int]$testResult.TotalCount), skipped $([int]$testResult.SkippedCount), not run $([int]$testResult.NotRunCount)" + throw "Pester ran no tests under [$($PSBPreference.Test.RootDir)] ($counts). Refusing to report success without running tests." + } + } + finally { + Pop-Location + Remove-Module -Name $PSBPreference.General.ModuleName -ErrorAction 'SilentlyContinue' + } +} + +# Note: -Depends replaces PowerShellBuild's default dependencies. 'UnitTest' above stands in +# for PowerShellBuild's 'Pester' task; 'Analyze' is still PowerShellBuild's. +Task -Name 'Test' -FromModule 'PowerShellBuild' -MinimumVersion '0.7.3' -Depends 'Init_Integration', 'UnitTest', 'Analyze'