For the next release of FEST-Swing, the number of automated tests has increased considerably: from 2,677 to 3,000+ (and counting!) Special attention has been paid to the quality of the tests: DRY and readable code, meaningful assertions, and proper naming. Tests are treated with the same TLC (Tender Loving Care) as the rest of the code base.
One important thing was still missing: organization. We have a one-to-one mapping between class and test, resulting in awfully long test classes. For example, all the tests for the class JTreeDriver are contained in JTreeDriverTest, which has 754 lines! 754 lines! When looking at it, I noticed how unmanageable this test class was:
- I lose focus by scrolling up and down, even with a 30-inch monitor
- it’s hard to find tests for a specific method: I have to look for references using my IDE, which can be time-consuming
- it’s hard to figure out if existing tests were covering all possible use cases, especially if they are not grouped by the method they are testing
- waste of resources: not all methods need the same test fixture
- when testing overloaded methods, the names for test methods tend to be too long
I thought that it would be better to create one test-class per method-to-test. To be honest, I wasn’t completely sure it was a good idea, until I read the excellent article "Big Flat Test Cases Suck (and what to do about it)" by Hamlet D’Arcy. Unfortunately, I cannot follow all of Hamlet’s advices, because I use Java and not the Groovy + easyb combo.
Hamlet’s article was a great help! I ended up creating one test-class per method-to-test, as I initially considered. I also reworked the names for the test methods, by adding underscores to improve readability.
As an example, going back to JTreeDriverTest, it originally contained the following methods to test JTreeDriver#expandRow(int):
shouldExpandRowshouldNotDoAnythingIfRowAlreadyExpandedshouldThrowErrorWhenExpandingRowInDisabledTreeshouldThrowErrorWhenExpandingRowInNotShowingTree
and the following methods to test JTreeDriver#expandPath(String):
shouldExpandPathshouldNotDoAnythingIfPathAlreadyExpandedshouldThrowErrorWhenExpandingPathInDisabledTreeshouldThrowErrorWhenExpandingPathInNotShowingTree
I moved those methods to their own test classes, JTreeDriver_expandNodeByRowIndex_Test and JTreeDriver_expandNodeByPath_Test, respectively. I particularly like this naming convention for test classes, because I can find all the tests for a particular method pretty quickly. IMHO, another well-received result is DRY-er method names, since, in this case, the distinction between row index and path is specified in the class name, once, instead of in each test method.
The new test classes have only the following methods:
should_expand_nodeshould_not_do_anything_if_node_already_expandedshould_throw_error_when_JTree_is_disabledshould_throw_error_when_JTree_is_not_showing
As far as I can tell, this new organization of test code has effectively solved the problems I mentioned earlier. It also resulted in better method naming. While performing these changes, I found that some use cases were not being tested, and I found a couple of bugs too!
I’m still in the middle of this reorganization. It is a lot of work. It may even delay the release date (originally scheduled for August 3rd.) But I think it’s worth it. I don’t dare to have a formal release if I cannot run the whole test suite successfully. On the meantime, I plan to release a "snapshot" for the users that are waiting for new features or bug fixes.
Oh BTW, I’m not claiming that I invented all the practices presented in this post. I’m only sharing something I learned, hoping it can be useful to others :)
Comments/suggestions are always welcome!
My name is Alex Ruiz. I'm a programmer with special interest in Java, API design, testing and OOP. I'm the creator of 
#1 by Hamlet D'Arcy on July 28, 2009 - 8:55 am
The other alternative is creating one TestCase per common setup/teardown. The results in a highly cohesive test… all the test methods use all the fields in the class. Your approach seems good when there aren’t a lot of cross-method dependencies (doFoo() depends on doBar()) but a shared fixture approach might be better there are more cross-method dependencies. AND… cross method dependencies is probably a code smell for temporal coupling or some such.
#2 by Michael Hüttermann on July 28, 2009 - 9:15 am
One test class per method results in a huge test class base. I prefer adding a couple of test methods in a test class with the restriction that they should all share the same setUp/tearDown. This leads to a more focussed test base. Of course this may also result in longer test classes .. if the length of test classes is important, you may use Checkstyle to monitor your expectations. But also here it is helpful to first define your expectations and then validate them (continuously). Me personally, I can work with longer classes, for me it is more critical how complex they are (e.g. McCabe). Longer ascii files can be managed by all IDEs, but only scoped ones are easy to maintain and to extend by the man in front of the desk (Cohesion vs Coupling are magic words here).
#3 by Alex Ruiz on July 28, 2009 - 9:30 am
@Michael Hüttermann
Hey my friend!
As I mentioned in our conversation on Twitter, so far I’m pleased with the results. It is very easy to find tests for a specific methods. Test classes are small and very focused. I also found that in some cases, one test-class per method-to-test is an overkill.
I’d like to kindly ask you to review the code before I check-in. I’m sure we can find a good balance! :)
#4 by Giorgio Sironi on July 29, 2009 - 2:40 am
You can also divide the test methods by scenario; the common setUp and tearDown is probably a sign of a common scenario, the “given” part of a test. Beware that a very long test case class can be a smell of a violation of Single Responsibility Principle by your system under test: often if you’re writing 40 test methods the class is doing too much, and you could factor out a collaborator that will handle the functionality which is less cohesive with the rest of your SUT.
#5 by Alex Ruiz on July 29, 2009 - 11:46 am
@Giorgio Sironi
I agree. That is something that I’ve been thinking about too.
JTreeDriveris doing to much. I have plans to divide it several classes as “gestures”, “assertions” and “queries”. That is a huge change in the code base, but necessary. This changes will be internal though, the public API won’t be affected :)Thanks!
#6 by Konstantin Scheglov on August 8, 2009 - 12:12 pm
One my colleague hates me because I use same naming scheme as you do – use underscores. I do this long time, probably 2-3 years and all this time he asks me to stop using it. He says that it is hard to read and it violates Sun Java code conventions.
What do you think about this? Do you feel yourself guilty because you violate standards?
#7 by Alex Ruiz on August 10, 2009 - 2:00 pm
@Konstantin Scheglov
Well, I don’t feel guilty at all :)
I’m using underscores for naming test methods only, any other code uses the regular Java code conventions. In my case, I found that underscores really make it easier to read test code.
At the same time, the team has to agree about using underscores for method naming or any other practice that is not considered “standard.” So far, nobody in the FEST team has complained ;)
Cheers,
-Alex