DEV Community

Enjoy Kumawat
Enjoy Kumawat

Posted on

My Commit-Message Script Has 8 Assertions in --selftest. None of Them Touch the Code That Can Actually Fail.

I have three files in this repo that shell out to something over the network or a subprocess and can fail in interesting ways: publish_devto.py, server.py, and git_commit.py. Two of them have --selftest blocks that stub the risky call and exercise the actual failure branches. One doesn't, and I only noticed because I went looking for a reason to be suspicious of my own test coverage after seeing a trending post about counting assertions in a test suite and not liking what you find.

git_commit.py reads a staged diff and calls claude -p to turn it into a commit message. It has five distinct exit paths, all guarding real failure modes I've hit before in this project:

try:
    diff = subprocess.check_output(["git", "diff", "--staged"], text=True, timeout=20)
except subprocess.TimeoutExpired:
    print("git diff --staged timed out after 20s", file=sys.stderr)
    raise SystemExit(1)
if not diff.strip():
    print("Nothing staged. Run `git add` first.")
    raise SystemExit(1)

try:
    raw = subprocess.check_output(
        ["claude", "-p", "--safe-mode", SYSTEM + "\n\n" + diff],
        text=True, timeout=20, stderr=subprocess.PIPE,
    ).strip()
except subprocess.TimeoutExpired:
    print("claude -p timed out after 20s", file=sys.stderr)
    raise SystemExit(1)
except subprocess.CalledProcessError as e:
    print(f"claude -p exited {e.returncode}: {(e.stderr or '').strip()[:200]}", file=sys.stderr)
    raise SystemExit(1)
except FileNotFoundError:
    print("claude CLI not found on PATH", file=sys.stderr)
    raise SystemExit(1)
Enter fullscreen mode Exit fullscreen mode

That's a held index lock hanging git diff, an empty staging area, a claude -p call that times out, one that exits non-zero, and one where the claude binary isn't even on PATH. Real scenarios — the timeout on this exact git diff --staged call was itself a bug I'd already found and fixed once (docs/project_notes/bugs.md, 2026-08-06: a prior fix claimed to add a timeout to "both" subprocess calls in this file and only actually touched one).

Here's the entire --selftest block:

if "--selftest" in sys.argv:
    _CASES = [
        ("co-authored-by: claude <noreply@anthropic.com>", True),
        ("🤖 generated with [claude code](https://claude.ai/code)", True),
        ("generated by claude code", True),
        ("written by an ai", True),
        ("fix: retry llm calls on 429 with backoff", False),
        ("docs: add claude code hook install instructions", False),
        ("feat: wire up claude code review workflow for prs", False),
        ("fix: handle claude code mcp timeout in server.py", False),
    ]
    for line, expect_stripped in _CASES:
        got = bool(_STRIP_RE.search(line))
        assert got == expect_stripped, (line, got, expect_stripped)
    print("selftest ok")
    raise SystemExit(0)
Enter fullscreen mode Exit fullscreen mode

Eight assertions, all against _STRIP_RE — the regex that strips AI-attribution lines from whatever claude -p returns. That regex is worth testing; it's regressed twice before (bugs.md, 2026-07-22 and 2026-07-26, both bare-substring over-matching). But it's a pure string-matching function with zero dependency on git, claude, or the network. selftest ok tells me the filter still behaves. It tells me nothing about whether this script survives a timed-out git diff, a claude binary that isn't installed, or a non-zero exit — the five branches above that are the actual reason those except clauses exist.

Compare that to the other two files' --selftest blocks, which I'd written the same week and apparently held to a different bar. publish_devto.py stubs urllib.request.urlopen itself to drive its failure paths:

def _fake_url_error(req, timeout=30):
    raise urllib.error.URLError("timed out")

urllib.request.urlopen = _fake_url_error
try:
    try:
        already_published("k", "anything")
        assert False, "URLError is the ambiguous case — must raise, not return None"
    except RuntimeError:
        pass
finally:
    urllib.request.urlopen = _orig_urlopen
Enter fullscreen mode Exit fullscreen mode

And a missing-credential case:

_saved_dev_key = os.environ.pop("DEV_TO_API", None)
try:
    try:
        main("this-file-does-not-exist.md")
        assert False, "missing DEV_TO_API must exit, not silently proceed"
    except SystemExit as e:
        assert e.code is not None and "DEV_TO_API not set" in str(e.code), e.code
    except KeyError:
        assert False, "must exit through ERROR: convention, not a bare KeyError"
finally:
    if _saved_dev_key is not None:
        os.environ["DEV_TO_API"] = _saved_dev_key
Enter fullscreen mode Exit fullscreen mode

server.py does the same thing to its own _gh/_dev helpers — swap in a fake, pop the credential, assert the exception shape. Both of those got this treatment specifically because a missing-credential KeyError was a real, previously-shipped bug (bugs.md, 2026-08-09) that a stub test could catch and a plain read-through couldn't.

Going back to git_commit.py with that pattern in mind, the fix isn't hard — subprocess.check_output is exactly as mockable as urllib.request.urlopen:

def _fake_timeout(*a, **k):
    raise subprocess.TimeoutExpired(cmd=a[0], timeout=20)

_orig_check_output = subprocess.check_output
subprocess.check_output = _fake_timeout
try:
    # exercise the git-diff-timeout branch here
    ...
finally:
    subprocess.check_output = _orig_check_output
Enter fullscreen mode Exit fullscreen mode

But it needs somewhere to plug in. publish_devto.py and server.py both wrap their risky calls in named functions — already_published(), main(), _gh(), _dev() — so a selftest can import or call them directly and swap out one dependency. git_commit.py has no such boundary. It's twenty lines of top-level script: read the diff, call claude -p, print the result, all at module scope, guarded only by an early if "--selftest" in sys.argv: ... raise SystemExit(0) at the top. There's no function to call with a stubbed subprocess.check_output without either refactoring the five try/except blocks into a callable or monkeypatching subprocess module-wide before the script's own top-level code runs — which, for a file this makes --selftest exit before ever reaching, doesn't actually help either.

So the honest fix here is two-part, not one: wrap the diff-read-and-generate logic in a function the way its two siblings already do, then give that function the same stub-and-assert treatment already_published() and _gh()/_dev() got. I haven't done that yet — flagging it here as the concrete next step rather than a vague "add more tests" note, since the shape of the fix (extract a function, then mock its one real dependency) is already sitting right there in the same repo, in the two files that did it first.

Top comments (2)

Collapse
 
alexshev profile image
Alex Shev

That gap between selftest coverage and the code path that actually fails is painfully common. I like tests that exercise the boring boundary: real argv parsing, real temp repo state, and the exact shell path the script uses in normal operation.

Collapse
 
eduzsh profile image
Edu Peralta

This is the trap that shows up constantly around agent tooling: a green selftest that only exercises a pure string filter trains you to trust the script, even though the real failure modes (timeout, missing CLI, failed process exit) never ran. The other two files already had the right shape. Wrap the risky call, stub the dependency, and assert the exit path you claim exists. Counting assertions is vanity. Counting which branches of the subprocess tree have ever been forced to fail is the metric that actually protects you the next time claude -p hangs during a commit.