Fichiers gargantuesques et duplications de tests

Certains fichiers Python dans le code de ZdS ont pris de l’embonpoint au cours du temps, jusqu’à atteindre plusieurs milliers de lignes ! En soi, avoir des fichiers très longs peut être acceptable, mais dans notre cas, ils posaient quelques soucis, du moins pour moi.

D’abord, cela causait un manque de réactivité des outils d’analyse temps-réel de l’IDE. Mon ordinateur n’est pas une machine guerre, sans être anémique. Quand l’IDE a besoin d’une dizaine de secondes pour reconnaître qu’un identifiant est désormais correctement défini dans le fichier, c’est qu’il y a un petit problème.

Plus gênant, il devenait difficile de naviguer dans le code sans utiliser intensivement les outils de l’IDE. Quand on se retrouve face à des dizaines de vues de quelques dizaines de lignes de code dans un fichier, on navigue avec les outils (aller à la définition, recherche des usages, etc.) jusqu’à ne plus savoir où est rangé quoi. Je me suis perdu plus d’une fois.

Un problème voisin était qu’il devenait délicat de trouver des choses dont on ne connaît pas le nom. Par exemple, si on cherche une fonctionnalité particulière et qu’on pense qu’elle se trouve quelque part parmi les 2000 lignes d’un fichier sur la base du nom de ce fichier, il faut potentiellement se résoudre à parcourir le fichier lui-même… Si on ne sait pas quels mots-clés chercher, l’IDE ne peut pas forcément beaucoup nous aider. Ce n’est pas non plus très facile d’abord pour de nouveaux contributeurs au code, qui ne peuvent pas comprendre aisément sa structure.

Pour trouver les gros fichiers sans se fatiguer à fureter dans le code, cette petite ligne de commande est fort utile :

git ls-files zds | xargs wc -l | awk '$1 > 1000' | sort -gr`

Concrètement, elle liste les fichiers versionnés dans le dossier zds (le dossier avec tous les fichiers Python du backend), compte leurs lignes, demande à awk de filtrer les fichiers trop petits (avec une limite arbitraire à 1000 lignes pour limiter le nombre de résultats) puis utilise grep pour les trier par nombre de lignes décroissant. Cette ligne pourrait probablement être ajustée, mais elle fait le boulot.

On obtient la liste suivante :

  73985 total
   5506 zds/tutorialv2/tests/tests_views/tests_content.py
   2042 zds/tutorialv2/tests/tests_views/tests_published.py
   1897 zds/forum/tests/tests_views.py
   1820 zds/member/tests/tests_views.py
   1542 zds/member/views.py
   1518 zds/tutorialv2/models/database.py
   1441 zds/tutorialv2/models/versioned.py
   1381 zds/tutorialv2/forms.py
   1213 zds/forum/tests/tests.py
   1175 zds/member/api/tests.py
   1152 zds/mp/api/tests.py

On voit qu’il y a un poids lourd avec 5 506 lignes ! Il s’en suit une dizaine de fichiers entre 1 000 et environ 2 000 lignes. On notera que la ligne total parle du total de lignes de code dans tous les fichiers, y compris ceux non affichés. On a donc un peu plus de 70 000 lignes de Python dans notre base de code pour faire tourner le site (sans compter tout Django derrière) !

Beaucoup de ces fichiers sont des fichiers de tests (ce qui ne rend pas leur taille plus pratique à gérer), mais il y a aussi quelques fichiers de code tels que views.py, database.py and versioned.py.

Atelier découpage

Découper des fichiers, c’est surtout intéressant si c’est fait de manière intelligente.

Malheureusement, une partie des fichiers est très complexe, notamment database.py et versioned.py, qui gèrent le cœur du site, à savoir les publications, j’ai préféré ne pas y toucher pour le moment, mais peut-être que cela viendra plus tard. D’autres gros fichiers ont aussi été laissés de côté, mieux vaut faire le boulot progressivement que de risquer de casser quelque chose.

Les fichiers concernés par le découpage ont ainsi été les suivants :

• zds/member/views.py, qui gère tout ce qui concerne le système de membres (login, logout, oubli de mot de passe, karma, ban et lecture seule, signalement de profils et j’en passe).
• zds/member/tests/tests_views.py qui contient les tests des fonctionalités du fichier précédent.

De manière assez intuitive, j’ai redécoupé les fonctionnalités par thème, aussi logiquement que possible et en essayant de trouver un compromis pour avoir une bonne unité des fonctionnalités tout en évitant le surdécoupage, nuisible lui aussi. Je ne dis pas que le résultat est parfait, mais c’est certainement mieux qu’avant.

On se retrouve avec un module views structuré ainsi :

• admin : tout ce qui concerne l’administration, pas très intéressant en soi.
• emailproviders.py : la gestion des fournisseurs de mails (un outil d’administration essentiellement invisible pour la plupart des utilisateurs).
• hats.py : pour ce qui concerne les casquettes.
• login.py : login, logout et associé.
• moderation.py : karma, bannissement, lecture seule…
• password_recovery.py : pour gérer les étourdis qui oublient leur mot de passe.
• profile.py : mise à jour des paramètres du profil (pseudo, mail, biographie, …).
• register.py : l’inscription ou la désinscription.
• reports.py : pour le signalement de profils.

Cela fait un sacré nombre de fichiers en plus, mais ils ne font chacun que quelques centaines de lignes et sont donc faciles à parcourir, voire même refactoriser à l’avenir.

Les tests sont réorganisés exactement pareil : chaque fichier ci-dessus se retrouve avec son fichier tests_*.py correspondant. Facile !

Atelier nettoyage

Quand j’ai farfouillé dans tests_content.py et tests_published.py et que j’ai décidé de ne pas les découper pour le moment, j’ai fait une découverte remarquable.

Dans tests_content.py, on trouve une classe de tests PublishedContentTests qui se retrouve à l’identique (à l’exception de deux tests ajoutés a posteriori) dans tests_published.py.

Je passe sur comment c’est arrivé parce que ce n’est pas le sujet, mais cela fait la modique somme de 1500 lignes de tests dupliqués qu’on peut retirer !

En plus de s’économiser des prises de têtes lors de l’ajout de tests (les deux classes dupliquées avaient commencé à diverger), on économise 2 min sur les environ 8 minutes nécessaires pour faire tourner cette partie des tests ! Si GitHub n’offrait pas la CI aux projets open source, ça ferait une petite économie financière. Quoi qu’il en soit, ce sont des calculs inutiles évités, et donc un gain environnemental en plus du gain de temps.

Résultats

Grâce à cela, on fait disparaître deux mastodontes (pas les plus gros malheureusement), et on vire 1500 lignes de code inutiles de la base de code.

  72524 total
   3577 zds/tutorialv2/tests/tests_views/tests_content.py
   2085 zds/tutorialv2/tests/tests_views/tests_published.py
   1897 zds/forum/tests/tests_views.py
   1518 zds/tutorialv2/models/database.py
   1441 zds/tutorialv2/models/versioned.py
   1389 zds/tutorialv2/forms.py
   1213 zds/forum/tests/tests.py
   1175 zds/member/api/tests.py
   1152 zds/mp/api/tests.py

La CI prend aussi environ 2 minutes de moins.

Pour les curieux, il est possible de regarder la PR ayant implémenté ces changements.