Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Suppose I have a class Manager derived from a base class Employee, and that Employee has a method getEmail() that is inherited by Manager. Should I test that the behaviour of a manager's getEmail() method is in fact the same as an employee's?

At the time these tests are written the behaviour will be the same, but of course at some point in the future someone might override this method, change its behaviour, and therefore break my application. However it seems a bit strange to essentially test for the absence of meddling code.

(Note that testing Manager::getEmail() method does not improve code coverage (or indeed any other code quality metrics (?)) until Manager::getEmail() is created/overridden.)

(If the answer is "Yes", some information about how to go about managing tests that are shared between base and derived classes would be useful.)

An equivalent formulation of the question:

If a derived class inherits a method from a base class, how do you express (test) whether you're expecting the inherited method to:

  1. Behave in exactly the same way as the base does right now (if the behaviour of the base changes, the behaviour of the derived method doesn't);
  2. Behave exactly the same way as the base for all time (if the behaviour of the base class changes, the behaviour of the derived class changes as well); or
  3. Behave however it wants to (you don't care about the behaviour of this method because you never call it).
share|improve this question
1  
IMO deriving a Manager class from Employee was the first major mistake. –  CodesInChaos Mar 5 '13 at 16:44
1  
@CodesInChaos Yes, that could be a bad example. But the same problem applies whenever you have inheritance. –  mjs Mar 5 '13 at 17:09
    
You don't even need to override the method. The base class method may call other instance methods that are overridden, and executing the same source code for the method still creates different behaviour. –  gnasher729 May 8 at 18:51
add comment

7 Answers

No, you do not need to test inherited methods. Classes and their test cases which rely on this method will break anyway if the behaviour changed in Manager.

Think of the following scenario: The email address is assembled as Firstname.Lastname@example.com:

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

You have unit tested this and it works fine for your Employee. You also have created a class Manager:

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

Now you have a class ManagerSort which sorts managers in a list based on their email address. Of yourse you suppose that the email generation is the same as in the Employee:

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

You write a test for your ManagerSort:

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

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

Everything works fine. Now someone comes and overrides the getEmail() method:

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

Now, what happens? Your testManagerSort() will fail because the getEmail() of Manager was overridden. You will investigate in this issue and will find the cause. And all without writing a seperate testcase for the inherited method.

Therefore, you do not need to test inherited methods.

Otherwise, e.g. in Java, you would have to test all methods inherited from Object like toString(), equals() etc. in every class.

share|improve this answer
    
You are not supposed to be clever with unit tests. You say "I don't need to test X because when X fails, Y fails". But unit tests are based on the assumptions of bugs in your code. With bugs in your code, why would you think that complex relations between tests work the way you expect them to work? –  gnasher729 May 8 at 18:58
    
@gnasher729 I say "I don't need to test X in the subclass because it is already tested in the unit tests for the superclass". Of course, if you change the implementation of X, you have to write appropriate tests for it. –  Uooo May 9 at 4:18
add comment

Yes, you should test inherited methods because in the future they may get overridden. Also, inherited methods may call virtual methods that are overridden, which would change the behavior of the non-overridden inherited method.

The way I test this is by creating an abstract class to test the (possibly abstract) base class or interface, like this (in C# using 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));
    }
}

Then, I have a class with tests specific to the Manager, and integrate the employee's tests like this:

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

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

The reason I can so this is Liskov's substitution principle: the tests of Employee should still pass when passed a Manager object, since it derives from Employee. So I have to write my tests only once, and can verify they work for all possible implementstions of the interface or base class.

share|improve this answer
    
Good point re Liskov's substitution principle, you're right that if this holds, the derived class will pass all the base class's tests. However, LSP is frequently violated, including by xUnit's setUp() method itself! And pretty much every web MVC framework that involves overriding an "index" method also breaks LSP, which is basically all of them. –  mjs Mar 6 '13 at 9:33
add comment

Robert Martin's rules for TDD are:

  1. You are not allowed to write any production code unless it is to make a failing unit test pass.
  2. You are not allowed to write any more of a unit test than is sufficient to fail; and compilation failures are failures.
  3. You are not allowed to write any more production code than is sufficient to pass the one failing unit test.

If you follow these rules, you wouldn't be able to get into a position to ask this question. If the getEmail method needs to be tested, it would have been tested.

share|improve this answer
    
+1 for Bob Martin reference. –  seggy Mar 6 '13 at 16:52
add comment

Don't unit test it. Do functional/acceptance test it.

Unit tests should test every implementation, if you are not providing a new implementation then stick by the DRY principle. If you want to spend some effort here then you can enhance the original unit test. Only if you override the method should you write a unit test.

At the same time, functional/acceptance testing should make sure that at the end of the day all of your code does what it is supposed to, and will hopefully catch any weirdness from inheritance.

share|improve this answer
add comment

I'd take the pragmatic approach here: If someone, in the future, overrides Manager::getMail, then it is that developer's responsibility to provide test code for the new method.

Of course that's only valid if Manager::getEmail really has the same code path as Employee::getEmail! Even if the method is not overridden, it might behave differently:

  • Employee::getEmail could call some protected virtual getInternalEmail which is overridden in Manager.
  • Employee::getEmail could access some internal state (e.g. some field _email), which can differ in the two implementations: For example, the default implementation of Employee could ensure that _email is always firstname.lastname@example.com, but Manager is more flexible in assigning mail addresses.

In such cases, it is possible that a bug manifests itself only in Manager::getEmail, even though the implementaion of the method itself is the same. In that case testing Manager::getEmail separately could make sense.

share|improve this answer
add comment

I would.

If you're thinking "well, it's really only calling Employee::getEmail() because I don't override it, so I don't need to test Manager::getEmail()" then you're not really testing the behavior of Manager::getEmail().

I would think of what only Manager::getEmail() should do, not whether or not it's inherited or overridden. If the behavior of Manager::getEmail() should be to return whatever Employee::getMail() returns, then that's the test. If the behavior is to return "pink@unicorns.com", then that's the test. It doesn't matter whether it's implemented by inheritance or overridden.

What does matter is that, if it changes in the future, your test catches it and you know something either got broken or needs to be reconsidered.

Some people may disagree with the seeming redundancy in the check, but my counter to that would be you're testing the behavior Employee::getMail() and Manager::getMail() as distinct methods, whether or not they're inherited or overridden. If a future developer needs to change the behavior of Manager::getMail() then they need to also update the tests.

Opinions may vary though, I think Digger and Heinzi gave reasonable justifications for the opposite.

share|improve this answer
    
I like the argument that behavioural testing is orthogonal to the way class happen to be constructed. It does seem a bit funny to completely ignore inheritance, though. (And how to you manage the shared tests, if you have a lot of inheritance?) –  mjs Mar 5 '13 at 16:05
1  
Well put. Just because Manager is using an inherited method, in no way means that it should not be tested. A great lead of mine once said, "If it is not tested, it's broken." To that end, if you make the tests for Employee and Manager required on code check-in you will be sure that the developer checking in new code for Manager that may alter the behavior of the inherited method will fix the test to reflect the new behavior. Or come knocking on your door. –  hurricaneMitch Mar 5 '13 at 18:34
    
You really want to test the Manager class. The fact that it is a subclass of Employee, and that getMail () is implemented by relying on inheritance, is just an implementation detail that you should ignore when creating unit tests. Next month you find out that inheriting Manager from Employee was a bad idea and you replace the whole inheritance structure and reimplement all methods. Your unit tests should continue testing your code without problems. –  gnasher729 May 8 at 18:54
add comment

Should I test that the behaviour of a manager's getEmail() method is in fact the same as an employee's?

I would say no as it would be a repeated test in my opinion I would test once in the Employee tests and that would be it.

At the time these tests are written the behaviour will be the same, but of course at some point in the future someone might override this method, change its behaviour

If the method is overridden then you would need new tests to check the overridden behaviour. That is the job of the person implementing the overriding getEmail() method.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.