Today a bunch of co-workers and me got together right after work to hone our skills, more specifically, our unit testing skills.
A couple of weeks before, I had discovered that the code coverage of PHPUnit is only at ~55%, so I thought it would be a great exercise for our after work hacking to help increase its code coverage by writing unit tests for it.
The plan was to try and write as many tests as we could for the Constraint classes PHPUnit uses to implement its assertions.
Those Constraint classes are pretty small, fairly easy to understand and not entirely covered by tests – in short, very well suited for our group, a mix of programmers having quite some experience in unit testing as well as others just having started to learn unit testing.
Well, our plan didn’t work out that way, we didn’t really succeed in writing a considerable amount of unit tests.
However, it still was a valuable experience, as it turned out the unit tests of the Constraints are a good example of how not to unit test.
So here’s what we learned (or were reminded of):
1. Don’t use one single test case class to test several different classes
The unit tests for the constraint classes of PHPUnit are lumped together in one large file. I think the reasoning behind that might have been that all constraints derive from the same parent class, PHPUnit_Framework_Constraint.
Don’t do that.
It renders it quite hard to find your way to the tests for one of the constraints, it’s quite annoying to navigate through that class.
If each class has its own test case, it’s much easier to find its test, too.
2. Name your tests well
You should name your tests such that they convey the intent of the test, describe what the test is about and how they’re different from other tests.
The tests in ConstraintTest.php wear names like:
- testConstraintIsEmpty
- testConstraintPCREMatch
- testConstraintClassNotHasStaticAttribute
- testConstraintStringMatches
- testConstraintStringMatches2
- testConstraintStringMatches3
- testConstraintStringMatches4
- testConstraintStringMatches5
- testConstraintStringMatches6 (sic!)
It’s really hard to understand what’s going on in tests named in such a way.
3. Avoid to test more than one behaviour in one single test
The tests in ConstraintTest.php test a lot of behavior in one single test, here’s an example:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 |
|
Especially in conjunction with #2, this makes it really hard to figure out what a given test is supposed to test. In our practice group we sat in pairs in front of our laptops and yet it still took us quite some time to find out what those tests were actually doing.
If you find yourself writing more than one assertion in a single test, take your hands from the keyboard and find out whether you’re about to test more than a single behavior.
Otherwise you will probably curse yourself when you come back to that test code in, say, 6 months…
Conclusion
If you plan to introduce others to unit testing by letting them loose on a real world codebase, I’d suggest not to pick PHPUnit, as ironic as it may sound.
PHPUnit itself is a very valuable tool and I’m very grateful for its existence, I’m using it daily at work. Yet, its test are no good example of how to write good unit tests, working on those tests will cause quite some confusion especially among programmers new to unit testing.
Anyways, for our next practice group meeting we’ll choose something more appropriate, we’re thinking about trying our luck on Symfony’s Assetic.
However, we will very probably come back to PHPUnit at some point in the future in order to try and refactor its tests.