10 trucs infaillibles pour rater ses tests unitaires en toutes circonstances (2/2)

Prenons un exemple : voici un programme de simulation  d’attaque « zombie » ; il y a d’un côté les zombies (à l’humour assez mordant), et de l’autre, une population saine, que ça ne fait pas rire du tout : chaque personne contaminée passe à son tour dans la catégorie zombie. On considère qu’en un jour, un seul zombie peut contaminer 10 personnes saines.

public class ZombieWarSimulator {

   /**
   * @param initialPeople initial number of healthy people
   * @param initialZombies initial number of zombies
   * @param days defining the simulation duration
   * @return new number of zombies after the number of days
   */
   public static int getZombieNumberAfterDays(int initialPeople, int initialZombies, int days) {
      int currentPeople = initialPeople;
      int currentZombies = initialZombies;

      for (int day = 0; day < days; day++) {
         // each day a zombie contaminate 10 people
         int newZombies = Math.min(currentZombies * 10, currentPeople);
         currentZombies = currentZombies + newZombies;
         currentPeople = currentPeople - newZombies;
      }

      return currentZombies;
   }
}

public class ZombieWarSimulatorTest {

   @Test
   public void simulateZDay() {
      // 3 days after 1 initial zombie
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(10000, 1, 3);
      assertThat(newZombies).isEqualTo(1331);
   }

}

Notre test est correct et valide le cas général : 3 jours après l’apparition du patient “0”, on compte 1331 personnes infectées. Mais le code fonctionne-t-il dans les cas suivants ?

• apparition simultanée de plusieurs patients “0”
• pas de patient “0”
• population entièrement contaminée
• cas du jour “Z”
• etc…

Pour avoir une bonne couverture de code, il nous faudra ajouter les cas aux limites à ce cas général :

public class ZombieWarSimulatorTest {

   @Test
   public void simulateZDay() {
      // 3 days after 1 initial  zombie
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(10000, 1, 3);
      assertThat(newZombies).isEqualTo(1331);
   }

   @Test
   public void simulateZDay2Zombies() {
      // 3 days after 2 initial zombies
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(10000, 2, 3);
      assertThat(newZombies).isEqualTo(2662);
   }

   @Test
   public void simulateNoZombie() {
      // no zombie
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(10000, 0, 3);
      assertThat(newZombies).isEqualTo(0);
   }

   @Test
   public void simulate28DaysLater() {
      // 28 days after 1 initial zombie
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(10000, 1, 28);
      assertThat(newZombies).isEqualTo(10001);
   }

   @Test
   public void simulateBeforeZDay() {
      // 1 zombie and no contamination days
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(10000, 1, 0);
      assertThat(newZombies).isEqualTo(1);
   }

}

Voilà qui est beaucoup mieux ! … Mieux, mais pas ultime. En effet, notre test en l’état manque d’élégance : nous avons beaucoup de duplication de code 🙁

Heureusement, les frameworks de test fournissent aujourd’hui des solutions de tests paramétrables, telles que  JUnitParams que j’utiliserai ici (ou encore le runner JUnit Parameterized, ou les Parameters de TestNG) :

@RunWith(JUnitParamsRunner.class)
public class ZombieWarSimulatorTest {

   @Test
   @Parameters(value = {
      "10000, 1, 3, 1331",   // 3 days after 1 initial  zombie in 10000 people
      "10000, 2, 3, 2662",   // 3 days after 2 initial  zombies in 10000 people
      "10000, 0, 3, 0",      // no initial zombies in 10000 people
      "10000, 1, 28, 10001", // 28 days after 1 initial zombie in 10000 people
      "10000, 1, 0, 1"       // 1 zombie and no contamination days in 10000 people
   })
   public void simulateZDay(int initialPeople, int initialZombies, int days, int finalZombies) {
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(initialPeople, initialZombies, days);
      assertThat(newZombies).isEqualTo(finalZombies);
   }

}

Convaincu ? C’en est tellement simple que l’on peut même « blinder » notre code, en étendant encore d’avantage notre couverture avec des cas de tests supplémentaires, pour pas plus cher !

@RunWith(JUnitParamsRunner.class)
public class ZombieWarSimulatorTest {

   @Test
   @Parameters(value = {
      "10000, 1, 3, 1331",         // 3 days after 1 initial  zombie in 10000 people
      "100000, 1, 4, 14641",       // 4 days after 1 initial  zombie in 100000 people
      "1000000, 1, 5, 161051",     // 5 days after 1 initial  zombie in 1000000 people
      "10000, 2, 3, 2662",         // 3 days after 2 initial  zombies in 10000 people
      "100000, 2, 4, 29282",       // 4 days after 2 initial  zombies in 100000 people
      "1000000, 2, 5, 322102",     // 5 days after 2 initial  zombies in 1000000 people
      "10000, 3, 3, 3993",         // 3 days after 3 initial  zombies in 10000 people
      "100000, 3, 4, 43923",       // 4 days after 3 initial  zombies in 100000 people
      "1000000, 3, 5, 483153",     // 5 days after 3 initial  zombies in 1000000 people
      "10000, 0, 3, 0",            // no initial zombies, 3 days later
      "10000, 0, 4, 0",            // no initial zombies, 4 days later
      "10000, 0, 5, 0",            // no initial zombies, 5 days later
      "10000, 1, 28, 10001",       // 28 days after 1 initial zombie in 10000 people
      "10000, 1, 280, 10001",      // 280 days after 1 initial zombie in 10000 people
      "10000, 1, 2800, 10001",     // 2800 days after 1 initial zombie in 10000 people
      "10000, 1, 0, 1",            // 1 zombie and no contamination days
      "10000, 2, 0, 2",            // 2 zombies and no contamination days
      "10000, 3, 0, 3"             // 3 zombies and no contamination days
   })
   public void simulateZDay(int initialPeople, int initialZombies, int days, int finalZombies) {
      int newZombies = ZombieWarSimulator.getZombieNumberAfterDays(initialPeople, initialZombies, days);
      assertThat(newZombies).isEqualTo(finalZombies);
   }

}

Formidable non ? Pas si sûr… En effet, les TU c’est bien, en abuser ça craint. Nous venons ici d’ajouter des cas des tests supplémentaires, qui n’ont pas augmenté notre couverture de code : en effet, ils sont redondants avec les cas précédents, ils n’apportent donc rien de plus. La situation est même pire que ça :

• Le coût récurrent de maintenance de ce test a été augmenté (si je change l’implémentation de getZombieNumberAfterDays(), je dois mettre à jour d’autant plus de valeurs de référence de  « newZombies »)
• Le test a perdu en clarté : les cas de test intéressants qu’il serait judicieux de mettre en lumière sont noyés dans un brouhaha de tests identiques
• Le temps d’exécution de mes tests est rallongé

7. Plus c’est long, plus c’est bon…

Comme nous venons de le voir, avoir une armada de tests unitaires sur son application, c’est très rassurant et sécurisant ; ça laisse supposer que l’on bénéficie d’une bonne couverture de test qui en cas de régression s’avérera salvatrice. « C’est pas faux ! »  diront certains… Mais si ça implique d’avoir des builds qui tournent pendant des plombes, cela risque fort d’être très mauvais pour la productivité et réactivité de l’équipe de développement…

Conséquences

Premièrement, les développeurs impatients, ne prennent plus le temps de lancer tous les tests avant de commiter leur code : augmentation de la fréquence des régressions sur le repository des sources, pouvant impacter toute l’équipe, et bloquer jusqu’à l’ensemble des développements en cours…

Par ailleurs, un commit « toxique» ne sera pointé du doigt par le build continu que trop tardivement ! La correction va nécessiter un effort de remise dans le contexte de la part du développeur, qui a eu largement le temps de commencer un autre développement (qu’il va devoir mettre en pause, voire même peut-être détricoter pour s’atteler à la correction du build)

Enfin, livrer un patch de production en urgence prend beaucoup plus de temps : pour construire le livrable, il faudra choisir entre

• faire un « skip » des tests (et livrer une version packagée potentiellement buggée)
• ou bien faire un build complet avec les tests, mais par conséquent, déployer le patch avec d’autant plus de délais.

Un temps global d’exécution trop long de vos tests nuira donc à leur bonne santé : il découragera les développeurs à les entretenir, et peut aboutir à terme à leur désactivation.

Alors Que faire ?

La question de la couverture

Déjà, posez-vous la question : la couverture de tests est-elle pertinente ?

• N’y a-t-il pas des doublons dans les classes/méthodes de tests ? Peut-on en supprimer ? Ou ré-factoriser ?
• N’y a-t-il pas des morceaux de code sur-testé (faut-il vraiment tester « apache-commons » et « log4j », parce que vous les utilisez dans votre application ?)
• N’y a-t-il pas même des tests coûteux et apportant peu de valeur ajoutée dont on peut se passer sans grand dommage ?
• Et surtout, peut-on alléger les jeux de données d’entrée ? Ne contiennent-ils pas des  redondances ? (ce qui diminuera le volume de données à traiter et donc les temps d’exécution)

Revue de l’architecture

Un build trop long n’est-il pas le signe d’un mauvais découpage applicatif (projet monolithique ?) A défaut de raccourcir les temps d’exécution des tests, on doit pouvoir faire des builds partiels, afin d’éviter de lancer systématiquement tous les tests (en particulier ceux qui n’ont pas de rapport avec le code modifié).

On peut donc tenter de redécouper le projet en plusieurs artefacts, ou sous-modules Maven. On dissocie ainsi en plusieurs builds (en définissant des interfaces claires entre modules, on doit pouvoir travailler sur un seul module à la fois).

Raffiner autrement les builds

Une solution alternative, qui nous évitera ce travail de découpage, consistera à ségréguer les tests par catégories pour n’en exécuter qu’une partie. On pourra ensuite utiliser les profils Maven pour définir des groupes de tests à exécuter.

Optimisations

Enfin, on pourra éventuellement chercher à optimiser les tests comme on optimise une application (optimisation d’algorithmes, factorisation, parallélisme…)

8. Accélérer les tests en parallélisant

Comme nous venons de le voir, une des actions possibles pour lutter contre des tests unitaires trop longs, c’est de les optimiser, notamment en profitant des architectures multi-cœurs des machines modernes afin de les exécuter en parallèle.

Le geste technique

Il existe désormais des Testrunners capables d’exécuter les tests unitaires en parallèle. Je prendrais, pour illustrer mes propos, l’exemple du plugin Maven Surefire qui offre des options de parallélisation :

• en exécutant les tests en parallèle sur un pool de threads

<plugins>
   [...]
   <plugin>
      <groupId>org.apache.maven.plugins</groupId>
      <artifactId>maven-surefire-plugin</artifactId>
      <version>2.16</version>
      <configuration>
         <!-- make tests classes & methods runnable in parallel -->
         <parallel>both</parallel>
         <!-- use a 10 thread thread pool -->
         <threadCount>10</threadCount>
      </configuration>
   </plugin>
   [...]
</plugins>

• ou en forkant l’exécution des tests sur plusieurs process en parallèle

<plugins>
   [...]
   <plugin>
      <groupId>org.apache.maven.plugins</groupId>
      <artifactId>maven-surefire-plugin</artifactId>
      <version>2.16</version>
      <configuration>
         <!-- run tests on a pool of 3 concurrent process -->
         <forkCount>3</forkCount>
         <!-- use a pool of JVMs -->
         <reuseForks>true</reuseForks>
      </configuration>
   </plugin>
   [...]
</plugins>

Le 2ème effet Kiss cool

Ces optimisations (par multi-threading ou forking) sont en apparence séduisantes, car non intrusives par rapport au code (les seules modifications se situent dans la configuration de Surefire dans le pom.xml). Dans la pratique, les choses ne sont pas si simples… Et oui, qui dit parallélisme dit problèmes de race conditions, deadlocks, et autres joyeusetés concurrentes ! Voici pêle-mêle, quelques exemples de problèmes que vous serez susceptibles de rencontrer…

Quand les tests couvrent du code non thread-safe…

Le problème n°1, comment peut-on tester du code non thread-safe dans un contexte d’exécution de tests que l’on souhaite multi-threadé ?  Prenons l’exemple suivant d’un cache (certes primitif et naïf je vous l’accorde mais néanmoins parfait pour cet exemple ;-)) :

public interface Cache {
   Object get(int id);
   int size();
   void flush();
}

//not thread safe impl
public class CacheImpl implements Cache {

   Map<Integer, Object> cache = new HashMap<Integer, Object>();
   int size = 0;

   public Object get(int id) {
      if (cache.get(id) == null) {
         cache.put(id, load(id));
         size++;
      }
      return cache.get(id);
   }

   protected Object load(int id) {
      // load from DB...
   }

   void flush() {
      cache = new HashMap<Integer, Object>();
      size = 0;
   }

   public int size() {
      return size;
   }

}

public class CacheTest {

   private static Cache cache = new CacheImpl();

   @Test
   public void testLoadFromCache() throws Exception {
      cache.flush();
      cache.get(1);
      assertEquals("1 object should be in cache", 1, cache.size());
   }

   @Test
   public void testLoad2FromCache() throws Exception {
      cache.flush();
      cache.get(1);
      cache.get(2);
      assertEquals("2 objects should be in cache", 2, cache.size();
   }

}

Les tests fonctionnent correctement en mode séquentiel. Néanmoins, lorsque l’on active le mode « parallel » de Surefire,  nos tests échouent…

Failed tests:

CacheTest.testLoad2FromCache:25 2 objects should be in cache expected:<2> but was:<3>

CacheTest.testLoadFromCache:18 1 object should be in cache expected:<1> but was:<3>

On ne peut cependant pas en vouloir au code de CacheImpl.java  (cette implémentation n’est pas supposée thread-safe…). C’est ici le test runner qui ne respecte plus le contrat d’utilisation de notre cache.

Alors que faire ?

Option n°1 : Rendre le code thread-safe ??!

Pourquoi ne pas modifier CacheImpl pour le rendre thread-safe ? Ce sera fait sans grande difficulté…  Mais est-ce malgré tout bien raisonnable ? Changer du code métier, juste pour accélérer les tests, avec toutes les conséquences que cela peut entraîner sur la « prod » ? (risque de deadlocks, impact sur les performances…)

Option n°2 : Rende le test thread-safe ??

Et sinon, ne peut-on pas tout simplement rendre le test CacheTest lui-même thread-safe ?

public class CacheTest {

   private static Cache cache = new CacheImpl();

   @Test
   public void testLoadFromCache() throws Exception {
      int size = 0;
      synchronized (cache) {
         cache.flush();
         cache.get(1);
         size = cache.size();
      }
      assertEquals("1 object should be in cache", 1, size);
   }

   @Test
   public void testLoad2FromCache() throws Exception {
      int size = 0;
      synchronized (cache) {
         cache.flush();
         cache.get(1);
         cache.get(2);
         size = cache.size();
      }
      assertEquals("2 objects should be in cache", 2, cache.size());
   }

}

Super me direz-vous, on a corrigé notre problème de concurrence, sans modifier le code du cache ! (le test repasse au vert…)

Mais encore une fois, est-ce bien raisonnable ? Nous venons de « ré-séquentialiser» les tests qu’on essayait de « paralléliser » ; on a rendu le code du test moins maintenable, pour un gain de performance nul :  retour à la case départ…

Option n°3 : forker les JVM pour passer les tests ???

Enfin, on peut paralléliser autrement que par multi-threading : en forkant l’exécution des tests sur plusieurs JVM, et ainsiéviter l’accès concurrent sur la même instance du cache). C’est probablement ici la solution à privilégier.

public interface Cache {
   Object get(int id);
   void flush();
   int size();
}

// thread-safe cache impl
public class CacheImpl implements Cache {

   Map<Integer, Object> cache = new HashMap<Integer, Object>();
   int size = 0;

   public synchronized Object get(int id) {
      if (cache.get(id) == null) {
         cache.put(id, load(id));
         size++;
      }
      return cache.get(id);
   }

   protected Object load(int id) {
      // load from DB...
   }

   public synchronized void flush() {
      cache = new HashMap<Integer, Object>();
      size = 0;
   }

   public synchronized int size() {
      return size;
   }

 }

public class CacheSpyImpl extends CacheImpl implements Cache {
   public static int loadNumber = 0;
   @Override
   protected Object load(int id) {
      loadNumber++;
      return super.load(id);
   }
}

public class CacheTest {

   private Cache cache = new CacheSpyImpl();

   @Before
   public void initCache() {
      CacheSpyImpl.loadNumber = 0;
      cache.flush();
   }

   @Test
   public void testLoadFromCache() throws Exception {
      cache.get(1);
      assertEquals("The object should have been loaded from DB", 1,      CacheSpyImpl.loadNumber);
   }

   @Test
   public void testReloadFromCache() throws Exception {
      cache.get(1);
      assertEquals("The cache should have been loaded from DB", 1, C     acheSpyImpl.loadNumber);
      // get from cache
      cache.get(1);
      assertEquals("The object should be in cache", 1, CacheSpyImpl.loadNumber);
   }

}

public class FnocServiceTest {

   String outputReport = "output.csv";
   FnocService service = new FnocService();

   @Test
   public void testCreateOrdersReport() throws Exception {
      // create orders report
      dumpReeport(new Order("Best of Star Acadebide"), outputReport);
      // check result from file system
      String actualReport = readFile(outputReport);
      String expectedReport = readFile("src/test/resources/order_report_cd.csv");
      assertEquals(expectedReport, actualReport);
   }

   @Test
   public void testCreateOrdersReport() throws Exception {
      // create orders report
      service.dumpReeport(new Order("Walking Beer", "World Bar Z"), outputReport);
      // check result from file system
      String actualReport = readFile(outputReport);
      String expectedReport = readFile("src/test/resources/order_report_bluray.csv");
      assertEquals(expectedReport, actualReport);
   }

}

public class FnocServiceTest {

   FnocService service = new FnocService();

   @Test
   public void testOrdersReportCD() throws Exception {
      // generate unique file name e.g /tmp/out~7549426430997112982.csv
      String outFile = File.createTempFile("out~", ".csv").getAbsolutePath();
      // create orders report
      service.dumpReeport(new Order("Best of Star Acadebide"), outFile);
      // check result from file system
      String actualReport = readFile(outFile);
      String expectedReport = readFile("src/test/resources/order_report_cd.csv");
      assertEquals(expectedReport, actualReport);
   }

   @Test
   public void testOrdersReportBluray() throws Exception {
      //generate unique file name e.g /tmp/out~54659864654654.csv
      String outFile = File.createTempFile("out~", ".csv").getAbsolutePath();
      // create orders report
      service.dumpReeport(new Order("Walking Beer", "World Bar Z"), outFile);
      // check result from file system
      String actualReport = readFile(outFile);
      String expectedReport = readFile("src/test/resources/order_report_bluray.csv");
      assertEquals(expectedReport, actualReport);
   }

}

/**
 * A JUnit4 Category for test that need to be executed in a monothread environnemnet (disable parallelisation of tests execution)
*/
public interface SequentialTestCategory { }

@Category(SequentialTestCategory.class)
public class TestComplicatedAndNotThreadSafeCode {

   public void testUnthreadsafeMethod1() {
      ...
   }

   public void testUnthreadsafeMethod2() {
      ...
   }

}

<properties>
   <test.parallel>both</test.parallel>
</properties>
...

<plugin>
   <groupId>org.apache.maven.plugins</groupId>
   <artifactId>maven-surefire-plugin</artifactId>

   <executions>
      <execution>
         <!-- thread safe tests -->
         <id>default-test</id>
         <phase>test</phase>
         <goals>
            <goal>test</goal>
         </goals>
         <configuration>
            <excludedGroups>SequentialTestCategory</excludedGroups>
            <parallel>${test.parallel}</parallel>
            <perCoreThreadCount>true</perCoreThreadCount>
            <threadCount>10</threadCount>
         </configuration>
      </execution>
      <execution>
         <!-- thread unsafe tests -->
         <id>sequential-tests</id>
         <phase>test</phase>
         <goals>
            <goal>test</goal>
         </goals>
         <configuration>
            <groups>SequentialTestCategory</groups>
            <parallel>none</parallel>
         </configuration>
      </execution>
   </executions>
</plugin>
...

/$ mvn test -Dtest.parallel=none

public class PwetterSearchTest {

   @BeforeClass
   public void static startup() {
      PwetServer.connect("mongodb://dbdev1.pwetter.net,dbdev2.pwetter.net:2500/?replicaSet=test");
   }

   @AfterClass
   public void static shutdown() {
      PwetServer.disconnect();
   }

   @Test
   public void testSearch() throws Exception {
      // Arrange
      PwetService api = PwetServer.getServiceInterface();
      api.post("Hey people ! So@t rocks !!!");
      api.post("Be smart, enjoy coding");
      api.post("So@t, what else ?");

      // Act
      List<Pwet> pwets = api.search(new KeywordFilter("So@t"));

      // Assert
      assertThat(pwets).containsExactly("Hey people ! So@t rocks !!", "So@t, what else ?");
   }

}