DEV Community

Cover image for A test said the server started. I deleted the server. It still passed.
Asuran
Asuran

Posted on

A test said the server started. I deleted the server. It still passed.

Here is a test from a real, well run Node project:

test('server starts', async (t) => {
  const app = build()
  await app.listen({ port: 0 })
  t.assert.ok(true, 'server started')
})
Enter fullscreen mode Exit fullscreen mode

It reads fine in review. It runs green. Now delete the body of build() so the server never comes up. The test is still green, because the only thing it asserts is true. In the same file two more of these caught the error in a catch and asserted true there too, so even the failure path was green.

That is not a made up example. I found it in fastify at a pinned commit and opened a PR to fix it. More on that at the end.

A whole class of tests cannot fail

Once you start looking, the pattern turns up in a few shapes:

  • A literal: assert.ok(true), expect(1).toBe(1), a snapshot of a constant.
  • An assertion parked in a catch the happy path never reaches, so nothing is checked when the code works and nothing is checked when it breaks.
  • A status list that accepts both outcomes: assert.ok([200, 500].includes(res.status)).

Each one runs, counts toward coverage and guards nothing. Coverage is the trap. The line executed, so the tool that counts executed lines is happy. Whether the line would go red on a regression is a different question. It is the one that matters.

Why review misses it

A reviewer reading the diff sees a test called server starts, an await listen and a green tick. The name states intent. The assertion is what actually runs, yet ok(true) does not look like a problem until you stop and ask what would ever turn this test red. A missing check does not show up in a diff the way a wrong line does.

Finding them

I wrote a small scanner for this. No account, no config file, no network call:

npx margyn-scan /path/to/repo
Enter fullscreen mode Exit fullscreen mode

One of its checks is cannot-fail: tests whose assertions hold whatever the code does. It also flags tests that assert nothing at all, files the build reads that git never committed, gates declared in package.json that no workflow invokes and linter exclusions that quietly drop tracked source. Every finding prints the command that reproduces it, so you confirm each one yourself instead of trusting a report.

The fastify case

The two tests above asserted ok(true) right after starting a server. I opened fastify/fastify#6962 to assert the server is actually listening instead. A maintainer reviewed it and suggested tightening it further to t.assert.ok(server.listening) with t.after for teardown. Both are in and the PR is open for review. The point is not that fastify is careless. It is one of the most carefully tested projects in the ecosystem, which is exactly why it matters that this still slips through.

The takeaway

A green suite tells you the tests ran. It does not tell you they would go red if the code they cover regressed. Those are two different claims. Teams merge on the first while believing the second. The gap is worth auditing, by hand or with something like margyn.

Top comments (0)