Search code examples
c++c++20googletestgooglemocknon-virtual-interface

Mocking problem on Non-Virtual functions with NVI Idiom


I followed Herb Sutter's NVI idiom on my class design because of its benefits. My abstract class is as follows in short.

class Communicator {
public:
    virtual ~Communicator() = default;
    bool Connect(void) {
        // some preoperations
        return DoConnect();
    }
    // 3 more similar public member functions

private:
    virtual bool DoConnect(void) = 0;
    // 3 more similar pure virtual member functions
};

I inject Communicator class to the class I want to test as follows (I found this method from Google docs) :

template <typename CommunicatorClass> // There will be some constraints
class TestingClass {
public:
    TestingClass(std::unique_ptr<CommunicatorClass> comm) : comm_{std::move(comm)} { }
private:
    std::unique_ptr<CommunicatorClass> comm_ = nullptr;
};

My Mock class :

class MockCommunicator : public Communicator {
public:
    MOCK_METHOD(bool, Connect, ());
    // 3 more mocks
};

My Test suite Setup:

// some code ..
auto mock = std::make_unique<MockCommunicator>();
TestingClass<MockCommunicator>(std::move(mock));

As you can guess since I did not override 4 private virtual functions MockCommunicator becomes abstract and the compiler gives error. Options I thought :

  1. Overriding virtual functions in MockCommunicator. (It seems ugly, but I am closest to this approach)
  2. I could convert pure virtual functions to virtual functions but I am not the only one who owns the code base. I want to force other developers to implement their own implementation.
  3. Making MockCommunicator the friend of Communicator class and mock private virtual functions. ( I think this is absolutely wrong. Is it?)
  4. Giving up on NVI because of its complexity to the code base

Is there any recommended way of handling this situation?

Note : I use C++20


Solution

  • Instead of mocking your public Connect it would be easier to mock the virtual private DoConnect (and all other pure virtual functions of course, it's not ugly, if you mock, you mock all the interface, not a fraction of it):

    class MockCommunicator : public Communicator {
    public:
        MOCK_METHOD(bool, DoConnect, (), (override));
        // 3 more mocks
    };
    

    Note that in mock DoConnect is public so that you can use it in EXPECT_CALL macros. Then assuming your TestingClass has some simple way to invoke the Communicator observable behavior:

        bool run() {
            return comm_->Connect();
        }
    

    you can write a simple test:

    TEST(xxx, yyy) {
        auto mock = std::make_unique<MockCommunicator>();
        auto* rawMock = mock.get();
        auto sut = TestingClass<MockCommunicator>(std::move(mock));
        EXPECT_CALL(*rawMock, DoConnect()).WillRepeatedly(testing::Return(true));
        ASSERT_TRUE(sut.run());
    }
    

    Having that, you don't even need your TestingClass to be a template:

    class TestingClass {
    public:
        TestingClass(std::unique_ptr<Communicator> comm) : comm_{std::move(comm)} { }
        bool run() {
            return comm_->Connect();
        }
    private:
        std::unique_ptr<Communicator> comm_;
    };
    

    And as a side note, your class Communicator must have a virtual destructor.

    godbolt demo