Clean Code avec le Kata Video Store » Refacto d’oncle Bob – Les tests

Equilibre DRY-DAMP

Dans une perspective BDD et de l’écriture de tests d’acceptation (acceptance tests), on pourrait pousser l’utilisation de méthodes helpers encore plus loin et écrire un véritable petit DSL suivant le format Given, When, Then :

    public void SingleNewRelease()
    {
      GivenRental(movie: newRelease1, daysRented: 3);
      WhenGenerateStatement();
      ThenAmountIs(9.0m);
      AndPointsAre(2);
    }

Cet exemple présente des qualités : fonction courte (4 lignes), phases AAA visibles, code expressif, niveau d’abstraction uniforme. En revanche, comme il cache plus les détails techniques, cela peut être plus difficile à déboguer et on perd le côté documentation de bas niveau.

Je vous déconseille de pousser loin la complexité d’un tel DSL. On pourrait en effet traiter le couplage temporel en forçant au niveau compilation l’ordre d’exécution Given puis When puis Then, ceci en chaînant les méthodes et en restreignant lesquelles sont possibles par l’emploi d’interfaces à la manière des fluent builders les plus perfectionnés. Cela donnerait dans le test :

    public void SingleNewRelease()
    {
      Given
        .Rental(movie: newRelease1, daysRented: 3)
      .When
        .GenerateStatement()
      .Then
        .AmountIs(9.0m)
        .And.PointsAre(2);
    }

Je trouve cela « classe ». Par contre, un tel DSL est coûteux à écrire et cela rajoute de la complexité dans le code de tests. Or, on cherche à ce que l’analyse d’un test qui échoue soit facile. Mieux vaut avoir du code de test simple, facile à comprendre et donc à maintenir, où l’on comprend comment est utilisé le SUT, ceci pour servir également de documentation de bas niveau. De même, la simplicité passe par une complexité cyclomatique basse : 1 est idéal, parfois 2 avec un forEach pour faire des tests paramétrés sans l’aide du framework de test, ce qui est le cas en JavaScript/TypeScript avec Jasmine, Jest…

On peut donc se permettre un peu de duplication dans les tests si cela favorise la simplicité tout en conservant la lisibilité et la maintenabilité. Il s’agit de chercher un équilibre entre les deux principes DRY et DAMP comme l’explique cette réponse sur Stack Overflow, avec le jeu de mots en anglais (dry-sec / damp-humide) comme moyen mnémotechnique.

Quand on développe une nouvelle fonctionnalité en TDD, le respect de la décomposition des phases Red, Green, Refactor implique de passer le plus rapidement possible de Red à Green, incluant la possibilité de faire des copier-coller, y compris dans les tests. Ce n’est qu’en phase Refactor que l’on traite les duplications et donc de l’équilibre DRY / DAMP.

Fin

Oncle Bob termine le refacto des tests lorsqu’il constate que “The tests say what they test in a pretty clear term”. Il n’est plus nécessaire d’aller plus loin. Le code des tests est suffisamment clair, clean. Il y a passé vingt minutes sur l’heure que prend tout l’exercice. C’est dire l’importance accordée aux tests, en particulier pour qu’ils soient en clean code.

Remarques

Encapsulation

Lors du refacto des asserts, oncle Bob a transformé les variables totalAmount et frequentRenterPoints en champs et les a exposés au moyen de getters pour pouvoir valider l’effet de la méthode testée. En faisant cela, on conserve un niveau d’encapsulation : les champs restent privés et exposés uniquement en lecture seule.

Malgré tout, on expose l’état interne d’un objet. Cela peut déranger d’un point de vue purement OOP dont l’encapsulation est l’un des piliers, avec le polymorphisme et l’héritage. Depuis la perspective clean code, ça l’est moins : la testabilité prime sur l’encapsulation. Oncle Bob utilise l’expression Tests trump Encapsulation et l’explique dans son article The Little Singleton.

Ceci est sujet à débat. Il existe d’ailleurs d’autres techniques plus ou moins propres pour contourner l’encapsulation dans les tests. Julien Fiaffé en présente quelques unes, plutôt pour les développeurs .NET, dans son article Unit tests and protected methods.

Test qui « pique les yeux »

Si l’on revient à notre cas, le problème est apparu dans les tests. Or, lorsque l’on tombe sur un test qui « pique les yeux », cela révèle souvent un problème dans le design du code de production. En effet, on constate que notre objet statement fait 2 choses : il calcule l’état pour un client et génère un rapport. Répartir ces responsabilités dans deux classes amène naturellement à la séparation opérée par oncle Bob dans les tests, ceci directement dans 2 classes de tests distinctes respectivement à ces 2 classes de prod.

Malgré tout, comme on est dans du refacto de legacy, les opérations réalisées par oncle Bob constituent une étape intermédiaire avant la séparation en deux classes.

Couplage temporel

Avec le design proposé à ce moment par oncle Bob, les champs totalAmount et frequentRenterPoints (et leur getter associé) n’ont de sens qu’après avoir appelé la méthode statement() qui les initialise. Oncle Bob est confronté au problème lors du refacto du test mais ne semble pas y voir un problème autre qu’une mauvaise manipulation temporaire, vite vue en jouant les tests qui échouent alors et aussi vite corrigée en remettant l’appel à statement() avant les asserts.

D’ailleurs, si l’on en revient aux trois phases AAA, on s’en rend compte rapidement : il manque la phase Act ! Soit dit en passant, si le test n’était pas passé rouge en dépit de cela, c’eût été signe de son inutilité.

Pourtant, il s’agit bien d’un problème dans le design : un couplage temporel. Il faut appeler generate() avant de pouvoir appeler getTotal(). De plus, ce n’est pas du tout explicite : il faut lire le code pour le comprendre. Enfin, heureusement que l’on appelle la méthode clearTotals() à chaque appel à generate(). Sinon, les totaux seraient doublés à chaque appel ! La séparation en 2 classes selon leur responsabilité (calcul et formatage) évoquée plus haut permet de résoudre ces problématiques.

Conclusion

Rien qu’en étudiant le refacto des tests, nous avons abordé beaucoup d’éléments de clean code :

• Améliorer la lisibilité en limitant le scroll vertical,
• Supprimer la duplication :
  • En utilisant des données de test initialisées dans le set up,
  • En anonymisant celles qui ne sont pas déterminantes pour les tests,
  • En ne gardant qu’un test d’UI, en transformant les autres en vrais tests unitaires vérifiant des données brutes, ce qui les rend moins fragiles.

Tout ceci rend les tests plus expressifs et permet de détecter des problèmes dans le design du code de production : la brèche dans l’encapsulation, le couplage temporel révélant une possible violation du SRP.

Pour ne pas faire trop d’un coup, nous verrons dans un prochain article la suite du refacto, celle du code de production. C’est là que l’on sera confronté à divers smells, à traiter par des refactos bien ciblés et en faisant un usage poussé de notre IDE.

© SOAT
Toute reproduction interdite sans autorisation de l’auteur

Pour aller plus loin

• Formation Clean Code C#
• Formation Clean Code Java
• Formation Refactoring Legacy code .Net
• Formation Refactoring Legacy code Java