Discussion :

[Benjamin Richner]

Le jour où j'ai commencé à croire aux tests unitaires, par Benjamin Richner

Juste après avoir obtenu mon diplôme, j'ai eu la chance d'être embauché dans un petit mais décent département de R&D en tant qu'ingénieur en logiciel embarqué. Ma première tâche a été de créer un projet de test unitaire et de le faire tourner dans notre pipeline de construction après compilation. Je n'étais pas particulièrement enthousiaste à l'idée de cette tâche, qui me semblait être une corvée. Je pensais que les tests unitaires étaient en grande partie du code jetable sans valeur. Ils ne voulaient probablement pas que je plonge immédiatement dans le code de production. Je me suis dit que c'était une bonne chose. Je suis le nouveau venu et c'est quelque chose qui est demandé par des ingénieurs bien plus expérimentés que moi. Pour l'instant, je devrais donc faire avec, même si je pense que c'est une perte de temps.

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 cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

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 cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

1. Ai-je déjà commis cette erreur ailleurs ?
2. Que se passe-t-il lorsque je corrige le bogue ?
3. 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

[esperanto]

Eh bien moi aujourd'hui je viens d'avoir une très bonne raison de ne pas croire aux tests unitaires.

Une application sur laquelle je travaille écrit sa configuration dans un fichier en utilisant une classe de sérialisation XML, plutôt ancienne.
Récemment une vulnérabilité a été détectée sur cette classe. Donc un collègue a cru bon de simplement remplacer par Jackson, qui il est vrai propose de la sérialisation.
Problème: chaque système de sérialisation a son propre format. Du coup le nouveau code n'est pas capable de lire un fichier avec l'ancien format.

Quand j'ai signalé le problème, rapport d'exception à l'appui, le collègue m'a répondu "pourtant tous les tests unitaires passent".
J'ai donc examiné la méthode où se trouvait la lecture du fichier et elle ressemble à ça:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
public Config readConfig(File source) {
    try {
          return serializer.unserialize(source);
    } catch (Exception ex) {
          return defaultConfig;
    }
}

Et voila le travail: en changeant la méthode de sérialisation, il y a une exception mais elle est absorbée immédiatement et ensuite on continue le test avec la configuration par défaut ... qui elle, passe les tests haut la main!

Alors quand je lis

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.

j'ai envie de répondre qu'à l'inverse, cet événement aurait pu (si ce n'était pas déjà le cas) m'apprendre à ne pas foncer tête baissée dans une méthodologie mal comprise sur base d'arguments marketing.

Faire des tests unitaires sur base de données de test qu'on sait par avance non représentatives, pour moi ça n'a pas de sens. Le jour où des vrais exemples arriveront, le test basé sur les anciennes données continuera à passer et on se sentira tranquille. C'est pourtant ce que les clients nous obligent souvent à faire.

Personnellement, j'ai tendance à faire des tests unitaires non pas au moment d'écrire une méthode, mais quand je m'apprête à la modifier: ça permet de vérifier rapidement que ma modification fait uniquement ce qu'elle est sensé faire, sans produire une régression. Surtout que quand ça arrive, longtemps après l'écriture initiale, j'ai eu le temps de trouver des exemples pertinents, ce qui n'était peut-être pas le cas au moment d'écrire la première version.

Maintenant, tant qu'on aura des managers pour qui les métriques comme le test coverage comptent plus que le résultat final...