What is the best way of testing private methods with GoogleTest?

Question

I would like to test some private methods using GoogleTest.

class Foo
{
private:
    int bar(...)
}

GoogleTest allows a couple of ways of doing this.

OPTION 1

With FRIEND_TEST:

class Foo
{
private:
    FRIEND_TEST(Foo, barReturnsZero);
    int bar(...);
}

TEST(Foo, barReturnsZero)
{
    Foo foo;
    EXPECT_EQ(foo.bar(...), 0);
}

This implies to include "gtest/gtest.h" in the production source file.

OPTION 2

Declare a test fixture as a friend to the class and define accessors in the fixture:

class Foo
{
    friend class FooTest;
private:
    int bar(...);
}

class FooTest : public ::testing::Test
{
protected:
    int bar(...) { foo.bar(...); }
private:
    Foo foo;
}

TEST_F(FooTest, barReturnsZero)
{
    EXPECT_EQ(bar(...), 0);
}

OPTION 3

The Pimpl idiom.

For details: Google Test: Advanced guide.

Are there any other ways to test private methods? What are some pros and cons of each option?

Answer 1

There are at least two more options. I'll list out some other options you should consider by explaining a certain situation.

Option 4:

Consider refactoring your code so that the part you want to test is public in another class. Typically when you're tempted to test a class's private method, it's a sign of bad design. One of the most common (anti)paterns that I see is what Michael Feathers calls an "Iceberg" class. "Iceberg" classes have one public method, and the rest are private (which is why it's tempting to test the private methods). It might look something like this:

For example, you might want to test GetNextToken() by calling it on a string successively and seeing that it returns the expected result. A function like this does warrant a test: that behavior isn't trivial, especially if your tokenizing rules are complex. Let's pretend it's not all that complex, and we just want to rope in tokens delimited by space. So you write a test, maybe it looks something like this:

TEST(RuleEvaluator, canParseSpaceDelimtedTokens)
{
    std::string input_string = "1 2 test bar";
    RuleEvaluator re = RuleEvaluator(input_string);
    EXPECT_EQ(re.GetNextToken(), "1");
    EXPECT_EQ(re.GetNextToken(), "2");
    EXPECT_EQ(re.GetNextToken(), "test");
    EXPECT_EQ(re.GetNextToken(), "bar");
    EXPECT_EQ(re.HasMoreTokens(), false);
}

Well, that actually looks pretty nice. We'd want to make sure we maintain this behavior as we make changes. But GetNextToken() is a private function! So we can't test it like this, because it wont even compile. But what about changing the RuleEvaluator class to follow the Single Responsibility Principle (Single Responsibility Principle)? For instance, we seem to have a parser, tokenizer, and evaluator jammed into one class. Wouldn't it be better to just separate those responsibilities? On top of that, if you create a Tokenizer class, then it's public methods would be HasMoreTokens() and GetNextTokens(). The RuleEvaluator class could have a Tokenizer object as a member. Now, we can keep the same test as above, except we are testing the Tokenizer class instead of the RuleEvaluator class.

Here's what it might look like in UML:

Note that this new design increases modularity, so you could potentially re-use these classes in other parts of your system (before you couldn't, private methods aren't reusable by definition). This is main advantage of breaking the RuleEvaluator down, along with increased understandability/locality.

The test would look extremely similar, except it would actually compile this time since the GetNextToken() method is now public on the Tokenizer class:

TEST(Tokenizer, canParseSpaceDelimtedTokens)
{
    std::string input_string = "1 2 test bar";
    Tokenizer tokenizer = Tokenizer(input_string);
    EXPECT_EQ(tokenizer.GetNextToken(), "1");
    EXPECT_EQ(tokenizer.GetNextToken(), "2");
    EXPECT_EQ(tokenizer.GetNextToken(), "test");
    EXPECT_EQ(tokenizer.GetNextToken(), "bar");
    EXPECT_EQ(tokenizer.HasMoreTokens(), false);
}

Option 5

Just don't test the private functions. Sometimes they aren't worth testing because they will be tested through the public interface. A lot of times what I see is tests that look very similar, but test two different functions/methods. What ends up happening is that when requirements change (and they always do), you now have 2 broken tests instead of 1. And if you really tested all your private methods, you might have more like 10 broken tests instead of 1. In short, testing private functions (by using FRIEND_TEST or making them public) that could otherwise be tested through a public interface cause test duplication. You really don't want this, because nothing hurts more than your test suite slowing you down. It's supposed to decrease development time and decrease maintenance costs! If you test private methods that are otherwise tested through a public interface, the test suite may very well do the opposite, and actively increase maintenance costs and increase development time. When you make a private function public, or if you use something like FRIEND_TEST, you'll usually end up regretting it.

Consider the following possible implementation of the Tokenizer class:

Let's say that SplitUpByDelimiter() is responsible for returning a std::vector<std::string> such that each element in the vector is a token. Furthermore, let's just say that GetNextToken() is simply an iterator over this vector. So your tests might look this:

TEST(Tokenizer, canParseSpaceDelimtedTokens)
{
    std::string input_string = "1 2 test bar";
    Tokenizer tokenizer = Tokenizer(input_string);
    EXPECT_EQ(tokenizer.GetNextToken(), "1");
    EXPECT_EQ(tokenizer.GetNextToken(), "2");
    EXPECT_EQ(tokenizer.GetNextToken(), "test");
    EXPECT_EQ(tokenizer.GetNextToken(), "bar");
    EXPECT_EQ(tokenizer.HasMoreTokens(), false);
}

// Pretend we have some class for a FRIEND_TEST
TEST_F(TokenizerTest, canGenerateSpaceDelimtedTokens)
{
    std::string input_string = "1 2 test bar";
    Tokenizer tokenizer = Tokenizer(input_string);
    std::vector<std::string> result = tokenizer.SplitUpByDelimiter(" ");
    EXPECT_EQ(result.size(), 4);
    EXPECT_EQ(result[0], "1");
    EXPECT_EQ(result[1], "2");
    EXPECT_EQ(result[2], "test");
    EXPECT_EQ(result[3], "bar");
}

Well, now let's say the requirements change, and you're now expected to parse by a "," instead of a space. Naturally, you're going to expect one test to break, but the pain increases when you test private functions. IMO, google test should not allow FRIEND_TEST. It is almost never what you want to do. Michael Feathers refers to things like FRIEND_TEST as a "groping tool", since it's trying to touch someone else's private parts.

I recommend avoiding option 1 and 2 when you can, as it typically causes "test duplication", and as a consequence, many more tests than necessary will break when requirements change. Use them as a last resort. Option 1 and 2 are the fastest ways to "test private methods" for here and now (as in the fastest to implement), but they'll really hurt productivity in the long run.

PIMPL can make sense too, but it still allows for some pretty bad design. Be careful with it.

I'd recommend Option 4 (refactoring into smaller testable components) as the right place to start, but sometimes what you really want is Option 5 (testing the private functions through the public interface).

P.S. Here's the relevant lecture about iceberg classes: https://www.youtube.com/watch?v=4cVZvoFGJTU

P.S.S. As for everything in software, the answer is it depends. There is no one size fits all. The option that solves your problem will depend on your specific circumstances.