Est-ce une bonne pratique d'exécuter des tests unitaires dans les hooks de contrôle de version?

43

D'un point de vue technique, il est possible d'ajouter des points d'ancrage pré / post-push qui exécuteront des tests unitaires avant de permettre à une validation spécifique d'être fusionnée à une branche distante par défaut.

Ma question est la suivante: est-il préférable de conserver les tests unitaires dans le pipeline de construction (donc, en introduisant des commits cassés dans le repo) ou de ne pas autoriser de "mauvais" commits.

Je me rends compte que je ne suis pas limité à ces deux options. Par exemple, je peux autoriser tous les validations vers des branches et des tests avant de pousser les validations de fusion vers un référentiel. Mais si vous devez choisir entre exactement ces deux solutions, laquelle choisir et pour quelles raisons?

shabunc
la source

Réponses:

35

Non, ce n'est pas pour deux raisons:

La vitesse

Les commits devraient être rapides. Un commit qui prend 500 ms, par exemple, est trop lent et encouragera les développeurs à s’engager avec plus de parcimonie. Etant donné que sur tout projet plus grand qu'un Hello World, vous aurez des dizaines ou des centaines de tests, leur exécution prendra trop de temps avant la validation.

Bien entendu, les choses s'aggravent pour les projets plus volumineux avec des milliers de tests exécutés à la minute près sur une architecture distribuée, ou des semaines ou des mois sur une seule machine.

Le pire, c'est qu'il n'y a pas grand chose que vous puissiez faire pour accélérer les choses. De petits projets Python comportant, par exemple, cent tests unitaires, prennent au moins une seconde pour s'exécuter sur un serveur moyen, mais souvent beaucoup plus longtemps. Pour une application C #, il faudra en moyenne quatre à cinq secondes, en raison du temps de compilation.

À partir de ce moment, vous pouvez soit payer 10 000 $ de plus pour un meilleur serveur, ce qui réduira le temps, mais pas beaucoup, ou exécuter des tests sur plusieurs serveurs, ce qui ne fera que ralentir les choses.

Tous deux paient bien lorsque vous avez des milliers de tests (ainsi que des tests fonctionnels, de système et d'intégration), ce qui vous permet de les exécuter en quelques minutes au lieu de plusieurs semaines, mais cela ne vous aidera pas pour des projets de petite envergure.

Ce que vous pouvez faire, au lieu de cela, est de:

  • Encouragez les développeurs à exécuter des tests étroitement liés au code qu'ils ont modifié localement avant d'effectuer une validation. Ils ne peuvent peut-être pas exécuter des milliers de tests unitaires, mais ils peuvent en exécuter cinq ou dix.

    Assurez-vous que trouver des tests pertinents et les exécuter est en fait facile (et rapide). Visual Studio, par exemple, est capable de détecter les tests susceptibles d'être affectés par les modifications apportées depuis la dernière exécution. D'autres IDE / plates-formes / langages / frameworks peuvent avoir des fonctionnalités similaires.

  • Gardez le commit aussi vite que possible. L'application de règles de style est acceptable, car souvent, c'est le seul moyen de le faire et parce que ces vérifications sont souvent incroyablement rapides. Effectuer une analyse statique est acceptable dès lors que cela reste rapide, ce qui est rarement le cas. Exécuter des tests unitaires n'est pas correct.

  • Exécutez des tests unitaires sur votre serveur d'intégration continue.

  • Assurez-vous que les développeurs sont automatiquement informés quand ils ont cassé la construction (ou quand les tests unitaires échouent, ce qui est pratiquement la même chose si vous considérez un compilateur comme un outil qui vérifie certaines des erreurs possibles que vous pouvez introduire dans votre code).

    Par exemple, consulter une page Web pour vérifier les dernières versions n'est pas une solution. Ils devraient être informés automatiquement . Afficher une popup ou envoyer un SMS sont deux exemples de la façon dont ils peuvent être informés.

  • Assurez-vous que les développeurs comprennent que casser la construction (ou échouer les tests de régression) n’est pas correct, et que dès que cela se produit, leur priorité est de la réparer. Peu importe qu'ils travaillent sur une fonction hautement prioritaire que leur supérieur hiérarchique a demandé à envoyer pour demain: ils ont échoué à la construction, ils doivent la réparer.

Sécurité

Le serveur qui héberge le référentiel ne doit pas exécuter de code personnalisé, tel que des tests unitaires, notamment pour des raisons de sécurité. Ces raisons ont déjà été expliquées dans CI Runner sur le même serveur de GitLab?

Si, par contre, votre idée est de lancer un processus sur le serveur de construction à partir du hook de pré-validation, cela ralentira encore plus les validations.

Arseni Mourzenko
la source
4
Je suis d’accord, c’est à quoi sert un serveur de build. Votre contrôle de source sert à gérer le code source, sans vous assurer que votre application fonctionne.
Matthew
4
Dans des cas extrêmes, les représailles peuvent être un outil de notification nécessaire lors de la rupture de la construction.
37
Une demi-seconde, c'est trop lent? C'est une goutte d'eau dans le panier comparé à un dernier regard sur ce qui est engagé, puis à la réflexion et à la saisie d'un commentaire de validation approprié.
Doval
5
@Doval: la différence est que lorsque vous jetez un dernier coup d'œil ou que vous réfléchissez au commentaire, vous êtes proactif et vous n'attendez donc pas. Ce n'est pas le temps que vous passez avant de taper le dernier caractère de votre IDE et le moment où vous pouvez recommencer à taper une fois le commit terminé, mais combien faut-il attendre. C'est pourquoi les compilateurs doivent être rapides. peu importe que vous passiez beaucoup plus de temps à lire et à écrire du code, car vous n'attendez pas alors que le code est en cours de compilation, vous êtes tenté de passer à une autre activité.
Arseni Mourzenko
10
@Thomas: il ne s'agit pas de distraction, mais de gêne. De la même manière, 100 ms. "a un impact mesurable" sur la manière dont les personnes utilisent un site Web. Même motif ici: 100 ms. n’est rien comparé au temps que vous passez à regarder une vidéo YouTube ou à démarrer votre PC: consciemment , vous ne remarquerez pas la différence entre 600 ms. et 700 ms. retard. Mais inconsciemment, cela influe sur votre comportement. De la même manière, des commits légèrement plus lents vous découragent de commettre tôt et fréquemment.
Arseni Mourzenko
41

Permettez-moi d'être le seul à être en désaccord avec mes collègues intervenants.

Ceci est connu sous le nom de Gated Check-ins dans le monde TFS, et je m'attends à ce que ce soit ailleurs. Lorsque vous essayez de vous enregistrer dans une branche avec l'enregistrement synchronisé, l'ensemble d'étagères est envoyé au serveur, ce qui garantit que vos modifications sont générées et que les tests unitaires spécifiés (en lecture) réussissent. S'ils ne le font pas, il vous avertit que vous êtes un mauvais singe qui a cassé la construction. S'ils le font, les modifications vont dans le contrôle de source (yay!).

D'après mon expérience, les enregistrements sécurisés sont l'un des processus les plus importants pour le succès des tests unitaires - et par extension de la qualité logicielle.

Pourquoi?

  • Parce que les enregistrements sécurisés forcent les gens à réparer les tests cassés. Dès que les tests cassés deviennent une tâche que les gens peuvent faire plutôt que devoir faire, ils sont déstabilisés par des ingénieurs paresseux et / ou des hommes d’affaires envahissants.
    • Plus le test est long, plus il est difficile à résoudre (et plus coûteux).
  • Parce que dès que les gens devraient exécuter les tests plutôt que doivent exécuter les tests, l' exécution des tests sont contournés par des ingénieurs paresseux / oublieux et / ou des gens d'affaires arrivistes.
  • Parce que dès que les tests unitaires ont un impact sur votre temps de validation, les gens commencent vraiment à se préoccuper de faire leurs tests unitaires . La vitesse compte. La reproductibilité compte. La fiabilité compte. L'isolement compte.

Et bien sûr, il y a l'avantage que vous avez évoqué à l'origine - lorsque vous avez des check-ins sécurisés et une suite solide de tests, chaque ensemble de modifications est "stable". Vous économisez toute la surcharge (et le potentiel d'erreur) de "quand était la dernière bonne construction?" - toutes les constructions sont assez bonnes pour se développer contre.

Oui, il faut du temps pour construire et exécuter les tests. D'après mon expérience, 5 à 10 minutes pour une application C # de bonne taille et des tests unitaires de ~ 5 000 unités. Et ça m'est égal. Oui, les gens devraient s’enregistrer fréquemment. Mais ils doivent également mettre à jour leurs tâches fréquemment, ou consulter leur courrier électronique, ou prendre un café ou des dizaines d'autres choses "ne travaillant pas avec du code" qui constituent le travail d'un ingénieur logiciel pour occuper ce temps. La vérification du mauvais code coûte beaucoup plus cher que 5 à 10 minutes.

Telastyn
la source
3
Je veux ajouter que beaucoup de projets open source ont une distinction claire entre contribution et engagement. Les raisons en sont très similaires à celles pour lesquelles il existe des enregistrements synchronisés.
JensG
Un problème important avec l’enregistrement synchronisé est qu’il empêche la résolution collaborative de problèmes de code difficile. Ceci est encore plus significatif avec les équipes distribuées.
CuriousRabbit
2
@CuriousRabbit - comment comptez-vous? Les gens s'engagent rarement en collaboration, même s'ils travaillent en collaboration. Sinon, les étagères ou les branches de tâches non datées fonctionnent bien pour cela sans entraver le reste de l'équipe.
Telastyn
@ Telastyn– Shelvesets est un nouveau terme pour moi. Je suppose que c'est une option MS VS, qui n'est pas une option pour un grand nombre de projets. Dans le domaine du développement d'applications mobiles, VS n'est pas un joueur.
CuriousRabbit
3
@ CuriousRabbit - vraiment? Une équipe répartie sur 17 heures se soucie plus d'attendre 10 minutes pour un engagement que la possibilité d'une construction cassée alors que la partie incriminée est endormie? Cela semble ... Moins qu'optimal.
Telastyn
40

Les commits devraient être rapides. Lorsque je commets du code, je souhaite qu’il soit transmis au serveur. Je ne veux pas attendre quelques minutes pendant qu'il exécute une batterie de tests. Je suis responsable de ce que je pousse sur le serveur et je n'ai besoin de personne pour me garder avec des crochets de validation.

Cela dit, une fois qu’il arrive sur le serveur, il doit être analysé, testé et construit immédiatement (ou dans un court délai). Cela m'indiquerait que les tests unitaires sont rompus, que la compilation n'a pas eu lieu ou que j'ai créé un désordre avec les outils d'analyse statique disponibles. Plus cela est fait rapidement (la construction et l’analyse), plus vite mes réactions et mes réponses sont rapides (les pensées n’ont pas complètement disparu de mon cerveau).

Donc non, ne mettez pas de tests et autres dans des hooks de commit sur le client. Si vous devez, mettez-les sur le serveur dans un post-commit (car vous n'avez pas de serveur de CI) ou sur le serveur de build de CI et avertissez-moi correctement des problèmes liés au code. Mais ne bloquez pas le commit en premier lieu.

Je devrais également souligner qu'avec certaines interprétations du développement piloté par les tests, il convient de vérifier un test unitaire qui se déclenche en premier . Ceci démontre et documente le bug est présent. Ensuite, un enregistrement ultérieur serait le code qui corrige le test unitaire. Empêcher toute vérification jusqu'à la réussite des tests unitaires réduirait la valeur effective de la vérification dans un test unitaire qui ne documente pas le problème.

Connexes: Devrais-je avoir des tests unitaires pour les défauts connus? et Quelle est l'utilité de vérifier les tests unitaires échoués?

Communauté
la source
2
Commits should run fast.Quels sont les avantages de cela? Je suis curieux parce que nous utilisons actuellement des checkins gated. Habituellement, mes checkins cumulent environ une heure de travail, donc 5 minutes d’attente ne sont pas un gros problème. En fait, j'ai constaté que c'est normalement lorsque je suis pressé que la compilation de validation est très utile pour détecter les erreurs stupides (suite à une précipitation)
Justin
1
@Justin Une attente de cinq minutes est une attente de cinq minutes, peu importe où il se trouve. L' un ne devrait pas avoir besoin de sortir les épées Nerf chaque fois que vous avez un commit. Et il n’est pas rare que je divise une heure de travail en plusieurs commits correspondant à des unités conceptuelles les unes des autres - "code de service restant", suivi de "commettre le code de la page du client servi" sous forme de deux commits distincts. (pour ne pas mentionner le tweak css comme un autre commit). Si chacune d’entre elles a pris 5 minutes, un 1/6 de mon temps est consacré à l’attente des tests. Cela conduirait à des commits de cumul plus importants, plus difficiles à suivre pour les bogues.
5 min d'attente pour exécuter des tests unitaires? Je pense que les projets sur lesquels vous travaillez doivent être divisés en composants plus petits.
user441521
@ Justement Justement catching silly mistakes (as a result of rushing). se précipiter en général est une mauvaise pratique en génie logiciel. Robert C. Martin recommande d'écrire du code comme pour une opération youtube.com/watch?v=p0O1VVqRSK0
Jerry Joseph
10

En principe, je pense qu'il est logique d'empêcher les gens d'apporter des modifications à la ligne principale qui briseraient la construction. En d’autres termes, le processus de modification de la branche principale de votre référentiel doit exiger que tous les tests passent toujours. Casser la construction est tout simplement trop coûteux en temps perdu pour que tous les ingénieurs du projet puissent faire autre chose.

Cependant, la solution particulière de commit hooks n'est pas un bon plan.

  1. Le développeur doit attendre que les tests soient exécutés lors de la validation. Si le développeur doit attendre sur son poste de travail la réussite de tous les tests, vous perdez un temps précieux en ingénierie. L'ingénieur doit pouvoir passer à la tâche suivante, même s'il devra revenir en arrière car les tests ont échoué.
  2. Les développeurs peuvent vouloir valider du code erroné dans une branche. Dans une tâche plus importante, la version du code destinée aux développeurs peut passer beaucoup de temps à ne pas passer. Évidemment, la fusion de ce code dans la ligne principale serait très mauvaise. Mais il est assez important que le développeur puisse toujours utiliser le contrôle de version pour suivre ses progrès.
  3. Il y a parfois de bonnes raisons pour ignorer le processus et contourner les tests.
Winston Ewert
la source
2
# 1 est évité en permettant aux développeurs de s'enregistrer dans une branche personnelle ou un référentiel local. Ce n'est que lorsqu'un développeur souhaite disposer de son code quelque part que les autres développeurs peuvent le voir que les tests unitaires doivent être exécutés. Comme avec le n ° 1, le n ° 2 est évité par les seuls crochets aux branches principales. Le n ° 3 est évité par le fait que A) Toute fonctionnalité de ce type peut être désactivée, même si c'est fastidieux (et devrait l' être) et B) Les tests unitaires défaillants individuels peuvent être désactivés.
Brian
@ Brian, je suis tout à fait d'accord, vous pouvez faire en sorte que cela fonctionne. Mais essayer de le faire en bloquant le hook côté serveur ne va pas fonctionner.
Winston Ewert
bons points. Breaking the build is simply too costly in terms of lost time for all engineers on the projectJe suggérerais d'utiliser une sorte d'outil de notification de build pour éviter que tous les ingénieurs finissent par perdre du temps sur chaque build cassé
Jerry Joseph
3

Non, vous ne devriez pas, comme d'autres réponses l'ont souligné.

Si vous voulez avoir une base de code qui ne présente aucun test en échec, vous pouvez plutôt développer sur des branches de fonctionnalités et extraire les demandes dans le maître. Vous pouvez ensuite définir les conditions préalables pour accepter ces demandes d'extraction. L'avantage est que vous pouvez pousser très vite et que les tests sont exécutés en arrière-plan.

Yogu
la source
2

Avoir à attendre une construction réussie et des tests sur chaque engagement dans la branche principale est vraiment horrible, je pense que tout le monde est d'accord sur ce point

Mais il existe d'autres moyens de créer une branche principale cohérente. Voici une suggestion, un peu similaire dans le sens des archivages contrôlés dans TFS, mais qui peut être généralisée à tout système de contrôle de version avec des branches, bien que j'utilise principalement les termes git:

  • Avoir une branche intermédiaire dans laquelle vous ne pouvez commettre que des fusions entre vos branches dev et la branche principale

  • Configurez un point d'ancrage qui démarre ou met en file d'attente une construction et teste les validations effectuées sur la branche intermédiaire, mais cela ne fait pas attendre le commetteur

  • Surchauffe et les tests avec succès, faire automatiquement la branche principale aller de l' avant si elle est à jour

    Remarque: ne fusionnez pas automatiquement dans la branche principale, car la fusion testée, sinon une fusion en aval du point de vue de la branche principale, peut échouer si elle est fusionnée dans la branche principale avec des validations intermédiaires.

En conséquence:

  • Interdire l'homme s'engage dans la branche principale, automatiquement si vous le pouvez, mais également dans le cadre du processus officiel en cas d'échappatoire ou s'il n'est pas techniquement possible de l'appliquer.

    Au moins, vous pouvez vous assurer que personne ne le fera de manière involontaire ou sans malice, une fois que ce sera une règle de base. Vous ne devriez pas essayer de le faire, jamais.

  • Vous devrez choisir entre:

    • Une seule branche de transfert, qui fera que les fusions autrement réussies échouent si une fusion précédente, mais pas encore créée ni testée, échoue

      Au moins, vous saurez quelle fusion a échoué et que quelqu'un la corrige, mais les fusions entre les deux ne sont pas traçables de manière triviale (par le système de contrôle de version) pour les résultats de la construction et des tests ultérieurs.

      Vous pouvez consulter les annotations de fichiers (ou blammer), mais parfois, une modification dans un fichier (par exemple, la configuration) génère des erreurs dans des endroits inattendus. Cependant, c'est un événement plutôt rare.

    • Plusieurs branches de transfert, ce qui permettra aux fusions réussies sans conflit d'accéder à la branche principale

      Même dans le cas où une autre branche intermédiaire possède des fusions défaillantes non conflictuelles . La traçabilité est un peu meilleure, moins le cas où une fusion ne s'attendait pas à un changement affectant d'une autre fusion. Mais encore une fois, c’est assez rare pour ne pas s’inquiéter de chaque jour ou de chaque semaine.

      Pour que les fusions ne soient pas en conflit la plupart du temps, il est important de séparer les branches de planification de manière judicieuse, par exemple, par équipe, par couche ou par composant / projet / système / solution (quel que soit le nom choisi).

      Si la branche principale a été transmise entre-temps à une autre fusion, vous devez fusionner à nouveau. Espérons que ce n’est pas un problème de fusions non conflictuelles ou de très peu de conflits.

Par rapport aux enregistrements synchronisés, l’avantage ici est que vous avez la garantie d’une branche principale opérationnelle, car la branche principale est uniquement autorisée à aller de l’avant, pas à fusionner automatiquement vos modifications avec les modifications effectuées entre celles-ci. Donc, le troisième point est la différence essentielle.

acélent
la source
Toutefois, le fait d’avoir une branche en activité n’est pas (généralement) d’assurer une version stable, mais de réduire le fouillis causé par le fait que les développeurs travaillent ensemble dans le même code.
Telastyn
Pas si vous avez un stock énorme avec de grandes équipes réparties dans le monde entier. Par exemple, Microsoft utilise une approche similaire, plus en couches, avec Windows.
acelent
2

Je préfère que "tests unitaires réussis" soit une porte pour la soumission de code. Cependant, pour que cela fonctionne, vous aurez besoin de quelques choses.

Vous aurez besoin d'un framework de construction qui mette en cache les artefacts.

Vous aurez besoin d'une infrastructure de test qui mette en cache le statut des tests exécutés (avec succès), avec n'importe quel artefact donné.

De cette façon, les archivages avec réussite des tests unitaires seront rapides (vérification croisée de la source sur l'artefact construit lors de la vérification des tests par le développeur), ceux dont les tests unitaires ont échoué seront bloqués et les développeurs qui oubliez pas de vérifier que les compilations avant de commettre seront encouragées à s'en souvenir la prochaine fois, car le cycle de compilation et de test est long.

Vatine
la source
1

Je dirais que cela dépend du projet et de la portée des tests automatisés exécutés sur le "commit".

Si les tests que vous souhaitez exécuter dans le déclencheur d’enregistrement sont très rapides, ou si le flux de travail du développeur va forcer un peu d’administration après un tel enregistrement, je pense que cela ne devrait pas avoir beaucoup d’importance et forcer les développeurs à vérifier uniquement dans des choses qui exécute absolument les tests les plus élémentaires. (Je suppose que vous n'exécuteriez que les tests les plus élémentaires sur un tel déclencheur.)

Et je pense que, si la vitesse et le flux de travail le permettent, c’est une bonne chose de ne pas transmettre les modifications aux autres développeurs qui échouent aux tests - et vous savez seulement si elles échouent si vous les exécutez.

Vous écrivez "commettre ... dans une branche distante" dans la question, ce qui impliquerait pour moi que ce n'est (a) pas quelque chose qu'un développeur fait toutes les quelques minutes, donc une petite attente peut être très bien acceptable, et (b) qu'après Une telle validation des modifications de code peut avoir un impact sur d'autres développeurs, de sorte que des vérifications supplémentaires peuvent être nécessaires.

Je suis d’accord avec les autres réponses sur "ne faites pas attendre vos développeurs en attendant" pour une telle opération.

Martin Ba
la source
1

Les commits cassés ne devraient pas être autorisés sur le tronc , car c'est le tronc qui peut entrer en production. Donc, vous devez vous assurer qu'il y a une passerelle à passer avant d'être connecté au tronc . Cependant, les commits cassés peuvent être totalement acceptables dans un repo tant qu'ils ne sont pas sur le tronc.

D'autre part, obliger les développeurs à attendre / résoudre les problèmes avant de transmettre les modifications au référentiel présente un certain nombre d'inconvénients.

Quelques exemples:

  • En TDD, il est courant de valider et de pousser des tests ayant échoué pour de nouvelles fonctionnalités avant de commencer à implémenter la fonctionnalité.
  • Il en va de même pour le signalement de bugs en validant et en repoussant un test qui a échoué.
  • Pousser un code incomplet permet à 2 personnes ou plus de travailler sur une fonctionnalité en parallèle facilement
  • Votre infrastructure de CI peut prendre son temps à vérifier, mais le développeur n'a pas à attendre
tuse
la source