Add a test suite; the repo had zero tests - #97
Merged
Merged
Conversation
John asked for American English throughout the project, including code comments and not just user-facing docs. - zsh/exports.zsh: "256 colours" -> "256 colors". Deliberately not touched, because neither is prose: - dot_tmux.conf, which spells it `colour235` in two commented-out lines. That is tmux's own option syntax, not our text; "correcting" it would break the config if those lines were ever uncommented. - LICENSE.md, which is verbatim GPL text. The only other pre-existing instance is a one-word comment fix in bin/secret, which lands with the test suite in the following commit because it sits inside that file's other changes.
CI proved that shell parses (zsh -n, shellcheck, py_compile) and that chezmoi apply is idempotent. Nothing anywhere asserted that a single line of this code behaves correctly. 112 tests now do: 69 bats + 43 stdlib unittest, run by ./tests/run and gated by a new matrixed CI job. Coverage is deliberately uneven. The git-* helpers are 1-13 line wrappers around one git command each; testing them would only restate them, so they have none. Effort went where a bug is invisible or dangerous: - bin/secret's shell_quote. Its output is eval'd by *every* interactive shell via `eval "$(secret env)"` in ~/.localrc, so a quoting bug there is code execution at shell startup, not a formatting nit. Thirteen adversarial values (embedded quote, $HOME, backtick, $(id), backslash, glob, `;rm -rf /`, a lone quote, an embedded newline) are round-tripped through eval in bash AND zsh and compared byte for byte. The zsh leg matters: the file rejects printf %q precisely because $'...' is not portable, and only a second-shell test defends that reasoning. - bin/herdr-reload-all's agent-pane exclusion. Reloading an agent pane types "reload" into a running agent's prompt; the select(.agent == null) filter that prevents it is now asserted by pane id, not assumed. - The chezmoi template traps CLAUDE.md documents, several of which already bit once: a bare `#` swallowing the next key in .chezmoi.yaml.tmpl, and dot_gitconfig.tmpl needing `get . "work"` rather than `.work` (a machine initialized before the key existed has no `work` entry, and `.work` against such a map is a hard error, not a falsy value -- it would break git config everywhere until someone re-ran chezmoi init). - Cross-file invariants no linter can see: .chezmoiignore covering every topic directory, the zsh cache path matching in dot_zshrc and zsh/completion.zsh, HOMEBREW_CASK_OPTS identical in the apply script and homebrew/exports.zsh, tmux<->herdr keybinding parity. The suite found a real bug. bin/herdr-reload-all's `|| true` guards only the first jq call, so plain-text output from `herdr pane list` sailed past it as an empty error code and then died in the reload pipeline with a raw "jq: parse error" and exit 5 via pipefail -- exactly the leaked-internals failure the protocol_mismatch branch exists to prevent. Fixed by validating the envelope up front, plus `// []` so a result with no panes key is a quiet no-op rather than "cannot iterate over null". Choices worth keeping: - bats-core is pinned to an exact release tarball with its sha256 verified, for the same reason shellcheck is: Ubuntu's apt bats trails Homebrew's by several minors and they disagree about which helpers exist, so an unpinned gate passes on one runner and fails on the other for identical code. bump-deps tracks the tag as a VERSION pin so it cannot rot; the checksum sits beside it in ci.yaml and is checked at install time, so a bumped tag with a stale hash fails loudly rather than silently skipping verification. - Python tests are stdlib unittest, keeping bump-deps' documented "no pip dependencies" promise and adding no install step to CI. - `security` and `herdr` are stubbed; jq is not. security is macOS-only, so without a stub none of bin/secret could be tested on the Ubuntu runner, and a test suite must not be able to reach a real login keychain even by accident ($SECRET_ACCOUNT/$SECRET_ENV_NAME are set to throwaway values as a second line of defense). herdr drives live panes. But herdr-reload-all's behavior largely *is* its jq filters, so stubbing jq would test nothing. - test_bump_deps.py runs against a fixture repo rather than the real tree, which is what makes check_invariants()'s violation branches reachable at all -- bump-deps --self-test cannot reach them without corrupting the actual files. Registering a new pin therefore means updating tests/python/helper.py too; that coupling is intentional. - tests/: new. run, README.md, four bats suites, three bats helpers, and the Python suite plus its fixture-repo helper. - .chezmoiignore: ignore tests/, or apply would copy the suite into $HOME. - bin/secret, bin/herdr-reload-all: sourced-guards so tests can call individual functions. if/then rather than `[ ... ] && main "$@"`, because that form leaves a non-zero status when sourced and set -e would kill the caller. herdr-reload-all's dependency checks moved inside main() for the same reason: at top level they abort the mere act of sourcing when herdr is absent, which is the normal case in CI. Also the envelope fix above, and one comment respelled to American English. - bin/bump-deps: register BATS_VERSION as a VERSION pin. - ci.yaml: new matrixed `test` job. Also asserts nothing *skipped* -- a missing jq/zsh/chezmoi makes tests skip rather than fail, which is deliberate so the suite runs on a half-configured laptop, but on a runner a skip means the job silently tested less than it claims. tests/run added to the shellcheck sweep. - homebrew template: bats-core, so local runs match CI. - CLAUDE.md: a Tests section and a CI table row. Verified: 112 tests pass; shellcheck, zsh -n, py_compile, bump-deps --self-test and `chezmoi status --exclude=scripts` all still clean, and tests/ is unmanaged. A test that cannot fail is worse than no test, so each was checked against a deliberate breakage: neutering shell_quote's sed, removing the agent filter, raising .chezmoiversion above install.sh's, dropping bin/ from .chezmoiignore, drifting HOMEBREW_CASK_OPTS, swapping `get . "work"` for `.work`, making env-import print values, and making bump-deps' version compare lexical. All nine turn the suite red. The ninth initially survived -- dropping the `, 1` bound in replace_value is unobservable with the current pin set, since no pattern's match contains its value twice -- so a test with a synthetic pin was added to make that bound observable rather than leave it unspecified.
The "every chezmoiscripts template renders" test rendered with no --config, so it picked up whatever ~/.config/chezmoi/chezmoi.yaml the machine happened to have. That passed on a configured laptop and failed on both CI runners: executing "stdin" at <.set_git_to_ssh>: map has no entry for key run_once_after_110_fix_git_upstream.sh.tmpl and the linux installs template read .set_git_to_ssh and .install_linux_apps out of the data map, and a fresh runner has installed chezmoi but never run `chezmoi init`, so that map is empty. Render against the config .chezmoi.yaml.tmpl generates instead, which is what the other tests in this file already do. Using the repo's own defaults rather than hardcoded values keeps the test honest about what a real init produces. Verified by running the suite with HOME and XDG_CONFIG_HOME pointed at an empty directory -- a far better CI proxy than a configured machine, and the check that should have caught this before it was pushed. 112 tests pass there.
The "assert nothing was skipped" guard did its job on its first real outing. Ubuntu runners ship neither jq nor zsh, and the suite skips rather than fails when a dependency is absent -- deliberate, so it stays runnable on a half-configured laptop. On a runner that meant ok 31 shell_quote: ... round-trips through zsh eval # skip zsh not installed which is the single most valuable test in the suite quietly opting out. The bash leg does not prove the zsh leg: the bundle is eval'd by zsh in real use, and bin/secret hand-rolls its quoting precisely because printf %q emits $'...' that is not portable across shells. Without zsh, that reasoning is undefended on half the matrix. Install both tools in one step, matching how the lint job installs zsh, and fail loudly rather than silently continuing if a non-Linux runner is ever missing one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI proved that shell parses (
zsh -n,shellcheck,py_compile) and thatchezmoi applyis idempotent. Nothing asserted that a single line of this code behaves correctly. 112 tests now do — 69 bats + 43 stdlibunittest, run by./tests/runand gated by a new matrixed CI job.Coverage is deliberately uneven
The
git-*helpers are 1–13 line wrappers around one git command each; testing them would only restate them, so they have none. Effort went where a bug is invisible or dangerous:secret.batsshell_quote's output iseval'd by every interactive shell viaeval "$(secret env)"in~/.localrc. A quoting bug there is code execution at shell startup, not a formatting nit. 13 adversarial values round-tripped throughevalin bash and zsh, compared byte for byte.herdr_reload_all.batsreloadinto a running agent's prompt. Theselect(.agent == null)filter that prevents it is asserted by pane id.templates.batsinvariants.batstest_bump_deps.pycheck_invariants()'s violation branches reachable —--self-testcannot reach them without corrupting the real files.The zsh leg of the quoting test matters:
bin/secretrejectsprintf %qprecisely because$'...'is not portable, and only a second-shell test defends that reasoning.The suite found a real bug
bin/herdr-reload-all's|| trueguards only the firstjqcall. Plain-text output fromherdr pane listsailed past it as an empty error code, then died in the reload pipeline with a rawjq: parse errorand exit 5 viapipefail— exactly the leaked-internals failure theprotocol_mismatchbranch exists to prevent. Fixed by validating the envelope up front, plus// []so a result with nopaneskey is a quiet no-op rather than "cannot iterate over null".Choices worth reviewing
shellcheckis: Ubuntu's aptbatstrails Homebrew's by several minors and they disagree about which helpers exist.bump-depstracks the tag as aVERSIONpin so it can't rot; the checksum sits beside it inci.yamland is checked at install time, so a bumped tag with a stale hash fails loudly rather than silently skipping verification.unittest— keepsbump-deps' documented "no pip dependencies" promise and adds no install step.securityandherdrare stubbed;jqis not.securityis macOS-only, so without a stub none ofbin/secretcould be tested on Ubuntu — and a test suite must not reach a real login keychain even by accident ($SECRET_ACCOUNT/$SECRET_ENV_NAMEare throwaway values as a second line of defense). Butherdr-reload-all's behavior largely is its jq filters, so stubbing jq would test nothing.if/then, not[ ... ] && main "$@"— the latter leaves a non-zero status when sourced andset -ekills the caller.herdr-reload-all's dependency checks moved insidemain()for the same reason.jq/zsh/chezmoimakes tests skip rather than fail — deliberate, so the suite runs on a half-configured laptop — but on a runner a skip means the job silently tested less than it claims.Three of my planned assertions were wrong
Worth flagging since they came out of reading CLAUDE.md:
verboseis deliberatelyfalse, and the rendered config has legitimate#comments. The real guard is anchored^key:matching plus a check that no comment line swallowed a key.zsh/exports.zsh:27does exportTERM— inside a guard that fires only whenzsh/terminforeports zero capabilities. Sanctioned; the test anchors at column zero to catch only an unguarded export. CLAUDE.md's flat "TERM is deliberately not set anywhere" is now slightly stale — left alone rather than edited unasked.# tmux:comments in the herdr config include prose lines, so parsing them yields false failures. The parity test uses a curated table instead.Verification
112 tests pass.
shellcheck,zsh -n,py_compile,bump-deps --self-testandchezmoi status --exclude=scriptsall still clean, andtests/is unmanaged.A test that cannot fail is worse than no test, so each was checked against a deliberate breakage — neutering
shell_quote's sed, removing the agent filter, raising.chezmoiversionaboveinstall.sh's, droppingbin/from.chezmoiignore, driftingHOMEBREW_CASK_OPTS, swappingget . "work"for.work, makingenv-importprint values, making the version compare lexical. All nine turn the suite red. The ninth initially survived: dropping the, 1bound inreplace_valueis unobservable with the current pin set, since no pattern's match contains its value twice — so a test with a synthetic pin was added to make that bound observable rather than leave it unspecified.The first commit is a small unrelated spelling fix (
colours→colorsin a comment).dot_tmux.conf'scolour235is tmux's own option syntax andLICENSE.mdis verbatim GPL, so neither was touched.What CI caught that local runs did not
Two follow-up commits, both worth reading as evidence the gating works:
b978334— the "every chezmoiscripts template renders" test rendered with no--config, so it silently used whatever~/.config/chezmoi/chezmoi.yamlmy machine had. A fresh runner has installed chezmoi but never runchezmoi init, so.set_git_to_sshand.install_linux_appswere missing and two templates failed to render. Now rendered against the config.chezmoi.yaml.tmplitself generates. Re-verified locally withHOME/XDG_CONFIG_HOMEpointed at an empty directory — a far better CI proxy than a configured laptop, and the check that should have caught this before the first push.796e395— the "assert nothing was skipped" guard fired on its first real outing:Ubuntu runners ship neither
jqnorzsh. Without that guard the job would have reported green while the single most valuable test in the suite quietly opted out — and the bash leg does not prove the zsh leg, which is the entire reasonbin/secrethand-rolls its quoting instead of usingprintf %q. Both tools are now installed in the job.All 9 checks pass on the final commit (
796e395):lint,test,applyandbootstrapacross macOS + Ubuntu, plussecrets.