test(bootstrap): behavioral coverage for sysload-writer and install_launch_agent#322
Conversation
There was a problem hiding this comment.
Prompt: evansenter/dotfiles/.../claude-review.md
Code Review
Summary
Adds four behavioral tests covering sysload-writer (CPU/RAM cache computation and top-failure tolerance) and the real install_launch_agent (__HOME__ substitution and the cmp-based idempotency guard). Verified against the actual sources: the writer 100 - idle to 15% and used/(used+unused) to 87% math is correct, the top || exit 0 failure path matches the no-cache assertion, and the eval-based loader redirects only the BASH_SOURCE-derived dotfiles_dir line (mirroring the existing _load_is_gateway_host pattern) while exercising the genuine sed/cmp/launchctl logic. These replace presence-only grep -q fn_name checks with real behavioral coverage. Lint/syntax look clean and the function-under-test setup faithfully matches bootstrap.sh line 395.
Findings
[Suggestion] tests/test-bootstrap.sh:430-431 — The substring assertions grep -q 15% and grep -q 87% would also pass on a buggy value (15% matches 115%; 87% matches 187%). Since the writer emits a fixed format string, asserting the full expected output (the glyph plus both values together) would both tighten the bound and pin the exact widget output that zjstatus consumes, rather than checking each percentage in isolation. Minor — the mutation testing already gives good confidence.
Verdict
APPROVE - Only one minor suggestion; no Critical or Important issues.
(Note: inline comments could not be posted from this environment — its static-analysis guards reject the JSON payload required by the reviews API, so the single finding is inlined here with its file:line.)
Automated review by Claude Code
…aunch_agent Closes the last open test gap from the new-machine-readiness audit (event-bus events 4152 / 4159): sysload-writer and the LaunchAgent installer had zero tests despite both being prior confirmed-failure surfaces. Adds four behavioral tests (not grep-presence): - sysload-writer parses stubbed `top` output into ~/.cache/sysload with the computed CPU% (100 - idle) and RAM% (used/(used+unused)) - sysload-writer exits 0 and writes no cache when `top` fails (KeepAlive sanity) - install_launch_agent expands __HOME__ in the plist and reloads via launchctl - install_launch_agent is idempotent: an unchanged plist is not reinstalled install_launch_agent derives its dotfiles_dir from BASH_SOURCE, which eval can't satisfy, so the loader redirects that single line at a fixture tree; the substitution / cmp-idempotency / launchctl-reload logic under test is the real bootstrap code. All four mutation-verified (removing each guard flips the test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ent loader 09a288d split `local dotfiles_dir="$(...)"` into declare-then-assign for shellcheck, so the loader's sed redirect stopped matching and the eval'd function fell back to BASH_SOURCE (empty under eval) — the substitute-and- reload test failed post-rebase. The loader now rewrites either declaration style. 39/39 bootstrap tests green on current main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrBLzQixo3zx8wSHVSGUXS
5dc1afd to
e5ede8d
Compare
There was a problem hiding this comment.
Prompt: https://github.com/evansenter/dotfiles/blob/main/home/.claude/contrib/prompts/claude-review.md
Code Review — Summary:
Adds four behavioral tests replacing presence-only grep-q-fn-name checks. Verified each assertion against the actual sources: sysload-writer 100-idle gives 15pct and used/(used+unused) gives 87pct; the top-failure no-cache path (top fails, writer exits 0, writes nothing); and the real install_launch_agent HOME sed plus the cmp-s idempotency guard, via an eval loader that redirects only the BASH_SOURCE-derived dotfiles_dir line (mirroring the existing _load_is_gateway_host pattern). Helper vars resolve and the idempotency test correctly discriminates (empty launchctl.calls, no Installing message). Solid, mutation-verified, well-constructed coverage.
Findings:
[Suggestion] tests/test-bootstrap.sh:430 — The substring assertions grep-q 15pct and grep-q 87pct are looser than the writer fixed-format output warrants (15pct also matches 115pct, 87pct matches 187pct). Since the writer emits one fixed-format printf line, asserting the full expected string in a single grep (both values together, or the whole glyph line) would tighten the bound and pin the exact widget output zjstatus consumes. Minor — the mutation testing already gives good confidence.
Verdict: APPROVE — one minor suggestion, no Critical or Important issues.
Note: inline comments via the reviews API could not be posted from this environment (static-analysis guards reject the JSON payload), so the single finding is inlined above with its file:line.
Automated review by Claude Code
What
Closes the last open test gap from the new-machine-readiness audit (event-bus events 4152 / 4159).
sysload-writerand the LaunchAgent installer had zero tests despite both being prior confirmed-failure surfaces (the sysload zjstatus widget flicker, the launchctl|| truecrash on re-bootstrap).The bus repeatedly flagged that
tests/test-bootstrap.shwas mostlygrep -q 'fn_name'presence checks — proving functions are named, not that they work. These four are behavioral.Tests added
sysload-writer writes cache from top outputtop, asserts~/.cache/sysloadgets computed CPU% (100 - idle) and RAM% (used/(used+unused)) — verified85% idle → 15%,20G/2949M → 87%sysload-writer tolerates top failuretopexits 1 → writer exits 0 and writes no cache (KeepAlive sanity / widget keeps last good value)launch agent substitutes __HOME__ and reloadsinstall_launch_agentexpands__HOME__in the plist, leaves no placeholder, callslaunchctl bootstraplaunch agent idempotent on unchanged plistcmp -sguard)Note on
install_launch_agentIt derives
dotfiles_dirfromBASH_SOURCE, whichevalcan't satisfy — so the loader redirects that one line at a fixture tree (same pattern as the existing_load_is_gateway_hosthelper). Everything actually under test — the__HOME__sed, thecmp-based idempotency guard, the launchctl reload — is the genuine bootstrap code.Verification
make checkgreen (lint, 39 bootstrap tests, hooks). All four mutation-verified: removing the__HOME__sed, thecmpguard, or the writer's failure guards each flips the corresponding test red; the broken-writer assertion was confirmed to discriminate against the real writer.🤖 Generated with Claude Code