Skip to content

test(gauntlet): pin stray-procs' set -e load path and its real ps call - #666

Merged
wasikarn merged 4 commits into
developfrom
claude/stray-procs-test-gaps
Oct 26, 2026
Merged

wasikarn merged 4 commits into
developfrom
claude/stray-procs-test-gaps

Conversation

@wasikarn

@wasikarn wasikarn commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Closes the two test gaps that the third fresh-context review of #632 left open. This PR is test-only: scripts/_lib/stray-procs.sh doesn't change. Refs #158.

Why

  • set -e load path: stray_procs_report promises it never returns non-zero. But the only set -e row covered the ps-failure path, so dropping || load="" left 21/21 green. A bash -e caller that found strays while the load reader failed would then have exited 1.
  • Real ps call: every row stubbed the private _sp_ps/_sp_load, so the real ps -U <uid> -o pid=,etime=,%cpu=,command= never ran. Changing etime= to etimes=, which macOS ps doesn't have, still passed.

What

tests/scripts/test-gauntlet-stray-procs.sh gets 4 more rows (21 to 25):

  • A bash -ec caller with strays and a failing _sp_load reaches the next command.
  • PATH shims, with the real readers running:
    • ps shim: behaves like macOS ps wherever the report depends on it. An unknown -o keyword is an error, and columns come out in the order -o asks for. With no -o list, it prints the default PID TTY TIME CMD columns under a header. It lists rows only for -U <your uid>. Checks that the real call feeds the report.
    • uptime shim: prints the macOS format (load averages: 3.21 2.00 1.00) and the Linux format (load average: 4.56, 2.00, 1.00). Checks that both parse.
  • The shim dir comes from tests/_lib/harness.sh (fresh_tmpdir, with _cleanup_trash on EXIT).

No version bump; merge-pr.sh does that at merge time.

Verified

  • 25/25 pass, and shellcheck (warning level) is clean.

  • Mutations of stray-procs.sh: each of these 7 makes the suite fail. The last two came from the first review of this PR, and the lib was restored after each (git diff empty):

    Mutation Failing rows
    drop || load="" 1
    etime= → etimes= 3
    ps -U "$(id -u)" → ps -A 3
    load pattern made macOS-only 1
    load split on spaces only 1
    -o columns reordered 1 for etime=,pid=,…; 3 for orders that move %cpu or command, such as pid=,%cpu=,etime=,command=
    -o list left out 3
  • audit.sh exits 0, and the pre-push gauntlet passed all layers on cce630fd.

Two gaps from PR #632's third review:
- A set -e caller that finds strays while the load reader fails must
  still continue; dropping '|| load=""' now fails a row.
- The real _sp_ps/_sp_load run against PATH shims: a ps that rejects
  unknown -o keywords (macOS has no etimes) and lists rows only for
  -U <uid>, and uptime in macOS and Linux formats. Mutating etime= to
  etimes=, -U to -A, or either load parse step fails a row.
@wasikarn

Copy link
Copy Markdown
Owner Author

Fresh-context review: FIX FIRST @ cce630f

Two-axis review (mattpocock-skills:code-review). A separate subagent ran each axis against merge-base a14ec7e.

Spec (MISSING=0 CREEP=0 WRONG=1)

  • Both gaps are closed, confirmed by mutation:
    • dropping || load="" fails 1 row
    • etimes= fails 3
    • renaming _sp_ps breaks 11 stub rows, while all 3 shim rows stay green, so they survive a helper rename.
  • Wrong: the PR body says the ps shim "behaves like macOS ps". But it only checks the column names. Deleting -o … or reordering the columns stays 25/25 green.

Standards (HARD=1 JUDGEMENT=3)

  • Hard (test-honesty pr-test-analyzer: add 'weakened-to-pass / gate-gaming' critical-gap class #5): the "real ps call" row's name claims more than it checks. Swapping to -o etime=,pid=,… stays green, though on a real machine awk would then read the wrong fields.
  • Judgement:
    • The new inline bash -c cases repeat a shape that run() can't take. Keeping them: a helper adds more than it saves.
    • harness.sh is sourced mid-file rather than at the top. Will fix.
    • SHIM=$(fresh_tmpdir) has no guard. Will fix.

Fix: the shim will print columns in the requested order, and without -o it prints macOS's default columns. Reordering or dropping -o then breaks the report the way it would on a real machine.

Review of cce630f (FIX FIRST): the shim checked -o keywords but always
printed pid etime %cpu command, so reordering the columns or leaving out
the -o list in stray-procs.sh stayed green though awk would then read the
wrong fields on a real machine. The shim now emits columns in the
requested order, and with no -o list prints the macOS default PID TTY TIME
CMD columns under a header; both mutations fail a row. harness.sh is
sourced at the top, house style, and an empty shim dir stops the test.
@wasikarn

Copy link
Copy Markdown
Owner Author

Fresh-context review: MERGE @ 5b80d35

Second two-axis review (mattpocock-skills:code-review). New subagents ran it against merge-base a14ec7e.

Spec (MISSING=0 CREEP=0 WRONG=1). Both gaps are closed, confirmed by mutation on a git archive copy:

  • dropping || load="" fails 1 row
  • etimes=, ps -A and leaving out the -o list each fail 3
  • every -o reorder tried fails 1 to 3
  • renaming _sp_ps breaks the 11 stub rows while all 3 shim rows stay green

The one "wrong" item was the PR table's reorder count, which depends on the order chosen. It's now corrected in the body.

Standards (HARD=0 JUDGEMENT=4). Clean on:

  • the harness contract (source plus its own EXIT trap, a guarded shim dir)
  • no rm or direct trash
  • no home paths or bare grep
  • mocks at the process boundary, not on the unit

Judgement calls, none blocking:

  • the uptime rows fail alongside a broken ps, since the report returns early with no strays
  • the unquoted shim heredoc needs many \$ escapes
  • the shim run line is repeated twice
  • the header says "last rows", but the wiring row comes after them

Verdict: MERGE. No hard violation, and no spec gap in the code.

@wasikarn
wasikarn merged commit 67e1b1b into develop Oct 26, 2026
@wasikarn
wasikarn deleted the claude/stray-procs-test-gaps branch October 26, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant