Comment tester à l'unité une fonction qui est refactorisée en modèle de stratégie?

10

Si j'ai une fonction dans mon code qui va comme:

class Employee{

    public string calculateTax(string name, int salary)
    {
        switch (name)
        {
            case "Chris":
                doSomething($salary);
            case "David":
                doSomethingDifferent($salary);
            case "Scott":
               doOtherThing($salary);               
       }
}

Normalement, je refactoriserais ceci pour utiliser le ploymorphisme en utilisant une classe d'usine et un modèle de stratégie:

public string calculateTax(string name)
{
    InameHandler nameHandler = NameHandlerFactory::getHandler(name);
    nameHandler->calculateTax($salary);
}

Maintenant, si j'utilisais TDD, j'aurais des tests qui fonctionnent sur l'original calculateTax()avant de refactoriser.

ex:

calculateTax_givenChrisSalaryBelowThreshold_Expect111(){}    
calculateTax_givenChrisSalaryAboveThreshold_Expect111(){}

calculateTax_givenDavidSalaryBelowThreshold_Expect222(){}   
calculateTax_givenDavidSalaryAboveThreshold_Expect222(){} 

calculateTax_givenScottSalaryBelowThreshold_Expect333(){}
calculateTax_givenScottSalaryAboveThreshold_Expect333(){}

Après refactoring, j'aurai une classe Factory NameHandlerFactoryet au moins 3 implémentations de InameHandler.

Comment dois-je procéder pour refactoriser mes tests? Dois-je supprimer le test unitaire claculateTax()de EmployeeTestset créer une classe de test pour chaque implémentation de InameHandler?

Dois-je également tester la classe Factory?

Songo
la source

Réponses:

6

Les anciens tests sont très bien pour vérifier que cela calculateTaxfonctionne toujours comme il se doit. Cependant, vous n'avez pas besoin de nombreux cas de test pour cela, seulement 3 (ou peut-être plus, si vous souhaitez également tester la gestion des erreurs, en utilisant des valeurs inattendues de name).

Chacun des cas individuels (actuellement mis en œuvre dans doSomethinget al.) Doit également avoir son propre ensemble de tests, qui testent les détails internes et les cas spéciaux liés à chaque implémentation. Dans la nouvelle configuration, ces tests pourraient / devraient être convertis en tests directs sur la classe de stratégie respective.

Je préfère supprimer les anciens tests unitaires uniquement si le code qu'ils exercent et la fonctionnalité qu'il implémente cessent complètement d'exister. Sinon, les connaissances encodées dans ces tests sont toujours pertinentes, seuls les tests doivent être refactorisés eux-mêmes.

Mise à jour

Il peut y avoir une certaine duplication entre les tests de calculateTax(appelons-les tests de haut niveau ) et les tests pour les stratégies de calcul individuelles ( tests de bas niveau ) - cela dépend de votre implémentation.

Je suppose que la mise en œuvre originale de vos tests affirme le résultat du calcul de la taxe spécifique, vérifiant implicitement que la stratégie de calcul spécifique a été utilisée pour le produire. Si vous conservez ce schéma, vous aurez en effet une duplication. Cependant, comme l'a laissé entendre @Kristof, vous pouvez également implémenter les tests de haut niveau en utilisant des simulations, pour vérifier uniquement que le bon type de stratégie (factice) a été sélectionné et invoqué par calculateTax. Dans ce cas, il n'y aura pas de duplication entre les tests de haut et de bas niveau.

Donc, si la refactorisation des tests concernés n'est pas trop coûteuse, je préférerais cette dernière approche. Cependant, dans la vraie vie, lors d'une refactorisation massive, je tolère une petite quantité de duplication de code de test si cela me fait gagner du temps :-)

Dois-je également tester la classe Factory?

Encore une fois, cela dépend. Notez que les tests de calculateTaxtester efficacement l'usine. Donc, si le code d'usine est un switchbloc trivial comme votre code ci-dessus, ces tests peuvent être tout ce dont vous avez besoin. Mais si l'usine fait des choses plus délicates, vous voudrez peut-être dédier des tests spécifiquement pour elle. Tout se résume à la quantité de tests dont vous avez besoin pour être sûr que le code en question fonctionne vraiment. Si, à la lecture du code - ou à l'analyse des données de couverture de code - vous voyez des chemins d'exécution non testés, dédiez quelques tests supplémentaires pour les exercer. Répétez ensuite ceci jusqu'à ce que vous ayez pleinement confiance en votre code.

Péter Török
la source
J'ai un peu modifié le code pour le rapprocher de mon code pratique actuel. Maintenant, une deuxième entrée salaryà la fonction a calculateTax()été ajoutée. De cette façon, je pense que je vais dupliquer le code de test pour la fonction d'origine et les 3 implémentations de la classe de stratégie.
Songo
@Songo, veuillez consulter ma mise à jour.
Péter Török
5

Je commencerai par dire que je ne suis pas un expert du TDD ou des tests unitaires, mais voici comment je testerais cela (j'utiliserai un code pseudo-similaire):

CalculateTaxDelegatesToNameHandler()
{
    INameHandlerFactory fakeNameHandlerFactory = Fake(INameHandlerFactory);
    INameHandler fakeNameHandler = Fake(INameHandler);

    A.Call.To(fakeNameHandlerFactory.getHandler("John")).Returns(fakeNameHandler);

    Employee employee = new Employee(fakeNameHandlerFactory);
    employee.CalculateTax("John");

    Assert.That.WasCalled(fakeNameHandler.calculateTax());
}

Je testerais donc que la calculateTax()méthode de la classe d'employé demande correctement son NameHandlerFactorypour un NameHandler, puis appelle la calculateTax()méthode du retourné NameHandler.

Kristof Claes
la source
hmmmm donc vous voulez dire que je devrais faire du test un test de comportement à la place (tester que certaines fonctions ont été appelées) et faire les affirmations de valeur sur les classes déléguées?
Songo
Oui, c'est ce que je ferais. J'écrirais en effet des tests séparés pour le NameHandlerFactory et le NameHandler. Lorsque vous en avez, il n'y a aucune raison de tester à nouveau leurs fonctionnalités dans la Employee.calculateTax()méthode. De cette façon, vous n'avez pas besoin d'ajouter des tests d'employés supplémentaires lorsque vous introduisez un nouveau NameHandler.
Kristof Claes
3

Vous prenez une classe (l'employé qui fait tout) et vous créez 3 groupes de classes: l'usine, l'employé (qui ne contient qu'une stratégie) et les stratégies.

Faites donc 3 groupes de tests:

  1. Testez l'usine isolément. Traite-t-il correctement les entrées. Que se passe-t-il lorsque vous passez dans un inconnu?
  2. Testez l'employé isolément. Pouvez-vous définir une stratégie arbitraire et cela fonctionne comme prévu? Que se passe-t-il s'il n'y a pas de stratégie ou de réglage d'usine? (si c'est possible dans le code)
  3. Testez les stratégies isolément. Chacun exécute-t-il la stratégie que vous attendez? Gèrent-ils les entrées aux limites impaires de manière cohérente?

Vous pouvez bien sûr faire des tests automatisés pour l'ensemble du shebang, mais ceux-ci sont maintenant plus comme des tests d'intégration et doivent être traités comme tels.

Telastyn
la source
2

Avant d'écrire un code, je commencerais par un test pour une usine. Se moquant des choses dont j'avais besoin, je me forcerais à penser aux implémentations et aux cas d'utilisation.

Ensuite, j'implémenterais une usine et continuerais avec un test pour chaque implémentation et enfin les implémentations elles-mêmes pour ces tests.

Enfin, je supprimerais les anciens tests.

Patkos Csaba
la source
2

À mon avis, vous ne devez rien faire, ce qui signifie que vous ne devez pas ajouter de nouveaux tests.

J'insiste sur le fait qu'il s'agit d'une opinion, et cela dépend en fait de la façon dont vous percevez les attentes de l'objet. Pensez-vous que l'utilisateur de la classe souhaite fournir une stratégie de calcul de la taxe? S'il ne s'en soucie pas, les tests devraient refléter cela, et le comportement reflété par les tests unitaires devrait être qu'ils ne devraient pas se soucier du fait que la classe a commencé à utiliser un objet de stratégie pour calculer la taxe.

J'ai rencontré ce problème plusieurs fois lors de l'utilisation de TDD. Je pense que la raison principale est qu'un objet de stratégie n'est pas une dépendance naturelle, par opposition à une dépendance de frontière architecturale comme une ressource externe (un fichier, une base de données, un service distant, etc.). Puisqu'il ne s'agit pas d'une dépendance naturelle, je ne fonde généralement pas le comportement de ma classe sur cette stratégie. Mon instinct est de ne changer mes tests que si les attentes de ma classe ont changé.

Il y a un excellent article d'oncle Bob, qui parle exactement de ce problème lors de l'utilisation de TDD.

Je pense que la tendance à tester chaque classe distincte est ce qui tue TDD. Toute la beauté de TDD est que vous utilisez des tests pour stimuler les schémas de conception et non l'inverse.

Rafi Goldfarb
la source