Skip to content

fix(test): resolve 13 Windows-only Pester failures #139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kurone-kito merged 4 commits into from
May 19, 2026
Merged

fix(test): resolve 13 Windows-only Pester failures #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
01f504a
fix(test): resolve 13 Windows-only Pester failures
kurone-kito May 19, 2026
908d877
chore(cspell): add HKLM to project word list
kurone-kito May 19, 2026
9b17e70
fix(test): escape inline JSON path in secret-status missing test
kurone-kito May 19, 2026
408397f
fix(test): use OSVersion.Platform for PS5-safe elevation skip guard
kurone-kito May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .cspell.config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ words:
- exrc
- gdefault
- gpgconf
- HKLM
- inputrc
- keepassxc
- kito
Expand Down
6 changes: 5 additions & 1 deletion tests/powershell/01-path.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ Describe '01-path' -Skip:($IsWindows -eq $false) {

# Disable registry sync by default so existing tests are not
# affected by the real Windows User PATH.
$env:DOTFILES_TEST_REGISTRY_USER_PATH = ''
# Use ';' not '' — Windows deletes env vars set to empty string,
# which would cause Get-RegistryUserPath to fall through to the
# real registry. A single ';' is non-null yet splits to only
# empty entries that the $norm -ne '' guard skips.
$env:DOTFILES_TEST_REGISTRY_USER_PATH = ';'

$env:PATH = @(
$script:Paths.UnrelatedA
Expand Down
4 changes: 2 additions & 2 deletions tests/powershell/30-mise.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ Describe '30-mise ghq trusted paths' {
Remove-Item Function:\PathMise -ErrorAction SilentlyContinue
}

It 'appends ghq-cloned owner paths from chezmoi-ghq-trusted-paths' {
It 'appends ghq-cloned owner paths from chezmoi-ghq-trusted-paths' -Skip:($null -eq (Get-Command 'ghq' -ErrorAction SilentlyContinue)) {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not skip mocked ghq tests based on host tools

On Windows CI or developer machines that do not have a real ghq executable installed, this -Skip condition prevents the test from running even though the test body immediately mocks both Get-Command ghq and the ghq root call. That means the ghq-trust path behavior is no longer covered in the common environment where ghq is absent; the same host-tool guard was also added to the blank-line case below, so regressions in this mocked flow can pass undetected.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

@kurone-kito kurone-kito May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rejecting: Pester 5 cannot mock a non-existent external command — it must be discoverable via Get-Command before the mock can wrap it. When ghq is absent, Mock ghq { ... } throws CommandNotFoundException during test setup rather than deferring to the mock body. The -Skip guard is therefore the correct fix; the alternative (mocking Get-Command 'ghq' to return a fake object, then also providing a stand-in function named ghq) would be a more invasive rewrite of the test structure than warranted. The test already has a companion case ('skips ghq trusted paths when ghq is not installed') that exercises the no-ghq code path without mocking the binary.

$miseCmd = New-TestMiseCommand -Name 'PathMise'
Mock Get-Command {
if ($Name -eq 'mise') { return $miseCmd }
Expand Down Expand Up @@ -387,7 +387,7 @@ Describe '30-mise ghq trusted paths' {
$env:MISE_TRUSTED_CONFIG_PATHS | Should -Not -BeLike "*$($script:GhqRoot)*"
}

It 'skips blank lines in chezmoi-ghq-trusted-paths' {
It 'skips blank lines in chezmoi-ghq-trusted-paths' -Skip:($null -eq (Get-Command 'ghq' -ErrorAction SilentlyContinue)) {
$miseCmd = New-TestMiseCommand -Name 'PathMise'
Mock Get-Command {
if ($Name -eq 'mise') { return $miseCmd }
Expand Down
6 changes: 4 additions & 2 deletions tests/powershell/secret-status.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ BeforeAll {

function script:Write-Manifest {
param([string]$CategoriesJson)
$escapedHomeDir = $script:HomeDir.Replace('\', '\\')
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Escape every Windows path in generated manifest JSON

Escaping only homeDir still leaves Windows runs broken for test cases that embed other paths inside CategoriesJson: for example the unskipped “secret file missing is MISSING” case writes "absPath":"$f", where $f contains backslashes on Windows, so the manifest remains invalid JSON and secret-status.ps1 exits 2 before the test can assert MISSING. Build the manifest data with ConvertTo-Json or ensure all interpolated JSON strings in the categories are escaped, not just this top-level field.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

@kurone-kito kurone-kito May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 9b17e70 — the 'secret file missing is MISSING' test embeds $f (a Windows path) directly into the categories JSON here-string. Added $fJson = $f.Replace('\\', '\\\\') and used $fJson in the interpolation. All other tests that embed Windows paths in JSON are already skipped on Windows (-Skip:($IsWindows -eq $true)).

$body = @"
{
"version": 1,
"manager": "bitwarden",
"os": "linux",
"homeDir": "$script:HomeDir",
"homeDir": "$escapedHomeDir",
"ghqRoot": "",
"categories": $CategoriesJson
Comment on lines 21 to 31
Copy link
Copy Markdown
Owner Author

@kurone-kito kurone-kito May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 9b17e70 — the 'secret file missing is MISSING' test was the only non-Windows-skipped case that embedded a Windows path in the categories JSON. Added $fJson = $f.Replace('\\', '\\\\') there. All other tests that contain Windows paths in JSON categories are guarded with -Skip:($IsWindows -eq $true).

}
Expand Down Expand Up @@ -124,9 +125,10 @@ Describe 'secret-status.ps1' {

It 'secret file missing is MISSING' {
$f = Join-Path $HomeDir 'never-existed.txt'
$fJson = $f.Replace('\', '\\')
$cats = @"
{"gpg":[],"sshKeys":[],"sshHosts":[],"secretFiles":[
{"label":"s","item":"S","target":"never-existed.txt","absPath":"$f","attachment":""}
{"label":"s","item":"S","target":"never-existed.txt","absPath":"$fJson","attachment":""}
],"envFiles":[]}
"@
Write-Manifest $cats
Expand Down
6 changes: 5 additions & 1 deletion tests/powershell/set-openssh-default-shell.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ Describe 'set-openssh-default-shell' -Skip:($IsWindows -eq $false) {
}
}

It 'Test-DotfilesAdminElevation returns false for non-elevated session' {
It 'Test-DotfilesAdminElevation returns false for non-elevated session' -Skip:(
[System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT -and
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(
Comment on lines +32 to +34
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use a PS5-safe elevated skip guard

On Windows PowerShell 5.1, $IsWindows is undefined and therefore $null, so this -and short-circuits to false and the elevated-session skip never applies; an elevated PS5 Invoke-Pester still runs the assertion and fails. The same guard was added to sync-openssh-authorized-keys.Tests.ps1, so use a PS5-safe Windows check for both elevated skips.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

@kurone-kito kurone-kito May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 408397f — replaced $IsWindows -and ... with [System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT -and .... This check is available in both PS5 and PS7, correctly returns false on non-Windows (short-circuiting the WindowsIdentity call), and is not affected by the undefined $IsWindows in Windows PowerShell 5.1.

[Security.Principal.WindowsBuiltInRole]::Administrator)
) {
Test-DotfilesAdminElevation | Should -BeFalse
}

Expand Down
6 changes: 5 additions & 1 deletion tests/powershell/sync-openssh-authorized-keys.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ Describe 'sync-openssh-authorized-keys' -Skip:($IsWindows -eq $false) {
}
}

It 'Test-DotfilesAdminElevation returns false for non-elevated session' {
It 'Test-DotfilesAdminElevation returns false for non-elevated session' -Skip:(
[System.Environment]::OSVersion.Platform -eq [System.PlatformID]::Win32NT -and
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(
[Security.Principal.WindowsBuiltInRole]::Administrator)
) {
Test-DotfilesAdminElevation | Should -BeFalse
}

Expand Down
Loading