Je suis donc allé de l'avant. J'ai implémenté la suite de tests et quelques tests de base, dont certains que j'ai portés de notre ancien framework de test obsolète vers googletest. Le test a été lancé par notre pipeline de compilation après la compilation, il a en fait transféré le binaire de test à un dispositif physique qui était (et est toujours) situé dans notre bureau et a exécuté le système d'exploitation QNX pour effectuer des tests sur la cible. Cela était nécessaire car le code utilisait de nombreuses primitives QNX et devait être testé sur ce système d'exploitation. Le test se situait donc entre un test unitaire et un test d'intégration. Le mécanisme a bien fonctionné et a été exécuté plusieurs fois par jour. Comme la couverture du test était assez faible et que personne ne touchait jamais à ce code de toute façon, il réussissait toujours - une autre raison pour laquelle je pensais qu'il s'agissait d'une perte de temps. De plus, il était rare que de nouveaux tests soient ajoutés. Il est très difficile d'écrire des tests unitaires pour les micrologiciels embarqués et les abstractions matérielles parce que la chose même que l'on veut tester est l'interaction avec le matériel, ce qui est précisément ce que l'on ne peut pas faire en code pur. Comme le test s'exécutait automatiquement et réussissait toujours, nous l'avons tous oublié et sommes passés à autre chose.
Un an plus tard, le test a été exécuté des centaines, voire des milliers de fois avec succès. Quelle perte de temps... Puis, un jour, nous avons commencé à observer des échecs de test. Pas beaucoup, peut-être trois en l'espace de quelques semaines. Le test s'est en fait arrêté avec un défaut de segmentation, il était donc clair qu'il s'agissait d'une erreur grave. Il est intéressant de noter qu'aucun des codes testés n'avait été modifié. C'est un point sur lequel nous devions absolument nous pencher ! Je vous épargne les détails de la recherche de l'erreur, mais finalement, j'ai pu reproduire le problème alors qu'un débogueur était connecté, de sorte que tout le contexte du problème m'a été offert sur un plateau d'argent.
Le problème était lié à la manière dont notre abstraction de threading, qui fonctionne avec l'héritage, était utilisée dans le cadre de test. Il y a une classe de base qui démarre le thread avec une fonction virtuelle et l'utilisateur de l'abstraction est supposé surcharger cette fonction, un peu comme ceci :
Code : | Sélectionner tout |
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 | /* Library code: */ class Thread { public: Thread() { /* ... */ } virtual ~Thread() { stop(); } void stop() { /* join the thread if running */ } protected: virtual void singlepassThreadWork() = 0; }; /* User code: */ class MyThread : public Thread { protected: void singlepassThreadWork() override { /* do stuff with foobar */ } private: std::vector<int> foobar; }; |
Maintenant, que se passe-t-il lorsque MyThread::singlepassThreadWork() utilise une variable membre de MyThread comme foobar et qu'on ajoute delete à l'objet MyThread alors que le thread est toujours en cours d'exécution ? La séquence de destruction est telle que MyThread est supprimé en premier et qu'ensuite, le destructeur de son objet parent Thread s'exécute et le thread est rejoint. Il y a donc une condition de course : Nous risquons d'accéder au vecteur foobar dans singlepassThreadWork() alors qu'il a déjà été supprimé. Nous pouvons corriger le code utilisateur en arrêtant explicitement le thread dans son destructeur :
Code : | Sélectionner tout |
1 2 3 4 5 6 7 8 9 10 11 | /* User code: */ class MyThread : public Thread { public: ~MyThread() { stop() } protected: void singlepassThreadWork() override { /* do stuff with foobar */ } private: std::vector<int> foobar; }; |
Ma déception était incommensurable. Le bogue se trouvait dans le cadre de test lui-même, et non dans le code testé. Les tests unitaires ne valent vraiment rien et c'est une perte de temps... N'est-ce pas ? Mais j'ai alors pensé à une bande dessinée que j'avais trouvée sur internet et que j'avais imprimée et accrochée au mur de notre bureau :
Même si cette bande dessinée est plus vieille que moi, datant d'une époque de pirates informatiques révolue depuis longtemps, elle m'a beaucoup touché parce que la sagesse qu'elle contient est toujours d'actualité. Chaque fois que vous rencontrez un bogue, posez-vous les trois questions suivantes :
- Ai-je déjà commis cette erreur ailleurs ?
- Que se passe-t-il lorsque je corrige le bogue ?
- Comment puis-je changer ma façon de faire pour rendre ce bogue impossible ?
Cela semble si simple, mais c'est une méthodologie très puissante pour prévenir d'autres erreurs et améliorer la qualité du code. Avec cette bande dessinée à l'esprit, je me suis demandé si cette erreur existait ailleurs dans le code - et j'ai trouvé un grand nombre d'exemples de cette condition de course. Il y en avait partout. Un collègue et moi-même avons passé au peigne fin l'ensemble du code et corrigé ce type d'erreur en quelques commits importants, en ajoutant un appel stop() dans le destructeur de la classe la plus basse dans la hiérarchie de l'héritage pour chaque thread. En outre, nous avons sensibilisé tous les développeurs à cet écueil et nous avons gardé un œil sur le nouveau code susceptible d'être affecté. Nous n'avons plus jamais observé ce bogue au cours des années qui ont suivi. En outre, nous savions désormais que cette abstraction basée sur l'héritage est défectueuse de par sa conception, puisque la plupart de ses utilisations souffrent d'une condition de course qui doit être atténuée manuellement par le programmeur. La conception de nouvelles abstractions ne serait pas sujette à de tels écueils car nous avons encouragé les développeurs à utiliser la composition et l'injection de dépendances plutôt que l'héritage, ce qui a permis d'améliorer considérablement la qualité globale du code.
Nous n'avons jamais découvert pourquoi la condition de course a soudainement commencé à provoquer des pannes après un an d'exécution réussie. Comme indiqué, aucun des codes concernés n'a été modifié. Pour autant que je sache, le système d'exploitation du système de test n'a pas été modifié pendant cette période. Les pannes ont dû être causées par des différences subtiles dans la façon dont les threads ont été programmés. Peut-être que l'ajout de tests non liés a augmenté la taille du binaire de test unitaire et a provoqué des effets secondaires concernant les caches et les timings du processeur ? Nous ne le saurons jamais.
Le jour où j'ai découvert cette condition de course grâce aux tests unitaires, j'ai vraiment commencé à croire en leur valeur. J'ai continué à développer le projet de test et j'ai même atteint une couverture de test de 100 % pour une bibliothèque cruciale dont nous dépendions, ce qui a permis d'éviter un désastre lorsqu'une modification du code a introduit un bogue subtil mais critique qui a été détecté par la suite de tests. Les efforts consacrés aux tests unitaires en valent la peine.
Cet événement m'a également appris à ne pas rejeter des concepts et des méthodologies de développement sur la base d'une demi-connaissance et de préjugés. En cas de doute, il suffit d'essayer. Quel est le pire qui puisse arriver ? Vous avez perdu un peu de temps. D'un autre côté, l'avantage potentiel est que vous avez ajouté un outil utile à votre boîte à outils pour le reste de votre vie.
Source : "The day I started believing in Unit Tests"
Et vous ?
Pensez-vous que l'avis de Benjamin Richner est crédible ou pertinent ?
Quel est votre avis sur le sujet ?
Voir aussi :
L'importance d'être un réviseur de code : Un cadre pour donner un feedback sur le code des autres, par Carlo Sales
Revue de code : un guide en 5 étapes pour que vos collègues vous détestent, par Mensur Durakovic
Les ingénieurs en logiciel ne sont pas (et ne devraient pas être) des techniciens, car un grand ingénieur en logiciel est celui qui automatise le travail répétitif ou manuel par Gabriella Gonzalez