Repository navigation
Get-StandInCommand: capture the test PID explicitly at dot-source time instead of relying on $PID default #128
Description
What
Get-StandInCommand in ProcessTracking.ps1 (around line 52) declares its parameter as:
function Get-StandInCommand {
param([int]$ProcessId = $PID)The default $PID resolves to the PID of the PowerShell process that is currently executing the function, not the test process that dot-sourced the script. This happens to be correct for every current call site (windows_pools_test.ps1:60, Start-DummyProcess at ProcessTracking.ps1:158, and Start-StandIn at process_tracking_test.ps1:44), but only by coincidence — all of them run in the same process scope as the dot-source.
Capture the test PID explicitly at dot-source time and use that as the default instead of relying on $PID.
Why
- Correctness risk: Any future caller that invokes
Get-StandInCommandfrom a child scope where$PIDdiffers (e.g. a background job, aStart-Job/ForEach-Object -Parallelblock, or a helper invoked from a separate runspace) will silently get the wrong PID. The stand-in command would then wait on the wrong parent and the test would either hang or pass for the wrong reason. - Maintainability: The current behavior is only correct because of an implicit invariant ("always called from the test process scope") that is not documented and not enforced. Making the dependency explicit removes a footgun.
- Review feedback: Flagged as a non-blocking follow-up in the AI review of PR Stop killing processes by bare PID — in the Windows tests, and in Stop-Slot #124.
How
-
At the top of
ProcessTracking.ps1(in the dot-source-time script scope), capture the test process PID once:$script:TestPid = $PID
Place this outside any function so it runs exactly once when the file is dot-sourced.
-
Change the parameter default in
Get-StandInCommand:function Get-StandInCommand { param([int]$ProcessId = $script:TestPid)
-
Add a short comment above the capture explaining why it exists (so a future reader doesn't "simplify" it back to
$PID):# Captured at dot-source time so Get-StandInCommand's default is the test # process PID, not whatever process happens to be executing the function. $script:TestPid = $PID
-
Verify existing call sites still behave identically — they should, since
$script:TestPidequals$PIDin all current call paths. -
Optionally add a small assertion in
process_tracking_test.ps1thatGet-StandInCommandwith no arguments produces a command referencing$script:TestPid(guards against regressions if the default is ever changed again).
Files Affected
ProcessTracking.ps1process_tracking_test.ps1
Constraints
- Must not change the observable behavior of
Get-StandInCommandfor any existing call site. - Must not change the signature in a way that breaks callers passing
-ProcessIdexplicitly. - Out of scope: the other follow-ups from the same review (mtime-vs-
StartTimerace inRegister-SlotPidFile, per-run timeout inInvoke-OverlappingPoolTests.ps1, draining$script:TrackedProcesses,Endfallback inInvoke-OverlappingPoolTests.ps1). Those should be tracked separately. - Out of scope: refactoring
Get-StandInCommandto take aProcessobject instead of an int PID.
LLM tier
Haiku — this is a mechanical, well-scoped change: add one script-scope variable at dot-source time and swap one parameter default. No design judgment required, and the review text already specifies the exact fix.
Success looks like
ProcessTracking.ps1contains a$script:TestPid = $PIDcapture at dot-source scope with an explanatory comment.Get-StandInCommand's$ProcessIdparameter defaults to$script:TestPid, not$PID.- All existing tests in
process_tracking_test.ps1andwindows_pools_test.ps1still pass unchanged. - A new (or extended) test asserts that calling
Get-StandInCommandwith no arguments uses$script:TestPid. - No call site needs to be modified to keep working.
Failure looks like
- The default is changed to something that no longer matches the
Activity