Testing private class member in C++ without friend [duplicate]

StackOverflow https://stackoverflow.com/questions/11458223

Question

Today I had a discussion with a colleague on whether to test or not to test private members or private state in the class. He almost convinced me why it makes sense. This question does not aim to duplicate already existing StackOverflow questions about the nature and reason of testing private members, like: What is wrong with making a unit test a friend of the class it is testing?

Colleagues suggestion was in my opinion a bit fragile to introduce the friend declaration to the unit test implementation class. In my opinion this is a no-go, because we introduce some dependency of tested code to the test code, whereas test code already depends on tested code => cyclic dependency. Even such innocent things like renaming a test class results in breaking unit tests and enforces code changes in tested code.

I'd like to ask C++ gurus to judge on the other proposal, which relies on the fact that we are allowed to specialize a template function. Just imagine the class:

// tested_class.h

struct tested_class 
{
  tested_class(int i) : i_(i) {}

  //some function which do complex things with i
  // and sometimes return a result

private:
  int i_;
};

I don't like the idea to have a getter for i_ just to make it testable. So my proposal is 'test_backdoor' function template declaration in the class:

// tested_class.h

struct tested_class 
{
  explicit
  tested_class(int i=0) : i_(i) {}

  template<class Ctx>
  static void test_backdoor(Ctx& ctx);

  //some function which do complex things with i
  // and sometimes return a result

private:
  int i_;
};

By adding just this function we can make the class' private members testable. Note, there is no dependency to unit test classes, nor the template function implementation. In this example the unit test implementation uses Boost Test framework.

// tested_class_test.cpp

namespace
{
  struct ctor_test_context
  {
    tested_class& tc_;
    int expected_i;
  };
}

// specialize the template member to do the rest of the test
template<>
void tested_class::test_backdoor<ctor_test_context>(ctor_test_context& ctx)
{
  BOOST_REQUIRE_EQUAL(ctx.expected_i, tc_.i_);
}

BOOST_AUTO_TEST_CASE(tested_class_default_ctor)
{
  tested_class tc;
  ctor_test_context ctx = { tc, 0 };
  tested_class::test_backdoor(ctx);
}

BOOST_AUTO_TEST_CASE(tested_class_value_init_ctor)
{
  tested_class tc(-5);
  ctor_test_context ctx = { tc, -5 };
  tested_class::test_backdoor(ctx);
}

By introducing just a single template declaration, which is not callable at all, we give the test implementer a possibility to forward test logic into a function. The function, acts on type safe contexts and is only visible from inside the particular test compilation unit, due to anonymous type nature of test context. And the best thing is, we can define as many anonymous test contexts as we like and specialize tests on them, without ever touching the tested class.

Sure, the users must know what template specialization is, but is this code really bad or weird or unreadable? Or can I expect from C++ developers to have the knowledge what C++ template specialization is and how it works?

Elaborating on using friend to declare unit test class I don't think this is robust. Imagine boost framework (or may be other test frameworks). It generates for every test case a separate type. But why should I care as long I can write:

BOOST_AUTO_TEST_CASE(tested_class_value_init_ctor)
{
  ...
}

If using friends, I had to declare each test case as a friend then... Or end up introducing some test functionality in some common type (like fixture), declare it as a friend, and forward all test calls to that type... Isn't that weird?

I would like to see your pro and cons practicing this approach.

Was it helpful?

Solution

What will follow is not technically speaking a straight answer to your question as it will still make use of the "friend" functionality but it does not require modification of the tested entity itself and I think it addesses the concern of breaking the encapsulation mentioned in some of the other answers; it does though require writing some boilerplate code.

The idea behind it is not mine and the implementation is entirely based on a trick presented and explained by litb on his blog(coupled with this Sutter's gotw for just a little bit more context, at least for me) - in short CRTP, friends, ADL and pointers to members (I must confess that to my dismay the ADL part I still don't get it entirely, but I'm relentesly working in figuring it out 100%).

I tested it with gcc 4.6, clang 3.1 and VS2010 compilers and it works perfectly.

/* test_tag.h */
#ifndef TEST_TAG_H_INCLUDED_
#define TEST_TAG_H_INCLUDED_

template <typename Tag, typename Tag::type M>
struct Rob
{
    friend typename Tag::type get(Tag)
    {
        return M;
    }
};

template <typename Tag, typename Member> 
struct TagBase
{
    typedef Member type;
    friend type get(Tag);
};


#endif /* TEST_TAG_H_INCLUDED_ */

/* tested_class.h */
#ifndef TESTED_CLASS_H_INCLUDED_
#define TESTED_CLASS_H_INCLUDED_

#include <string>

struct tested_class
{
    tested_class(int i, const char* descr) : i_(i), descr_(descr) { }

private:
    int i_;
    std::string descr_;
};

/* with or without the macros or even in a different file */
#   ifdef TESTING_ENABLED
#   include "test_tag.h"

    struct tested_class_i : TagBase<tested_class_i, int tested_class::*> { };
    struct tested_class_descr : TagBase<tested_class_descr, const std::string tested_class::*> { };

    template struct Rob<tested_class_i, &tested_class::i_>;
    template struct Rob<tested_class_descr, &tested_class::descr_>;

#   endif

#endif /* TESTED_CLASS_H_INCLUDED_ */

/* test_access.cpp */
#include "tested_class.h"

#include <cstdlib>
#include <iostream>
#include <sstream>

#define STRINGIZE0(text) #text
#define STRINGIZE(text) STRINGIZE0(text)

int assert_handler(const char* expr, const char* theFile, int theLine)
{
    std::stringstream message;
    message << "Assertion " << expr << " failed in " << theFile << " at line " << theLine;
    message << "." << std::endl;
    std::cerr << message.str();

    return 1;
}

#define ASSERT_HALT() exit(__LINE__)

#define ASSERT_EQUALS(lhs, rhs) ((void)(!((lhs) == (rhs)) && assert_handler(STRINGIZE((lhs == rhs)), __FILE__, __LINE__) && (ASSERT_HALT(), 1)))

int main()
{
    tested_class foo(35, "Some foo!");

    // the bind pointer to member by object reference could
    // be further wrapped in some "nice" macros
    std::cout << " Class guts: " << foo.*get(tested_class_i()) << " - " << foo.*get(tested_class_descr()) << std::endl;
    ASSERT_EQUALS(35, foo.*get(tested_class_i()));
    ASSERT_EQUALS("Some foo!", foo.*get(tested_class_descr()));

    ASSERT_EQUALS(80, foo.*get(tested_class_i()));

    return 0; 
}

OTHER TIPS

I think unit testing is about testing the observable behavior of the class under test. Therefore there is no need to test private parts as they themselves are not observable. The way you test it is by testing whether the object behaves the way you expect it (which implicitly implies that all private internal states are in order).

The reason for not to be concerned about the private parts is that this way you can change the implementation (e.g. refactoring), without having to rewrite your tests.

So my answer is don't do it (even if technically possible to) as it goes against the philosophy of unit tests.

Pros

  • You can access the private members to test them
  • Its a fairly minimal amount of hack

Cons

  • Broken encapsulation
  • Broken encapsulation that is more complicated and just as brittle as friend
  • Mixing test with production code by putting test_backdoor on the production side
  • Maintance problem ( just like friending the the test code, you've created an extremely tight coupling with your test code )

All of the Pros/Cons aside, I think you are best off making some architectural changes that allow better testing of whatever complex stuff is happening.

Possible Solutions

  • Use the Pimpl idiom, put the complex code in the pimpl along with the private member, and write a test for the Pimpl. The Pimpl can be forward declared as a public member, allowing external instantiation in the unit test. The Pimpl can consist of only public members, making it easier to test
    • Disadvantage: Lots of code
    • Disadvantage: opaque type that can be more difficult to see inside of when debugging
  • Just test the public/protected interface of the class. Test the contract that your interface lays out.
    • Disadvantage: unit tests are difficult/impossible to write in an isolated manner.
  • Similar to the Pimpl solutions, but create a free function with the complex code in it. Put the declaration in a private header ( not part of the libraries public interface ), and test it.
  • Break encapsulation via friend a test method/fixture
    • Possible variation on this: declare friend struct test_context;, put your test code inside of methods in the implementation of struct test_context. This way you don't have to friend each test case, method, or fixture. This should reduce the likelyhood of someone breaking the friending.
  • Break encapsulation via template specialization

I am sorry to advice this, but it helped me when most methods in those answers are not achievable without strong refactoring: add before the header for the file with the class whose private members you wish to access,

#define private public

It is evil, but

  • doesn't interfere with production code

  • does not break encapsulation as friend / changing access level does

  • avoids heavy refactoring with PIMPL idiom

so you may go for it...

Testing private members is not always about verifying the state by checking if it equals some expected values. In order to accommodate other, more intricate test scenarios, I sometimes use the following approach (simplified here to convey the main idea):

// Public header
struct IFoo
{
public:
    virtual ~IFoo() { }
    virtual void DoSomething() = 0;
};
std::shared_ptr<IFoo> CreateFoo();

// Private test header
struct IFooInternal : public IFoo
{
public:
    virtual ~IFooInternal() { }
    virtual void DoSomethingPrivate() = 0;
};

// Implementation header
class Foo : public IFooInternal
{
public:
    virtual DoSomething();
    virtual void DoSomethingPrivate();
};

// Test code
std::shared_ptr<IFooInternal> p =
    std::dynamic_pointer_cast<IFooInternal>(CreateFoo());
p->DoSomethingPrivate();

This approach has the distinct advantage of promoting good design and not being messy with friend declarations. Of course, you don't have to go through the trouble most of the time because being able to test private members is a pretty nonstandard requirement to begin with.

I don't usually feel the need to unit test private members and functions. I might prefer to introduce a public function just to verify correct internal state.

But if I do decide to go poking around in the details, I use a nasty quick hack in the unit test program:

#include <system-header>
#include <system-header>
// Include ALL system headers that test-class-header might include.
// Since this is an invasive unit test that is fiddling with internal detail
// that it probably should not, this is not a hardship.

#define private public
#include "test-class-header.hpp"
...

On Linux at least this works because the C++ name mangling does not include the private/public state. I am told that on other systems this may not be true and it wouldn't link.

I used a function to test private class members which was just called TestInvariant().

It was a private member of the class and, in debug mode, was called at the beginning and end of every function (except the beginning of the ctor and end of the dctor).

It was virtual and any base class called the parent version before it's own.

That allowed me to verify the internal state of the class all of the time without exposing the intenals of the class to anyone. I had very simple tests but there is no reason why you could not have complicated ones, or even set it on or off with a flag etc.

Also you can have public Test functions which can be called by other classes which call your TestInvariant() function. Therefore when you need to change inner class workings you do not need to change any user code.

Would this help?

I think the first thing to ask is: Why is friend considered to be something that must be used with caution?

Because it breaks encapsulation. It provides another class or function with access to the internals of your object, thus expanding the visible scope of your private members. If you have a lot of friends, it's much harder to reason about the state of your object.

In my opinion, the template solution is even worse than friend in that regard. Your main stated benefit of the template is that you no longer need to explicitly friend the test from the class. I argue that, on the contrary, this is a detriment. There are two reasons for that.

  1. The test is coupled to the internals of your class. Anyone changing the class should know that by changing the privates of the object that they may be breaking the test. friend tells them exactly what objects might be coupled to the internal state of your class, but the template solution doesn't.

  2. Friend limits the scope expansion of your privates. If you friend a class, you know that only that class may access your internals. Thus, if you friend the test, you know that only the test can read or write to private member variables. Your template back door, however, could be used anywhere.

The template solution is ineffective because it hides the problem rather than fixing it. The underlying issue with the cyclic dependency still exists: someone changing the class must know about every use of the back door, and someone changing the test must know about the class. Basically, the reference to the test from the class was removed only by making all private data into public data in a roundabout way.

If you must access private members from your test, just friend the test fixture and be done with it. It's simple and understandable.

There's a theory that if it's private it shouldn't be tested alone, if it needs so then it should be redesigned.

For me that's Shi'ism.

On some project people create a macro for private methods, just like:

class Something{
   PRIVATE:
       int m_attr;
};

When compiling for test PRIVATE is defined as public, otherwise it's defined as private. that simple.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top