Plus de tests pour avancer sereinement
Zeste de Savoir est un projet au long cours, avec presque 8 ans depuis la sortie officielle du site, encore plus depuis le début du développement, et plus encore quand on considère que la base de code est issue d’un projet antérieur. Avec le temps et la succession des développeurs, il n’est pas rare de voir apparaître quelques signes d’obsolescence, qu’il est de bon ton de corriger au fur et à mesure pour maintenir le code dans un état le plus optimal possible, sur la durée.
Cette série de billet intitulée Archéocode vise à montrer comment le code de Zeste de Savoir tente de garder une certaine jeunesse, loin des nouvelles fonctionnalités visibles pour les utilisateurs.
Dans ce quatrième billet, je parle de tests : notre politique à ce sujet, comment on en rajoute, et le temps qu’ils peuvent nous faire gagner.
Notre politique de tests
Pendant longtemps, Zeste de Savoir n’a pas eu de politique explicite au sujet des tests : ils étaient clairement bienvenus, beaucoup de développeurs en écrivaient pour leurs modifications, mais on tolérait parfois leur absence.
Il y a quelques mois, l’équipe de développement s’est mise d’accord (de mémoire sous l’impulsion de @philippemilink) pour formaliser un peu notre position vis-à-vis des tests.
En bref, les tests sont désormais obligatoires pour tout ce qui touche au backend en Python. Leur intérêt est trop grand pour éviter de casser le code, en particulier certaines parties qui sont fragiles. De plus, leur absence peut ralentir des évolutions théoriquement faciles, comme nous le verrons plus loin.
Les quelques paragraphes ci-dessous, issus de la documentation, constituent l’expression détaillée de notre politique.
La politique actuelle pour les tests du backend est que tout ce qui est écrit en Python doit être testé. Ce n’est pas entièrement le cas actuellement, mais c’est l’objectif vers lequel on tend.
En conséquence, toute mise à jour du code doit s’accompagner de la création (ou adaptation) de tests pour au minimum couvrir le comportement ajouté ou modifié. L’ajout de tests manquants mais hors du périmètre de la modification n’est pas requis.
La présence de tests adéquats est vérifiée lors de l’assurance qualité des PR. Toute fusion de PR sans tests associés est exceptionnelle et cantonnée à des situations où la balance bénéfices-risques est acceptable. Accepter une PR sans tests associés est laissé à l’appréciation de l’équipe qui fusionne les PR.
Les contributions destinées purement à l’amélioration des tests sont les bienvenues. En effet, l’historique du projet fait que certaines parties du projet sont peu, mal, ou pas testées automatiquement.
Le reste de la page dont est issu cet extrait donne également quelques astuces pour l’écriture de tests dans le cadre du projet.
À noter qu’avec une telle politique, la couverture (ou la qualité générale) des tests ne peut qu’augmenter !
Plus de tests !
Rajouter des tests en passant pour assurer l’avenir
Lors qu’on fait une refactorisation, il est intéressant d’ajouter des tests s’il en manque (ou s’il n’y en a pas du tout). Cela peut être après, pour vérifier que ce qu’on a changé marche comme prévu, ou mieux encore, juste avant de refactoriser, pour vérifier qu’on ne change pas le comportement par accident. Parfois, en écrivant les tests en vue d’une refactorisation, on trouve même des bugs !
Quoi qu’il en soit, le résultat est le même : il y a plus de tests (ou meilleurs) avant qu’après. Cela garantit que la refactorisation fonctionne bien, n’a rien cassé et qu’on ne cassera pas à l’avenir le code en question et qu’il fonctionnera bien jusqu’à ce que le besoin évolue.
Il n’est pas facile de donner de bons exemples de ça, car chaque correction de bug tend à avoir de petits ajouts en passant. Pour citer ce que je connais, il y a par exemple cette correction d’un petit bug/refacto, accompagnée de tests plus complets qu’avant, ou alors cette évolution de l’espace de rédaction avec des tests visant à être plus robustes qu’auparavant.
Tester pour mieux avancer (maintenant, ou plus tard)
Récemment, lors du développement d'une évolution que j’espère bientôt disponible, j’ai dû ajouter un comportement très simple et peu intrusif à de multiples parties du code, et normalement facile à tester.
Ce qui devait être une promenade de santé s’est révélé un travail plutôt laborieux, parce que ces parties du code n’étaient pas testées du tout ! Cela a abouti à quatre PR (#6188, #6190, #6193 et #6194), juste pour ajouter des tests. Le faire dans la PR pour la nouvelle fonctionnalité n’était pas très judicieux ; garder séparer permet en effet de garder une PR de taille raisonnable et avec un périmètre bien défini.
Quoi qu’il en soit, j’aurais tout aussi bien pu me dire « tant pis » et passer outre, et ne pas tester cet ajout. Cependant, c’est une mauvaise idée.
- Déjà, si on fait ça à chaque fois, on ne teste jamais les ajouts et on finit par avoir de gros trous dans la raquette et se retrouver dans la situation exacte dans laquelle je me suis trouvé.
- Pour cette PR spécifiquement, le comportement en question est implémenté avec le système de signaux de Django. Ça a des avantages en termes de découplage de code, mais c’est notoirement difficile à débuguer en cas de soucis. C’est vraiment une partie du comportement à blinder.1
- C’est contraire à notre politique de tests (qui est justifiée notamment par les arguments ci-dessus
).
- On utilise aussi cette technique pour les notifications sur le site. Les notifications constituent une partie très fragile du code, dure à faire évoluer sereinement, notamment parce que les tests ne sont entièrement suffisants pour aider à garantir le bon fonctionnement et que c’est très facile d’introduire des bugs discrets mais terribles.↩
J'aime nos tests !
Je parle essentiellement pour moi, mais je pense que le sentiment est partagé au sein de l’équipe : tester représente un investissement rentable, en particulier sur un projet avec des développeurs bénévoles comme Zeste de Savoir.
Investir sur les tests compense notre réactivité et disponibilités réduites. Ne pas faire assez de tests et corriger ensuite des bugs évitables est un luxe que peuvent se payer les entreprises avec des développeurs à temps plein. S’il y a un bug important, quelqu’un changera son emploi du temps pour le corriger. Sur Zeste de Savoir, faute de disponibilité, les bugs peuvent perdurer longtemps. Quand c’est vraiment grave, l’équipe fait évidemment le nécessaire. Cependant, ce qui n’est ni critique ni facile à corriger survivra des mois, voire des années.
Nos tests augmentent notre vitesse de développement. On passe finalement assez peu de temps à corriger des bugs graves (il y en a quand même !), et plus de temps à faire des évolutions. Faire des évolutions qui marchent est aussi beaucoup plus facile quand on a des tests : on progresse d’un pas plus assuré, même pour des modifications complexes, quand on a confiance dans le fait que les grosses boulettes seront détectées.
Voilà, de la poussière en moins sous le tapis ! Les tests dans le code de Zeste de Savoir me font gagner un temps fou, le temps pris pour s’en occuper est un investissement. Payons notre dette technique !
À bientôt, je l’espère, pour parler d’une autre vieillerie découverte dans le code de Zeste de Savoir et du coup de ménage associé !
