Don't use `define private public` in test files

[horenmar]

Bug Report

I wanted to check whether Catch 2.1.0 fixed all perf. regression from Catch Classic on a real world test suite, but using #define private public in test files causes compilation errors because of ODR violations.

One example:

In file included from /mnt/c/ubuntu/json/src/json.hpp:1800:0,
                 from /mnt/c/ubuntu/json/test/src/unit-iterators1.cpp:32:
/usr/include/c++/5/sstream:300:7: error: ‘struct std::__cxx11::basic_stringbuf<_CharT, _Traits, _Alloc>::__xfer_bufptrs’ redeclared with different access
       struct __xfer_bufptrs
       ^
test/CMakeFiles/test-iterators1.dir/build.make:62: recipe for target 'test/CMakeFiles/test-iterators1.dir/src/unit-iterators1.cpp.o' failed

This is caused by Catch no longer including <sstream> everywhere, so an attempt to redefine private to public also hits <sstream> header. It can be temporarily fixed by just force including <sstream> before including the json.hpp header, but it should be noted that doing #define private public and having it affect any part of the standard library in any TU using the standard library is UB.

Note: There are also several test files that rely on transitive stdlib includes from Catch and will not compile with newer version.

OS & Compiler: g++ 5.4 under WSL

[nlohmann]

I only added #define private public to test suites that contain tests calling private functions or classes. Is there a better way to achieve this without changing the actual library?

[nlohmann]

(And I like the idea of using a more recent version of Catch. I tried an early 2.0 version, but the performance of the tests was worse.)

[horenmar]

As far as I know, there is no way of doing this with no changes to the header.

There are two ways these things are usually solved:

1. Add a friend declaration for class that holds the test fixtures.
2. Have a conditional access modifier:

#ifndef UNDER_TEST
#define INTERNAL_BLOCK private
#else
#define INTERNAL_BLOCK public
#endif
class foo {
public:
    foo() { ... }
INTERNAL_BLOCK:
    bool helper_1(){ ... }
    int helper_2() { ... }
    ...
};

A common argument is also that if a private method needs to be made accessible to be tested, then the class is too complex and should be breaked apart (because a class should be defined by its external api, not implementation details), but I admit that I don't particularly subscribe to that philosophy.

[theodelrieu]

With the recent basic_json refactoring, I think tests could be rewritten to use the low-level constructs (e.g. iterators/lexer etc...).

Those would remain unexposed by the basic_json class, but the need for the #define private public hack should disappear.

[nlohmann]

This still would not allow a unit test to check the value of a private member variable. Though such a check may be expressed in terms of the public API in some cases, refactoring the code for the edge cases would be a lot of work.

[chen3feng]

#define keyword is an undefined behavior in C++ std. And in gcc, access may change managed function name. I think using friend is a better way.