Flagging Tests That Assert Nothing, When All You Can See Is the Diff

The hole in my merge card

My agent fleet writes code without me. A daemon polls a project board, runs each task as a headless coding agent inside a container, and then puts a review card in Telegram where I press merge. Between those two points sits a family of mechanical scans that read the diff and annotate the card with things a human reviewer should not have to notice by hand.

That family was built around a single idea: a pull request can go green by suppressing the thing that would have turned it red. So the scanner looks for assertions deleted from a test, test files emptied out, skip and xfail markers, noqa and type: ignore and eslint-disable comments, empty except and catch blocks, a commit pushed with hooks off, TLS verification downgraded, a CI job quietly removed, continue-on-error switched on.

Read that list again and the shape of what it misses becomes obvious. Every class describes a check that used to exist and stopped existing. Not one of them describes a check that was never there in the first place.

A brand new test that asserts nothing rides through completely clean. It is added, so nothing was removed. It lives in a test file, so the suite count goes up. It executes, so it passes. And it proves precisely nothing.

Two weaker things were covering that gap. The reviewer prompt tells the reviewing agent to flag tests that only assert True or over-mock the unit under test, which is a model judgment, and mechanising model judgments is the entire reason the scan family exists. The second cover is a repro-evidence gate that would catch a vacuous test by its exit code against the merge base, except that gate is injected for bug-typed tasks only. Features and chores are the bulk of my board. They had no mechanical check on new tests at all.

The nudge to close it came from a Reddit thread that landed in my daily digest. Someone was running one vendor's model to write and another's to review, and the single concrete thing their reviewer caught was a test that did not exercise the real logic. That is exactly the case my scanner could not see.

What the check actually is

The new class is called vacuous_test, and the rule fits in a sentence. In a file that looks like a test path, an added test callable whose new-side body carries no assertion marker gets a line on the merge card.

Assertion marker is deliberately broad. A bare assert counts. So do expect(, check(, pytest.raises, toThrow, and any call to a function named assert_something or _assert_something. Real suites assert through helpers constantly, and a check that recognises only the literal keyword will spend its first week flagging perfectly good tests.

Three things are skipped outright: conftest files, underscore-prefixed helpers, and callables carrying a fixture decorator. A fixture named test_client is a fixture, not a test, and the decorator above it is the only reliable way to know that.

There is exactly one whitelist, and it is worth being precise about the mechanism, because a vague answer here would undercut the point of the whole exercise. Each suppression class owns a small set of regexes over the task text, and this class matches phrasings like smoke test, smoke-only, assertion-free test, vacuous test, no-op test. A match alone is not enough: the phrase has to sit in a clause carrying a verb of the right direction, add for this class, with no negation attached.

So a task saying "add a smoke test for the health endpoint" suppresses the hit. A task saying "do not leave this as a smoke test" does not, because the clause is negated. And a task that merely mentions smoke tests in passing while asking for something else does not either, because there is no add verb attached to the phrase. No model call, no embedding, nothing that can drift between runs. The same machinery already decides whether a task genuinely asked you to delete a test.

One thing is conspicuously absent from that description: an abstract syntax tree. Every guide to this kind of check reaches for AST parsing, and for good reason, since call structure beats text shape every time. I did not use one, and the reason turned out to matter more than the check itself.

A diff is not a file

The scanner never receives a file. It receives a unified diff, which is a partial view of a file, cropped to the changed lines plus three lines of context on either side. Error-tolerant parsers exist, and tree-sitter will happily build a tree over a fragment, so the honest framing is a tradeoff rather than an impossibility. But a tree over a fragment tells you about the fragment. The question this check has to answer is whether the fragment is the whole test, and that is a property of the diff, not of the syntax. The pattern-matching approach every guide warns against is the one that has access to the information the decision actually needs.

Working from a cropped view carries a failure mode that has nothing to do with pattern quality. I did not see it. My reviewing agent did, and filed it as blocking on the first review cycle.

Here is the shape it found. Renaming a test touches exactly one line, the def line, or the test( line in a JavaScript suite. Adding a fixture parameter touches that same single line. The body below does not change, so it arrives as context, and git hands you three lines of it by default:

-def test_old_name():
+def test_new_name():
     step1()
     step2()
     step3()

The assertion is on the next line down, and the scanner cannot see it. Meanwhile a genuinely vacuous new test looks like this:

+def test_smoke_path():
+    client.get("/health")

To a scanner reading line text, those two bodies are the same: a couple of statements, no assertion anywhere. And the first one is the shape it meets most often, since renaming a test and adding a parameter to a test are the two most common test edits in the repository. The check would have opened its career by crying wolf on ordinary work.

The two diffs above are not the same, though, and the difference is sitting in plain sight. In the first, the body lines are unchanged context. In the second, every line of the body is an addition. That distinction is the fix, and the scanner was already holding it and throwing it away, because every diff line carries its kind. Git also has a property worth memorising here: an added block is always shown in full. It is never cropped, because there is no older version of it to crop against.

So when a body scan runs off the end of a hunk, the kind of the last line it consumed answers the only question that matters. If that line was added, the scanner saw the whole body, and a body with no assertion genuinely is vacuous. If that line was unchanged context, the scanner saw a window that ran out before the body did, and the honest response is to stay quiet.

Fail open on a partial view. That posture already applied to a wrapped multi-line signature whose closing parenthesis never arrives inside the hunk, and it now applies to bodies as well. It also settles the case of a test body split across two hunks, which sounds like an unhandled edge until you notice it is the same situation wearing a different hat: the scan works within one hunk, so the second half of that body is simply not in view, the scan runs off the end while still on context, and the check stays quiet. A body that got split across hunks has unchanged lines in the middle by definition, which is exactly what the truncation signal keys on.

The fix is not a better regex. It is the scanner learning to ask whether it is looking at all of something before passing judgment on it.

Back-running the check before it reached the card

One line in the plan mattered more than any line of code, and I wrote it before implementation started. Before this check goes on the merge card, run it across the last thirty merged agent pull requests and paste the hit list on the task. If it flags noise, tighten the pattern rather than shipping a card line I will learn to skip.

That final clause is the whole point. A merge card is a small surface holding a limited amount of my attention. A line that is wrong often enough to feel wrong does not become mildly annoying. It becomes invisible, and on the way out it takes some of the credibility of the lines sitting next to it.

First back-run: six hits across three pull requests. Two noise shapes, both of them ordinary patterns in my own code rather than exotic corner cases. Wrapped multi-line signatures, where the scanner mistook a closing parenthesis for the end of the suite and never reached the assertions underneath. And assertions made through helper wrappers with names like _assert_refused, which the assertion pattern did not yet recognise.

Both got fixed in the pattern rather than excused in the report.

Second back-run: one hit across thirty pull requests. That hit is a test which deliberately calls a function and asserts nothing, because the point of the test is that the call must not raise. It belongs to the class the check exists to surface, so I left it on the card instead of carving out another exemption.

One in thirty, measured on real merged history, with the single survivor being a true positive. That is a number no unit test could have given me. Fixtures agree with whoever wrote them. Merged history does not.

Why it annotates instead of blocking

The check never blocks a merge. It writes a class name on the card, records it in review metrics, and leaves the decision to me.

That is a read of the population rather than timidity. Tests that legitimately assert nothing exist and are not rare: must-not-raise probes, wiring checks, harness smoke tests whose only job is proving a path is reachable. A blocking rule aimed at a category that small but that genuine buys you an argument and an exemption mechanism, and exemption mechanisms rot until nobody remembers what they were for. An annotation buys you a signal at the exact moment you can act on it, which is while the card is on screen and the merge has not happened yet.

So the honest description of what I built is not a gate. I annotate new tests that assert nothing, and after the back-run I trust the annotation enough to read it.

What I would tell anyone building the same thing

Measure the false-positive rate against your own merged history before the check ever reaches a human, rather than against the fixtures you wrote for it. Fixtures come from the same person who wrote the pattern and inherit every one of that person's assumptions, which is exactly the blind spot you are trying to cover.

When your input is a partial view of something, the first question is not what does this say. It is whether you are looking at all of it. Most of the wrong answers I have shipped out of diff-based tooling trace back to confidently reading a window as though it were the whole room.

Recognise assertions made through helpers, or the check will teach the people around it that it does not understand their code.

Prefer annotation over blocking when the legitimate exception class is real and small. Prefer blocking when that class is empty.

And put the measurement step in the plan ahead of the implementation step, so that tightening the pattern becomes the expected outcome of the back-run rather than a disappointment.

FAQ

Does it catch a test whose assertion is technically present but meaningless?

No, and it is not trying to. Something like an assertion that compares a value to itself satisfies this check completely. Oracle strength is a separate problem, and the tool for it is mutation testing, which changes the production code and asks whether the suite notices. This check is the floor underneath that, not a substitute for it.

What about weak tests that were merged before the check existed?

They are invisible to it, permanently. The rule fires on test callables added in the diff under review, so a vacuous test written last year is never revisited. Closing that would need a separate sweep across the existing suite, which is a different job with a different noise profile.

Which languages does it cover?

Python test functions and the Jest, Vitest, and Playwright call forms today. Everything else falls through without a hit, which is worth stating plainly, because a check that silently covers only part of your repository is easy to mistake for one that covers all of it.

What does it cost to run?

Effectively nothing. The patch is already fetched once for the whole scan family, and this class is another pass over lines that are already in memory. That cheapness is what makes it reasonable to run on every pull request rather than on a schedule.

Related reading