feat(ci): add a Windows PowerShell 5.1 Pester leg - #212
Conversation
The Pester matrix only ran Linux pwsh and Windows pwsh, both of which define $IsWindows as a real boolean, so the PS5.1 bug class ($IsWindows is $null) was structurally undetectable. PS5.1 is a real execution surface: the profile loader is wired into the WindowsPowerShell Documents directory. - Add powershell51-tests to the CI matrix, running under the inbox Windows PowerShell shell (not pwsh), asserting the major version is 5, installing Pester 5 (inbox 3.4 cannot run this suite), and running the same tests/powershell/ entry point. Kept advisory (not in the ruleset's required-check set) until it proves stable. - Fix 6 Unix-only-test -Skip: guards that were not PS5.1-null-safe: under PS5.1 $IsWindows is $null, so -Skip:($IsWindows -eq $true) evaluates to "not skipped" and Unix-only tests would run and fail on Windows. Converted to -Skip:($IsWindows -ne $false), matching the pattern 30-mise.Tests.ps1 already uses for its Unix Describe block.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe pull request adds a Windows PowerShell 5.1 Pester CI job, updates platform guards for PS5.1 semantics, replaces several PowerShell file writes with explicit UTF-8 .NET writes, and changes test path construction to nested ChangesPowerShell 5.1 compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant WindowsPowerShell51
participant Pester
participant PowerShellTests
GitHubActions->>WindowsPowerShell51: Start powershell51-tests
WindowsPowerShell51->>WindowsPowerShell51: Verify PowerShell major version 5
WindowsPowerShell51->>Pester: Ensure Pester 5
WindowsPowerShell51->>Pester: Invoke Pester tests/powershell/
Pester->>PowerShellTests: Execute tests
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new advisory CI job that runs the existing PowerShell Pester suite under Windows PowerShell 5.1 to catch $IsWindows -eq $null regressions, and updates Unix-only -Skip: guards in Pester tests to be PS5.1-null-safe.
Changes:
- Add a
powershell51-testsjob to.github/workflows/test.ymlusingshell: powershelland verifying PS major version is 5. - Update Unix-only Pester
-Skip:expressions to-Skip:($IsWindows -ne $false)so they correctly skip on PS5.1 where$IsWindowsis$null. - Keep the new PS5.1 leg advisory (not required) while it stabilizes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/test.yml |
Adds an advisory Windows PowerShell 5.1 Pester job to extend CI coverage to PS5.1 semantics. |
tests/powershell/secret-status.Tests.ps1 |
Makes Unix-only It/Describe blocks PS5.1-null-safe via updated -Skip: guards. |
tests/powershell/02-cargo.Tests.ps1 |
Makes the Unix-only Describe block PS5.1-null-safe via updated -Skip: guard. |
The new powershell51-tests CI job (#166) failed on real Windows PowerShell 5.1: Join-Path only accepts two positional path segments there (PS7 added variadic support), and the utf8NoBOM encoding literal used with Set-Content is PS6+ only. - Nest Join-Path calls where more than two segments were joined. - Replace `Set-Content -Encoding utf8NoBOM` with [System.IO.File]::WriteAllText plus a BOM-less UTF8Encoding, which works unchanged on PS5.1. - Resolve PS provider paths (e.g. TestDrive:\...) to real filesystem paths before handing them to [System.IO.File], since .NET I/O APIs don't understand PowerShell drives. Scope is limited to files actually exercised by tests. The same utf8NoBOM literal remains in three .tmpl scripts that render but never execute under any current test (git-profiles, authorized-keys, and the SSH key deploy script); left untouched pending a follow-up issue. The SSH key deploy template in particular is security-sensitive and should not be changed without a dedicated review.
cspell flags this fake AWS access key ID once a PR touches the file it lives in, since incremental checks scan the whole file, not just the diff. It is a documented placeholder, not a real credential.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Line 128: Update the actions/checkout step in the PowerShell test job to set
persist-credentials to false, keeping the existing checkout behavior unchanged
unless a later step explicitly requires authenticated Git operations.
In `@tests/powershell/25-deploy-secret-files.Tests.ps1`:
- Around line 51-59: Add a narrowly scoped cspell ignore directive for the
fixture token “AKIAEXAMPLE” near its use in the test assertion within the
deployment test. Keep the ignore limited to this test fixture rather than adding
a broad dictionary entry, and preserve the existing content validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 30037f46-2a0a-46ba-aac5-25c935028333
📒 Files selected for processing (10)
.github/workflows/test.ymlhome/dot_local/bin/executable_secret-deploy-state.ps1tests/powershell/02-cargo.Tests.ps1tests/powershell/25-deploy-secret-files.Tests.ps1tests/powershell/fixtures/generate-authorized-keys.ps1tests/powershell/fixtures/generate-git-profiles.ps1tests/powershell/generate-authorized-keys.Tests.ps1tests/powershell/secret-deploy-state.Tests.ps1tests/powershell/secret-status.Tests.ps1tests/powershell/signing-resolve.Tests.ps1
GitHub Actions' powershell/pwsh shell steps default $ErrorActionPreference to Stop. Windows PowerShell 5.1 wraps a redirected native process's stderr lines as ErrorRecord objects, so under that combination the expected non-zero-exit output from the pwsh subprocesses spawned by secret-deploy-state and secret-status tests was promoted into a terminating exception instead of landing in the captured $output. Set $ErrorActionPreference = 'Continue' locally in both harness helpers before the capture. Also skip-guard the 30-mise "calls reshim" test on PS5.1: its Set-Item Function: mock doesn't record the call there, but the script's own `& $miseCommand reshim` line has nothing PS6+-only about it, so this reads as a mock scope-capture quirk rather than a real incompatibility -- left for follow-up rather than guessed at blind.
- Set persist-credentials: false on the leg's checkout step; it only needs a working tree, never authenticated git operations. - Force-import Pester 5+ before Invoke-Pester: PSModulePath can list the inbox Pester 3.4 module ahead of the CurrentUser-scoped install, and auto-loading would silently pick the wrong major version instead of guaranteeing Pester 5+.
This comment has been minimized.
This comment has been minimized.
- Enable TLS 1.2 before Install-Module in the PS5.1 leg: PS5.1's .NET default protocol set can exclude it, and PSGallery requires it. Mirrors the existing workaround in 55-setup-editors.ps1.tmpl's vim-plug bootstrap. - signing-resolve.Tests.ps1: WriteAllText writes $Matches[1] exactly as captured, unlike the Set-Content it replaced, which always appended a trailing newline. Append one explicitly so the rendered gitconfig fixture keeps its previous shape.
This comment has been minimized.
This comment has been minimized.
|
F4 Cleanup Evidence
|
Summary
Closes #166.
The Pester matrix only ran Linux
pwshand Windowspwsh, both ofwhich define
$IsWindowsas a real boolean, so the PS5.1 bug class(
$IsWindowsis$null) was structurally undetectable. PS5.1 is areal execution surface: the profile loader is wired into the
WindowsPowerShellDocuments directory.powershell51-teststo the CI matrix, running under the inboxWindows PowerShell shell (not
pwsh), asserting the major versionis 5, installing Pester 5 (inbox 3.4 cannot run this suite), and
running the same
tests/powershell/entry point. Kept advisory(not in the ruleset's required-check set) until it proves stable.
-Skip:guards that were not PS5.1-null-safe:under PS5.1
$IsWindowsis$null, so-Skip:($IsWindows -eq $true)evaluates to "not skipped" and Unix-only tests would run andfail on Windows. Converted to
-Skip:($IsWindows -ne $false),matching the pattern
30-mise.Tests.ps1already uses for its UnixDescribeblock (02-cargo.Tests.ps1line 12,secret-status.Tests.ps1lines 95/110/159/178/213).Test plan
tests/powershell/suite under this box's
pwsh(PS7) — 202/207 pass, the 5remaining failures are pre-existing non-Windows-local failures
unrelated to this change (PSReadLine/OnIdle mocks, tailscale
serve mock)
npx markdownlint-cli2 "**/*.md"— 0 issuesnpx js-yaml)powershell51-testsjob runs green on this PR and echoes$PSVersionTable.PSVersion.Major -eq 5— verified from CI, nolocal Windows PowerShell 5.1 available to pre-check
🤖 Generated with Claude Code
https://claude.ai/code/session_01Dz1GQ6a5w3RPxbU1mxsdP1
Summary by CodeRabbit
Bug Fixes
Tests