Sustainable C++ Testing

Riddle #4: Uneventful

We have a class counting events. In order to do so, it registers a listener with an event manager in its constructor.

#include <gtest/gtest.h>
#include <gmock/gmock.h>

class EventManager {
public:
  class EventListener {
  public:
    virtual ~EventListener() = default;
    virtual void onEvent() = 0;
  };

  virtual ~EventManager() = default;
  virtual void registerEventListener(EventListener* listener) = 0;
};

class EventCounter :
  // private inheritance as it's an implementation detail and this EventCounter
  // should be free to refactor (e.g. move the listener implementation to a separate class)
  private EventManager::EventListener {
public:
  explicit EventCounter(EventManager& eventManager) {
    eventManager.registerEventListener(this);
  }
  
private:
  void onEvent() override {
    ++eventCount;
  }
  
public:
  int eventCount = 0;
};

class MockEventManager : public EventManager {
public:
  MOCK_METHOD1(registerEventListener, void(EventListener*));
};

TEST(EventCounterTest, eventDispatch_incrementsCounter) {
  MockEventManager eventManager;
  EventManager::EventListener* listener = nullptr;
  
  EXPECT_CALL(eventManager, registerEventListener(testing::_))
    .WillOnce(testing::SaveArg<0>(&listener));
  EventCounter eventCounter(eventManager);
  
  EXPECT_EQ(0, eventCounter.eventCount);
  listener->onEvent();
  EXPECT_EQ(1, eventCounter.eventCount);
}

#include <gtest/gtest.h>
#include <gmock/gmock.h>

class EventManager {
public:
  class EventListener {
  public:
    virtual ~EventListener() = default;
    virtual void onEvent() = 0;
  };
  
  virtual ~EventManager() = default;
  virtual void registerEventListener(EventListener* listener) = 0;
};

class EventCounter :
  // private inheritance as it's an implementation detail and this EventCounter
  // should be free to refactor (e.g. move the listener implementation to a separate class)
  private EventManager::EventListener {
public:
  explicit EventCounter(EventManager& eventManager) {
    eventManager.registerEventListener(this);
  }
  
private:
  void onEvent() override {
    ++eventCount;
  }
  
public:
  int eventCount = 0;
};

class MockEventManager : public EventManager {
public:
  MOCK_METHOD1(registerEventListener, void(EventListener*));
};

TEST(EventCounterTest, eventDispatch_incrementsCounter) {
  MockEventManager eventManager;
  EventManager::EventListener* listener = nullptr;
  
  EXPECT_CALL(eventManager, registerEventListener(testing::_))
    .WillOnce(testing::SaveArg<0>(&listener));
  EventCounter eventCounter(eventManager);
  ASSERT_NE(listener, nullptr);
  
  EXPECT_EQ(0, eventCounter.eventCount);
  listener->onEvent();
  EXPECT_EQ(1, eventCounter.eventCount);
}

#include <gtest/gtest.h>
#include <gmock/gmock.h>

class EventManager {
public:
  class EventListener {
  public:
    virtual ~EventListener() = default;
    virtual void onEvent() = 0;
  };
  
  virtual ~EventManager() = default;
  virtual void registerEventListener(EventListener* listener) = 0;
};

class EventCounter :
  // private inheritance as it's an implementation detail and this EventCounter
  // should be free to refactor (e.g. move the listener implementation to a separate class)
  private EventManager::EventListener {
public:
  explicit EventCounter(EventManager& /*eventManager*/) {
//    eventManager.registerEventListener(this);
  }
  
private:
  void onEvent() override {
    ++eventCount;
  }
  
public:
  int eventCount = 0;
};

class MockEventManager : public EventManager {
public:
  MOCK_METHOD1(registerEventListener, void(EventListener*));
};

TEST(EventCounterTest, eventDispatch_incrementsCounter) {
  MockEventManager eventManager;
  EventManager::EventListener* listener = nullptr;
  
  EXPECT_CALL(eventManager, registerEventListener(testing::_))
    .WillOnce(testing::SaveArg<0>(&listener));
  EventCounter eventCounter(eventManager);
  
  EXPECT_EQ(0, eventCounter.eventCount);
  listener->onEvent();
  EXPECT_EQ(1, eventCounter.eventCount);
}

Can you spot a weakness in the test?

Give me a hint

What if EventCounter does not always register a listener? show me

Reveal the answer

Diagnosis

The test assumes a listener will always be registered and dereferences it without any checks, which may lead to a crash, preventing the test suite from executing fully. It's a common mistake in assuming that just expressing the expectations is good enough for the test. The expectations, however, are only verified automatically upon mock destruction. In some cases, this is not good enough.

Reveal the remedy

Remedy

The most straightforward way to improve the test is to assert on the listener being set (show me).
Note there is another GMock mechanism that seems tempting in this context, testing::Mock::VerifyAndClear. It, however, does not generate a fatal failure, so it is not easy to use it to interrupt the test execution and avoid the problem - a potential crash.

Feedback on this riddle? 3c6120687265663d226d61696c746f3a666565646261636b407375737461696e61626c6563707074657374696e672e636f6d3f7375626a6563743d5375737461696e61626c6520432b2b2054657374696e672c20726964646c65233420666565646261636b223e446f2067657420696e20746f756368213c2f613e