diff options
Diffstat (limited to 'languages/elisp')
| -rwxr-xr-x | languages/elisp/claude/hooks/validate-el.sh | 7 | ||||
| -rw-r--r-- | languages/elisp/claude/rules/elisp-testing.md | 2 | ||||
| -rw-r--r-- | languages/elisp/claude/scripts/coverage-summary.el | 21 | ||||
| -rwxr-xr-x | languages/elisp/githooks/pre-commit | 41 | ||||
| -rw-r--r-- | languages/elisp/tests/test-coverage-summary.el | 45 | ||||
| -rw-r--r-- | languages/elisp/tests/test-pre-commit-hook.bats | 126 | ||||
| -rw-r--r-- | languages/elisp/tests/test-validate-el-hook.bats | 100 |
7 files changed, 328 insertions, 14 deletions
diff --git a/languages/elisp/claude/hooks/validate-el.sh b/languages/elisp/claude/hooks/validate-el.sh index 2529fcc..870eefe 100755 --- a/languages/elisp/claude/hooks/validate-el.sh +++ b/languages/elisp/claude/hooks/validate-el.sh @@ -39,8 +39,6 @@ f="$(jq -r '.tool_input.file_path // .tool_response.filePath // empty')" [ -z "$f" ] && exit 0 [ "${f##*.}" = "el" ] || exit 0 -MAX_AUTO_TEST_FILES=20 # skip if more matches than this (large test suites) - # --- Phase 1: syntax + byte-compile --- case "$f" in */init.el|*/early-init.el) @@ -55,6 +53,7 @@ case "$f" in # under a tests/ subdir) so cross-project edits compile against their # own modules, not just this project's. if ! output="$(emacs --batch --no-site-file --no-site-lisp \ + --eval '(setq load-prefer-newer t)' \ -L "$(dirname "$f")" \ -L "$(dirname "$f")/.." \ -L "$PROJECT_ROOT" \ @@ -95,15 +94,17 @@ case "$f" in esac count="${#tests[@]}" -if [ "$count" -ge 1 ] && [ "$count" -le "$MAX_AUTO_TEST_FILES" ]; then +if [ "$count" -ge 1 ]; then load_args=() for t in "${tests[@]}"; do load_args+=("-l" "$t"); done if ! output="$(emacs --batch --no-site-file --no-site-lisp \ + --eval '(setq load-prefer-newer t)' \ -L "$PROJECT_ROOT" \ -L "$PROJECT_ROOT/modules" \ -L "$PROJECT_ROOT/tests" \ -L "$PROJECT_ROOT/themes" \ --eval '(package-initialize)' \ + --eval "(cd \"$PROJECT_ROOT/tests\")" \ -l ert "${load_args[@]}" \ --eval "(ert-run-tests-batch-and-exit '(not (tag :slow)))" 2>&1)"; then # Terminal gets a compact summary (the run tally + the failing test names); diff --git a/languages/elisp/claude/rules/elisp-testing.md b/languages/elisp/claude/rules/elisp-testing.md index 7c3a9ef..1ac76a0 100644 --- a/languages/elisp/claude/rules/elisp-testing.md +++ b/languages/elisp/claude/rules/elisp-testing.md @@ -43,6 +43,8 @@ The bundle ships a coverage summary at `.claude/scripts/coverage-summary.el` and The number to watch is the missing-file count. A module no test loads never appears in the SimpleCov report, so a line-weighted total skips it silently — the suite looks healthier than it is. The summary counts every `modules/*.el` on disk that's absent from the report as 0%, so an untested module drags the project number down where you can see it. Copy the fragment's targets into your own Makefile to adopt it; the bundle never edits your Makefile. +This is a local-only helper by design. `.claude/scripts/` is gitignored in code projects, so `coverage-summary.el` is untracked and CI never runs `make coverage-summary` against it — it's a developer-run check, not a CI gate. A gitignored install is intentional, not a coverage gap; don't move the script to a tracked `scripts/` dir to make CI pick it up. + ## TDD Workflow Write the failing test first. A failing test proves you understand the change. Assume the bug is in production code until the test proves otherwise — never fix the test before proving the test is wrong. diff --git a/languages/elisp/claude/scripts/coverage-summary.el b/languages/elisp/claude/scripts/coverage-summary.el index eb30c66..ed7ecfc 100644 --- a/languages/elisp/claude/scripts/coverage-summary.el +++ b/languages/elisp/claude/scripts/coverage-summary.el @@ -11,8 +11,15 @@ ;; by file rather than by line, so untested modules are visible. ;; ;; Self-contained on purpose — it ships into a project's =.claude/scripts/= and -;; must run with nothing but stock Emacs (`json' is built in). The SimpleCov -;; JSON shape it parses is: +;; must run with nothing but stock Emacs (`json' is built in). +;; +;; Local-only helper (Craig, 2026-06-28). =.claude/scripts/= is gitignored in +;; code projects, so this file is not tracked and CI cannot run +;; `make coverage-summary' against it. That is intentional: it stays a +;; developer-run helper, not shipped to a tracked =scripts/= dir and not a CI +;; gate. A gitignored install here is the design, not a coverage gap. +;; +;; The SimpleCov JSON shape it parses is: ;; { <suite>: { "coverage": { <abs-path>: [null | 0 | int, ...] } } } ;; where a null entry is a non-executable line, 0 is executable-but-unhit, and ;; any positive integer is a hit. Data unions across multiple suite keys. @@ -91,11 +98,15 @@ missing or malformed." (defun cj/coverage-summary--source-files (source-dir project-root) "Return *.el files directly under SOURCE-DIR, relative to PROJECT-ROOT. -Sorted; compiled files and subdirectories are out of scope." +Sorted. Compiled files and subdirectories are out of scope, as are generated +package files (`*-autoloads.el', `*-pkg.el') -- a build tool writes those, no +test covers them, and counting them as untested source skews the number." (let ((source-dir (file-name-as-directory (expand-file-name source-dir))) (project-root (file-name-as-directory (expand-file-name project-root)))) - (sort (mapcar (lambda (p) (file-relative-name p project-root)) - (directory-files source-dir t "\\.el\\'")) + (sort (seq-remove + (lambda (p) (string-match-p "\\(?:-autoloads\\|-pkg\\)\\.el\\'" p)) + (mapcar (lambda (p) (file-relative-name p project-root)) + (directory-files source-dir t "\\.el\\'"))) #'string<))) (defun cj/coverage-summary--missing (tracked source-dir project-root) diff --git a/languages/elisp/githooks/pre-commit b/languages/elisp/githooks/pre-commit index 909cde2..a87bedf 100755 --- a/languages/elisp/githooks/pre-commit +++ b/languages/elisp/githooks/pre-commit @@ -5,15 +5,37 @@ set -u REPO_ROOT="$(git rev-parse --show-toplevel)" -cd "$REPO_ROOT" +cd "$REPO_ROOT" || exit 1 # --- 1. Secret scan --- # Patterns for common credentials. Scans only added lines in the staged diff. -SECRET_PATTERNS='(AKIA[0-9A-Z]{16}|sk-[a-zA-Z0-9_-]{20,}|-----BEGIN (RSA|DSA|EC|OPENSSH|PGP)( PRIVATE)?( KEY| KEY BLOCK)?-----|(api[_-]?key|api[_-]?secret|auth[_-]?token|secret[_-]?key|bearer[_-]?token|access[_-]?token|password)[[:space:]]*[:=][[:space:]]*["'"'"'][^"'"'"']{16,}["'"'"'])' +# +# Two passes because case-sensitivity differs. AWS keys are uppercase, sk- keys +# lowercase, PEM headers fixed, so those match case-SENSITIVELY: under -i, +# AKIA[0-9A-Z]{16} matches any mixed-case 20-char run, which random base64 in an +# embedded image blob hits ~6% of the time per 100KB and blocks real commits. +# Only the keyword=value patterns need -i. +SECRET_PATTERNS_CS='(AKIA[0-9A-Z]{16}|sk-[a-zA-Z0-9_-]{20,}|-----BEGIN (RSA|DSA|EC|OPENSSH|PGP)( PRIVATE)?( KEY| KEY BLOCK)?-----)' +SECRET_PATTERNS_CI='(api[_-]?key|api[_-]?secret|auth[_-]?token|secret[_-]?key|bearer[_-]?token|access[_-]?token|password)[[:space:]]*[:=][[:space:]]*["'"'"'][^"'"'"']{16,}["'"'"']' -secret_hits="$(git diff --cached -U0 --diff-filter=AM \ - | grep '^+' | grep -v '^+++' \ - | grep -iEn "$SECRET_PATTERNS" || true)" +# Read the diff on its own so a git failure is distinguishable from "grep +# matched nothing". Both end in a non-zero status, but only one of them means +# there is nothing to scan; piping them together and swallowing the result with +# `|| true` made a broken git look like a clean commit — the scan searched an +# empty string, found nothing, and the secret went in. +if ! staged_diff="$(git diff --cached -U0 --diff-filter=AM)"; then + echo "pre-commit: cannot read the staged diff — refusing to skip the secret scan" >&2 + exit 1 +fi + +# The greps keep their `|| true`: exiting 1 on no match is their normal result. +added_lines="$(printf '%s\n' "$staged_diff" | grep '^+' | grep -v '^+++' || true)" + +cs_hits="$(printf '%s\n' "$added_lines" | grep -nE "$SECRET_PATTERNS_CS" || true)" +ci_hits="$(printf '%s\n' "$added_lines" | grep -niE "$SECRET_PATTERNS_CI" || true)" +# awk dedupes lines both passes matched, keeping first-seen order. +secret_hits="$(printf '%s\n%s' "$cs_hits" "$ci_hits" \ + | grep -v '^[[:space:]]*$' | awk '!seen[$0]++' || true)" if [ -n "$secret_hits" ]; then echo "pre-commit: potential secret in staged changes:" >&2 @@ -25,7 +47,14 @@ if [ -n "$secret_hits" ]; then fi # --- 2. Paren check on staged .el files --- -staged_el="$(git diff --cached --name-only --diff-filter=AM | grep '\.el$' || true)" +# Same split as the secret scan above: a git failure must not read as "no files +# staged", which would skip the language check silently. +if ! staged_names="$(git diff --cached --name-only --diff-filter=AM)"; then + echo "pre-commit: cannot read the staged file list — refusing to skip the check" >&2 + exit 1 +fi + +staged_el="$(printf '%s\n' "$staged_names" | grep '\.el$' || true)" if [ -n "$staged_el" ]; then paren_fail="" diff --git a/languages/elisp/tests/test-coverage-summary.el b/languages/elisp/tests/test-coverage-summary.el index 5be03b3..a4525db 100644 --- a/languages/elisp/tests/test-coverage-summary.el +++ b/languages/elisp/tests/test-coverage-summary.el @@ -109,6 +109,33 @@ is a JSON array string like \"[1, 0, null]\"." (ert-deftest cs-file-pct-fully-covered () (should (= 100.0 (cj/coverage-summary--file-pct 4 4)))) +;; --- source-file scan and under-dir filtering ------------------------------ + +(ert-deftest cs-source-files-is-non-recursive () + "Only top-level *.el under SOURCE-DIR are source; files in subdirectories +are out of scope." + (cs-test--with-project + (list :sources '(("top.el" . ";; t") ("sub/nested.el" . ";; n")) + :report (cs-test--report '(("top.el" . "[1]")))) + (let ((sources (mapcar #'file-name-nondirectory + (cj/coverage-summary--source-files src root)))) + (should (member "top.el" sources)) + (should-not (member "nested.el" sources))))) + +(ert-deftest cs-under-dir-filters-outside-source-and-rekeys () + "Report entries outside SOURCE-DIR are dropped; survivors are keyed +relative to PROJECT-ROOT." + (cs-test--with-project + (list :sources '(("in.el" . ";; i")) + :report (cs-test--report '(("in.el" . "[1, 1]") + ("../out.el" . "[1, 0]")))) + (let* ((table (cj/coverage-summary--under-dir + (cj/coverage-summary--parse-file report) src root)) + (keys (let (ks) (maphash (lambda (k _v) (push k ks)) table) ks))) + (should (equal keys (list (file-relative-name + (expand-file-name "src/in.el" root) root)))) + (should (= 1 (hash-table-count table)))))) + ;; --- missing-file detection (the kernel) ----------------------------------- (ert-deftest cs-missing-finds-ondisk-file-absent-from-report () @@ -135,6 +162,24 @@ is a JSON array string like \"[1, 0, null]\"." (missing (cj/coverage-summary--missing tracked src root))) (should (null missing))))) +(ert-deftest cs-missing-excludes-generated-package-files () + "Generated -autoloads.el / -pkg.el are not source, so a build tool writing +them does not drag the number down; a genuinely untested source is still +flagged (the filter is not over-broad)." + (cs-test--with-project + (list :sources '(("real.el" . ";; r") ("untested.el" . ";; u") + ("proj-autoloads.el" . ";; gen") + ("proj-pkg.el" . ";; gen")) + :report (cs-test--report '(("real.el" . "[1, 1]")))) + (let* ((table (cj/coverage-summary--under-dir + (cj/coverage-summary--parse-file report) src root)) + (tracked (let (ks) (maphash (lambda (k _v) (push k ks)) table) ks)) + (missing (mapcar #'file-name-nondirectory + (cj/coverage-summary--missing tracked src root)))) + (should (member "untested.el" missing)) + (should-not (member "proj-autoloads.el" missing)) + (should-not (member "proj-pkg.el" missing))))) + ;; --- project number (unit-weighted, missing as 0%) ------------------------- (ert-deftest cs-project-pct-unit-weighted-with-missing-as-zero () diff --git a/languages/elisp/tests/test-pre-commit-hook.bats b/languages/elisp/tests/test-pre-commit-hook.bats new file mode 100644 index 0000000..413c71d --- /dev/null +++ b/languages/elisp/tests/test-pre-commit-hook.bats @@ -0,0 +1,126 @@ +#!/usr/bin/env bats +# Tests for githooks/pre-commit — the secret scan and paren check. +# +# The scan reads its input through a pipeline: +# +# added_lines="$(git diff --cached ... | grep '^+' | grep -v '^+++' || true)" +# +# `grep` exits 1 when it matches nothing, which is the ordinary case, so the +# `|| true` has to stay. But with no `pipefail` it also swallows a failure of +# `git diff` itself, and an empty `added_lines` makes the scan search nothing, +# find nothing, and report clean. A gate that passes without looking is the +# failure this file exists to pin: the fail-open test drives a broken `git diff` +# and asserts the hook refuses rather than exiting 0. +# +# Each test builds a throwaway git repo in BATS_TEST_TMPDIR, so nothing touches +# the real repository or its hooks. + +setup() { + HOOK="${BATS_TEST_DIRNAME}/../githooks/pre-commit" + REPO="${BATS_TEST_TMPDIR}/repo" + mkdir -p "$REPO" + cd "$REPO" || return 1 + git init -q . + git config user.email t@example.com + git config user.name Test + # Split so the fixtures never appear as credential-shaped literals here. + AWS_TAIL="IOSFODNN7EXAMPLE" + WORD_TAIL="word" +} + +# Put a stub `git` ahead of the real one that fails for the staged-diff call +# and delegates everything else, so only the pipeline under test breaks. +break_staged_diff() { + mkdir -p "${BATS_TEST_TMPDIR}/bin" + cat > "${BATS_TEST_TMPDIR}/bin/git" <<'STUB' +#!/usr/bin/env bash +if [ "${1:-}" = "diff" ] && [ "${2:-}" = "--cached" ] && [ "${3:-}" = "-U0" ]; then + echo "simulated git failure" >&2 + exit 128 +fi +exec /usr/bin/git "$@" +STUB + chmod +x "${BATS_TEST_TMPDIR}/bin/git" + PATH="${BATS_TEST_TMPDIR}/bin:$PATH" +} + +# ------------------------------- Normal cases ------------------------------- + +@test "secret scan: blocks a staged AWS key" { + # Assembled at runtime: a literal key-shaped string in this file would trip + # the very hook under test on every commit that touches it, and this repo + # mirrors to a public remote. + printf 'aws = "%s"\n' "AKIA${AWS_TAIL}" > creds.txt + git add creds.txt + run "$HOOK" + [ "$status" -eq 1 ] + [[ "$output" == *"potential secret"* ]] +} + +@test "secret scan: blocks a staged keyword=value password" { + printf '%s = "%s"\n' "pass${WORD_TAIL}" "correcthorsebatterystaple" > conf.txt + git add conf.txt + run "$HOOK" + [ "$status" -eq 1 ] + [[ "$output" == *"potential secret"* ]] +} + +@test "secret scan: allows an ordinary staged file" { + printf 'just some prose\n' > notes.txt + git add notes.txt + run "$HOOK" + [ "$status" -eq 0 ] +} + +# ------------------------------ Boundary cases ------------------------------ + +@test "secret scan: allows a commit with nothing staged" { + run "$HOOK" + [ "$status" -eq 0 ] +} + +@test "paren check: blocks an unbalanced staged .el file" { + printf '(defun broken ()\n (message "no close"\n' > bad.el + git add bad.el + run "$HOOK" + [ "$status" -eq 1 ] + [[ "$output" == *"paren check failed"* ]] +} + +@test "paren check: allows a balanced staged .el file" { + printf '(defun fine ()\n (message "ok"))\n' > good.el + git add good.el + run "$HOOK" + [ "$status" -eq 0 ] +} + +# -------------------------------- Error cases ------------------------------- + +@test "secret scan: refuses to pass when the staged diff cannot be read" { + # The scan must not report clean after searching nothing. Without a + # pipefail-aware guard the broken diff yields an empty added_lines and the + # hook exits 0, letting a real secret through unscanned. + printf 'aws = "%s"\n' "AKIA${AWS_TAIL}" > creds.txt + git add creds.txt + break_staged_diff + run "$HOOK" + [ "$status" -ne 0 ] +} + +@test "paren check: refuses to pass when the staged file list cannot be read" { + printf '(defun broken ()\n (message "no close"\n' > bad.el + git add bad.el + mkdir -p "${BATS_TEST_TMPDIR}/bin2" + cat > "${BATS_TEST_TMPDIR}/bin2/git" <<'STUB' +#!/usr/bin/env bash +if [ "${1:-}" = "diff" ] && [ "${2:-}" = "--cached" ] && [ "${3:-}" = "--name-only" ]; then + echo "simulated git failure" >&2 + exit 128 +fi +exec /usr/bin/git "$@" +STUB + chmod +x "${BATS_TEST_TMPDIR}/bin2/git" + PATH="${BATS_TEST_TMPDIR}/bin2:$PATH" + run "$HOOK" + [ "$status" -ne 0 ] +} diff --git a/languages/elisp/tests/test-validate-el-hook.bats b/languages/elisp/tests/test-validate-el-hook.bats new file mode 100644 index 0000000..d4d6f23 --- /dev/null +++ b/languages/elisp/tests/test-validate-el-hook.bats @@ -0,0 +1,100 @@ +#!/usr/bin/env bats +# Tests for .claude/hooks/validate-el.sh — the auto-test runner. +# +# The runner used to skip entirely above MAX_AUTO_TEST_FILES=20, with no else +# branch: nothing printed, exit 0, indistinguishable from a passing run. That +# was live for the three largest families here (calendar-sync 63 test files, +# music 45, ai-term 35), so every edit to those ran parens and byte-compile and +# zero tests, silently. +# +# The cap was removed rather than made loud, because its premise did not hold. +# Measured on this machine, running a whole family takes about a second: +# ai-term 208 tests in 1.0s, music 403 in 1.7s, calendar-sync 633 in 0.9s. It +# was also concealing a real cross-test pollution bug in calendar-sync that +# only appears when that family runs in one process. +# +# These tests pin that no file count is skipped. Each builds a synthetic +# project in BATS_TEST_TMPDIR and points CLAUDE_PROJECT_DIR at it, so nothing +# runs against the real tree. + +setup() { + # Bundle layout: the hook ships at claude/hooks/ here and installs to + # .claude/hooks/ in a consuming project. This test was written against the + # installed layout, so re-homing it needed the path adjusted. + HOOK="${BATS_TEST_DIRNAME}/../claude/hooks/validate-el.sh" + PROJ="${BATS_TEST_TMPDIR}/proj" + mkdir -p "$PROJ/modules" "$PROJ/tests" + export CLAUDE_PROJECT_DIR="$PROJ" + printf '(provide (quote widget))\n' > "$PROJ/modules/widget.el" +} + +# N green test files matching the widget stem. +make_tests() { + local n="$1" i + for ((i = 1; i <= n; i++)); do + printf '(require (quote ert))\n(ert-deftest test-widget-%d () (should t))\n' \ + "$i" > "$PROJ/tests/test-widget-${i}.el" + done +} + +# One failing test file, to prove the run is real rather than merely quiet. +make_failing_test() { + printf '(require (quote ert))\n(ert-deftest test-widget-bad () (should nil))\n' \ + > "$PROJ/tests/test-widget-bad.el" +} + +hook_input() { + printf '{"tool_input":{"file_path":"%s"}}' "$PROJ/modules/widget.el" +} + +run_hook() { + run bash -c "$(printf '%q' "$HOOK") <<< '$(hook_input)'" +} + +# ------------------------------- Normal cases ------------------------------- + +@test "a small family runs and passes quietly" { + make_tests 3 + run_hook + [ "$status" -eq 0 ] +} + +@test "a failing test blocks, so a quiet pass means the tests really ran" { + make_tests 3 + make_failing_test + run_hook + [ "$status" -eq 2 ] + [[ "$output" == *"TESTS FAILED"* ]] +} + +# ------------------------------ Boundary cases ------------------------------ + +@test "at the old cap of 20 files: runs" { + make_tests 20 + run_hook + [ "$status" -eq 0 ] +} + +@test "past the old cap: still runs, no longer skipped" { + make_tests 21 + run_hook + [ "$status" -eq 0 ] + [[ "${output,,}" != *"skipped"* ]] +} + +@test "well past the old cap: a failure in file 63 is still caught" { + # The regression this guards: at 63 files the runner used to skip, so a red + # test in a big family reported clean. calendar-sync is exactly this size. + make_tests 63 + make_failing_test + run_hook + [ "$status" -eq 2 ] + [[ "$output" == *"TESTS FAILED"* ]] +} + +# -------------------------------- Error cases ------------------------------- + +@test "no matching tests: exits clean without running anything" { + run_hook + [ "$status" -eq 0 ] +} |