Back to engineering

Engineering

Green for the wrong reason

The Bookatu engineering team10 min read

We put a bug back into the code to find out which of the tests written to catch it would go red. Three different ways of breaking it, and the suite stayed green through all three. This is what was wrong with those tests, and the cheap loop that came out of it, which then turned up a rule nothing in our suite has ever checked.

The short version

  • The tests written around a fix were green. We put the bug back in three different ways and they stayed green through all three, so they were proving nothing.
  • The cause was the fixture. It had been edited during the fix, so it described the world after the fix and no longer set up the conditions the bug needed.
  • A coverage report would have been perfectly happy. Every line ran. A line that runs is not an assertion that would have noticed it changing.
  • The rewrite targets one rule per test, then breaks each rule in turn. Two of the three mutants were killed, each by exactly one test. The third survived.
  • That survivor is the finding. The test we would have named as covering that rule passes down a different code path entirely.

This is a companion piece to Six tables, one booking a night, published alongside it. That post is about the bug we fixed. This one is about the part of the same work we got wrong. All you need of the bug is one sentence: a dining room with several free tables offered exactly one reservation a night, because the availability loop gave up the moment its single staff member was occupied. The fix was a few lines. The tests around it were worthless, and it took us a while to notice, because they were green.

The fix came with tests. They ran fast and their comments described the scenario properly. Then, out of a habit that has earned its keep, we put the bug back and ran them again. Green. Three separate ways of breaking the code, three green runs.

The fixture had moved with the code

The fix did not only change a condition. It changed the shape of the row a reservation writes. Before, every booking claimed a staff member, because when no particular person was requested the engine assigned the first free one. After, a reservation claims a table and no staff member at all. Why it has to work that way is the companion post's subject. What matters here is only that the row changed shape while the tests were being written.

So while writing the fix we also updated the test helper that seats a party, because otherwise it was writing rows the new code would never produce. Here is that helper as it stands.

ts
/** Seat a party at 19:00 on the given table, holding the owner as staff. */
async function seat(resourceId: string | null, serviceId = dinnerId) {
  const startAt = new Date("2026-06-16T07:00:00Z"); // 19:00 NZST
  await tdb.insert(schema.appointments).values({
    orgId,
    customerId,
    serviceId,
    serviceName: "Dinner",
    durationMin: 90,
    // A reservation claims no staffer. That is what lets a second table be
    // seated at the same time, and what the unique index enforces.
    staffId: serviceId === dinnerId ? null : ownerStaffId,
    resourceId,
    startAt,
    endAt: new Date(startAt.getTime() + 90 * 60_000),
    status: "confirmed",
    priceCents: 0,
  });
}

Read the staffId line as a description of the world, because that is what a fixture is. It says that in this world, a seated party occupies no person. That is true after the fix. It was not true before it. The bug we were trying to reproduce needed the single staff row to be busy, and this fixture never makes anybody busy, so breaking the code changed nothing any of those tests could see. They were not testing the fix. They were testing a world in which the bug could not occur.

The docstring is still sitting there saying the helper holds the owner as staff, which the line below it stopped doing. Nobody updated it, because nothing made anybody read it. That is the same failure in miniature: a sentence describing the old world, left in place next to code that has moved on, and no mechanism anywhere that notices the disagreement.

A fixture is not setup. It is a claim about the world, and editing one during a fix is a way of quietly agreeing with yourself.

The uncomfortable part is that nothing about this looked like a mistake while it was happening. Updating the fixture was necessary. Rows in the old shape would have been rows the new code cannot produce, and a test built on those is testing a ghost. Both edits were correct in isolation. What was missing was any test whose setup did not depend on the thing being fixed.

Coverage would have said we were fine

Every line of the fix ran during those tests. The condition was evaluated and the new branch was taken. A coverage report would have shaded all of it green, and it would have been telling the truth about the only question it can answer, which is whether control flow reached a line.

That is worth knowing, because a line nothing reaches is definitely untested. But the question you actually have is a different one: if this line were wrong, would anything complain. The two come apart the moment a line runs for reasons unrelated to what it decides. A condition can be evaluated hundreds of times in a test run and never once be the reason an assertion passed.

There is no static way to close that gap. You cannot read your way to it either, because reading the test is how it got written in the first place, and you will read it the same way the second time. The only method available is to change the code and see whether the suite objects.

One rule per test, then delete the rule

The rewrite started by writing down what the fix actually asserted, which turned out to be three separate rules rather than one. Two of them were new. The third came from an earlier piece of restaurant work, five weeks before, and had come along for the ride.

ts
// Three rules, three separate places in the file, collected here.

// RULE 1 (new), in the slot loop: a reservation does not need a free staffer.
const isTableReservation = p.requiresResource && p.partySize != null;
if (!freeStaff.length && !isTableReservation) continue;

// RULE 2 (new), while collecting busy intervals: a seated party holds its own
// table and nothing else.
if (a.staffId === null && a.resourceId) continue;

// RULE 3 (older), the body of fits(): a table only counts as free if the
// party fits it.
if (party == null) return true;
const cap = p.resourceCaps.get(rid);
if (!cap) return true;
if (cap.seats < party) return false;
if (cap.minParty != null && party < cap.minParty) return false;
if (cap.maxParty != null && party > cap.maxParty) return false;
return true;

Then one test per rule, each written so that the rule is the only thing standing between it and a failure. Rule one gets a test that makes the owner busy with something that is not a reservation at all, which is the setup the updated fixture could no longer produce on its own.

ts
it("offers a table even when the only staffer is busy with something else", async () => {
  // THE BUG, reproduced the way it actually bit. The owner is the single staff
  // row a restaurant is seeded with. Put them on anything at 19:00 and the old
  // code bailed out before it ever looked at the tables, so a dining room with
  // four free tables offered nothing.
  await seat(null, cutId); // the owner is occupied, no table involved
  const slots = await slotsAt1900(dinnerId, 90, 2);
  expect(slots).toHaveLength(1);
  expect(slots[0].resourceIds?.length).toBeGreaterThan(0);
});

Delete the second clause of rule one, so the condition reads as it did before the fix, and run the file. One test fails out of nine, and it is that one. Restore, and move to the next.

ts
it("a seated party does not block anyone, only its own table", async () => {
  // A reservation carries no staffer, and a booking with no staffer normally
  // blocks the whole org. If a seated party were treated that way, one table
  // at 19:00 would shut every other service in the building.
  await seat(tableIds[0]);
  expect(await slotsAt1900(cutId, 60)).toHaveLength(1);
});

Delete the continue that makes up rule two and run again. One test fails out of nine, and again it is the one whose name describes the rule. Two for two, and by this point the exercise feels like a formality.

The third mutant survived

Rule three is the capacity gate. It decides whether a given table is large enough for the party in front of it, and it predates the reservation work. We neutered it in the cheapest way available, by returning early so that every table fits every party.

ts
const fits = (rid: string): boolean => {
  if (party == null) return true;
  return true; // MUTANT: capacity gate removed
  const cap = p.resourceCaps.get(rid);
  if (!cap) return true;
  if (cap.seats < party) return false;
  // ... and the min/max party bounds, now unreachable
};

Nine passed. Nothing went red. And if you had stopped us in the corridor and asked which test covers table sizing, we would have pointed at this one without hesitating.

ts
it("still respects which tables fit the party", async () => {
  // Two four-seaters left, a party of four: bookable. The two-seaters must
  // not be offered to them.
  await seat(tableIds[0]);
  await seat(tableIds[1]);
  const slots = await slotsAt1900(dinnerId, 90, 4);
  expect(slots).toHaveLength(1);
  expect(slots[0].resourceIds).toEqual(expect.arrayContaining([tableIds[2], tableIds[3]]));
  expect(slots[0].resourceIds).not.toContain(tableIds[0]);
});

Read the setup rather than the assertion. The two tables seated before the check are the two-seaters. The assertion says a two-seater is not offered to a party of four, and that is true, but it is true because those tables are occupied. The busy filter removes them several lines before the capacity gate is ever consulted. The gate could return anything at all and the arrays would come out identical.

The test is not wrong. It asserts something real and it will keep asserting it. It simply passes down a different code path from the one its name implies, and the only rule it actually pins is one that already had a test of its own.

The case that would close the gap is the one we did not write: a party of four, a two-seater sitting empty on the floor, and an assertion that it is not offered to them. Nothing in our suite has ever asked that question, and nothing in the suite would have told us so. We know it because we broke a rule and the suite shrugged.

The point of mutating is not to feel good about the tests that work. It is to find the one that was never there.

The loop, on an ordinary afternoon

Most teams will never adopt a mutation testing framework, and the reasons are fair. They are slow on a real codebase, and getting one usefully configured is a project rather than an afternoon. The manual version costs a minute or two per rule, most of it spent editing rather than waiting, and it gets you the part that matters.

  1. Name the rule in one sentence. Write down the decision the code makes. If you cannot get it into a sentence, you do not have a rule, you have a paragraph, and it needs splitting before it can be tested properly.
  2. Predict which test goes red. Say the test name out loud before touching anything. This is the step people skip and it is the one doing the work, because the prediction is your actual belief about your suite. Everything after it is just checking that belief against the machine.
  3. Break exactly one thing. Invert the condition, or delete the guard entirely. One rule at a time, or a failure tells you nothing about which change caused it.
  4. Run only that file. A few seconds for us once the cache is warm, and that includes replaying every migration into an in-memory Postgres before the first assertion runs. Never the whole suite. The loop has to stay cheap enough that you bother doing it for the third rule, because the third rule is where the surprise lives.
  5. Revert from git, not from memory. Check the file out again. Never hand-undo a mutant, because a hand-undone mutant is how one eventually ships.

The failure has to match the prediction, and that is stricter than it sounds. A different test going red is a real result rather than a pass. It means the rule is load bearing somewhere you had not modelled, and usually that both tests are less specific than you believed. The same test going red on a different assertion than you expected is a result too.

Which rules deserve it

Not every test, and not most of them. Mutating a test that asserts an obvious mapping is a way to spend an afternoon confirming what you already knew. The signal is worth paying for in a few specific places.

  • A rule you wrote in the same sitting as its test. This is the fixture drift case, and it is by far the most common. If your hands touched the code and the setup on the same afternoon, the setup may have absorbed the fix.
  • A condition with more than one clause. Something like A && !B has two ways to be wrong and a test usually only reaches one of them. Delete each clause separately, not the whole line.
  • Anything a bug report produced. That test exists so a specific thing never comes back. If deleting the fix leaves it green, it is not doing that job, whatever the comment above it says.
  • Rules that filter rather than reject. A guard that throws is hard to get wrong. A filter that quietly removes candidates can be neutered and still return the right answer for the wrong reason, which is precisely what happened to our capacity gate.
  • Code you are about to refactor. Mutate first. A suite that does not notice a rule disappearing will not notice you moving it either, and refactoring under that suite is not refactoring.

The ones to skip are tests over pure shapes and mappings, where the assertion is the function written backwards. Mutating those tells you that a equals a. The interesting rules are the ones that decide whether something is offered or refused, because those are the rules a user experiences, and they are the ones that can be neutralised without anything looking broken.

What it costs against what it is worth

Our suite is several thousand tests and we are, on the whole, proud of it. That number tells you what it cost, and a large number invites you to read it as a measure of something else. It is not. Every one of those tests went green against a codebase that already worked, which is the least surprising outcome available.

The tests in this story would have shipped. They were green and reviewed, and their comments described the behaviour accurately. The only thing separating them from tests that meant something was the minute it took to break the code on purpose. Two of the three that replaced them have now been watched to fail, which is the only reason we believe them. The third has not, and that is worth more than the two that did, because it is the only thing here we did not already know.

If you have never deliberately broken your own code to see whether the suite notices, you know what your suite cost. You do not know what it is worth.

testingmutation testingcoveragevitestengineering

Building on Bookatu?

Bookatu has a public REST API and webhooks. Have a look at the developer docs.

Developer docs