Dois-je tester des méthodes héritées?

30

Supposons que j'ai un gestionnaire de classe dérivé d'un employé de classe de base et que l'employé ait une méthode getEmail () héritée par le gestionnaire . Dois-je tester que le comportement de la méthode getEmail () d'un manager est en fait le même que celui d'un employé?

Au moment où ces tests sont écrits, le comportement sera le même, mais bien sûr, à un moment donné, quelqu'un pourrait remplacer cette méthode, changer son comportement et donc casser mon application. Cependant, il semble un peu étrange de tester essentiellement l' absence de code d'ingérence.

(Notez que le test de la méthode Manager :: getEmail () n'améliore pas la couverture du code (ou en fait toute autre métrique de qualité du code (?)) Jusqu'à ce que Manager :: getEmail () soit créé / remplacé.)

(Si la réponse est "Oui", certaines informations sur la façon de gérer les tests partagés entre les classes de base et dérivées seraient utiles.)

Une formulation équivalente de la question:

Si une classe dérivée hérite d'une méthode d'une classe de base, comment exprimez-vous (testez) si vous vous attendez à ce que la méthode héritée:

  1. Se comportent exactement de la même manière que la base en ce moment (si le comportement de la base change, le comportement de la méthode dérivée ne change pas);
  2. Se comportent exactement de la même manière que la base pour tous les temps (si le comportement de la classe de base change, le comportement de la classe dérivée change également); ou
  3. Comportez-vous comme il veut (vous ne vous souciez pas du comportement de cette méthode car vous ne l'appelez jamais).
mjs
la source
1
L'OMI dérivant une Managerclasse a Employeeété la première erreur majeure.
CodesInChaos
4
@CodesInChaos Oui, cela pourrait être un mauvais exemple. Mais le même problème s'applique chaque fois que vous avez un héritage.
mjs
1
Vous n'avez même pas besoin de remplacer la méthode. La méthode de classe de base peut appeler d'autres méthodes d'instance qui sont remplacées et l'exécution du même code source pour la méthode crée toujours un comportement différent.
gnasher729

Réponses:

23

Je prendrais l'approche pragmatique ici: si quelqu'un, à l'avenir, remplace Manager :: getMail, alors c'est la responsabilité du développeur de fournir le code de test pour la nouvelle méthode.

Bien sûr, cela n'est valable que s'il a Manager::getEmailvraiment le même chemin de code que Employee::getEmail! Même si la méthode n'est pas remplacée, elle peut se comporter différemment:

  • Employee::getEmailpourrait appeler un virtuel protégé getInternalEmailqui est remplacé dans Manager.
  • Employee::getEmailpourrait accéder à un état interne (par exemple, un champ _email), qui peut différer dans les deux implémentations: Par exemple, l'implémentation par défaut de Employeepourrait garantir que _emailc'est toujours [email protected], mais Managerest plus flexible dans l'attribution des adresses de messagerie.

Dans de tels cas, il est possible qu'un bogue ne se manifeste que dans Manager::getEmail, même si l'implémentation de la méthode elle - même est la même. Dans ce cas, tester Manager::getEmailséparément pourrait avoir un sens.

Heinzi
la source
14

Je voudrais.

Si vous pensez "eh bien, c'est vraiment seulement appeler Employee :: getEmail () parce que je ne le remplace pas, donc je n'ai pas besoin de tester Manager :: getEmail ()" alors vous ne testez pas vraiment le comportement de Manager :: getEmail ().

Je penserais à ce que seul Manager :: getEmail () devrait faire, pas s'il est hérité ou non. Si le comportement de Manager :: getEmail () doit être de retourner tout ce que Employee :: getMail () retourne, alors c'est le test. Si le comportement est de retourner "[email protected]", alors c'est le test. Peu importe qu'il soit implémenté par héritage ou remplacé.

Ce qui importe, c'est que, s'il change à l'avenir, votre test le détecte et vous savez que quelque chose s'est cassé ou doit être reconsidéré.

Certaines personnes peuvent être en désaccord avec la redondance apparente dans le chèque, mais mon contre serait que vous testiez le comportement Employee :: getMail () et Manager :: getMail () en tant que méthodes distinctes, qu'elles soient héritées ou non remplacé. Si un futur développeur doit changer le comportement de Manager :: getMail (), il doit également mettre à jour les tests.

Les opinions peuvent cependant varier, je pense que Digger et Heinzi ont donné des justifications raisonnables pour le contraire.

seggy
la source
1
J'aime l'argument selon lequel les tests de comportement sont orthogonaux à la façon dont la classe se construit. Cependant, cela semble un peu drôle d'ignorer complètement l'héritage. (Et comment gérer les tests partagés, si vous avez beaucoup d'héritage?)
mjs
1
Bien placé. Tout simplement parce que Manager utilise une méthode héritée, cela ne signifie en aucun cas qu'elle ne doit pas être testée. Une de mes grandes pistes a dit un jour: "Si ce n'est pas testé, c'est cassé." À cette fin, si vous effectuez les tests pour l'employé et le gestionnaire requis lors de l'archivage du code, vous serez sûr que le développeur archivant le nouveau code pour le gestionnaire qui peut modifier le comportement de la méthode héritée corrigera le test pour refléter le nouveau comportement . Ou venez frapper à votre porte.
ouraganMitch
2
Vous voulez vraiment tester la classe Manager. Le fait qu'il s'agisse d'une sous-classe de Employee et que getMail () soit implémenté en s'appuyant sur l'héritage, n'est qu'un détail d'implémentation que vous devez ignorer lors de la création de tests unitaires. Le mois prochain, vous découvrez que l'héritage de Manager de Employee était une mauvaise idée et vous remplacez toute la structure d'héritage et réimplémentez toutes les méthodes. Vos tests unitaires devraient continuer à tester votre code sans problème.
gnasher729
5

Ne le testez pas à l'unité. Faites un test fonctionnel / d'acceptation.

Les tests unitaires devraient tester chaque implémentation, si vous ne fournissez pas une nouvelle implémentation, respectez le principe DRY. Si vous souhaitez y consacrer quelques efforts, vous pouvez améliorer le test unitaire d'origine. Ce n'est que si vous remplacez la méthode que vous devez écrire un test unitaire.

Dans le même temps, les tests fonctionnels / d'acceptation devraient s'assurer qu'à la fin de la journée, tout votre code fait ce qu'il est censé faire et, espérons-le, attrapera toute bizarrerie de l'héritage.

MrFox
la source
4

Les règles de Robert Martin pour le TDD sont les suivantes:

  1. Vous n'êtes pas autorisé à écrire un code de production, sauf s'il s'agit de réussir un test unitaire échoué.
  2. Vous n'êtes pas autorisé à écrire plus d'un test unitaire qu'il ne suffit pour échouer; et les échecs de compilation sont des échecs.
  3. Vous n'êtes pas autorisé à écrire plus de code de production qu'il n'en faut pour réussir le test unitaire ayant échoué.

Si vous suivez ces règles, vous ne serez pas en mesure de poser cette question. Si la getEmailméthode devait être testée, elle l'aurait été.

Daniel Kaplan
la source
3

Oui, vous devez tester les méthodes héritées, car à l'avenir, elles pourraient être remplacées. En outre, les méthodes héritées peuvent appeler des méthodes virtuelles qui sont remplacées , ce qui changerait le comportement de la méthode héritée non substituée.

La façon dont je teste cela est en créant une classe abstraite pour tester la classe de base (éventuellement abstraite) ou l'interface, comme ceci (en C # en utilisant NUnit):

public abstract class EmployeeTests
{
    protected abstract Employee CreateInstance(string name, int age);

    [Test]
    public void GetEmail_ReturnsValidEmailAddress()
    {
        // Given
        var sut = CreateInstance("John Doe", 20);

        // When
        string email = sut.GetEmail();

        // Then
        Assert.IsTrue(Helper.IsValidEmail(email));
    }
}

Ensuite, j'ai une classe avec des tests spécifiques à la Manager, et j'intègre les tests de l'employé comme ceci:

[TestFixture]
public class ManagerTests
{
    // Other tests.

    [TestFixture]
    public class ManagerEmployeeTests : EmployeeTests
    {
        protected override Employee CreateInstance(string name, int age);
        {
            return new Manager(name, age);
        }
    }
}

La raison pour laquelle je peux le faire est le principe de substitution de Liskov: les tests de Employeedevraient toujours passer quand on passe un Managerobjet, car il dérive de Employee. Je dois donc écrire mes tests une seule fois, et je peux vérifier qu'ils fonctionnent pour toutes les implémentations possibles de l'interface ou de la classe de base.

Daniel AA Pelsmaeker
la source
Bon point concernant le principe de substitution de Liskov, vous avez raison: si cela se vérifie, la classe dérivée passera tous les tests de la classe de base. Cependant, LSP est fréquemment violé, y compris par la setUp()méthode de xUnit elle-même! Et à peu près tous les frameworks Web MVC qui impliquent de remplacer une méthode "index" cassent également LSP, qui est fondamentalement tous.
mjs
C'est absolument la bonne réponse - je l'ai entendu appeler le "modèle de test abstrait". Par curiosité, pourquoi utiliser une classe imbriquée plutôt que de simplement mélanger les tests via class ManagerTests : ExployeeTests? (c.-à-d. refléter l'héritage des classes testées.) Est-ce principalement esthétique, pour aider à visualiser les résultats?
Luke Usherwood
2

Dois-je tester que le comportement de la méthode getEmail () d'un manager est en fait le même que celui d'un employé?

Je dirais non car ce serait un test répété à mon avis, je testerais une fois dans les tests des employés et ce serait tout.

Au moment où ces tests sont écrits, le comportement sera le même, mais bien sûr, à un moment donné, quelqu'un pourrait remplacer cette méthode, changer son comportement

Si la méthode est remplacée, vous aurez besoin de nouveaux tests pour vérifier le comportement substitué. C'est le travail de la personne qui met en œuvre la getEmail()méthode prioritaire .


la source
1

Non, vous n'avez pas besoin de tester les méthodes héritées. Les classes et leurs cas de test qui s'appuient sur cette méthode s'arrêteront de toute façon si le comportement change Manager.

Imaginez le scénario suivant: l'adresse e-mail est assemblée en tant que pré[email protected]:

class Employee{
    String firstname, lastname;
    String getEmail() { 
        return firstname + "." + lastname + "@example.com";
    }
}

Vous avez testé l'unité et cela fonctionne bien pour votre Employee. Vous avez également créé une classe Manager:

class Manager extends Employee { /* nothing different in email generation */ }

Vous avez maintenant une classe ManagerSortqui trie les gestionnaires dans une liste en fonction de leur adresse e-mail. De votre part, vous supposez que la génération d'e-mails est la même que dans Employee:

class ManagerSort {
    void sortManagers(Manager[] managerArrayToBeSorted)
        // sort based on email address omitted
    }
}

Vous écrivez un test pour votre ManagerSort:

void testManagerSort() {
    Manager[] managers = ... // build random manager list
    ManagerSort.sortManagers(managers);

    Manager[] expected = ... // expected result
    assertEquals(expected, managers); // check the result
}

Tout fonctionne bien. Maintenant, quelqu'un vient et remplace la getEmail()méthode:

class Manager extends Employee {
    String getEmail(){
        // managers should have their lastname and firstname order changed
        return lastname + "." + firstname + "@example.com";
    }
}

Maintenant, que se passe-t-il? Votre testManagerSort()échouera parce que le getEmail()de Managera été surchargée. Vous allez enquêter sur ce problème et en trouver la cause. Et tout cela sans écrire un testcase séparé pour la méthode héritée.

Par conséquent, vous n'avez pas besoin de tester les méthodes héritées.

Sinon, par exemple en Java, vous devrez tester toutes les méthodes héritées de Objectlike toString(), equals()etc. dans chaque classe.

Uooo
la source
Vous n'êtes pas censé être intelligent avec les tests unitaires. Vous dites "je n'ai pas besoin de tester X parce que quand X échoue, Y échoue". Mais les tests unitaires sont basés sur les hypothèses de bogues dans votre code. Avec des bogues dans votre code, pourquoi pensez-vous que les relations complexes entre les tests fonctionnent comme vous vous attendez à ce qu'ils fonctionnent?
gnasher729
@ gnasher729 Je dis "Je n'ai pas besoin de tester X dans la sous-classe car il est déjà testé dans les tests unitaires pour la super-classe ". Bien sûr, si vous changez l'implémentation de X, vous devez lui écrire des tests appropriés.
Uooo