Tester du code qui n'a pas été pensé pour
Dans mon article InvoiceService ne devrait pas exister, j'ai fini sur un bout de code de test.
Un test hyper simple à mettre en place, sans aucune dépendance. Normal : le code était « bien fait ». Des interfaces, des entrées et des sorties claires, de l'inversion de dépendance…
Mais ce ne serait pas honnête de dire que c'est tout le temps comme ça en entreprise.
Avouons-le : 90 % du temps, on vit avec des InvoiceService, pas avec des Invoice.
Et généralement, soit on n'a pas de tests, soit on a de mauvais tests.
Ce que j'entends par mauvais test : au moindre mouvement dans le code, il faut bouger, voire réécrire les tests.

Ce n'est pas du tout le but d'un test.
- Unitaire, vraiment ?
- InvoiceService (encore lui 👀)
- London style
- Detroit style
- Ce qu'il faut retenir
- Précisions
- Notes
Unitaire, vraiment ?#
D'ailleurs, revenons sur le test unitaire.
Il existe deux écoles1 :
- l'école de Londres, dite « mockiste » : on teste une classe à la fois, et on mocke tout ce qu'elle utilise ;
- l'école de Detroit, dite « classique » : on teste un comportement, avec les vrais objets (le plus possible, du moins).
Pour l'école de Detroit, « unitaire » ne veut pas dire tester une fonction, ni une classe, mais un comportement.
On veut vérifier que notre comportement fonctionne, peu importe, finalement, comment le code a été écrit.
Au fond, l'étiquette compte peu : unitaire ou intégration.
La vraie question, c'est : est-ce que mon test vérifie un comportement, ou la façon dont le code est écrit ?
C'est ce que je vais essayer de décrire dans cet article : comment écrire des tests qu'on ne réécrit pas tous les jours, et qui nous couvrent justement le jour où on voudra refactorer.
Car c'est ça, le but du test : pouvoir refactorer autant de fois qu'il le faut, avec un retour hyper rapide.
Test vert, ou test rouge.
InvoiceService (encore lui 👀)#
Tout le code de cet article, tests compris, est dans le repo article-invoice-legacy-test.
Analysons ce que fait notre InvoiceService.
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;
}
En regardant cette fonction, assez commune dans les projets, on voit 4 dépendances : le repository des clients, l'API de TVA, le logger et l'entity manager.
Et même une cinquième : le repository des factures, inutilisé ici, mais demandé par le constructeur.
London style#
Si je devais faire un test de création de facture avec des mocks (London style), ça donnerait :
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());
}
Ce qu'on peut affirmer avec ce test :
- le total est bien calculé ;
computeVatreçoit le bon montant et le bon pays ;- la TVA vaut ce qu'on a demandé au mock de renvoyer… bon ;
persistetflushont été appelés (pas que la facture a été enregistrée) ;- un log a été écrit, avec ce texte-là.
Très bien. Maintenant, faisons un petit refactoring : au lieu de charger tout le client pour lire son pays, on ne charge que son pays, avec une nouvelle méthode findCountry() dans le repository.
$country = $this->customers->findCountry($invoice->getCustomerId()) ?? 'FR'; // changement ici
$invoice->setTotal($total);
$invoice->setVatAmount($this->taxApi->computeVat($total, $country)); // changement ici
Le code fonctionne toujours. Le test, lui :
✘ 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.
Rien de très grave, me direz-vous. Mais imaginez ça sur x appels, x mocks.
Et oui, on peut aussi résoudre ce problème avec l'IA, pour corriger le test vite.
Mais soyons honnêtes deux minutes.
Le test aurait-il dû exploser pour ce simple changement ?
Le comportement a-t-il changé ?
A-t-on cassé ce qu'on avait déjà construit ?
Non.
Detroit style#
Maintenant, faisons ce même test en 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;
}
}
Ici, on ne vérifie pas si find, persist ou flush ont été appelés.
On donne un input à createInvoice, et on interroge la base pour récupérer notre facture.
Ensuite, on vérifie ses valeurs.
Oui, avec une vraie base, c'est techniquement un test d'intégration. C'est voulu.
Avant chaque test, setUp recrée le schéma de la base2.
Pour créer le client, j'utilise un simple helper customer(). Ici, nous pourrions utiliser une factory3.
Étant donné que l'API de TVA est une dépendance externe à notre application, on fake ce qu'elle peut nous envoyer.
TaxApiClient passe par le HttpClient de Symfony : on peut donc lui brancher FakeTaxApi via la config, sans toucher au 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)]);
}
}
Et notre findCountry() ? Ce test reste vert : il ne sait pas comment on récupère le pays, il vérifie seulement la TVA enregistrée en base.
Remarquez qu'on n'a pas du tout touché à notre InvoiceService pour écrire ce test !
Mais maintenant, grâce à lui, on est capable de le refactorer sans en avoir peur.
Donc, quelle différence avec le London style ?
Ici, on ne teste que le comportement. Je pourrais même dire qu'on teste l'enveloppe qui crée la facture.
Demain, vous pouvez modifier complètement l'intérieur de la fonction createInvoice, comme ci-dessous :
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;
}
Je ne dis pas que ce code est meilleur que le précédent. Je dis juste qu'il peut être écrit différemment, sans rien casser.
$ 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.
Même comportement :
- Detroit : vert ✅
- London : rouge ❌
Et finalement, n'est-ce pas ce qu'on veut ?
Ce qu'il faut retenir#
Je n'oppose pas le Detroit style au London style. Les deux ont des buts différents.
Detroit est plutôt le choix par défaut, et il est précieux pour du refactoring et du legacy.
London, plutôt pour faire émerger la conception, et commencer à travailler sans connaître ses dépendances.
Pour le cas de notre article, Detroit a plus de sens : comme on l'a vu, il nous fallait tester le comportement, peu importe comment la fonction est construite.
Maintenant, il n'y a plus qu'à refactorer tranquillement. Et pourquoi pas vers des Invoice, comme dans mon article précédent.
Précisions#
Étant donné qu'on a besoin de vrais objets pour faire nos tests Detroit, il peut arriver que ce soit compliqué de les initialiser.
Pour cela, on peut utiliser des factories sous Symfony, et j'imagine que chaque framework a son lot d'outils de test pour ça.
Toutefois, si cela devient vraiment très compliqué, cela veut dire que la fonction fait beaucoup, beaucoup, beaucoup trop de choses. Et que le refactoring ne fera que du bien.
Notes#
Martin Fowler les compare dans Mocks Aren't Stubs. L'école de Londres a son livre de référence : Growing Object-Oriented Software, Guided by Tests, de Steve Freeman et Nat Pryce. ↩
Recréer le schéma devient lent quand les tests se multiplient. DAMA DoctrineTestBundle ouvre une transaction avant chaque test et l'annule à la fin : la base reste propre, sans rien recréer. ↩
Sous Symfony, la référence est Foundry :
CustomerFactory::createOne(['country' => 'FR'])crée le client en base, avec des valeurs par défaut pour tous les autres champs. ↩