The test that broke on every refactor was testing the wrong thing

TL;DR#

A test that asserts how code works fails when you change how it works — which is the definition of a refactor. So the suite turns into a tax on improvement: every rename, extraction or reordering produces red tests and none of them indicate a defect.

The usual tell is mock verification. verify(repo).findById(42) asserts that a specific collaborator was called with specific arguments in a specific order. None of that is behaviour; it is a transcript of one implementation.

Assert what a caller can observe: the return value, the state change, the message that left the system. Then a refactor that preserves behaviour is green, and that is the only signal worth having.

The case this post is about#

A use case that checks out a cart, tested the way mock-heavy suites teach:

class CheckoutUseCase(
    private val cart: CartRepository,
    private val pricing: PricingService,
    private val payments: PaymentGateway,
    private val orders: OrderRepository,
    private val email: EmailService,
)

@Test
fun checkoutChargesAndSaves() = runTest {
    whenever(cart.find("c1")).thenReturn(sampleCart)
    whenever(pricing.total(sampleCart)).thenReturn(Money(2500))
    whenever(payments.charge(Money(2500), "tok")).thenReturn(Charge("ch_1"))

    useCase.checkout("c1", "tok")

    inOrder(cart, pricing, payments, orders, email) {
        verify(cart).find("c1")
        verify(pricing).total(sampleCart)
        verify(payments).charge(Money(2500), "tok")
        verify(orders).save(any())
        verify(email).sendReceipt(any(), any())
    }
    verifyNoMoreInteractions(cart, pricing, payments, orders, email)
}

Green, thorough-looking, and it encodes five collaborators, their argument shapes, and the exact order of five calls.

The symptom#

Every refactor turned it red, with no defect. Four examples from one quarter:

caching the price lookup          → FAILED  pricing.total called 0 times (cache hit)
moving email to an event listener → FAILED  verifyNoMoreInteractions: email never called
extracting a CartLoader          → FAILED  cart.find called 0 times (now via loader)
sending the receipt before save   → FAILED  inOrder: unexpected order

In every case the behaviour a user experiences — a charge, an order, a receipt — was unchanged.

So people stopped refactoring. The 90-line checkout method stayed 90 lines, because improving it meant rewriting 40 lines of test assertions that nobody could confidently reproduce.

And then it missed a real bug. A currency rounding error shipped: the charge was correct and the saved order total was off by one cent. The test asserted orders.save(any()) — it verified that saving happened, never what was saved.

charged:  £25.00
order:    £24.99      ← test green

A test suite that fails on every harmless change and passes on a real defect has the sign of its own value inverted.

Why the obvious fix failed#

"Loosen the mocks." Replacing inOrder with unordered verify removes one coupling and keeps the rest. The cache refactor still failed, because the test asserts a call that is now legitimately skipped.

"Delete verifyNoMoreInteractions." Fixes the email-listener case and is the right call — though on its own it just makes a bad test less brittle rather than useful.

"Use verify(..., atLeastOnce()) everywhere." Now the assertions are so weak they would pass on a method that did the right calls in a wrong way. Weakening a structural assertion does not turn it into a behavioural one.

"Add an integration test alongside." We did, and it caught the rounding bug immediately — which told us what the unit test should have been doing all along rather than that we needed both.

The actual mechanism#

Two kinds of assertion, and they have opposite responses to change.

A behavioural assertion describes what a caller can observe: a returned value, a changed stored state, an emitted message, a thrown error. It is stated in the vocabulary of the problem.

A structural assertion describes how the subject achieved it: which collaborators it called, with what, in what order, how many times. It is stated in the vocabulary of the current code.

A refactor is defined as changing the structure while preserving the behaviour. So:

Behavioural assertions stay green through refactors and go red on defects. Exactly what you want.

Structural assertions go red on refactors and can stay green through defects. The correlation is with the wrong thing.

The second half of that sentence is the one worth demonstrating, because "brittle" undersells it. A structural assertion does not merely fail at the wrong times — it can pass while the code is wrong:

// the test
verify(exactly = 1) { repository.save(any()) }
verify { analytics.track("order_placed") }

// an implementation that satisfies both and is broken
fun placeOrder(order: Order) {
    repository.save(order.copy(total = Money.ZERO))   // wrong amount — `any()` accepts it
    analytics.track("order_placed")
    // …and never calls paymentGateway.charge(), which no assertion mentions
}

Both verifications pass. The order is saved with a zero total and nobody is charged. The test is green because it asked was save called once rather than is the order now recorded at the right amount and paid for — and any(), which exists to keep these tests from breaking on every field addition, is precisely what lets the wrong value through.

The omission is the sharper half. A behavioural assertion fails when a step is missing, because the observable outcome is absent. A structural one only fails for the interactions it happens to name, so forgetting to charge the card is invisible to a suite that was never told charging was part of the behaviour. Mock-based tests therefore have a failure mode in proportion to what you forgot to think of, which is the opposite of what tests are for.

And there is a reason the structural style is so tempting: mocking frameworks make verify the path of least resistance, and a subject with five injected collaborators gives you little else to assert against. The test shape follows the design.

A test is only as useful as its correlation with defects. Structural assertions correlate with edits, which is why they feel like coverage and behave like a tax.

The fix#

Replace the mocks with fakes you can inspect, and assert on observable outcomes.

// test doubles that behave, rather than record
class FakeOrderRepository : OrderRepository {
    val saved = mutableListOf<Order>()
    override suspend fun save(order: Order) { saved += order }
}

class FakePaymentGateway : PaymentGateway {
    val charges = mutableListOf<Money>()
    var nextResult: ChargeResult = ChargeResult.Ok(Charge("ch_1"))
    override suspend fun charge(amount: Money, token: String): ChargeResult {
        charges += amount
        return nextResult
    }
}

@Test
fun checkoutChargesTheCorrectTotalAndStoresAMatchingOrder() = runTest {
    val orders = FakeOrderRepository()
    val payments = FakePaymentGateway()
    val useCase = CheckoutUseCase(cart = FakeCart(sampleCart), pricing = RealPricing(),
                                  payments = payments, orders = orders, email = FakeEmail())

    val result = useCase.checkout("c1", "tok")

    assertIs<CheckoutResult.Success>(result)
    assertEquals(Money(2500), payments.charges.single())      // charged the right amount
    assertEquals(Money(2500), orders.saved.single().total)    // ← would have caught the rounding bug
    assertEquals(OrderStatus.Paid, orders.saved.single().status)
}

Four things changed:

RealPricing() instead of a mocked PricingService. Pricing is pure logic with no I/O, so using the real one makes the test exercise the actual calculation — which is where the rounding bug lived. Mocking a pure function replaces the thing you want to test with a restatement of what you expect.

Fakes expose state, not call records. orders.saved.single().total asserts the content of what was stored. verify(orders).save(any()) could never have.

No ordering assertions. The order of internal calls is not observable to a caller and not part of the contract.

And the email assertion moved to what matters:

@Test
fun aReceiptIsSentForASuccessfulCheckout() = runTest {
    val email = FakeEmail()
    useCase(email = email).checkout("c1", "tok")
    assertEquals(1, email.sent.size)
    assertEquals(Money(2500), email.sent.single().total)   // the receipt says the right thing
}

Whether the receipt is sent inline or via an event listener is now free to change.

One case where verify is the right tool, to be fair to it: when the interaction is the observable behaviour. "We must not charge the card twice" is a statement about calls:

@Test
fun aRetriedCheckoutDoesNotChargeTwice() = runTest {
    useCase.checkout("c1", "tok")
    useCase.checkout("c1", "tok")                  // same idempotency key
    assertEquals(1, payments.charges.size)         // the contract IS the call count
}

The distinction: here the call count is what the user cares about, not an implementation detail.

How we proved it#

That the new tests survive the refactors that used to break them. Each applied in turn:

refactor                            old test   new test
cache the price lookup               FAILED     PASSED
move email to an event listener      FAILED     PASSED
extract a CartLoader                 FAILED     PASSED
reorder save and receipt             FAILED     PASSED

And that they catch the defect the old ones missed. Reintroducing the rounding bug:

// pricing rounds down when building the Order but not the charge
old test (verify orders.save(any()))   PASSED    ← the shipped bug
new test (assert saved.total)          FAILED    expected Money(2500) but was Money(2499)

Those two tables together are the argument. Four harmless changes, one real defect, and the two suites disagree on every row.

Then the consequence that justified the work — the 90-line method finally got split:

CheckoutUseCase.checkout
  before:  94 lines, cyclomatic complexity 17
  after:   21 lines + 3 extracted functions, complexity 4
  tests:   unchanged, still green

Refactoring without touching a test is the property being bought, and it is the one that compounds.

One honest limit. The new suite needs hand-written fakes — about 60 lines for five collaborators — which is more code than whenever/verify. They are reusable across the module's tests, so the cost amortises, and I would not pretend the first test is cheaper to write. It is cheaper to keep.

What this generalizes to#

Judge a test by its correlation with defects. Not coverage, not assertion count, not how thorough it looks. A test that fails when nothing is broken and passes when something is has negative value, and it is usually one of the longer ones.

If it is not observable to a caller, it is not the contract. Call order, intermediate values, which collaborator did the work, how many times — none of it is promised to anyone. Asserting it converts freedom into obligation.

Prefer fakes over mocks for anything with state. A fake that records what was stored lets you assert content; a mock that records calls lets you assert shape. Content is what breaks in production.

Do not mock pure logic. Mocking a calculator replaces the computation with your assumption about it, which is exactly where the rounding bug hid. Pure functions are cheap to run for real, and running them for real is the entire point.

Related#

• runTest skips your delays, which is why the flaky test passed — another test that proved less than it looked
• commonTest runs on every target, which is the point and the cost — what a failing test is telling you
• 163 checks reported green. None of them read the published page. — checks that cannot fail

Keep reading#

This is part of 100 practices of senior engineers — the habits that separate five years of experience from one year five times. Follow MobileByteSensei for the rest.