At this week’s ACCU Conference I went to an excellent talk by Sven Rosvall entitled “Unit Testing Beyond Mock Objects”.
The talk covered the newer Java and C# unit testing frameworks that allow inserting mock objects even where legacy code is using some undesired dependency directly, meaning there is no seam where you can insert a different implementation.
These tools solve a real problem, and could be useful.
However, I want to move the discussion in a different direction: can we avoid mocking altogether, and end up with better code?
Sven gave us an example of some legacy code a little bit like this. (I translated into C++ just for fun.)
// Return true if it's between 12:00 and 14:00 now. bool is_it_lunch_time_now() { system_clock::time_point now = system_clock::now(); time_t tt = system_clock::to_time_t( now ); tm local_tm = ( *localtime( &tt ) ); int hour = local_tm.tm_hour; return ( 12 <= hour && hour < 14 ); }
To test this code, we would have something like this:
TEST_CASE( "Now: It's lunchtime now (hopefully)" ) { // REQUIRE( is_it_lunch_time_now() ); // NOTE: only run at lunch time! REQUIRE( !is_it_lunch_time_now() ); // NOTE: never run at lunch time! }
Which is agony. But the production code is fine:
if ( is_it_lunch_time_now() ) { eat( sandwich ); }
So, the normal way to allow mocking out a dependency like this would be to add an interface, making our lunch-related code take in a TimeProvider. To avoid coupling the choice of which TimeProvider is used to the calling code, we pass it into the constructor of a LunchCalculator that we plan to make early on:
class TimeProvider { public: virtual tm now_local() const = 0; }; class RealTimeProvider : public TimeProvider { public: RealTimeProvider() { } virtual tm now_local() const { system_clock::time_point now = system_clock::now(); time_t tt = system_clock::to_time_t( now ); return ( *localtime( &tt ) ); } }; class HardCodedHourTimeProvider : public TimeProvider { public: HardCodedHourTimeProvider( int hour ) { tm_.tm_hour = hour; } virtual tm now_local() const { return tm_; } private: tm tm_; }; class LunchCalc { public: LunchCalc( const TimeProvider& prov ) : prov_( prov ) { } bool is_it_lunch_time() { int hour = prov_.now_local().tm_hour; return ( 12 <= hour && hour < 14 ); } private: const TimeProvider& prov_; };
and now we can write tests like this:
TEST_CASE( "TimeProvider: Calculate lunch time when it is" ) { HardCodedHourTimeProvider tp( 13 ); // 1pm (lunch time!) REQUIRE( LunchCalc( tp ).is_it_lunch_time() ); } TEST_CASE( "TimeProvider: Calculate lunch time when it isn't" ) { HardCodedHourTimeProvider tp( 10 ); // 10am (not lunch :-( ) REQUIRE( ! LunchCalc( tp ).is_it_lunch_time() ); }
Innovatively, these tests will pass at all times of day.
However, look at the price we’ve had to pay: 4 new classes, inheritance, and class names ending with unKevlinic words like “Provider” and “Calculator”. (Even worse, in a vain attempt to hide my embarrassment I abbreviated to “Calc”.)
We’ve paid a bit of a price in our production code too:
// Near the start of the program: config.time_provider = RealTimeProvider(); // Later, with config passed via PfA if ( LunchCalc( config.time_provider ).is_it_lunch_time() ) { eat( sandwich ); }
The code above where we create the RealTimeProvider is probably not usefully testable, and the class RealTimeProvider is also probably not usefully testable (or safely testable in the case of some more dangerous dependency). The rest of this code is testable, but there is a lot of it, isn’t there?
The advantage of this approach is that we have been driven to a better structure. We can now switch providers on startup by providing config, and even code way above this stuff can be fully exercised safe in the knowledge that no real clocks were poked at any time.
But. Sometimes don’t you ever think this might be better?
tm tm_local_now() // Not testable { system_clock::time_point now = system_clock::now(); time_t tt = system_clock::to_time_t( now ); return ( *localtime( &tt ) ); } bool is_lunch_time( const tm& time ) // Pure functional - eminently testable { int hour = time.tm_hour; return ( 12 <= hour && hour < 14 ); }
The tests look like this:
TEST_CASE( "Functional: Calculate lunch time when it is" ) { tm my_tm; my_tm.tm_hour = 13; // 1pm (lunch time!) REQUIRE( is_lunch_time( my_tm ) ); } TEST_CASE( "Functional: Calculate lunch time when it isn't" ) { tm my_tm; my_tm.tm_hour = 10; // 10am (not lunch time :-( ) REQUIRE( ! is_lunch_time( my_tm ) ); }
and the production code looks very similar to what we had before:
if ( is_lunch_time( tm_local_now() ) ) { eat( sandwich ); }
Where possible, let’s not mock stuff we can just test as pure, functional code.
Of course, the production code shown here is now untestable, so there may well be work needed to avoid calling it in a test. That work may involve a mock. Or we may find a nicer way to avoid it.
… What about just mocking what you use? Stop trying to make C++ into Java with EasyMock or C# with RhinoMock, use something tailored to C++ (and I mean all of C++).
#include “hippomocks.h”
TEST_CASE( “Calculate lunch time when it is” )
{
MockRepository mocks;
mocks.ExpectCall(system_clock::now).Return(system_clock::lunchtime);
REQUIRE( is_lunch_time() );
}
TEST_CASE( “Calculate lunch time when it isn’t” )
{
MockRepository mocks;
mocks.ExpectCall(system_clock::now).Return(system_clock::dinnertime);
REQUIRE( ! is_lunch_time() );
}
Stop using C++ as if it’s a badly implemented Java or a badly implemented Haskell. Use it for what it’s worth.
I have been thinking on how to mock avoiding to,write an interface.
You could do this, though it is not superior to the last solution:
extern std::function is_it_lunch_time_now;
Default implementation goes in .cpp.
In test code:
is_lunch_time_now = /*mock impl*/;
function is bool() but the website swallowed things
between less than and greater than symbols
due to markup I guess.
You can avoid writing an interface doing this, but needs source code modification:
extern std::function is_it_lunch_time_now;
.cpp file:
is_it_lunch_time_now = []() -> bool { … };
test file, rebind function:
is_it_lunch_time_now = /*mock implementation*/
Peter: it looks like hoppomocks gives you the ability to insert a seam where there isn’t one, like PowerMock which was the subject of the talk that inspired this post. This sounds like a helpful thing to be able to do when you’re in a situation where there is a lot of code already using untestable code, but I am trying to think about how we would strucure code ideally. In my opinion, in the ideal case we wouldn’t rely on “magic” like that. Of course, that argument relies on us sharing understanding of what is “magic” and what is perfectly good and normal code.
Germán: thank you, I fixed the less-thans.
Germán: yes, that is another way to avoid mocks. But isn’t the functional style for is_lunch_time() nicer anyway, even if we also implement is_it_lunch_time_now() based on it?
> Of course, that argument relies on us sharing understanding of what is “magic†and what is perfectly good and normal code.
To me it is the discussion of what exactly an interface is. Is it only an interface if you have a C++ class with pure virtual methods that you can replace with a mock object? Or is a set of C functions with a coherent goal also an interface?
I tend to the latter. Having code that does not require mocks to test is inherently better, but you will have to tie those bits of code together *somehow*, and you’ll need to check that the whole of those bits of code does what you expect it to do. You can check a key and a lock, but until you check them together you don’t know you have the *right* key.
Hi Peter, yes – it is legitimate to use natural seams (e.g. a mock implementation that matches the declared function signature of the real dependency) that are available in C++ that are not in a language like Java. However, I’d still prefer to write code whose behaviour depends only on its input where possible, and doesn’t use an untestable depenency call in its implementation. I do admit you’ve suggested a good way to replace that dependency call, but I’d still rather not have it.
First – good to see you using Catch for your test framework :-)
Second – I completely agree with your thought processes here. It’s one of the reasons that truly functional code is so much easier to reason about (once you get your head around the paradigm in the first place) – but it all has it’s start in straightforward stuff like this.
Thirdly – despite a different language and focus – it echoes a lot of what I wrote here: http://www.levelofindirection.com/journal/2013/7/11 (which I know you’ve seen because you were the first commenter ;-) )
Phil – yes, this was my first go with CATCH. I enjoyed it.
We are in total agreement :-) If only everyone listened to us …?
(That link should be http://www.levelofindirection.com/journal/2013/7/11/injecting-singletons-in-objective-c-unit-tests.html )