I'm doing some development at the moment in order to sharpen my skills and I'm mostly playing around with TDD. Part of my solution involves two concrete classes which share some common functionality and therefore via the magic of refactoring inherit from the same abstract base class. I knew from the start that I'd end up with such a base class but because I wanted to stick rigidly to letting my tests define my classes I didn't 'discover' the base class until one of the subclasses was mostly written.
So refactoring my concrete class means moving some functionality into the new base class, does this also means that I should also move the related tests into a new test fixture? No, not move them anyway; the tests should remain where they are in the concrete class test fixture since its contract hasn't changed in the refactoring, only the implementation has changed. However I should still test the base class, with what essentially will be a copy and paste of the tests in the concrete class.
So now I have two sets of near identical tests that test essentially same thing. Remembering that I mentioned above that there are two subclasses this now becomes three sets of the same tests. So now I think I can be sly and do some refactoring of my tests to avoid duplication. I create a base test class from which all three tests fixtures inherit (a-la a solution proposed by Mark Needham), in this base test class I can put all the common tests. Done.
Though this works swimmingly I can't shake the feeling that I'm heading down the wrong path. Sure now I have a scalable solution for testing X number of subclasses but in doing so I've had to couple my unit tests together, they now know about each other and dependencies exist. The solution works, but only if the subclasses stick to the same contract as the parent and do not try to override behaviour.
I firmly believe that unit tests should test code in as much isolation as possible so I'm thinking now that maybe I should have three separate test fixtures again. My tests are the contract that my code needs to fulfill in order to be accepted, and in that sense I have three separate contracts anyway. Sure the contracts look the same, just the names change, but the crucial thing I gain is I can change the terms and conditions on one without invalidating my others.
Perhaps I'm being overly pedantic, but the duplication sits easier with me than the inheritance model. I'd be interested in what others think of this as I know I'm probably in a minority here.
If the classes inherit from one another because they share some common contract, then I don't really see the problem sharing a superclass which tests the common functionality. Likewise sharing setup/teardown code at a higher level (say all tests which use the database) can warrant a superclass.
ReplyDeleteIf you're worried about the different classes under test having no logical connection, though, then you're possibly right to worry. If the commonality in testing is in setup, you could probably alleviate this using some external set up object you can pull in via. composition. If the classes actually share no contractual commonality, I'm not sure why you would have inherited tests in the first place, though.
My final thought, though, is that if you're having this much of an issue testing this, maybe consider pulling the superclass out as a separate class that can be pulled in by composition and test the interactions between it and its subclasses. Over-testing class hierarchies is an easy rat-hole to fall down, depressingly.
+1 for composition of the common code.
ReplyDeleteAlso, if you are interested in testing the contract provided by your subclasses, you maybe don't need to test the base class (just an implementation detail)?
I agree with John on that last point; the superclass, if never used on its own, doesn't need to be tested in isolation unless there is some specific reason to do so. Its behaviour affects the contract of the system only through its subclasses.
ReplyDeleteThanks for the replies. Yeah I thought about just leaving the superclass untested but I can't gaurantee that it won't be used in the future, moreover that'd mean my tests knowing something about the implementation that I believe it shouldn't.
ReplyDeleteI think I'll go for composition instead in this particular instance as it makes sense.
Or maybe I'll just add a comment to my test class calling it an itegration test and wipe my hands off the whole sorry mess :)