Testable Designs (Answering Earl's Questions)
Earl Damron added a comment to a recent blog post about the TDD comic at TheServerSide.net.
Earl says:
I'm having a heck of a time understanding how to thoroughly exercise my class, despite being confident that it would help. It seems like there are a number of factors that simply prevent much testing with Nunit
Earl goes on to list the conceptual challenges he's facing. I'm certainly not willing to say that I've got the right answers. If I was gonna take a crack at cracking some of these nuts I'd rather do it with another set of eyes and maybe a whiteboard. It'd be great if someone could sanity check some of my sketches.
Earl's enquires about:
a class with one public method, which performs the majority of its work (likely to fail) in private methods
This is a canonical question about unit testing. Whether to test private methods as well as public methods is the source of a lot of discussion on the XP and TDD forums and in QA circles. There are folks who support both approaches and back up their views with some good reasons.
Since TDD is a design methodology, engaging in it often leads to designs that are well factored and more amenable to testing. It turns out that when you design your classes from a testability perspective, you end up with some pretty solid OO designs.
To be blunt, it sounds like you've got more of a design problem that is starting to express its negative potential in terms of its testability, rather than a testing problem. So you might consider redesigning your class, or adding some test hooks. You could also write a private method invoker to call those private methods through reflection (this is an out-of-the-box feature of Visual Studio 2005).
I'm in the test-public-methods-only camp - about 97% percent of the time. Sometimes I add test hooks, but I usually only add test hooks to set an object's state. I can’t say I've ever added a test hook that lets me directly invoke a private method.
The problem with having a big ol' god method that calls a bunch of private methods is that you need test scenarios that will give you complete coverage over all branches in your code. If your god method has a switch block or a bunch of if blocks, you need to code scenarios that will put your code through each permutation. There's not much you can do about this. You still need to support all the business cases, so you're gonna need test scenarios for each. You can, however, use designs that atomize the domain code so that the test code gets a fighting chance at growing up clear and concise.
In this particular case, there's one design (or set of designs) that occurs to me off the bat. I can't say categorically if my idea is right. Writing about it certainly doesn't prove that this is a good design. The only thing that can say if the design is right is putting it in practice. Nonetheless, here's an idea...
Methods that have switch statements or flat conditional branching are prime candidates for applying a pattern that uses polymorphism. Switch statements and flat conditional blocks are sometimes indicators of polymorphism phobia. Sometimes you have switch statements that aren’t indicators of polymorphobia, like in factory classes. If you're coming from a procedural programming, you wouldn’t have had objects and inheritance and subsequently no polymorphism-enabled patterns like Factory, so some of this stuff might seem like overkill. I'm still getting used to a lot of it. I often question whether all this stuff is overkill. Sometimes it is. Sometimes it isn't. I find it's often helpful paint it, and step back and take a look. If it ain't right, roll it back.
If you could take each one of the conditional branches in your god method, and turn each one into an object, you could test each object in isolation with its own test fixture. Lets just say that you have a class called Foo() and it has a god method called Bar(). Bar() has a switch statement with two branches. Branch 1 calls SomeMethod() and branch 2 calls SomeOtherMethod(). SomeMethod() and SomeOtherMethod() are private. To test Foo.Bar() and get complete coverage, you need two test cases - one for each branch in Bar() so that both SomeMethod() and SomeOtherMethod() get called. Imagine what your test fixture might look like if you had 5 or 10 braches. It'll likely be on the spaghetti side of the code food groups.
You could refactor SomeMethod() and SomeOtherMethod() to classes based on the Command pattern. So, you'd end up with a SomeCommand class and a SomeOtherCommand class. Each one of these would have a method called Execute().
Once you have the command objects, you can unit test each one directly using their own test fixtures. At this point you'd have a Foo class, a SomeCommand class, and a SomeOtherCommand class, and you'd also have a test fixture for each. Since the actual logic is contained in the command objects, you arguably only need to unit test the logic in the command fixtures rather than the Foo fixture, making the Foo fixture much cleaner.
Because the command objects are relatively harmless on their own, you can make the Execute() method public. The Execute() method signature would likely have an argument that allows the passing of data from Foo.Bar() into the appropriate command. It's fairly likely that each command's Execute() method would accept an argument of type Foo. Execute might operate on the Foo instance using internal-scope members. Asserts would be made against the public state of the Foo instance based on any changes made to state by the command object.
You still need about as much code to get the coverage, but the factoring is moving towards a more (I would say) comfortable level of granularity and isolation. As you apply more appropriate factoring to your application code, the factoring of the test code will change right along, gaining the same readability and maintainability benefits. This issue is really a matter of taking the notion of class responsibility into consideration in your code.
Classes should be designed to have a fairly limited set of responsibilities. You're aiming for a sweet spot where things are neither too granular or too chunky. If you're like me and you don’t really consider yourself an expert in class design - at least not compared to our teachers - then this design might not be immediately apparent to you. However, if you drive towards testability in your design (i.e.: find ways around god methods with a bunch of private helper methods) chances are you'll end up in the sweet spot. This is why TDD is considered design!
The challenge you have remaining in testing Foo.Bar() is making sure that the right command object is instantiated based on the switch block in the Bar() method. Remember, Bar() just instantiates the appropriate command object based on the switch condition.
This is another OO and testability design issue. The Factory Pattern is a likely solution. Move the switch logic from Foo.Bar() into a FooCommandFactory class. FooCommandFactory has a factory method called CreateCommand() (if you've used ADO .NET, you should be seeing some familiar stuff). The CreateCommand() method signature specifies an argument that represents the value that the switch evaluates. The switch still exists, it's just been moved into the CreateCommand() method in the factory.
CreateCommand() returns an instance of either SomeCommand or SomeOtherCommand. In order for FooCommandFactory.CreateCommand() to return instances of different classes, those classes need to have a common super type. You could declare the return type of CreateCommand() as System.Object, since everything in .NET inherits from Object. That would solve it, but that typing is a bit to broad for comfort. It would be better to create an ICommand interface that both SomeCommand and SomeOtherCommand implements. The ICommand interface declares the Execute() method. FooCommandFactory.CreateCommand() will return an instance of ICommand.
The Foo.Bar() method now just makes a call to FooCommandFactory.CreateCommand() to get the appropriate command object according to the criteria, and executes it's Execute() method:
FooCommandFactory.CreateCommand(switchCriteria).Execute(this)
Now you just need to write a test fixture for FooCommandFactory that makes sure that the CreateCommand() method returns the correct command types.
This begs the question, "So what's left to unit test in Foo.Bar()?" The answer may seem odd, but here it is nonetheless… There may be nothing left to unit test in Foo.Bar(). Exercising Foo.Bar() is now a matter of user acceptance testing or some other higher order of testing rather than unit testing. Depending on your case, you might consider getting rid of Foo.Bar() completely since it's really just a convenience method that makes use of your new, refactored API. There's often not much left of procedural patterns once you objectify them. Going OO means more than translating lines of code from VB6 to VB .NET. To really go OO, you gotta go native - or at least try. After all 3 million VB developers can’t be wrong (hmm... except maybe for the millions still writing VB6 code on the CLR rather than learn some techniques that were invented less than thirty years ago :)
Earl's next quandary:
A class that provides functionality that is highly dependant on the state of a larger process, in which my class only performs one logical unit of work (amongst a half dozen others)
Sounds the Strategy pattern with the Template Method pattern. You need to test each strategy in isolation. That means that each strategy class needs to be isolatable. You need to create the appropriate state in the test fixture's setup. This might take some doing if it's a complicated process. Each strategy test fixture might inherit from a common abstract test fixture whose setup creates the general-purpose state.
Process testing isn't always unit testing. You can use the same tools to frame and execute the tests. Each step in the process may be a unit. Test the units in unit testing. Process testing often requires the creation of a non-trivial amount of test code to get complete coverage. You might defer to acceptance of functional testing to the results of an end-to-end process.
And finally, Earl inquires about external dependencies:
A class that's highly dependant on external resources, like message queues and database tables, that would have data in them that the class is supposed to operate on.
When you're testing a class, you're testing the class. You want to isolate it from dependency so that the presence or configuration of that dependency has no impact on the outcome of the unit tests.
When you're testing code that has dependencies on external resources (and this could apply to the question about process testing as well), it's often in your best interest to create mockups of those external resources. The mockups are stubs that you code, and have completely predictable results. The results that come from calls to the mockups are hard coded. It's hard to get more predictable than that.
For example, if you have a data access component that retrieves a person's name based on an ID, you would hard code a data access library that looks just like your real library. However, rather than calling out to your database to get the user's name, you would simply hard code the returned value. So every time I call into Dal.GetPersonName(int id) for the ID 12345, the mock data access object returns "Joe Schmoe".
If you're testing some business logic code, and somewhere embedded in that code a call is made to an external resource, that you don’t want to have to unit test it as well as the business code. Business component unit tests test the code in the business component. Everything else is often not relevant to proving that the business code works.
As you might imagine, it might become a bit cumbersome to code up all of these mock objects. You don’t have to hand code them, you would use a mock objects framework like NMock to dynamically create (at runtime) mock instances based on the interface of real object. Check out Scott Dockendorf's Uber testing tools list for references to mock object libraries. You can get a leg up on NMock use from the NMock MSDN article.
Well, that's my 2 cents.
Cheers!