First of all - I haven't been writing here for a while. Various things kept me busy, and I didn't think I had anything new to talk about. Probably this topic isn't very new either, but recent events made me elaborate on it.
Refactoring is probably one of the most practiced activities nowadays - specially if you're aiming to be good at writing software. More domain knowledge and experience will make that beautiful piece of code you wrote a couple of months ago look terribly ugly now. Modern IDEs and a good safety net provided by our tests make this task easy and enjoyable.
However, and specially when you're working (like I am at the moment) on a very big project, maintaining your tests is actually as much important as maintaining your production codebase.
Code sometimes may not be crystal clear at first sight (and in some cases not even after reading it a couple of times), so what really helps us understanding code is tests. Tests should express requirements - exactly what the code is supposed to do. But happens quite often that, even with a proper test coverage, tests are the dusty corner of our codebase: people tend to write them, but seldom refactor them. This makes really difficult both understanding the test itself but also to extend it, if there's a need to.
Few problems I usually come along, and how I try to solve them:
- Poorly named tests: lots of people start their test names with
test...in reminiscence of the old JUnit 3.x days. A test name describes one of the requirement our tested part of the system needs to implement. A test name should make sense as read in plain English, and it should describe one of the capabilities of the system (i.e.acceptsRequestMoreRecentThanThreeDays). - Tens of assertions, sometimes unrelated to what the test is testing: people usually say that you should have only one assert per test. I'm not that strict, but assertions that are in the same test should be related (yeah, that's the same high cohesion thing!). If a test is checking whether some additional work is done in case of a particular input, as long as you've covered the other work your method will always do in another test there's no need to repeat the assertions. It will mislead the reader from the real purpose of your test. Remove them: it will make the test clearer and shorter.
- Mocks, mocks everywhere: again a controversial topic. I read many people that tell that mocks are useless - I think that they are a powerful tool when doing unit testing. Some units of work won't change state of objects, while they will produce side effects (e.g. call a DAO). By mocking dependencies (I should rather say by creating stubs, but some tools like Mockito made the name mock more universal) and verifying invocations, we test that our code is really doing what we want. But lots of times I've seen mocking value objects owned by the application, and even setting answers to getters. Interactions on value objects should be always tested by checking the state of the objects: it's easier and less error prone (you remember that guy that added some hidden logic in a getter, don't you...). As a rule of thumb, if a dependency is mocked to return something, that something probably shouldn't be a mock itself.