...and all is well. However, since the functionality is now in the HerbivoreEater class, it now feels like there's something wrong with having tests for each of these behaviors on each subclass. Yet it's the subclasses that are actually being consumed, and it's only an implementation detail that they happen to share overlapping behaviors (Lions and Tigers may have totally different end-uses, for instance).

It seems redundant to test the same code multiple times, but there are cases where the subclass can and does override the functionality of the base class (yes, it might violate the LSP, but lets face it, IHerbivoreEater is just a convenient testing interface - it may not matter to the end-user). So these tests do have some value, I think.

What do other people do in this situation? Do you just move your test to the base class, or do you test all subclasses for the expected behavior?

EDIT:

Based on the answer from @pdr I think we should consider this: the IHerbivoreEater is just a method signature contract; it does not specify behavior. For instance:

For the sake of argument, shouldn't you have an Animal class which contains Eat? All animals eat, and therefore the Tiger and Lion class could inherit from animal.
–
The Muffin ManNov 1 '11 at 21:54

1

@Nick - that is a good point, but I think that's a different situation. As @pdr pointed out, if you put the Eat behavior in the base class, then all subclasses should exhibit the same Eat behavior. However, I'm talking about 2 relatively unrelated classes that happen to share a behavior. Consider, for example, the Fly behavior of Brick and Person which, we may assume, exhibit a similar flying behavior, but it doesn't necessarily make sense to have them derive from a common base class.
–
Scott WhitlockNov 2 '11 at 1:06

3 Answers
3

This is great because it shows how tests truly drive the way you think about design. You are sensing problems in the design and asking the right questions.

There are two ways of looking at this.

IHerbivoreEater is a contract. All IHerbivoreEaters must have an Eat method which accepts a Herbivore. Now, your tests do not care how it is eaten; your Lion might start with the haunches and Tiger might start at the throat. All your test cares about is that after it calls Eat, the Herbivore is eaten.

On the other hand, part of what you're saying is that all IHerbivoreEaters eat the Herbivore in exactly the same way (hence the base class). That being the case, there is no point in having an IHerbivoreEater contract at all. It offers nothing. You may as well just inherit from HerbivoreEater.

Or maybe do away with Lion and Tiger completely.

But, if Lion and Tiger are different in every sense apart from their eating habits, then you need to start wondering if you are going to run into problems with a complex inheritance tree. What if you also want to derive both classes from Feline, or only Lion class from KingOfItsDomain (along with Shark, perhaps). This is where LSP truly comes in.

Now, here's a beautiful thing developing (beautiful because I didn't intend it). If you just make that private constructor available to the test, you can pass in a fake IHerbivoreEatingStrategy and simply test that the message is passed on to the encapsulated object properly.

And your complex test, the one you were worrying about in the first place, only has to test the StandardHerbivoreEatingStrategy. One class, one set of tests, no duplicated code to worry about.

And if, later, you want to tell Tigers that they should eat their herbivores a different way, none of these tests have to change. You're simply creating a new HerbivoreEatingStrategy and testing that. The wiring is tested at integration-test level.

+1 The Strategy Pattern was the first thing that came into my head reading the question.
–
StuperUserNov 1 '11 at 19:03

Very good, but now replace "unit test" in my question with "integration test". Don't we end up with the same problem? IHerbivoreEater is a contract, but only in so much as I need it for testing. It seems to me that this is one case where duck typing would really help. I just want to send them both to the same test logic now. I don't think the interface should make a promise of the behavior. The tests should do that.
–
Scott WhitlockNov 1 '11 at 19:33

great question, great answer. You don't have to have a private constructor; you can wire IHerbivoreEatingStrategy to StandardHerbivoreEatingStrategy using an IoC container.
–
azheglovNov 1 '11 at 20:05

@ScottWhitlock: "I don't think the interface should make a promise of the behavior. The tests should do that." That's exactly what I'm saying. If it does make a promise of the behaviour, you should get rid of it and just use the (base-)class. You don't need it for testing at all.
–
pdrNov 1 '11 at 21:34

It seems redundant to test the same code multiple times, but there are cases where the subclass can and does override the functionality of the base class

In a round-about way you're asking whether its appropriate to use white-box knowledge to omit some tests. From a black-box perspective, Lion and Tiger are different classes. So someone without familiarity with the code would test them, but you with deep implementation knowledge know that you can get away with simply testing one animal.

Part of the reason for developing unit tests is to allow yourself to refactor later but maintain the same black-box interface. The unit tests are helping you ensure your classes continue to meet their contract with clients, or at the very least forces you to realize and think carefully how the contract might change. You yourself realize that Lion or Tiger might override Eat at some later point. If this is remotely possible, a simple unit test testing that each animal you support can eat, in the manner of:

I wonder if this question really just boils down to a preference of black-box vs. white-box testing. I lean towards the black-box camp, which is perhaps why I'm testing the way I am. Thanks for pointing that out.
–
Scott WhitlockNov 1 '11 at 19:58

You're doing it right. Think of a unit test as the test of the behavior of a single usage of your new code. This is the same call you would make from production code.

In that situation, you're completely correct; a user of a Lion or Tiger will not (won't have to, at least) care that they're both HerbivoreEaters and that the code that actually runs for the method is common to both up in the base class. Similarly, a user of a HerbivoreEater abstract (provided by the concrete Lion or Tiger) won't care which it has. What they care about is that their Lion, Tiger, or unknown concrete implementation of HerbivoreEater will Eat() a Herbivore correctly.

So, what you're basically testing is that a Lion will Eat as intended, and that a Tiger will Eat as intended. It's important to test both, because it may not always be true that they both eat exactly the same way; by testing both, you ensure that the one you didn't want to change, didn't. Because those are both of the HerbivoreEaters defined, at least until you add Cheetah you have also tested that all HerbivoreEaters will Eat as intended. Your testing both completely covers and adequately exercises the code (provided that you also make all expected assertions of what should result from a HerbivoreEater eating a Herbivore).