Catch2: testing private class members?

[cseed]

Is there a best practice for accessing private members in test cases? I didn't see anything in the documentation. One option is to use:
#define private public

in the test case file, but that doesn't seem ideal. I'm happy to use a friend declaration, but I don't see how to refer to the test case.

[PureAbstract]

As Bjarne says is so many words in _Design and Evolution_, it's by design that private and public control accessiblity, and not visibility - so #define private public shouldn't change the semantics of your code. Adding friend declarations, on the other hand, can do so in subtle ways. (Yes, there's a rule that says Thou Shalt Not Define Preprocessor Macros With Keyword Names, but I've yet to find a compiler that does anything other than what you expect in this circumstance.)

When I find myself needing to do this, my personal preference is actually to add -Dprivate=public (or it's moral equivalent) to the compiler command line when you build your tests.

Of course, there's a very good argument to say that in well-factored code, you shouldn't need to test private members (which I agree with wholeheartedly - except on those occasions where I find it disagreeable ;-)

[philsquared]

@cseed - the direct to answer to your question of "is there a best practice for accessing private members in test cases?" is this:

Don't.

At least in my opinion. Usually.

Tests should only be testing the observable behaviour. Private members are there to form part of the implementation of the behaviour - not its semantics.
That said there are some cases where it seems desirable. Off the top of my head:

1. Testing legacy code. If you need to do this before you can get to the point of cleaning things up properly then #define private public is usually the weapon of choice. See Michael Feather's "Working With Legacy Code'.
2. A part of the class you are working on has internal invariants that you want to want to verify. These are perhaps better tested with normal asserts in the code itself - but it may also indicate that you have another class hiding within the class under test. Separate that class out and _maybe_ test it in isolation (in terms of its now public interface) - but when used as part of the implementation of the original class you do not need to test it.
3. You're just getting carried away and wanting to test everything. Think about the reasons for change that your codebase has. If a requirement changes then you'd expect the tests to fail (and you should probably change them to fail before changing the code to fix them, if you can). If implementation changes - as part of a refactoring, say - but meets the same external specification then your tests really shouldn't break. If they do they are probably testing implementation, not requirements. You might sometimes end up in that situation if you're TDDing as you grow small units into more useful components. In which case you are left with some "scaffolding tests" which should probably come down now.
4. Some actual requirements are hard or impossible to test in terms of the public interface. Things like, "should cache this value", specifically don't impact the externally observable state/ behaviour. Testing them is, strictly speaking, outside the realm of unit testing - but that doesn't mean they shouldn't be tested. I take these on a case-by-case basis. Sometimes I can construct contexts in which they can be detected (like changing an underlying source so cached values are out of sync). Sometimes I just verify manually (gasp!). Sometimes I do provide a backdoor for testing - but that's pretty rare.

So, in short, I mostly agree with @PureAbstract.

[cseed]

@philsquared

Tests should only be testing the observable behaviour.

I don't understand this position. Isn't the point of testing, roughly, to make our code more reliable and (more likely to be) more correct? In that case, why limit the notion of testing? I agree, testing the interface and testing details of the implementation should be clearly separated. However, if there are internal routines with well-defined interfaces, it seems quite valuable to test them.

My situation is similar to this, combined, admittedly, with a bit of laziness. I have internal routines that can be used to set up state that is tedious, but not impossible, to achieve through the public interface. I would like to test these interfaces, yes, but in addition, use them to set up tests of the public interface in situations that are hard to get to otherwise. The balance here seems to be between the difficulty in writing tests, and the cost of maintaining them going forward.

Perhaps you would argue that I have a class within a class, but if the inner class is (1) not conceptually distinct, and (2) not used elsewhere, then breaking it out purely for testing seems as bad as -Dprivate=public, if not worse, because it makes the code larger and less transparent.

In response to point 4,

[] Things like, "should cache this value", specifically don't impact the externally observable state/ behaviour.

I think performance is part of the external behavior. Trying running f(1000) on a linear vs exponential function, and tell me if the observable behavior changes. However, one could well argue that if "should cache this value" is part of the observable behavior, it should be part of the public interface. You might also argue performance unit testing belongs elsewhere. But it belongs somewhere. "You can't control what you don't measure."

@PureAbstract - I was planning to leave the friend declarations in even when I'm not testing.

Thanks for the thoughtful replies. I'll keep this in mind as I go forward using Catch.

[hendrics]

@cseed you seem to answer your own question: "The balance here seems to be between the difficulty in writing tests, and the cost of maintaining them going forward."

Testing through public interface helps to make your tests less brittle and more maintainable. "I have some private stuff which is useful" - if it is useful why is it private? The benefit of making something private is the knowledge that nothing depends on it externally and you are free to change the private code knowing that the behaviour of a class or a subsystem is not going to change. When you grow your system to more > 1k unit tests - you want to keep them as maintainable as possible (e.g. not to break when you refactor you private code).

In my personal experience, I had to refactor a relatively big class with unit tests, and someone defined private as public for testing. It was a very difficult and was clear that it allowed a bad design (not resilient to change) through, without any other perceived convenience. There are exceptions obviously but in general doing that is just an alarm to highlight inflexible design/implementation.

[nabijaczleweli]

This is horribly UB and _will_ fail horribly somewhere down the line

[nabijaczleweli]

Yes: do _not_ test class internals.

[philsquared]

In a bizarre twist of fate I have just been bitten by #define public private!
It's in a set of tests testing some legacy code. The tests were added years ago but needed to reach into one of the objects to inspect some internal state. The object is implemented in a DLL (this is Windows).

Anyway, I just added another header file to the test file and it started giving linker errors.
Much head-scratching later I discovered that the accessibility of the exported functions was impacting their mangled names so the linker wasn't matching them up!

I think this is the the flip-side of ODR (the problem was multiple definitions mapping to the same implementation, rather than vice-versa). Either way nasty stuff.

To my shame I've fixed it to move the header file around for now (and added a comment!). But I do need to go back and address that properly. It's really not a good situation to be in!

[horenmar]

IIRC on Windows the visibility of members in an object also affects their layout. This can only bite you if you have mix of private and public member variables, but still.

DO NOT DO #define public private.

[philburr]

I _generally_ think it is best to unit test your _interface_. However, I do disagree that one should _never_ test internals. One should always be very careful about testing internal details because internal details can change and it makes your unit tests fragile and prone to maintenance. However, there are cases that in my code I've found that are useful to unit test. For example, I'm currently working on a class that is very math heavy and I need to be very careful that even internal functions do the right thing. I will do unit tests on them initially to validate behavior and will probably eventually remove them (the unit tests) once I validate the implementation.

Anyway to answer the original question, what I do is put the testable code in a class in an unnamed namespace (I do almost all class implementation in headers), have the ultimate implementation derive from that class (either protectedly or privately as needed). Then you have an implementation to test and an interface to test. You can then easily consolidate when you've reached code maturity.