Espionner une classe testée est-il une mauvaise pratique?

13

Je travaille sur un projet où les appels internes de classe sont habituels mais les résultats sont souvent des valeurs simples. Exemple ( pas de vrai code ):

public boolean findError(Set<Thing1> set1, Set<Thing2> set2) {
  if (!checkFirstCondition(set1, set2)) {
    return false;
  }
  if (!checkSecondCondition(set1, set2)) {
    return false;
  }
  return true;
}

L'écriture de tests unitaires pour ce type de code est vraiment difficile car je veux juste tester le système de conditions et non l'implémentation des conditions réelles. (Je le fais dans des tests séparés.) En fait, il serait préférable de passer des fonctions qui implémentent les conditions et dans les tests, je fournis simplement des simulations. Le problème avec cette approche est le bruit: nous utilisons beaucoup de génériques .

Une solution de travail; cependant, il faut faire de l'objet testé un espion et se moquer des appels aux fonctions internes.

systemUnderTest = Mockito.spy(systemUnderTest);
doReturn(true).when(systemUnderTest).checkFirstCondition(....);

Le problème ici est que l'implémentation du SUT est effectivement modifiée et il peut être problématique de garder les tests synchronisés avec l'implémentation. Est-ce vrai? Existe-t-il une meilleure pratique pour éviter ce ravage d'appels de méthode interne?

Notez que nous parlons de parties d'un algorithme, donc le répartir en plusieurs classes peut ne pas être une décision souhaitée.

java unit-testing allprog
la source

Réponses:

15

Les tests unitaires doivent traiter les classes qu'ils testent comme des boîtes noires. La seule chose qui compte, c'est que ses méthodes publiques se comportent comme prévu. La façon dont la classe y parvient grâce à des méthodes internes et privées n'a pas d'importance.

Lorsque vous sentez qu'il est impossible de créer des tests significatifs de cette façon, c'est un signe que vos classes sont trop puissantes et font trop. Vous devriez envisager de déplacer certaines de leurs fonctionnalités vers des classes distinctes qui peuvent être testées séparément.

Philipp
la source
1
J'ai compris l'idée des tests unitaires il y a longtemps et j'en ai écrit plusieurs avec succès. C'est juste trompeur que quelque chose a l'air simple sur papier, ça a l'air pire dans le code, et finalement je suis confronté à quelque chose qui a une interface vraiment simple mais qui m'oblige à se moquer de la moitié du monde autour des entrées.
allprog
@allprog Lorsque vous devez faire beaucoup de moqueries, il semble que vous ayez trop de dépendances entre vos classes. Avez-vous essayé de réduire le couplage entre eux?
Philipp
@allprog si vous êtes dans cette situation, la conception de la classe est à blâmer.
itsbruce
C'est le modèle de données qui cause le mal de tête. Il doit respecter les règles ORM et de nombreuses autres exigences. Avec la logique métier pure et le code sans état, il est beaucoup plus facile d'obtenir les bons tests unitaires.
allprog
3
Les tests unitaires n'ont pas nécessairement besoin de traiter le SUT comme une boîte arrière. C'est pourquoi ils sont appelés tests unitaires. En se moquant des dépendances, je peux influencer l'environnement et pour savoir ce que je dois me moquer, je dois également connaître certains des internes. Mais bien sûr, cela ne signifie pas que le SUT doit être modifié en aucune façon. Espionner, cependant, permet de faire quelques changements.
allprog
4

Si les deux findError()et checkFirstCondition()etc. sont des méthodes publiques de votre classe, alors findError()c'est effectivement une façade pour les fonctionnalités qui sont déjà disponibles à partir de la même API. Il n'y a rien de mal à cela, mais cela signifie que vous devez écrire des tests très similaires aux tests déjà existants. Cette duplication reflète simplement la duplication dans votre interface publique. Ce n'est pas une raison pour traiter cette méthode différemment des autres.

Kilian Foth
la source
Les méthodes internes sont rendues publiques simplement parce qu'elles doivent être testables et je ne veux pas sous-classer le SUT ou inclure les tests unitaires dans la classe SUT en tant que classe interne statique. Mais je comprends votre point. Cependant, je n'ai pas pu trouver de bonnes lignes directrices pour éviter ce genre de situations. Les didacticiels sont toujours bloqués au niveau de base qui n'a rien à voir avec un vrai logiciel. Sinon, la raison de l'espionnage est précisément d'éviter la duplication du code de test et de délimiter l'unité de test.
allprog
3
Je ne suis pas d'accord que les méthodes d'assistance doivent être publiques pour que les tests unitaires soient corrects. Si le contrat d'une méthode stipule qu'elle vérifie diverses conditions, il n'y a rien de mal à écrire plusieurs tests par rapport à la même méthode publique, un pour chaque "sous-traitance". Le but des tests unitaires est d'obtenir une couverture de tout le code, et non une couverture superficielle des méthodes publiques via une correspondance méthode-test 1: 1.
Kilian Foth
L'utilisation de l'API publique uniquement pour les tests est beaucoup plus complexe que de tester les pièces internes une par une. Je ne conteste pas, je comprends que cette approche n'est pas la meilleure et elle a son recul que ma question montre. Le plus gros problème est que les fonctions ne sont pas composables en Java et les solutions de contournement sont extrêmement laconiques. Mais il ne semble pas y avoir d'autre solution pour les tests unitaires réels.
allprog
4

Les tests unitaires devraient tester le contrat; c'est la seule chose importante pour eux. Tester tout ce qui ne fait pas partie du contrat n'est pas seulement une perte de temps, c'est une source potentielle d'erreur. Chaque fois qu'un développeur modifie les tests lorsqu'il modifie un détail d'implémentation, des sonneries d'alarme doivent sonner; ce développeur peut (intentionnellement ou non) cacher ses erreurs. Tester délibérément les détails de l'implémentation force cette mauvaise habitude, ce qui rend plus probable le masquage des erreurs.

Les appels internes sont un détail d'implémentation et ne devraient être utiles que pour mesurer les performances . Ce qui n'est généralement pas le travail des tests unitaires.

itsbruce
la source
Super. Mais en réalité, la "chaîne" que je dois taper et l'appeler code est dans un langage qui connaît très peu les fonctions. En théorie, je peux facilement décrire un problème et faire des substitutions ici et là pour le simplifier. Dans le code, je dois ajouter beaucoup de bruit syntaxique pour obtenir cette flexibilité qui me décourage de l'utiliser. Si la méthode acontient un appel à la méthode bdans la même classe, les tests de adoivent inclure les tests de b. Et il n'y a aucun moyen de changer cela tant qu'il bn'est pas passé aen paramètre. Mais il n'y a pas d'autre solution, je vois.
allprog
1
Si bfait partie de l'interface publique, il doit quand même être testé. Si ce n'est pas le cas, il n'a pas besoin d'être testé. Si vous l'avez rendu public simplement parce que vous vouliez le tester, vous vous êtes trompé.
itsbruce
Voir mon commentaire à la réponse de @ Philip. Je ne l'ai pas encore mentionné mais le modèle de données est la source du mal. Le code pur et sans état est un jeu d'enfant.
allprog
2

Tout d'abord, je me demande ce qui est difficile à tester sur l'exemple de fonction que vous avez écrit? Pour autant que je sache, vous pouvez simplement passer diverses entrées et vérifier que la valeur booléenne correcte est renvoyée. Qu'est-ce que je rate?

En ce qui concerne les espions, le genre de tests dits "à boîte blanche" qui utilise des espions et des simulations représente des ordres de grandeur plus de travail à écrire, non seulement parce qu'il y a tellement plus de code de test à écrire, mais chaque fois que l'implémentation est changé, vous devez également changer les tests (même si l'interface reste la même). Et ce type de test est également moins fiable que les tests en boîte noire, car vous devez vous assurer que tout ce code de test supplémentaire est correct, et même si vous pouvez être sûr que les tests unitaires en boîte noire échoueront s'ils ne correspondent pas à l'interface , vous ne pouvez pas faire confiance à cela à propos du code utilisant trop de mocks parce que parfois le test ne teste même pas beaucoup de vrai code - seulement des mocks. Si les simulations sont incorrectes, il est probable que vos tests réussiront, mais votre code est toujours cassé.

Quiconque a de l'expérience avec les tests en boîte blanche peut vous dire qu'ils ont du mal à écrire et à maintenir. Couplés au fait qu'ils sont moins fiables, les tests en boîte blanche sont tout simplement bien inférieurs dans la plupart des cas.

BT
la source
Merci pour la note. L'exemple de fonction est plus simple que tout ce que vous avez à écrire dans un algorithme complexe. En fait, la question se pose plutôt: est-il problématique de tester des algorithmes avec des espions en plusieurs parties. Ce n'est pas un code avec état, tous les états sont séparés en arguments d'entrée. Le problème est le fait que je veux tester la fonction complexe dans l'exemple sans avoir à fournir des paramètres sensés pour les sous-fonctions.
allprog
Avec l'aube de la programmation fonctionnelle dans Java 8, cela est devenu un peu plus élégant, mais conserver les fonctionnalités dans une seule classe peut être un meilleur choix dans le cas des algorithmes plutôt que d'extraire les différentes parties (seules inutiles) en «utilisation unique». classes juste à cause de la testabilité. À cet égard, les espions font la même chose que les simulacres mais sans avoir à faire exploser visuellement le code cohérent. En fait, le même code d'installation est utilisé que pour les simulacres. J'aime rester à l'écart des extrêmes, chaque type de test peut être approprié dans des endroits spécifiques. Tester d'une manière ou d'une autre est bien meilleur que de toute façon. :)
allprog
"Je veux tester la fonction complexe .. sans avoir à fournir des paramètres sensés pour les sous-fonctions" - je ne comprends pas ce que vous voulez dire. Quels sous-fonctions? Parlez-vous des fonctions internes utilisées par la «fonction complexe»?
BT
C'est pour ça que l'espionnage est utile dans mon cas. Les fonctions internes sont assez complexes à contrôler. Pas à cause du code, mais parce qu'ils implémentent quelque chose de logiquement complexe. Déplacer des éléments dans une classe différente est une option naturelle, mais ces fonctions seules ne sont pas du tout utiles. Par conséquent, garder la classe ensemble et la contrôler par la fonctionnalité d'espionnage s'est avéré être une meilleure option. Fonctionne parfaitement depuis près d'un an et pourrait facilement résister aux changements de modèle. Je n'ai pas utilisé ce modèle depuis, je pensais juste de mentionner qu'il est viable dans certains cas.
allprog
@allprog "logiquement complexe" - Si c'est complexe, vous avez besoin de tests complexes. Il n'y a aucun moyen de contourner cela. Les espions vont simplement rendre les choses plus difficiles et plus complexes pour vous. Vous devez créer des sous-fonctions compréhensibles que vous pouvez tester par vous-même, plutôt que d'utiliser des espions pour tester leur comportement spécial dans une autre fonction.
BT