De l'art de trancher ses tests unitaires

Lorsque j'ai commencé à faire du TDD, j'ai souvent suivi le process une classe à tester = une classe de tests unitaires et ce, pendant très longtemps.

Si par exemple je devais coder une classe nommée BikeList, j'écrivais une classe BikeListTest dans laquelle je mettais tous les tests unitaires de BikeList.

C'est une pratique très répandue chez les développeurs: je l'ai vue au sein de nombreuses équipes, dans des livres et des tutoriels en ligne.

Mais rien ne nous lie vraiment à ce modèle une classe = une classe de tests.

Voyons un cas concret avec une classe de tests qui suit l'approche fréquemment utilisée:

public class BikeListTest {
   private BikeList bikeList;
   private Bike ferrus;
   private Bike tern;

   @BeforeEach
   void setUp() {
       bikeList = new BikeList();
       ferrus = new Bike();
       tern = new Bike();
   }

   @Test
   void empty() {
       assertEquals(0, bikeList.size(), "Size of empty list should be 0.");
   }

   @Test
   void addOneToEmpty() {
       bikeList.add(ferrus);

       assertEquals(1, bikeList.size(), "Size of one bike list should be 1.");
   }

   @Test
   void addManyToEmpty() {
       bikeList.add(ferrus);
       bikeList.add(tern);

       assertTrue(bikeList.size() > 1, "Size of many bike list should be greater than 1.");
   }

   @Test
   void contains() {
       bikeList.add(ferrus);
       bikeList.add(tern);

       var scott = new Bike();

       assertTrue(bikeList.contains(ferrus), "List should contain ferrus");
       assertTrue(bikeList.contains(tern), "List should contain tern");
       assertFalse(bikeList.contains(scott), "List should not contain scott");
   }
}

Comme vous pouvez le voir, la classe testée se nomme BikeList et il y a quelques variables de type Bike que j'utilise pour créer et initialiser mes fixtures.

(Mon exemple n'a pas beaucoup de tests pour ne pas rallonger l'article mais vous pouvez supposer qu'il y a d'autres méthodes pour tester des features comme remove, isEmpty, ...)

Le code est simple à suivre...il y a un peu de duplication entre addManyToEmpty() et contains() dans la façon dont on ajoute des éléments dans la variable bikeList mais ça n'a pas l'air trop grave, n'est-ce pas ? Pourquoi ne pas laisser tous les tests ici ?

Ce qui me perturbe ici est cette variable d'instance bikeList qui n'est pas configurée de la même manière entre tous les tests: dans certains cas, elle est vide, dans d'autres elle ne contient qu'un élément et ailleurs, elle en contient deux.

De même, les variables d'instance ferrus et tern ne sont pas utilisées dans tous les tests.

Notre classe souffre de ce que l'on appelle un problème de cohésion.

La cohésion est une caractéristique qui peut être utilisée à différents niveaux. Elle définit le degré auquel les éléments d'un module sont liés. Plus clairement, il s'agit de regrouper dans un même module les éléments susceptibles d'être modifiés ensemble.

La cohésion est liée à la notion de couplage avec laquelle les développeurs sont un peu plus familiers mais elle en est différente et il ne faut donc pas les confondre.

On cherche en général à avoir une forte cohésion et un faible couplage dans notre code. On y pense parfois quand il s'agit de restructurer notre code de production pendant le refactoring et un peu moins quand il s'agit des tests. Pourtant, cela peut justement améliorer ceux-ci.

Pour améliorer nos tests, nous allons regrouper dans une même classe les tests qui utilisent les mêmes variables d'instance.

Cela veut donc dire que l'on aura trois classes:

• une avec le test empty qui utilise une liste vide
• une avec le test addOneToEmpty qui utilise une liste contenant un seul élément
• une avec les tests addManyToEmpty et contains qui utilisent une liste contenant plus d'un élément

La question qui se pose est comment nommer ces classes ?

Ma recommandation est en général de la nommer en fonction de la fixture qui va être utilisée dans les tests. Avec notre exemple, je propose:

• EmptyBikeListTest
• OneBikeListTest
• ManyBikeListTest

La fixture définit le contexte des tests et tous les tests de chaque se partagent la même fixture.

Essayons avec le premier test empty:

public class EmptyBikeListTest {
   private BikeList bikeList;

   @BeforeEach
   void setUp() {
       bikeList = new BikeList();
   }

   @Test
   void empty() {
       assertEquals(0, bikeList.size(), "Size of empty list should be 0.");
   }
}

C'est pas mal mais le nom du test fait écho avec le nom de la classe, ce n'est pas terrible. Cela vient du fait que dans la classe BikeListTest, mes tests étaient nommés en fonction de la fixture qui était utilisée dans le test et non en fonction du comportement que je testais.

Cette information étant maintenant dans le nom de la classe, il est beaucoup mieux de renommer la méthode en fonction du comportement. Et dans ce cas précis, le comportement en question est la méthode size().

Renommons également la variable bikeList en empty vu que c'est la seule liste de nos tests et cela rend plus clair qu'il n'y a rien dedans.

public class EmptyBikeListTest {
   private BikeList empty;

   @BeforeEach
   void setUp() {
       empty = new BikeList();
   }

   @Test
   void size() {
       assertEquals(0, empty.size(), "Size of empty list should be 0.");
   }
}

Voilà, c'est beaucoup mieux. Je peux facilement ajouter d'autres tests ici pour valider des features telles que isEmpty, remove dans le cas où la liste est vide.

Si on reproduit la même réflexion pour les tests restants, on obtient:

public class OneBikeListTest {
    private BikeList one;
    private Bike ferrus;

    @BeforeEach
    void setUp() {
        one = new BikeList();
        ferrus = new Bike();
        one.add(ferrus);
    }

    @Test
    void size() {
        assertEquals(1, bikeList.size(), "Size of one bike list should be 1.");
    }
}

et:

public class ManyBikeListTest {
    private BikeList many;
    private Bike ferrus;
    private Bike tern;

    @BeforeEach
    void setUp() {
        many = new BikeList();
        ferrus = new Bike();
        tern = new Bike();

        many.add(ferrus);
        many.add(tern);
    }

    @Test
    void size() {
        assertTrue(many.size() > 1, "Size of many bike list should be greater than 1.");
    }

    @Test
    void contains() {
        var scott = new Bike();

        assertTrue(many.contains(ferrus), "List should contain ferrus");
        assertTrue(many.contains(tern), "List should contain tern");
        assertFalse(many.contains(scott), "List should not contain scott");
    }
}

Je trouve à présent les tests beaucoup plus lisibles: dans chaque classe, one ne configure qu'une seule fixture utilisée par tous les tests.

En conclusion, pour découper nos tests unitaires, il est parfois plus judicieux de regrouper ensemble les tests qui partagent une même configuration.

Et si vous ne le faisiez pas déjà, pensez également à évaluer la cohésion de votre code en général.