02Ingénierie5 min

Le test qui disait vert et mentait

Six SCRIPT ERROR dans un test qui affichait TOUS LES TESTS OK et sortait avec le code 0. Comment on découvre qu'un vert est faux et ce que je fais maintenant avant de m'y fier.

Par Salva Castells

Image de couverture : Roland Tanglao, CC BY 2.0.
Dans cet article

Un test d'un jeu multijoueur sous Godot affichait « TODOS LOS TESTS OK » et sortait avec le code 0. Dans cette même exécution, il y avait six SCRIPT ERROR. Deux d'entre eux disaient Compilation failed et un autre Trying to assign value of type 'CanvasLayer' to a variable of type 'character_select_panel.gd'. Le runner regardait le code de sortie, le code de sortie valait 0, et personne ne regardait plus loin.

Un test qui ne peut pas échouer est pire que pas de test du tout. Sans test, vous savez que vous ne savez pas. Avec un faux vert, vous croyez savoir.

Six erreurs à l'intérieur d'un vert

Le bloc qui échouait instanciait toute la scène du niveau pour compter les objets ramassables qu'elle déclare. Cette scène entraîne deux zones d'UI dont les scripts utilisent des autoloads qui, en lançant le moteur avec --script, n'existent pas. Les scripts ne compilaient pas, le nœud sortait en CanvasLayer au lieu du type attendu, et Godot crachait les erreurs sur stderr sans faire tomber le processus.

Le plus vilain n'est pas l'erreur, c'est que le test continuait et que les assertions suivantes étaient évaluées sur un arbre à moitié construit.

Le correctif a été d'arrêter d'instancier. La scène est lue avec PackedScene.get_state(), qui donne les noms de nœuds, l'instance= et les propriétés surchargées sans exécuter le moindre _ready ni charger les scripts des zones. La précondition reste ancrée au fichier de production : si quelqu'un retire un objet du niveau, le test passe au rouge.

L'assertion qui était une tautologie

Ce même test avait un autre problème, pire, parce qu'il ne fait pas de bruit sur stderr. La précondition disait « le niveau apporte N modèles » avec N dérivé de la scène elle-même, puis vérifiait que la conversion renvoyait N. Ça passe toujours. Ça ne vérifie pas qu'il convertit les modèles présents, ça vérifie que le nombre que vous venez de lire est égal à lui-même.

Maintenant ce nombre est écrit à la main :

const PLAYGROUND_TEMPLATE_COUNT := 7  # les 7 pickups placés dans l'éditeur

Et chaque payload converti est apparié un à un avec sa déclaration du .tscn par position, avec une tolérance de 1 px parce que l'hôte a déjà intégré une frame de gravité avant de les convertir, plus son type et sa quantité. Fixer le 7 à la main, c'est ce qui distingue « il convertit toutes celles qu'il y a » de « il convertit les 7 qu'il y a ».

Comment on vérifie qu'un rouge est possible

La façon de savoir si un test sert à quelque chose, c'est de casser le code exprès et de regarder s'il s'en aperçoit. Dans ce correctif, j'ai laissé écrites les trois mutations que j'ai essayées : retirer le remove_child de la fonction qui détache les modèles donnait rouge sur trois assertions ; mettre la quantité à 1 dans le payload donnait rouge sur deux ; supprimer un objet de la scène donnait rouge sur la précondition.

Trente secondes par mutation, et c'est la seule preuve réelle qu'un test protège quelque chose. Un test que vous n'avez jamais vu au rouge est une hypothèse.

La course qui faisait passer le test à deux instances

Un script lançait deux instances du jeu, hôte et client via ENet sur localhost, et capturait l'état des deux. Ça sortait vert. J'avais trois faux verts à la fois, et je les ai tous les trois reproduits avant de toucher à quoi que ce soit :

  • L'hôte mourait en cours de route et donnait 0. Le client avait déjà capturé ses frames avec un joueur local, donc le script se déclarait satisfait. Maintenant on exige en plus que l'hôte meure du SIGTERM du script lui-même ; le code mesuré est 143.
  • Le pkill -f injectait des chemins non échappés dans une ERE. Avec un + dans le chemin ça ne matchait pas et l'hôte vivait jusqu'au timeout : trois minutes au lieu de vingt secondes. Avec un . il pouvait tuer le Godot d'une autre exécution. Maintenant on tue par PID de l'enfant direct.
  • Le client devinait au numéro de frame quand l'hôte existait. Et on ne peut pas deviner, parce qu'avant d'héberger, Godot installe un OfflineMultiplayerPeer : get_unique_id() renvoie 1 et is_server() renvoie true dans le menu, y compris dans l'instance qui va être cliente. Le seul marqueur réel de « il est en train d'héberger », c'est que son JSON contienne un joueur avec is_local.

Au passage, il est ressorti que les at de l'autopilot comptent des frames dessinées, pas des secondes : avec la vsync, une fenêtre masquée ou sans focus attend le compositeur et tombe à 1-3 fps. Mesuré : 85 frames en 90 secondes. Avec --disable-vsync --max-fps 60, 51 fps stables.

Tester l'artefact que vous distribuez, pas celui que vous compilez

Le cas qui m'a coûté le plus était dans une application de bureau de transcription. La CI était au vert et publiait un .deb avec le micro mort, parce que rien n'exécutait jamais ce qu'elle construisait.

PyInstaller empaquetait le PortAudio du wheel de sounddevice, compilé sans backend PulseAudio, plus un libasound.so.2 sans ses modules alsa-lib. Dans le bundle, ça supprime les PCM pulse, pipewire et default et ne laisse que les hw: en accès exclusif. PipeWire a déjà pris le micro, donc l'enregistrement échouait avec Invalid sample rate [PaErrorCode -9997].

Ça ne se reproduisait que dans le .deb installé : en développement, l'édition de liens se fait contre le PortAudio du système, qui a bien Pulse. Le correctif a été d'exclure libportaudio, libasound et libjack du bundle, déjà exigés dans les Depends.

Ce qui a changé dans la CI compte plus que le correctif. Maintenant, avant d'empaqueter, elle vérifie qu'il n'y a pas de bibliothèques audio dans le bundle et que le binaire démarre sous xvfb. Et le .deb déclarait libc6 (>= 2.43), un plancher qui ne s'installe sur aucune Ubuntu : il venait du fait de prendre le tag GLIBC maximal du bundle, c'est-à-dire de bibliothèques copiées depuis la machine qui compile. Il décrivait le constructeur, pas l'application. Maintenant il est déclaré à la main pour ce qu'il est, une décision de release : 2.35, celle du runner.

J'ai aussi retiré le build Windows de la CI : je l'ai entièrement commenté et j'ai sorti build-windows des needs de la release. Garder au vert un artefact que personne ne testait coûtait plus que ce qu'il rapportait.

Ce que je me demande aujourd'hui devant un nouveau test

Deux choses, et il suffit qu'une seule échoue pour que je ne fasse plus confiance. La première : est-ce que je l'ai vu au rouge à cause d'un changement qui casse vraiment le comportement, et non d'un qui casse la forme ? La seconde : est-ce que la valeur attendue vient d'ailleurs que du code jugé, ou est-ce que je la dérive de lui ?

Et une troisième pour la CI : ce que vous testez doit être le fichier que quelqu'un va télécharger, pas celui qui sort de votre run.