test: keep the hook spawn-cost measurement, gated off by default - #6
Conversation
The budgets in internal/hooks were set from a measurement rather than a guess, and the instrument that produced it was thrown away with the throwaway branch. This keeps it, so the next person to suspect the hook path can re-measure instead of arguing. TestHookSpawnCost breaks a Windows hook launch into tiers that each add one thing to the one above, so the difference between two rows is the cost of that thing: a bare CreateProcess says whether the machine is contended, PowerShell adds the interpreter, stdin and the hook script add this package's plumbing, the tree-cancel wiring and Runner add what internal/hooks costs, and pwsh sits beside powershell because "use the other one" is always the first suggestion and the numbers say it is slower. The September 2026 reference reading is in the doc comment. It is skipped unless PACKETCODE_HOOK_TIMING=1: it takes about fifteen seconds and asserts nothing. That variable also stands TestMain's warm-up down. The warm-up exists to keep PowerShell's first-start cost out of the other tests' budgets, and that cost is the most useful number this measurement reports -- warming first would turn it into a second reading of the warm case, an instrument that always agrees with itself. The other tests in a measurement run then pay the cold start themselves, which their scaled budgets already cover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 57e6d66dc9
ℹ️ 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".
| // here so the next person to suspect the hook path can measure it rather than | ||
| // guess, which is how the budgets in this package were set. | ||
| // | ||
| // PACKETCODE_HOOK_TIMING=1 go test -run TestHookSpawnCost -v ./internal/hooks/ |
There was a problem hiding this comment.
Add -count=1 to this invocation; otherwise, after the first successful measurement with the same environment, subsequent runs can replay stale timing output rather than measure the current machine. go help test confirms that package-list-mode results using only cacheable flags such as -run and -v are cached and says the idiomatic way to disable caching is -count=1, which is essential for a diagnostic intended to compare repeated readings under changing load.
Useful? React with 👍 / 👎.
| go func(i int) { | ||
| defer wg.Done() | ||
| s := time.Now() | ||
| _ = raw("powershell", ps(hookScript), payload)() |
There was a problem hiding this comment.
When any of the eight concurrent PowerShell processes fails to start or exits nonzero—exactly the kind of resource-pressure failure this tier may encounter—the error is discarded and its elapsed time is still presented as a valid spawn duration. Unlike report, which at least logs invocation errors, this row can therefore produce deceptively fast or otherwise invalid measurements without warning; collect and log the per-spawn errors before reporting the timings.
Useful? React with 👍 / 👎.
1 participant
Follow-up to #3. The budgets in
internal/hookswere set from a measurement, but the instrument that produced it went away with the throwaway diagnostic branch. This keeps just that instrument — not the 120s diagnostic budgets or the temporary CI steps from the closed #2.What it is
TestHookSpawnCostbreaks a Windows hook launch into tiers, each adding exactly one thing to the one above, so the difference between two rows is the cost of that thing:cmd.exeCreateProcesspowershell, no stdinpowershell+ stdin, then + the hook scriptRunnerinternal/hookscostspwshbesidepowershellPlus the same under CPU load and 8 concurrent spawns, since the suite never runs on an idle machine.
The September 2026 reference reading is recorded in the doc comment, so a future run has something to be compared against.
Gating
Skipped unless
PACKETCODE_HOOK_TIMING=1— it takes ~15s and asserts nothing.PACKETCODE_HOOK_TIMING=1 go test -run TestHookSpawnCost -v ./internal/hooks/That variable also stands
TestMain's warm-up down. The warm-up exists to keep PowerShell's first-start cost out of the other tests' budgets, and that cost is the single most useful number this measurement reports — warming first would turn it into a second reading of the warm case, an instrument that always agrees with itself. Tests in a measurement run then pay the cold start themselves, which their scaled budgets already cover.Checks
go test ./...green locally.go vetandgolangci-lint(--max-issues-per-linter=0 --max-same-issues=0) underGOOS=linux,darwinandwindows: 0 issues ininternal/hooks. The new file is//go:build windows, so theGOOS=windowspass is the one that actually reads it.internal/hooksstill 3.3s.🤖 Generated with Claude Code