« Sometimes crappy tests are better than none at all | Main | Fatally flawed password schemes #27 »

Unit tests should play nice

Listen to this articleListen to this article

I've seen a few blog entries around of late demonstrating nifty things you can do to achieve setup/cleanup in your unit tests using statics or classloaders etc. And whilst I admire the creativity and ingenuity, I have to say that just like testing private methods, to me it smells.

Firstly, If unit tests rely on static state and class loaders for setup and cleanup then the tests are just plain broken and wrong. The ultimate goal for me is to be able to run test classes in parallel. Just splitting them up to achieve this won't help because you likely have no way of knowing which test classes interact with which static parts of the system. The good old abstract static factory is a prime example of this. Unit tests should probably create any data and mock any infrastructure they need for the test. They certainly shouldn't be relying on the behaviour of a class loader! If there's too much infrastructure to mock, thats a smell too!

Next, ideally we'd like to run test methods in a single test class on a single instance of said test class, implementing setUp() and tearDown() as appropriate. Maybe the idea of a single instance isn't always practical (I can't think why) but just assuming the tests will be run in a new instance isn't acceptable either. What could I possibly want to do in a constructor that I couldn't do in setUp()? Again, relying on the behaviour of the test runner to create new instances or the class loader to unload static state is just wrong!

There should be no assertions in tearDown(). Assertions failures in tearDown() will mask any failures in the test itself making it all but impossible to track down the actual problem.

And lastly, avoid extending other test classes as much as possible. Remember, any tests that are defined in the super class wil also be run in the subclass. Not only do we end up redundantly running the super class tests but if you have inadvertently overidden some public or protected methods (say setUp() or tearDown() for example) and forgotten to call the super method of the same signature, all hell breaks loose.

As a general rule, excessive refactoring of unit tests can make them difficult to understand. Sure it might be testing the code but it WILL make maintenence of the tests very difficult and surely hinders the ability of anyone to get a handle on the expected behaviour/role of the class under test.

Comments

Hey Mate,

I have to slightly disagree with you on the last point. I think most tests can do with a little refactoring. Things like extract method etc to remove duplication in the test. I wouldn't however go extracting super classes etc. I just want my tests to be easy to read with little duplication. I think a little refactoring on tests can go a long way!

Cheers,
Damian

Hey Damo. I totally agree. I refactor tests all the time so I guess I had better well agree hehehe. I probably mis-stated what I meant.

The problem I see is that tests start off (hopefully) conveying the intent nicely. Then as you add more and more tests and refactor away, that intent can be lost quite easily but not always obviously to the developer who wrote them in the first place because they still know in their own mind what the class does.

If that does happen, of course, it's probably a smell that the code under test is doing too much.

That's really all I meant :-)

Post a comment