Est-il utile de tester des méthodes unitaires où la seule logique est les gardes?

12

Disons que j'ai une méthode comme celle-ci:

public void OrderNewWidget(Widget widget)
{
   if ((widget.PartNumber > 0) && (widget.PartAvailable))
   {
        WigdetOrderingService.OrderNewWidgetAsync(widget.PartNumber);
   }
}

J'ai plusieurs de ces méthodes dans mon code (la moitié avant d'un appel de service Web asynchrone).

Je me demande s'il est utile de les couvrir de tests unitaires. Oui, il y a de la logique ici, mais ce n'est qu'une logique de garde. (Cela signifie que je m'assure d'avoir les éléments dont j'ai besoin avant d'autoriser l'appel du service Web.)

Une partie de moi dit "bien sûr que vous pouvez les tester à l'unité, mais cela ne vaut pas le temps" (je suis sur un projet qui est déjà en retard).

Mais l'autre côté de moi dit, si vous ne les testez pas à l'unité et que quelqu'un change les gardes, alors il pourrait y avoir des problèmes.

Mais la première partie de moi dit en retour, si quelqu'un change les gardes, alors vous faites juste plus de travail pour eux (car maintenant ils doivent changer les gardes et les tests unitaires pour les gardes).

Par exemple, si mon service assume la responsabilité de vérifier la disponibilité des widgets, je ne veux plus de ce gardien. S'il est sous test unitaire, je dois changer deux endroits maintenant.

Je vois les avantages et les inconvénients dans les deux sens. J'ai donc pensé demander ce que les autres avaient fait.

c# unit-testing Vaccano
la source
17
Vous ne faites pas "plus de travail" pour les mainteneurs. S'ils changent de logique, ils doivent changer les tests unitaires correspondants. Voilà comment ça fonctionne. Je ne vois pas vos inconvénients: si votre test unitaire ne nécessitait pas de modification, il ne testerait rien, n'est-ce pas? Vous pourriez tout aussi bien vous demander si les tests unitaires sont utiles.
Andres
9
C'est hors sujet, mais je changerais la logique pour lever une exception si le numéro de pièce est 0 ou moins, ou si la pièce n'est pas disponible, car à mon avis, ce serait un bug pour permettre à quelqu'un d'appeler cette méthode avec un widget bidon, masquant silencieusement un autre problème.
Matthew
2
@Matthew très bon point. Cette fonction réside. La dénomination vous dit que ça va commander quelque chose. Et puis ce n'est pas le cas, mais vous ne le saurez jamais, à moins que vous n'appliquiez la même logique que celle qui est à l'intérieur, ce qui ivole DRY. En d'autres termes: si le design est modifié pour être plus correct, cette question n'aurait peut-être pas été posée en premier lieu.
stijn
2
but it is not worth the time" (I am on a project that is already behind schedule).Nous sommes des développeurs de logiciels. La seule fois où nous sommes dans les délais, c'est quand nous sommes morts :)
maple_shaft

Réponses:

26

Une partie de moi dit "bien sûr que vous pouvez les tester à l'unité, mais cela ne vaut pas le temps" (je suis sur un projet qui est déjà en retard).

Ce sont trois tests très courts. Vous avez passé autant de temps à vous poser la question.

Mais l'autre côté de moi dit, si vous ne les testez pas à l'unité et que quelqu'un change les gardes, alors il pourrait y avoir des problèmes.

Écoutez ce côté.

Mais la première partie de moi dit en retour, si quelqu'un change les gardes, alors vous faites juste plus de travail pour eux (car maintenant ils doivent changer les gardes et les tests unitaires pour les gardes).

Si votre responsable est un écrou TDD, vous lui rendez la tâche plus difficile. Tout changement que je fais sans qu'il y ait de changement ou d'ajout de tests m'amène à y réfléchir sérieusement. En fait, j'ajouterais probablement les tests avant d'aller de l'avant et de faire le changement.

La première partie de vous est tout simplement fausse. Donnez une tape dans le dos à la deuxième partie et arrêtez d'y penser.

pdr
la source
+1 pour la brièveté même si j'ai aussi écrit une réponse heh
Jimmy Hoffa
3
+1. Oui, si les exigences changent, certains tests devront être adaptés.
Olivier Jacot-Descombes
Bien que j'aime ce que vous dites, j'ai plusieurs exemples de ce type d'appel dans mon code (la moitié avant d'un appel asynchrone). Il ne s'agit donc pas seulement de 3 tests unitaires. Pourtant, si c'est la "bonne" façon, je veux les faire.
Vaccano
@Vaccano: Plus vous devez en écrire, plus vous n'avez pas de tests pour de chemins logiques et plus il est nécessaire de les écrire.
pdr
9

Cela simplifierait les tests unitaires si la logique de garde et l'ordre réel étaient des méthodes distinctes.

En Widgetclasse

public bool IsReadyForOrdering { get { return PartNumber > 0 && PartAvailable; } }

ou une méthode équivalente ailleurs

public bool IsWidgetReadyForOrdering(Widget widget)
{
    return widget.PartNumber > 0 && widget.PartAvailable;
}

La méthode de commande

public void OrderNewWidget(Widget widget)
{
   if (IsWidgetReadyForOrdering(widget)) {
        WigdetOrderingService.OrderNewWidgetAsync(widget.PartNumber);
   }
}

Maintenant, les tests sont IsWidgetReadyForOrderingdevenus faciles. N'y pensez plus longtemps. Essaye-le!

Olivier Jacot-Descombes
la source
1
Oof, logique avec état dans une propriété .. -1 qui devrait être une méthode et qui devrait prendre le PartNumber, PartAvailable en raison de sa simplicité. Faites-le protégé s'il n'a pas besoin de faire partie de l'API publique mais pas d'une propriété .. Je suis également généralement en désaccord. La logique de garde à l'intérieur de la méthode est tout à fait correcte, et à mes yeux meilleure parce que c'est une si petite méthode que vous polluez simplement la classe pour rendre une méthode déjà petite plus petite. Ajoutez-le à la classe dans ce cas uniquement s'il s'agit d'une logique répétitive.
Jimmy Hoffa
1
Tout d'abord, cela ne répond pas à la question. Deuxièmement, c'est faux. Vous devez écrire trois tests de toute façon. Troisièmement, il expose inutilement les détails de l'implémentation au code appelant.
pdr
8
@JimmyHoffa: où voyez-vous une logique avec état dans cette propriété? En fait, l'exemple montre comment séparer la logique de changement d'état (OrderNewWidget) de la logique sans changement d'état (ReadyForOrdering). Et je suis convaincu que l'extraction même de petites fonctions améliorera le code et ne l'aggravera pas, comme vous le dites. Donc +1.
Doc Brown
2
La méthode OrderNewWidgetest probablement dans une autre classe que Widget, car elle a un Widgetargument. Étant donné que la méthode n'a pas de valeur de retour, le tester n'est pas évident. Vous devez injecter un WigdetOrderingService-mock qui suit l' OrderNewWidgetAsyncappel.
Olivier Jacot-Descombes
2
@JimmyHoffa: en fait, votre définition de "sans état" signifie "statique" en C #. Ainsi, ce que vous dites est "les propriétés ne doivent pas accéder à l'état d'un objet". Mais même les getters stupides accèdent aux variables d'état, ce qui signifierait "n'écrire que des propriétés statiques" - ce qui ne semble pas avoir beaucoup de sens.
Doc Brown
6

Si vous n'avez pas de temps dans votre calendrier pour les tests unitaires mais que vous avez du temps de côté pour une utilisation solide de l'AQ, demandez si vous pouvez voler une partie de ce temps d'AQ pour écrire des tests unitaires, ou si vous pouvez passer une partie de la période d'AQ faire des tests unitaires, ou peut-être simplement traiter du code non testé unitaire .. Malheureusement, les horaires qui sont immuables vous obligent à faire des concessions ou à travailler vous-même à mort, je suggère généralement la première option car la deuxième vous rendra incapable de supporter / maintenir le système correctement pendant sa durée.

Cela dit, à votre question générale de tester les déclarations des gardes; Oui! Testez absolument les déclarations des gardes! Ce sont des parties importantes du comportement de cette méthode, vous ne voudriez pas découvrir que quelqu'un a mal compris quelque chose en corrigeant un bug et a supprimé vos gardes ou changé le &&en un le ||feriez-vous? Les tests unitaires permettront de s'assurer que a) vous avez réellement la logique de vos gardes correcte et b) personne ne casse cette logique plus tard sans se plaindre lorsqu'ils exécutent les tests unitaires en leur disant que cela devrait être le cas pour une raison quelconque.

Jimmy Hoffa
la source
0

Il y a d'excellentes réponses ci-dessus, et les points qu'elles soulèvent sont très importants. Mais celui qui semble avoir été oublié est que vous avez un ensemble complet de tests unitaires, ils se lisent comme une spécification extrêmement détaillée du code. Si vous omettez de tester la validation simplement parce que le code est si facile qu'il est difficile de voir comment cela pourrait mal tourner, une partie de vos spécifications est manquante. Si je faisais partie d'une équipe où ces tests avaient été manqués, je supposerais en fait que votre service ne validerait pas ses arguments.

Jules
la source
-5

Le code est le code. Vous devriez essayer d'obtenir une couverture à 100% lors des tests. Si ce n'était pas important, ce ne serait pas là.

SuperUser
la source