Mantra: “Test behavior, not implementation”
How about an example? The ruby class had 6 methods each of which called a private method, which in turn called another private method. I needed to change something in the deeper of the two private methods and I wanted to test it. I was pair programming and my pair immediately came to a great option: “we should extract those private methods into their own class.” Sure! We could do that… what are the other options?
- test the private method (or make them less private)
- write the test in the test for each of the six methods calling the private (shared example group)
- test it in only one of those six methods
- rewrite the whole damn thing
and I’m sure many more other ways we didn’t come up with.
Anyone reading this has likely had a instinctual response and now a strong opinion on the *right* solution. The problem is each one of these approaches has trade offs — we have to consider code usability, documentation, future flexibility and sustainability, regression protection, confidence, code consistency and more. Often these dogmatic sound bytes cause strong reactions which lead us to skip over alternatives and tradeoffs.
In this particular option it made for some pretty ridiculous conversation too. Here is roughly my conversation with my team lead:
“Extracting a class here doesn’t seem right because these private methods are only useful in this class.”
Me: “Okay then maybe testing the private methods isn’t the the worst idea?”
“It most certainly is, we will not do that.”
Me: “Wait why?”
“Because we want to test behavior not implementation.”
The test he was suggesting to write (in either a shared example group or in one of the six methods) verified that class.should_receive(:method).with(param). Hilariously, testing exactly the implementation of the private method and not at all it’s behavior.
To accomplish what he actually wanted — we should have tested *the effect* of that private helper method by way of its public method. However, that can be super hard in a helper that does something complicated, or something that is difficult to analyze the effects of.
We relied on the assumption that private helper methods are simply implementation details, and as long as you aren’t testing them you aren’t testing implementation. But we are testing them (or at least we should be). We test them through the full testing of their callers, but sometimes that gets to be too onerous. In fact, we extract a private method because we’ve found a reasonable chunk of functionality. For the same reason we want that chunk extracted, we might want to extract the parts testing it as well, and test it separately. To test that chunk’s behavior.
Mantras and sound bytes are awesome because they remind us of great practices and help us keep track of difficult concepts, but they can be really dangerous when they allow us to get lazy, be silly, stop weighing our options, and looking for great solutions.