DEV Community

g.sunilkumar
g.sunilkumar

Posted on Fully Autonomous

Three bugs with passing tests, and what each test actually checked

#javascript #codereview #ai #testing

The ticket says 10% off orders of more than 10 units. The pull request says this:

// Ticket: "10% off orders of more than 10 units."
export function lineTotalCents(unitCents, qty) {
  const rate = qty >= 10 ? 0.9 : 1;
  return Math.round(unitCents * qty * rate);
}
Enter fullscreen mode Exit fullscreen mode

And it ships with this test:

for (const qty of [1, 5, 10, 25]) {
  const expected = Math.round(999 * qty * (qty >= 10 ? 0.9 : 1));
  assert.equal(lineTotalCents(999, qty), expected);
}
Enter fullscreen mode Exit fullscreen mode

It passes, and it covers quantity 10. It also copies qty >= 10 from the code, so it can only ever agree with the code. At exactly 10 units the customer gets a discount the ticket does not allow.

I wrote three small examples of this shape. With AI-generated code they get easier to miss, because the code looks neat whether it is right or wrong. Here are the other two.

The kill switch that fails open

The ticket: ops must be able to switch checkout off during an incident.

export async function isCheckoutEnabled(fetchFlags) {
  try {
    const flags = await fetchFlags();
    return flags.checkout !== false;
  } catch (err) {
    console.warn("flag service unavailable, using default", err.message);
    return true; // keep the shop open
  }
}
Enter fullscreen mode Exit fullscreen mode

The test:

const down = async () => { throw new Error("timeout"); };
await assert.doesNotReject(isCheckoutEnabled(down));
Enter fullscreen mode Exit fullscreen mode

"Handles errors gracefully" is true. It is also beside the point. If the outage that made ops reach for the switch also takes down the flag service, checkout reopens in the middle of the incident.

The fixture too small to fail

The ticket: delete every expired session.

export async function purgeExpired(store, pageSize = 100) {
  let offset = 0;
  while (true) {
    const page = await store.listExpired({ offset, limit: pageSize });
    if (page.length === 0) return;
    await store.deleteMany(page.map((row) => row.id));
    offset += pageSize;
  }
}
Enter fullscreen mode Exit fullscreen mode

The test uses a store with 3 rows, which never reaches a second page. Run it on 250 rows and 100 expired sessions stay behind, because deleted rows shift up under the offset. The job reports success.

What the three have in common

Each test is real and passes honestly. Each one checks the code against itself, a mock or an easy case, and never against the ticket.

So read the ticket first, then the tests, then the diff. For each test, ask what it would take to make it fail. If the only answer is changing the code and the test together, it says nothing about the requirement.

A rule helps with the rest: whoever opens the PR owns every line, however it was produced. It never asks where a line came from, so nobody has to detect AI code or argue about it. The weak spot is that a written explanation can be generated too, so a live follow-up question is the part that holds.

Nobody has measured whether this speeds up review or reduces incidents. I would like to hear where it breaks on a real team.

The three examples run on Node.js with nothing to install: https://github.com/sunilkumarvalmiki/own-the-diff-examples. Run node --test in the examples folder.

An AI assistant drafted this article for me, and I published it after approving it. The examples illustrate each pattern and their tests run. They are not from a real incident.

Top comments (0)