Testing code that wasn't designed for it

4 Oct 2026 10 min read tests · php · legacy

In my article InvoiceService shouldn't exist, I ended with a bit of test code.
A dead simple test to set up, with no dependencies at all. No surprise: the code was "well made". Interfaces, clear inputs and outputs, dependency inversion…

But it wouldn't be honest to say it's always like that at work.
Let's admit it: 90% of the time, we live with InvoiceServices, not with Invoices.

And usually, either we have no tests, or we have bad tests.

What I mean by a bad test: at the slightest move in the code, you have to change the tests, or even rewrite them.

That's not the point of a test at all.

• Unit, really?
• InvoiceService (again 👀)
• London style
• Detroit style
• What to remember
• Clarifications
• Notes

Unit, really?#

Speaking of which, let's go back to the unit test.

There are two schools¹:

• the London school, known as "mockist": you test one class at a time, and you mock everything it uses;
• the Detroit school, known as "classicist": you test a behavior, with the real objects (as much as possible, at least).

For the Detroit school, "unit" doesn't mean testing a function, or a class, but a behavior.

We want to check that our behavior works, no matter, in the end, how the code was written.

Deep down, the label matters little: unit or integration.
The real question is: does my test check a behavior, or the way the code is written?

That's what I'll try to describe in this article: how to write tests you don't rewrite every day, and that cover you precisely on the day you want to refactor.
Because that's the point of a test: being able to refactor as many times as needed, with super fast feedback.
Green test, or red test.

InvoiceService (again 👀)#

All the code from this article, tests included, is in the article-invoice-legacy-test repo.

Let's look at what our InvoiceService does.

public function __construct(
    private readonly InvoiceRepository $invoices,
    private readonly CustomerRepository $customers,
    private readonly TaxApiClient $taxApi,
    private readonly EntityManagerInterface $em,
    private readonly LoggerInterface $logger,
) {}

public function createInvoice(array $data): Invoice
{
    $invoice = new Invoice();
    $invoice->setCustomerId($data['customer_id'] ?? null);

    $total = 0;
    foreach ($data['lines'] ?? [] as $row) {
        $line = new InvoiceLine();
        $line->setLabel($row['label']);
        $line->setAmount($row['amount']);
        $invoice->getLines()->add($line);
        $total += $row['amount'];
    }

    $customer = $invoice->getCustomerId() !== null
        ? $this->customers->find($invoice->getCustomerId())
        : null;

    $invoice->setTotal($total);
    $invoice->setVatAmount($this->taxApi->computeVat($total, $customer?->getCountry() ?? 'FR'));

    $this->em->persist($invoice);
    $this->em->flush();
    $this->logger->info('Facture créée', ['invoice' => $invoice->getId()]);

    return $invoice;
}

Looking at this function, which is pretty common in projects, we see 4 dependencies: the customer repository, the VAT API, the logger and the entity manager.
And even a fifth one: the invoice repository, unused here, but required by the constructor.

London style#

If I had to write an invoice creation test with mocks (London style), it would look like this:

public function testCreatesADraftInvoiceWithItsLinesTotalAndVat(): void
{
    $customers = $this->createMock(CustomerRepository::class);
    $taxApi = $this->createMock(TaxApiClient::class);
    $em = $this->createMock(EntityManagerInterface::class);
    $logger = $this->createMock(LoggerInterface::class);

    $customers->expects($this->once())->method('find')->with(42)->willReturn($this->customer('FR'));
    $taxApi->expects($this->once())->method('computeVat')->with(150_000, 'FR')->willReturn(30_000);
    $em->expects($this->once())->method('persist')->with($this->isInstanceOf(Invoice::class));
    $em->expects($this->once())->method('flush');
    $logger->expects($this->once())->method('info')->with('Facture créée');

    $service = new InvoiceService(
        $this->createStub(InvoiceRepository::class),
        $customers,
        $taxApi,
        $em,
        $logger,
    );

    $invoice = $service->createInvoice([
        'customer_id' => 42,
        'lines' => [
            ['label' => 'Développement', 'amount' => 100_000],
            ['label' => 'Recette', 'amount' => 50_000],
        ],
    ]);

    self::assertSame('draft', $invoice->getStatus());
    self::assertSame(150_000, $invoice->getTotal());
    self::assertSame(30_000, $invoice->getVatAmount());
    self::assertCount(2, $invoice->getLines());
}

What we can claim with this test:

• the total is computed correctly;
• computeVat receives the right amount and the right country;
• the VAT equals what we asked the mock to return… well;
• persist and flush were called (not that the invoice was saved);
• a log was written, with that exact text.

Fine. Now, let's do a small refactoring: instead of loading the whole customer to read its country, we only load its country, with a new findCountry() method in the repository.

$country = $this->customers->findCountry($invoice->getCustomerId()) ?? 'FR'; // change here

$invoice->setTotal($total);
$invoice->setVatAmount($this->taxApi->computeVat($total, $country)); // change here

The code still works. The test, though:

 ✘ Creates a draft invoice with its lines total and vat
   Expectation failed for method name is "find" when invoked 1 time.
   Method was expected to be called 1 time, actually called 0 times.

Nothing serious, you'll tell me. But imagine that across x calls, x mocks.
And yes, you can also fix it with AI, to correct the test quickly.

But let's be honest for two minutes.
Should the test have blown up for such a simple change?
Did the behavior change?
Did we break what we had already built?

No.

Detroit style#

Now, let's write the same test, Detroit style.

final class CreateInvoiceTest extends KernelTestCase
{
    private EntityManagerInterface $em;
    private InvoiceService $service;

    protected function setUp(): void
    {
        self::bootKernel();
        $this->em = self::getContainer()->get(EntityManagerInterface::class);
        $this->service = self::getContainer()->get(InvoiceService::class);

        $schema = new SchemaTool($this->em);
        $metadata = $this->em->getMetadataFactory()->getAllMetadata();
        $schema->dropSchema($metadata);
        $schema->createSchema($metadata);
    }

    public function testCreatesADraftInvoiceWithItsLinesTotalAndVat(): void
    {
        $customer = $this->customer('FR');

        $invoice = $this->service->createInvoice([
            'customer_id' => $customer->getId(),
            'lines' => [
                ['label' => 'Développement', 'amount' => 100_000],
                ['label' => 'Recette', 'amount' => 50_000],
            ],
        ]);

        $this->em->clear();
        $saved = $this->em->find(Invoice::class, $invoice->getId());

        self::assertSame('draft', $saved->getStatus());
        self::assertSame(150_000, $saved->getTotal());
        self::assertSame(30_000, $saved->getVatAmount());
        self::assertCount(2, $saved->getLines());
    }

    private function customer(string $country): Customer
    {
        $customer = (new Customer())->setName('ACME')->setCountry($country);
        $this->em->persist($customer);
        $this->em->flush();

        return $customer;
    }
}

Here, we don't check whether find, persist or flush were called.
We give an input to createInvoice, and we query the database to get our invoice back.
Then, we check its values.

Yes, with a real database, it's technically an integration test. That's on purpose.

Before each test, setUp recreates the database schema².
To create the customer, I use a simple customer() helper. Here, we could use a factory³.

Since the VAT API is a dependency external to our application, we fake what it can send us.
TaxApiClient goes through Symfony's HttpClient: so we can plug FakeTaxApi into it through the config, without touching the code:

# config/packages/framework.yaml
when@test:
    framework:
        http_client:
            mock_response_factory: App\Tests\FakeTaxApi

# config/services_test.yaml
services:
    App\Tests\FakeTaxApi: ~

final class FakeTaxApi
{
    private const RATES = ['FR' => 20, 'DE' => 19];

    public function __invoke(string $method, string $url, array $options): JsonMockResponse
    {
        $body = json_decode($options['body'], true);

        return new JsonMockResponse(['vat' => intdiv($body['amount'] * self::RATES[$body['country']], 100)]);
    }
}

And our findCountry()? This test stays green: it doesn't know how we get the country, it only checks the VAT saved in the database.

Notice that we didn't touch our InvoiceService at all to write this test!
But now, thanks to it, we can refactor it without fear.

So, what's the difference with the London style?
Here, we only test the behavior. I could even say we test the envelope that creates the invoice.

Tomorrow, you can completely change the inside of the createInvoice function, like below:

public function createInvoice(array $data): Invoice
{
    $lines = array_map(
        fn (array $row) => (new InvoiceLine())->setLabel($row['label'])->setAmount($row['amount']),
        $data['lines'] ?? [],
    );
    $total = array_sum(array_map(fn (InvoiceLine $line) => $line->getAmount(), $lines));

    $country = $this->em->createQueryBuilder()
        ->select('c.country')
        ->from(Customer::class, 'c')
        ->where('c.id = :id')
        ->setParameter('id', $data['customer_id'] ?? 0)
        ->getQuery()
        ->getOneOrNullResult()['country'] ?? 'FR';

    $invoice = (new Invoice())
        ->setCustomerId($data['customer_id'] ?? null)
        ->setTotal($total)
        ->setVatAmount($this->taxApi->computeVat($total, $country));
    array_walk($lines, fn (InvoiceLine $line) => $invoice->getLines()->add($line));

    $this->em->wrapInTransaction(fn () => $this->em->persist($invoice));
    $this->logger->info('Facture créée', ['invoice' => $invoice->getId()]);

    return $invoice;
}

I'm not saying this code is better than the previous one. I'm just saying it can be written differently, without breaking anything.

$ php bin/phpunit --testdox

Time: 00:00.499, Memory: 48.50 MB

Create Invoice
 ✔ Creates a draft invoice with its lines total and vat
 ✔ Vat depends on the customer country
 ✔ Refuses an invoice without customer only once the database complains

Create Invoice Mocked
 ✘ Creates a draft invoice with its lines total and vat
   Expectation failed for method name is "find" when invoked 1 time.
   Method was expected to be called 1 time, actually called 0 times.

 ✘ Vat depends on the customer country
   Expectation failed for method name is "computeVat" when invoked 1 time
   Parameter 1 for invocation App\Client\TaxApiClient::computeVat(100000, 'FR'): int does not match expected value.
   -'DE'
   +'FR'

 ✘ Creates an invoice without customer with french vat
   Expectation failed for method name is "flush" when invoked 1 time.
   Method was expected to be called 1 time, actually called 0 times.

Same behavior:

• Detroit: green ✅
• London: red ❌

And in the end, isn't that what we want?

What to remember#

I'm not pitting the Detroit style against the London style. They have different goals.

Detroit is rather the default choice, and it's valuable for refactoring and legacy.
London, rather to let the design emerge, and to start working without knowing your dependencies.

In the case of this article, Detroit makes more sense: as we've seen, we needed to test the behavior, no matter how the function is built.

Now, all that's left is to refactor in peace. And why not towards Invoices, like in my previous article.

Clarifications#

Since we need real objects for our Detroit tests, setting them up can sometimes get complicated.
For that, you can use factories in Symfony, and I imagine every framework has its own set of testing tools for it.

However, if it gets really complicated, it means the function does way, way, way too many things. And refactoring will only do it good.

Notes#