DEV Community

Revin
Revin

Posted on

I grepped 61 merged pull requests for three tells: 9 had a test file with zero assertions

Martin Fowler published a bliki entry today called LLM Voice. It is about reading, not about writing. He describes the growing feeling that the thing in front of him came out of a model, and how his desire to keep reading drops off once that feeling arrives.

I can't necessarily define why, although there are the common tells (it's not the em-dashes, it's the load-bearing tapestries).

He also says he distrusts the tools that claim to detect model prose, partly on accuracy, partly because detection misses the point. The reaction is a human one.

I get that same feeling in code review, and I get it most weeks. The pull request description is tidy. The sections line up. Commit messages are in the imperative mood. Then I open the test file and nothing in it asserts anything.

The trouble is that the feeling is useless as a review comment. "This reads like a model wrote it" gives the author nothing to fix, it insults people who write carefully in a second language, and I have been wrong about it more than once. So about five weeks ago I stopped reviewing the voice and started checking the consequences with grep.

What I measured

61 merged pull requests across three repositories. TypeScript, Node, Jest, roughly five weeks. Mixed authorship: some people on those repos use a coding assistant all day, some never open one, and nothing in the repo records which is which.

Three checks, all cheap enough to run against a diff while I read it.

Check one: test files that never assert

fd -e spec.ts -e test.ts . src | while read -r f; do
  rg -qP '\b(expect|assert|should)\s*\(' "$f" || echo "no assertion: $f"
done
Enter fullscreen mode Exit fullscreen mode

First pass on the largest repo:

no assertion: src/billing/invoice.service.spec.ts
no assertion: src/orders/cancel.handler.spec.ts
no assertion: src/auth/token.guard.spec.ts
Enter fullscreen mode Exit fullscreen mode

All three had been merged weeks earlier. All three were green. One of them looked like this:

it('rejects an expired token', async () => {
  const res = await request(app)
    .get('/orders')
    .set('Authorization', `Bearer ${expiredToken}`);

  console.log(res.status);
});
Enter fullscreen mode Exit fullscreen mode

That test passes if the guard returns 401. It also passes if someone deletes the guard, because the only thing it can fail on is a thrown exception. The name of the test carries the whole claim, and the name is not executable.

Part of this is solved by eslint-plugin-jest with the expect-expect rule, and I turned it on. It did not help as much as I wanted. Two of the three repos did not have the plugin at all, and the rule does not know about our custom helpers, so it needs an allowlist that nobody maintains.

Check two: expects that only verify the mock

rg -l 'expect\(' -g '*.spec.ts' src | while read -r f; do
  total=$(rg -c 'expect\(' "$f")
  calls=$(rg -c 'expect\([^)]*\)\.toHaveBeenCalled' "$f")
  [ "$total" = "$calls" ] && echo "$f: $calls/$total assertions are call checks"
done
Enter fullscreen mode Exit fullscreen mode
src/pricing/apply-discount.spec.ts: 4/4 assertions are call checks
src/notify/send-receipt.spec.ts: 3/3 assertions are call checks
Enter fullscreen mode Exit fullscreen mode

A file where every assertion is toHaveBeenCalled proves that the wiring of the test works. The function under test can return a wrong number, a null, or a string where money should be, and the suite stays green. This one is harder to argue about than check one, because the author did write assertions. They just all point at the test's own scaffolding.

Check three: catch blocks that log and swallow

I tried a regex here and wasted an afternoon on multiline matches. Ast-grep did it in one line.

ast-grep --lang ts --pattern 'try { $$$ } catch ($E) { logger.$M($$$) }' src
Enter fullscreen mode Exit fullscreen mode

The shape it finds:

try {
  await publishInvoice(order);
} catch (err) {
  logger.error('publishInvoice failed', { err });
}
Enter fullscreen mode Exit fullscreen mode

No rethrow, no return of a failure, no retry. The caller gets a resolved promise and marks the order as published. The log line exists, so a dashboard somewhere will even show it, and nothing in the system acts on it. The metric is there, the guarantee is not.

The numbers

Of the 61 pull requests, 9 carried at least one test file with zero assertions, a shade over 14%. In 6 more, every assertion added by the diff was a call check. 4 introduced a catch that swallowed an error on a path the same diff created. Three pull requests tripped two checks at once, so the unique count is 16.

The part I did not expect: at least 4 of those 9 came from people on the team who do not use a coding assistant at all. I know, because I asked. Whatever these checks find, it is not authorship. It is the moment in a review where the description was convincing enough that nobody opened the second file.

What I threw away

My first attempt was the obvious one. I wrote a scorer for pull request descriptions: a word list, sentence length variance, how symmetrical the bullet lists were, output as a number between 0 and 1. It lasted two days. It flagged the two most careful writers on the team, both of them writing in a second language, and it gave a clean score to a pull request whose code was clearly generated and whose description someone had typed by hand. Fowler's warning about detectors transfers to code review without a single edit, and the conversation the scorer started was worse than the problem it was meant to solve.

I also tried failing the build on check one. That lasted until the first snapshot suite, where the assertion lives inside a helper and the regex cannot see it. Now the check posts a comment listing the files and lets the merge through. The number in the comment does the work.

On two of the services I went further and ran mutation testing restricted to the files the diff touched. Line coverage on those files sat in the high eighties, the mutation score came back in the forties. It is slow, so I only reach for it when one of the three checks fires twice in the same service.

What changed in review

Review time went up by something like three minutes per pull request, which I would pay again, because the three checks hand me a sentence I can write without accusing anybody of anything: this file has no assertion, add one. Nobody argues with that sentence. People argue with vibes, and they are right to.

I do not know how this holds up in a repo with 40 contributors, or in a language where the test runner does not make assertions this greppable. Three repos and five weeks is a small sample, and two of the repos share a tooling setup, so the checks were easy to write by construction.

So, the question for you: what is your mechanical tell? The thing you can grep, not the thing you feel. And if you have found a way to measure assertion quality in review without it turning into a bot telling someone their test is bad, I would like to read how you set it up.

I write about review and legacy work at the Revin blog, including a longer piece in Portuguese on how far you can take the human out of the pull request.

Top comments (0)