2026-09-04 · 6 min read

Your tests pass and the bug is still there, because the tests were never checking for it

There's a thread on r/ExperiencedDevs that comes back every couple of weeks in slightly different words: "Is AI code review good enough?" The answers are always some flavor of "it depends," which tells you nothing you can act on. So here's something you can act on: there's a specific class of bugs that AI reviewers almost never catch, that ships constantly because the test suite is green, and that you can learn to recognize in about ten minutes once someone shows you what to look for. The class is deceptive tests, and the three patterns below cover most of what you'll see in practice.

The tautological assertion, or: the test that agrees with whatever you tell it

Say a PR lands with a tiered discount function and three tests, all green. The first test is doing its job:

it("leaves small orders untouched", () => {
  expect(applyDiscount(80)).toBe(80);
});

That 80 is a literal that somebody (or something) derived from the spec: orders under $100 get no discount, so 80 stays 80. You can read the assertion against the requirements and it checks out. But look at the next two:

it("applies the discount above the threshold", () => {
  const subtotal = 240;
  const expected = subtotal - subtotal * DISCOUNT_RATE;
  expect(applyDiscount(subtotal)).toBe(expected);
});

it("caps the discount on large orders", () => {
  const subtotal = 900;
  const expected = subtotal - Math.min(subtotal * DISCOUNT_RATE, DISCOUNT_CAP);
  expect(applyDiscount(subtotal)).toBe(expected);
});

Both of them import DISCOUNT_RATE and DISCOUNT_CAP from the same config file the implementation reads, and then they rebuild the expected value using the same formula the function uses. So when you change the discount rate from 10% to 20%, or set the cap to $5 instead of $50, or introduce an off-by-one at the threshold boundary, all three tests stay green, because the expected value drifts along with the bug. The test isn't checking the code against the spec; it's checking the code against itself. You could rename this pattern "the mirror test": the code looks at its own reflection and confirms it sees a face.

What gives it away in review: any expected value that imports a constant, calls a helper, or does arithmetic instead of being a plain number you can trace back to the requirements. The fix is almost always just hardcoding the spec's answer: expect(applyDiscount(240)).toBe(216). That 216 came from a human (or at least a calculator), not from running the formula again and hoping it agrees with itself.

The mocked-away rule, or: 100% coverage of a function that never executes

Here's one that's subtler. A PR adds an overdue-invoice reminder: the business rule lives in overdueInvoices(), which filters invoices past a 30-day grace period, and the integration function sendReminders() calls it and emails each result. So far so good. Now look at the test:

jest.mock("../lib/mailer", () => ({
  sendEmail: jest.fn().mockResolvedValue(undefined),
}));

jest.mock("./overdue", () => ({
  overdueInvoices: jest.fn(() => [
    { id: "inv-1", customerEmail: "[email protected]" },
    { id: "inv-2", customerEmail: "[email protected]" },
  ]),
}));

it("sends reminders only for invoices overdue past 30 days", async () => {
  const result = await sendReminders([], Date.now());
  expect(sendEmail).toHaveBeenCalledTimes(2);
  expect(result.reminded).toBe(2);
});

Read the test name out loud: "sends reminders only for invoices overdue past 30 days." Now read the second jest.mock: the overdueInvoices function is stubbed to always return two hardcoded invoices regardless of what you pass in. The grace-period logic, the thing the test name promises to verify, never executes. The input array is literally empty, and it doesn't matter because the mock ignores it. What this test actually proves is that sendReminders can loop over an array and call sendEmail for each item, which is the kind of insight you'd get from reading the function for three seconds.

Mocking the mailer is totally fine here, that's a side-effect boundary you don't want your tests reaching. But mocking the overdue-invoice filter is mocking away the business rule the test claims to cover, and once you do that, you can change the grace period from 30 days to 30 microseconds and the test won't notice.

What gives it away in review: the test name promises a business rule, but a jest.mock (or unittest.mock.patch, or vi.mock) replaces the exact module that contains the rule. One mock is usually fine because you're isolating a side effect. Two mocks, where one of them is the thing the test name says it's covering: that's the tell. If the mock makes the test independent of the rule, the test doesn't test the rule, no matter what the describe block says.

The unawaited assertion, or: the test that literally cannot fail

This is the most common one in generated code, and it's also the most invisible during a quick scroll through the diff. A PR adds tests for an async profile loader:

it("returns the user's name", () => {
  loadProfile(7).then((profile) => {
    expect(profile.name).toBe("Ada");
  });
});

it("falls back to the default avatar", () => {
  loadProfile(7).then((profile) => {
    expect(profile.avatar).toBe("/img/default-avatar.png");
  });
});

Neither test returns or awaits the promise, so Jest considers the test finished as soon as the synchronous body ends. The .then callback with the assertion runs later (or never, depending on microtask timing), by which point the test has already passed. You could change the return value to anything you want, or delete the loadProfile function entirely, or make the assertion check for "Napoleon Bonaparte" instead of "Ada", and the suite would stay green. These tests cannot fail. They are decoration.

This pattern ships constantly because it produces a green test on the very first run, which the model interprets as "test is working, move on." The fix is genuinely one keyword:

it("returns the user's name", async () => {
  const profile = await loadProfile(7);
  expect(profile.name).toBe("Ada");
});

What gives it away in review: a test calling an async function without await or return, with the assertion living inside a .then() or a callback. In Python it's the same shape: an assertion inside a coroutine that nobody awaits. The test function is synchronous, the assertion is asynchronous, and the framework doesn't wait for one to catch up to the other.

Why these survive AI review and what that tells you about where reviewing is heading

All three patterns share a property that makes them nearly invisible to automated reviewers: the code is syntactically correct, the tests are syntactically correct, and the tests pass. An AI reviewer evaluates the diff against its training distribution of "what good tests look like," and these look like excellent tests. Descriptive names, clean structure, reasonable assertions, correct imports. The problem is that they're asking the wrong question, or no question at all, and catching that requires you to read what the test claims to verify, then think about whether there's actually a bug that could make it fail. That's a reasoning task about the relationship between a spec and its encoding, not a pattern-matching task about code shape, and it's the part of code review that is still squarely yours.

Try it yourself

DiffDojo has nine PRs built around deceptive tests, each a realistic diff where everything is green and the verdict is yours to defend. Your profile tracks whether test-reading is one of your blind spots (for most reviewers, it quietly is). Start with the discount test from above and see whether you'd have caught it before reading the canonical answer.