Context
Follow-up from review of PR #774.
internal/execution/process_unix_test.go has robustness gaps that could let regressions in ps-based zombie detection go unnoticed:
-
Three tests (TestTerminateProcessTreeTreatsZombieGroupAsExited, TestTerminateProcessTreeStopsRunningGroup's setup implicitly, TestSignalTargetRunningTreatsZombieIndividualPIDAsExited) use t.Skip when ps is unavailable or the process doesn't reach the zombie state in time. ps is present on CI images today, so this doesn't bite currently, but the tests can go quietly green (as skips) if ps parsing or availability ever regresses in CI.
-
TestTerminateProcessTreeStopsRunningGroup's final assertion:
if running, ok := processGroupHasRunningMember(pid); ok && running {
t.Fatal("the group still has a running member after termination")
}
passes vacuously when ok is false (i.e., when ps state couldn't be determined at all) — the same "silently degrades" failure mode as the skips above.
Proposed fix
- Consider failing loudly (rather than skipping) when
ps is expected to be available (e.g., detect a hard CI environment variable, or simply let these fail instead of skip so a broken ps pipeline surfaces immediately instead of showing as skipped-green).
- Change
TestTerminateProcessTreeStopsRunningGroup's final assertion to t.Fatal on !ok too, so an undetectable state is treated as a failure rather than silently passing.
Not blocking; ps is available on all CI images used today.
Context
Follow-up from review of PR #774.
internal/execution/process_unix_test.gohas robustness gaps that could let regressions inps-based zombie detection go unnoticed:Three tests (
TestTerminateProcessTreeTreatsZombieGroupAsExited,TestTerminateProcessTreeStopsRunningGroup's setup implicitly,TestSignalTargetRunningTreatsZombieIndividualPIDAsExited) uset.Skipwhenpsis unavailable or the process doesn't reach the zombie state in time.psis present on CI images today, so this doesn't bite currently, but the tests can go quietly green (as skips) ifpsparsing or availability ever regresses in CI.TestTerminateProcessTreeStopsRunningGroup's final assertion:passes vacuously when
okisfalse(i.e., whenpsstate couldn't be determined at all) — the same "silently degrades" failure mode as the skips above.Proposed fix
psis expected to be available (e.g., detect a hard CI environment variable, or simply let these fail instead of skip so a brokenpspipeline surfaces immediately instead of showing as skipped-green).TestTerminateProcessTreeStopsRunningGroup's final assertion tot.Fatalon!oktoo, so an undetectable state is treated as a failure rather than silently passing.Not blocking;
psis available on all CI images used today.