Make the uninstaller test suites run on macOS and Windows PowerShell 5.1 - #7643
Conversation
The suites added in #7631 pass on the ubuntu runner but abort elsewhere. tests/sh/test_uninstall_arg_guard.sh built its fixture with TMPDIR="$_TMP_ROOT" mktemp -d. BSD mktemp with no template implies -t and resolves the directory itself, so on macOS the result is a sibling of _TMP_ROOT rather than a child, the validation below it fires, and the whole suite exits at the first --help case. Pass an explicit template instead. tests/sh/test_uninstall_shared_icon.sh used mktemp -p, which is GNU only. tests/studio/test_uninstall_arg_guard.ps1 set $ErrorActionPreference to Stop and then merged a native command's stderr with 2>&1. Windows PowerShell 5.1 turns that into a terminating NativeCommandError, so the suite died on the first rejected-argument case; PowerShell 7.1 and later do not. Scope the preference down around the two native invocations. The suite is self-hosting through (Get-Process -Id $PID).Path, so the Windows workflow now also runs it under powershell.exe. Nothing in CI covered 5.1 before, which is why this went unnoticed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 958207de55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function Invoke-Uninstaller([string[]]$ScriptArgs) { | ||
| $argv = @("-NoProfile", "-File", $uninstallPath) + $ScriptArgs | ||
| $out = & $pwshPath @argv 2>&1 | Out-String | ||
| $out = Invoke-Native { & $pwshPath @argv 2>&1 } |
There was a problem hiding this comment.
When Invoke-Uninstaller launches the child through Invoke-Native, the native process sets $LASTEXITCODE only in the helper's child scope; after the helper returns, line 59 therefore reads the caller's previous value (initially $null) rather than the uninstaller's exit code. Consequently every help case is recorded as nonzero because $null -eq 0 is false, the suite accumulates failures, and both Windows workflow invocations added by this change exit 1. Return/capture the exit code inside Invoke-Native along with the output instead of reading it afterward.
Useful? React with 👍 / 👎.
1 participant
Problem
The uninstaller suites added in #7631 pass on the ubuntu runner, which is the only place CI runs them. On the other two platforms they abort before finishing.
Verified on real runners (macos-14, windows-latest, and Windows PowerShell 5.1) before this change:
tests/sh/test_uninstall_arg_guard.shtests/studio/test_uninstall_arg_guard.ps1Root causes
macOS.
make_homebuilt its fixture withTMPDIR="$_TMP_ROOT" mktemp -d. BSDmktempwith no template implies-tand resolves the directory itself, so the result lands beside_TMP_ROOTinstead of inside it. The validation immediately below then fires andexit 1s the whole suite:An explicit template is portable across GNU and BSD and keeps the existing containment check meaningful.
tests/sh/test_uninstall_shared_icon.shhad the same class of problem withmktemp -p, which is GNU only.Windows PowerShell 5.1. The suite sets
$ErrorActionPreference = "Stop"and then merges a native command's stderr with2>&1. Windows PowerShell turns that into a terminatingNativeCommandError, so the run dies on the first rejected-argument case, which is the first case that writes to stderr:PowerShell 7.1 and later do not do this, which is why the pwsh runs are green. The preference is now scoped down around the two native invocations.
Why 5.1 matters here
The suite is self-hosting through
(Get-Process -Id $PID).Path, so launching it frompowershell.exeexercises 5.1, and 5.1 is the shell the README one-liner lands in on a stock Windows box. Nothing in CI ran 5.1 before, which is why this went unnoticed, so the Windows workflow now runs the guard suite there too.Verification
Re-run on the same runners after the change:
Locally:
test_uninstall_arg_guard.sh25 passed,test_uninstall_shared_icon.sh11 passed,test_uninstall_arg_guard.ps139 checks passed;dash -n/bash -n/busybox sh -nclean and the ps1 parses;tests/studio/test_ci_shell_suite_coverage.py15 passed.No production code changes.
scripts/uninstall.shandscripts/uninstall.ps1are untouched.