Skip to content

hooks_test.sh: run_hook dead exit-code check silently weakens every test #73

Description

@5uck1ess

Surfaced by the silent-failure hunter during review of #72. Pre-existing bug in the test harness; not introduced by that PR.

Problem

`hooks/hooks_test.sh:48-49`:

```bash
OUTPUT=$(echo "$input" | bash "$HOOK_DIR/$script" 2>/dev/null) || true
EXIT=$?
```

After `|| true`, `$?` is the exit code of `true`, i.e. always `0`. The subsequent `if [ $EXIT -ne 0 ]` check at line 52 is dead code — every test using `run_hook` passes on exit-code regardless of what the hook actually returned.

This means any hook whose contract includes exit codes (e.g. `safety-check.sh` exit 2 for blocks, `devkit-guard.sh` exit 2 for step violations) cannot be enforced via `run_hook`. The tests that check exit codes today work because they bypass `run_hook` and invoke `bash …` directly with their own `$?` capture (see the safety-check exit-2 and guard tests).

Fix

```bash
set +e
OUTPUT=$(echo "$input" | bash "$HOOK_DIR/$script" 2>/dev/null)
EXIT=$?
set -e
```

Or equivalent: capture into a file and separately read `$?`.

Why this matters

The #72 review described three bugs that "hid in plain sight for months." A test harness that silently swallows exit codes is the exact class of infrastructure that lets bugs hide. Worth fixing before adding more tests that route through `run_hook`.

Scope

  • Fix `run_hook` to capture `$?` before `|| true`
  • Audit existing `run_hook` callers — any test that was "passing" because of this would need its assertion tightened
  • Consider adding a deliberately-failing fixture that must actually fail to prove the harness is working

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions