How NOT to use mocks in googletest framework and why singletons are bad

Lets make a game*. It is about marmot – a great, wild beast that can do one thing: it can sleep. Marmot’s state depends on the superposition of his universe: in the winter he sleeps. And during the night he sleeps. And if he not sleeps, he is awake. Our Universe interface will look like this:

class Universe
{
public:
    virtual ~Universe() = default;

    virtual bool IsDay() const = 0;
    virtual bool IsWinter() const = 0;
};

Now comes a brilliant idea: let’s model The Universe as a singleton class – in the end there should be only one and its state should be same for all the clients (marmots). We add this factory method.

Universe& UniverseFactory::getInstance()
{
	static RandomUniverse singleton;
	return singleton;
}

Marmot can now use it to get a Universe and figure out if he supposed to sleep or not.

MarmotState Marmot::getState() const
{
    if(UniverseFactory::getInstance().IsWinter()) {
        return MarmotState::MARMOT_IS_SLEEPING;
    }

    if(UniverseFactory::getInstance().IsDay()) {
        return MarmotState::MARMOT_IS_AWAKE;
    }

    // Oh oh - we made a mistake here! Marmot should not be awake
    return MarmotState::MARMOT_IS_AWAKE;
}

Of course everything is unit tested and all the tests are green so lets “ship it!“.

Next day the support phone line gets red hot as hundreds of people are calling, asking why their marmots are not sleeping in the middle of the night. What happened here? Why our tests did not catch this? Let’s look at them.

static UniverseMock universe;

Universe& UniverseFactory::getInstance()
{
	return universe;
}

class SomeClassTest: public ::testing::Test {
public:

	void SetUp() {}

	void TearDown() {}
};

TEST_F(MarmotTest, When_Not_Winter_And_Day_Should_Be_Awake) {
    Marmot marmot;

    EXPECT_CALL(universe, IsWinter()).WillOnce(Return(false));
    EXPECT_CALL(universe, IsDay()).WillOnce(Return(true));
    ASSERT_EQ(MarmotState::MARMOT_IS_AWAKE, marmot.getState());
}

TEST_F(MarmotTest, When_Winter_Should_Sleep) {
    Marmot marmot;

    EXPECT_CALL(universe, IsWinter()).WillRepeatedly(Return(true)); // oh!

    EXPECT_CALL(universe, IsDay()).WillOnce(Return(true));
    ASSERT_EQ(MarmotState::MARMOT_IS_SLEEPING, marmot.getState());

    EXPECT_CALL(universe, IsDay()).WillOnce(Return(false));
    ASSERT_EQ(MarmotState::MARMOT_IS_SLEEPING, marmot.getState());
}

TEST_F(MarmotTest, When_Night_Should_Sleep) {
    Marmot marmot;

    EXPECT_CALL(universe, IsDay()).WillOnce(Return(false));
    ASSERT_EQ(MarmotState::MARMOT_IS_SLEEPING, marmot.getState());
}

The last test should fail but it does not because we’ve made two terrible things: the universe mock is a static variable and there is no expectation on IsWinter in the last test. The first issue is the real problem here. The mock object does hold its state until destroyed and in the second test we asked it to always return true when ‘IsWinter’ gets called. Fine. Let’s just create and destroy the mock for every test run so the expectations don’t propagate between tests.

static UniverseMock* universe;

Universe& UniverseFactory::getInstance()
{
    return *universe;
}

class MarmotTest: public ::testing::Test {
public:

    void SetUp() {
        universe = new UniverseMock;
    }

    void TearDown() {
        delete universe;
    }
};

And this will work until one day somebody does this:

auto& universe = UniverseFactory::getInstance();

MarmotState Marmot::getState() const
{
if(universe.IsWinter()) {
return MarmotState::MARMOT_IS_SLEEPING;
}

if(universe.IsDay()) {
return MarmotState::MARMOT_IS_AWAKE;
}

...

Now we get a segfault during testing — because our singleton is recreated for each test, while the marmot updates its reference only once. Which brings us to the main conclusion of this post: singletons are bad, and we should think carefully before deciding to use them. And if we do choose to use a singleton, we need to think even harder to ensure our code remains testable.

Does that mean we have to redesign our marmot game?
Not necessarily. We can leave the bad code untouched — with the help of a magical function that verifies and clears expectations between tests. It’s called (surprise, surprise): VerifyAndClearExpectations. And we can just call it after every test run, like this:

class MarmotTest: public ::testing::Test {
public:

    void SetUp() {}

    void TearDown() {
        Mock::VerifyAndClearExpectations(&universe);
    }
};

Now tests go red and we can clearly see the root-causes – one because of wrong expectations and second because of the bug. Fix it, ship it, and go for a… not yet! First, add a “Refactor Marmot Universe” ticket to the technical backlog. Now you can go for a beer — or to sleep, if you’re a marmot and it’s winter. Or night.

*All source code can be found here

Leave a comment