From fc073e7e2a90bcb0c9905a3ef385542bcb4dea6a Mon Sep 17 00:00:00 2001 From: "goodolclint-claude[bot]" <323206664+goodolclint-claude[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:55:54 -0500 Subject: [PATCH] ci: pin Pester by exact version everywhere it is installed or imported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pester was installed with -MinimumVersion 5.0 and no ceiling in the CI job image, both install steps in unit-tests.yml, and both Import-Module calls, plus the suite's own import inside the container. The image is rebuilt on every CI run and Pester is installed fresh on every unit-test run, so PSGallery chose the version — a new major could reach the required PR checks with no commit here, surfacing as unexplained test breakage on whichever PR ran next. It had already happened. Steps named "Install Pester 5" were resolving 6.1.0 on both legs, because Pester 6 declares PowerShellVersion 5.1 and so installs on Windows PowerShell too. Nothing broke — the suite uses only constructs common to 5 and 6, and runs 1566/0 under 6.1.0 with no deprecation warnings — but nobody chose it. The step names are corrected; they had been describing an install that stopped happening some time ago. Pinning the install alone is not enough, in two ways review found: An unset variable does not fail. -RequiredVersion accepts an empty value and degrades to "latest" for Install-Module and to "any" for Import-Module, both exiting 0, so a renamed or dropped env key would silently restore the float this commit removes. A guard step now fails the job instead. The point of use was still floored. run-integration.sh imported the suite's Pester with -MinimumVersion 5.0, so a second Pester reaching PSModulePath would win regardless of what was installed. The Dockerfile now promotes the ARG to ENV so the version is discoverable at runtime, and that import is pinned to it. The pin lives in two files, so shell-selfchecks asserts they agree — split-brain between the workflow and the image is precisely the unexplained breakage this is meant to prevent. CONTRIBUTING.md and CLAUDE.md are updated too; the contributor instructions were a third floating install site. Recorded as an amendment to D017 — the same principle as the nested PVE package pin, applied to the lane's own tooling. --- .github/workflows/unit-tests.yml | 37 ++++++++++++++++--- CLAUDE.md | 3 +- CONTRIBUTING.md | 3 +- DECISIONS.md | 21 +++++++++++ tests/Dockerfile.test | 16 ++++++-- .../infrastructure/scripts/run-integration.sh | 6 ++- 6 files changed, 74 insertions(+), 12 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 242f61a..b48c698 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -1,5 +1,11 @@ name: Unit Tests +# Pinned, not floored. Pester is installed fresh on every run, so a version range +# lets a new major reach the required checks with no commit to this repo (#112). +# Keep in step with PESTER_VERSION in tests/Dockerfile.test. +env: + PESTER_VERSION: 6.1.0 + on: push: branches: [ main ] @@ -81,17 +87,26 @@ jobs: name: module-${{ matrix.framework }}-${{ matrix.artifact_os }} path: ./publish/${{ matrix.framework }}/ - - name: Install Pester 5 (PS 5.1) + # -RequiredVersion accepts an empty value and degrades to "latest" with + # exit 0, so an unset variable would silently restore the float this pin + # removes. Fail the job instead. + - name: Verify PESTER_VERSION is set + shell: pwsh + run: | + if ([string]::IsNullOrWhiteSpace($env:PESTER_VERSION)) { throw 'PESTER_VERSION is not set' } + "Pinning Pester to $env:PESTER_VERSION" + + - name: Install Pester (PS 5.1) if: matrix.ps_version == '5.1' shell: powershell run: | - Install-Module -Name Pester -MinimumVersion 5.0 -Force -Scope CurrentUser -SkipPublisherCheck + Install-Module -Name Pester -RequiredVersion $env:PESTER_VERSION -Force -Scope CurrentUser -SkipPublisherCheck - - name: Install Pester 5 (PS 7.x) + - name: Install Pester (PS 7.x) if: matrix.ps_version != '5.1' shell: pwsh run: | - Install-Module -Name Pester -MinimumVersion 5.0 -Force -Scope CurrentUser + Install-Module -Name Pester -RequiredVersion $env:PESTER_VERSION -Force -Scope CurrentUser - name: Copy module to module path (PS 5.1) if: matrix.ps_version == '5.1' @@ -121,7 +136,7 @@ jobs: if: matrix.ps_version == '5.1' shell: powershell run: | - Import-Module Pester -MinimumVersion 5.0 + Import-Module Pester -RequiredVersion $env:PESTER_VERSION $config = New-PesterConfiguration $config.Run.Path = "tests/PSProxmoxVE.Tests" $config.Run.Exit = $true @@ -136,7 +151,7 @@ jobs: if: matrix.ps_version != '5.1' shell: pwsh run: | - Import-Module Pester -MinimumVersion 5.0 + Import-Module Pester -RequiredVersion $env:PESTER_VERSION $config = New-PesterConfiguration $config.Run.Path = "tests/PSProxmoxVE.Tests" $config.Run.Exit = $true @@ -172,6 +187,16 @@ jobs: - name: report-package-currency self-check run: bash tests/infrastructure/scripts/report-package-currency.test.sh + # The pin lives in two files; split-brain is exactly the unexplained + # breakage D017 exists to prevent, so assert they agree. + - name: Pester pin agrees across workflow and image + run: | + wf=$(sed -n 's/^ PESTER_VERSION: *//p' .github/workflows/unit-tests.yml) + img=$(sed -n 's/^ARG PESTER_VERSION=//p' tests/Dockerfile.test) + echo "workflow=$wf image=$img" + [ -n "$wf" ] && [ -n "$img" ] || { echo "could not read one of the pins" >&2; exit 1; } + [ "$wf" = "$img" ] || { echo "PESTER_VERSION differs: workflow=$wf image=$img" >&2; exit 1; } + - name: Shell syntax check run: | for f in tests/infrastructure/scripts/*.sh; do diff --git a/CLAUDE.md b/CLAUDE.md index d0693aa..196ef90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,8 @@ C# binary PowerShell module for managing Proxmox VE (PVE) infrastructure. Two pr - `src/PSProxmoxVE/` — Cmdlets and module surface (targets netstandard2.0) - `src/PSProxmoxVE.Core/` — Services, models, HTTP client (targets netstandard2.0) -Tests: xUnit (`tests/PSProxmoxVE.Core.Tests/`) and Pester 5 (`tests/PSProxmoxVE.Tests/`). +Tests: xUnit (`tests/PSProxmoxVE.Core.Tests/`) and Pester (`tests/PSProxmoxVE.Tests/`), pinned by +`PESTER_VERSION` in `.github/workflows/unit-tests.yml` and `tests/Dockerfile.test`. ## Development Workflow diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22b11cb..2a21adc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,7 +8,8 @@ Thank you for your interest in contributing! This document provides guidelines a - [.NET SDK 10.0+](https://dotnet.microsoft.com/download) - [PowerShell 7.2+](https://github.com/PowerShell/PowerShell) (for running Pester tests) -- [Pester 5](https://pester.dev/) (`Install-Module Pester -MinimumVersion 5.0 -Force`) +- [Pester](https://pester.dev/) — match CI's pin: `Install-Module Pester -RequiredVersion 6.1.0 -Force` + (the version is `PESTER_VERSION` in `.github/workflows/unit-tests.yml`) - An IDE with C# support (Visual Studio, VS Code with C# Dev Kit, Rider) ### Building diff --git a/DECISIONS.md b/DECISIONS.md index ed5ca63..567b558 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -634,6 +634,27 @@ still fails the job. `run-integration.sh` returns 3 for a genuine test failure a reach or authenticate to a node; only 3 is suppressed. Suppressing both would let a botched reboot report success while the lane learned nothing. +### Amendment 2026-09-01 — the pin covers the test tooling, not just the nested PVE packages + +The gating lane's own tooling is pinned by exact version for the same reason its nested PVE packages +are: `Pester` in `tests/Dockerfile.test` (`ARG PESTER_VERSION`) and in `.github/workflows/unit-tests.yml` +(`env.PESTER_VERSION`), installed and imported with `-RequiredVersion` at every site, including the +suite's own import inside the container. The Dockerfile promotes the ARG to `ENV` so the version is +discoverable at runtime. Both files must name the same version, and `shell-selfchecks` asserts it. + +Before this, both sites used `-MinimumVersion 5.0` with no ceiling. The image is rebuilt on every CI +run and Pester is installed fresh on every unit-test run, so PSGallery decided the version — a new +major could reach the merge gate with no commit to this repository, surfacing as unexplained test +breakage on whichever PR happened to run next. That is the same class of moving input the lane split +exists to eliminate; the difference is only that it moves in the test runner rather than in PVE. + +It had already happened silently: steps named "Install Pester 5" were resolving 6.1.0 on both the +PowerShell 5.1 and 7.x legs, because Pester 6 declares `PowerShellVersion 5.1` and so installs on +Windows PowerShell too. Nothing broke — the suite uses only constructs common to 5 and 6 — but +nobody chose it. + +Bumping is a deliberate commit that changes both files together. + ### Anti-pattern (do not reintroduce) ```bash # NEVER in first-boot.sh — this is the mismatch that left a node unclustered diff --git a/tests/Dockerfile.test b/tests/Dockerfile.test index e64e8e6..dcb3af1 100644 --- a/tests/Dockerfile.test +++ b/tests/Dockerfile.test @@ -36,12 +36,22 @@ RUN if [ "$(dpkg --print-architecture)" = "amd64" ]; then \ && ln -s /root/.dotnet/tools/pwsh /usr/local/bin/pwsh; \ fi -# Install Pester and prepare module directory +# Install Pester and prepare module directory. +# Pinned, not floored: this image is rebuilt on every CI run, so a version range +# lets a new Pester major reach the gating lane with no commit to this repo. +# Bump deliberately, the way D017 treats the nested PVE package set. +ARG PESTER_VERSION=6.1.0 RUN pwsh -NoProfile -Command \ - 'Set-PSRepository -Name PSGallery -InstallationPolicy Trusted; \ - Install-Module -Name Pester -MinimumVersion 5.0 -Scope AllUsers -Force' \ + "Set-PSRepository -Name PSGallery -InstallationPolicy Trusted; \ + Install-Module -Name Pester -RequiredVersion $PESTER_VERSION -Scope AllUsers -Force" \ + && pwsh -NoProfile -Command \ + "if (-not (Get-Module -ListAvailable Pester | Where-Object Version -eq '$PESTER_VERSION')) { throw 'Pester $PESTER_VERSION not installed' }" \ && mkdir -p /usr/local/share/powershell/Modules/PSProxmoxVE +# Promoted to ENV so the suite can pin its import too — the install pin alone +# does not bind the point of use if a second Pester ever reaches PSModulePath. +ENV PESTER_VERSION=$PESTER_VERSION + WORKDIR /repo CMD ["pwsh", "-NoProfile"] diff --git a/tests/infrastructure/scripts/run-integration.sh b/tests/infrastructure/scripts/run-integration.sh index eeaae73..f214d28 100644 --- a/tests/infrastructure/scripts/run-integration.sh +++ b/tests/infrastructure/scripts/run-integration.sh @@ -565,7 +565,11 @@ cmd_test() { pwsh -NoProfile -Command " \$PveVersion = '$v' \$TestFilter = '$pester_filter_arg' - Import-Module Pester -MinimumVersion 5.0 + \$pesterVersion = \$env:PESTER_VERSION + if ([string]::IsNullOrWhiteSpace(\$pesterVersion)) { + throw 'PESTER_VERSION is not set — refusing to import whichever Pester happens to be present' + } + Import-Module Pester -RequiredVersion \$pesterVersion \$config = New-PesterConfiguration if (\$TestFilter) {