Writing mockable code in C++

Question

I am focusing on writing easy to test code these days and I realize that most of my classes follow the skeleton below:

Some class ObjectManager needs a DatabaseRepo, while there is only one type DatabaseRepo in production (OracleRepo), I am still implementing it against an interface because I want to be able to mock out the DatabaseRepo, thus I need to make use of polymorphism. I inject the DatabaseRepo as a unique_ptr to the ObjectManager's constructor, in order to indicate that ObjectManager owns the DatabaseRepo.

One thing, for example, which is flawed here is that in the unit test, I access the state of the MockRepo after I moved it into the ObjectManager. While its is safe (ObjectManager) is still alive it looks like a hack.

---

Question: Using this as a general pattern for the situation where class A has an object of type B, are there any design aspects I could improve?

---

class ObjectManager {
  public:
     ObjectManager(std::unique_ptr<DatabaseRepo> initRepo) : repo(initRepo) {

     }

     void process() {
       repo->updateRecords();
     }


  private:
    std::unique_ptr<DatabaseRepo> repo;
};


class DatabaseRepo {
  public:
    virtual void updateRecords = 0;
} 

class OracleRepo : public DatabaseRepo {
  public:
    void updateRecords() overrides {

    }
}

class MockRepo : public DatabaseRepo {
  public:
    void updateRecords() overrides {

    }
}

testObjectManager() {
   std::unique_ptr<DatabaeRepo> mockrepo = 
       std::unique_ptr<DatabseRepo>(new MockRepo);

   ObjectManager om(std::move(mockrepo)); 
   om.process();

   EXPECT_CALLED_ONCE(mockrepo.get()->updateRecords());
}

Answer 1

You know that the pointer is going to remain valid until ObjectManager is destroyed, but you want to avoid deferencing a moved-from unique_ptr. In this case it should be OK to keep the original pointer and use that:

testObjectManager() {
   DatabaseRepo* mockrepo = new MockRepo;

   ObjectManager om(std::unique_ptr<DatabaseRepo>(mockrepo)); 
   om.process();

   EXPECT_CALLED_ONCE(mockrepo->updateRecords());
}

Answer 2

Why do you pass it as an std::unique_ptr<> ?

I would use a non-const reference instead.

One thing, for example, which is flawed here is that in the unit test, I access the state of the MockRepo after I moved it into the ObjectManager.

You will be ensured that the mock lives as long as you need it to, as you have full control over the life-time yourself.

Your code will even be simpler, as there is no need to use new:

testObjectManager() {
   DatabaseRepo mockrepo;

   ObjectManager om(mockrepo);
   om.process();

   EXPECT_CALLED_ONCE(mockrepo.updateRecords());
}

Answer 3

One possible solution would be to create mocks as unique_ptrs, then set up the expectations, then create the class under test (thus invalidating the unique_ptrs). So the mocks can be members of the fixture, but the class under test cannot be.

This assumes that the expectations are set up ahead of time rather than checked after the fact. I use gmock, so the assumption holds. Your mocking system may be different.

Answer 4

You can take action in the destructor of MockRepo to inform other parts of the test code that the mock has been destroyed.

class MockRepo : public DatabaseRepo {
    MockRepo *& signal;
  public:
    MockRepo(MockRepo *& signal) : signal(signal) { signal = this; }
    ~MockRepo() { signal = nullptr; }
    void updateRecords() override {

    }
}

testObjectManager() {
   MockRepo * mock; 

   ObjectManager om(std::make_unique<MockRepo>(mock)); 
   om.process();

   EXPECT_NOT_NULL(mock);
   EXPECT_CALLED_ONCE(mock->updateRecords());
}

Note that your program has undefined behaviour, as DatabaseRepo lacks a virtual destructor.

class DatabaseRepo {
  public:
    DatabaseRepo(const DatabaseRepo &) = delete;
    DatabaseRepo& operator=(const DatabaseRepo &) = delete;
    DatabaseRepo(DatabaseRepo &&) = delete;
    DatabaseRepo& operator=(DatabaseRepo &&) = delete;
    virtual ~DatabaseRepo() = default;
    virtual void updateRecords() = 0;
}