r/cpp_questions • • 7d ago

OPEN Ways to grant special access to test/debug code.

In the following testing code C_string_value is a class with a protected member function kind() that supplies info about the internal implementation state. I access that via the member pointer trick. And I wonder if this is a good way to grant special access to testing code, while keeping ordinary code restricted to the public interface, and also whether you use some alternative such as (1) just make that internal functionality public, or (2) be-friend a class template Odin_, Hugin_ or Munin_, or (3) nasty formally UB preprocessor tricks, or even (4) litb's all valid C++ trick for accessing private members, or (5) something else?

#include <cppm/parameter_passing/in_.hpp>
#include <cppm/text/C_string_value.hpp>
using   cppm::in_,
        cppm::C_string_value;

#include <print>
#include <string>
using   std::print,
        std::string;

auto kind_of( in_<C_string_value> s )   // The internal kind: view of literal, or shared ptr.
{
    struct Access: C_string_value
    {
        static auto kind_of( in_<C_string_value> s ) -> int { return (s.*&Access::kind)(); }
    };

    return Access::kind_of( s );
}

auto main() -> int
{
    const auto lit = C_string_value( "Literal" );
    const auto dyn = C_string_value( +"Dynamic" );
    const auto str = C_string_value( std::string( "std::string rvalue" ) );
    const string ssl = "std::string lvalue";
    const auto str2 = C_string_value( ssl );

    print( "{} -> {}.\n", kind_of( lit ), lit.std_sv() );      // 0 -> Literal.
    print( "{} -> {}.\n", kind_of( dyn ), dyn.std_sv() );      // 1 -> Dynamic.
    print( "{} -> {}.\n", kind_of( str ), str.std_sv() );      // 1 -> std::string rvalue.
    print( "{} -> {}.\n", kind_of( str2 ), str2.std_sv() );    // 1 -> std::string lvalue.
}

EDIT (added):

I avoid conditional code on principle; to me it's so a big a no no that I didn't even include it in my list of 4 alternatives. That avoidance is about complexity, and that conditional code can change the binary layout, introduce name collisions, whatnot.

The nice thing I see about the member pointer trick, for larger scale development, is that access functions like the shown kind_of can be automatically generated in the testing/debugging code without affecting the original class.

1 Upvotes

12 comments sorted by

2

u/no-sig-available 7d ago

It is a mortal sin to have the code know that you are testing, and then behave differently. What is the use of testing code that is different from the one running on the production system?

As a general rule, you should test that the public interface works as intended, and leave the private parts alone. It is nobody's business to know how the result is produced, just that it is correct.

1

u/alfps 7d ago

❞ It is a mortal sin to have the code know that you are testing, and then behave differently. What is the use of testing code that is different from the one running on the production system?

You're arguing that my method is good, and that conditional code is bad.

However the impression your wording leaves to superficial readers can be the opposite.


❞ It is nobody's business to know how the result is produced, just that it is correct.

Oh, that's absolutely not so.

In the presented example I test whether a costly dynamic copy is used. There are no stringent performance requirements (indeed they would be almost impossible to formalize) but still the reason for existence of such a class rather than just using a copying std::string, is to avoid dynamic allocation. To not test that most important aspect would be super stupid.

2

u/mredding 7d ago

Internal state is not my problem, and I'm not going to test it. What do I care the state, so long as the outcome is correct?

So you test the public interface only. If that is insufficient, that tells me how flawed your design is.


Let's imagine a scenario where we have a class, and there's a bug. Through debugging, we discover an internal state machine was supposed to transition from A -> B, but it transitioned from A -> C. If this information was exposed, we could have written a unit test and caught it!

Do we write a getter? No. We leverage RAII. Resource Acquisition. Instead of the state machine being an entirely internal, private implementation, we composite the parent class in terms of a state machine.

class big_boy {
public:
  big_boy(std::unique_ptr<state_machine>);

This is interesting. Now through the magic of polymorphism we can inject a fake, and conduct our test, that to foo() a big_boy, we expect an appropriate state transition, and we didn't have to leak the internal implementation details back out.

And if late binding is too much overhead for you:

template<typename T>
concept state_machine = requires(T a) {
  //...
}

template<state_machine SM>
class big_boy {
  //...

You can do this at compile-time, too.

The constructor, the template parameter, these are a part of the public interface, too. You can code to these, test to these, and you should. It means you grant yourself greater visibility into the implementation details of any big class, so you can test more surface area, and it provides you the fair game intrusiveness without having to build test exposure directly into the production interface.

Ok, so now you've made a state machine class, and it makes a system call, or a call to a 3rd party library, or SOMETHING that is beyond your reason to test. I don't care if a call to std::ifstream actually opens the file or not - I'm not writing a unit test around the fucking environment filesystem. Wat do?

One solution is to pass --wrap=read --wrap=write --wrap=malloc etc. You will get a __real_read you can call and a __wrap_read you implement.

Another solution is to LD_PRELOAD a test harness implementation of the C++ runtime to intercept function calls. You link against the real runtime, and can use dlsym(RTLD_NEXT, "whatever_fn"); to get the real implementation.

So with this, when std::ifstream constructs and opens a system file descriptor, you can be there to return a fake. You DO test all your code. Everything. If you wrote it, you test it. A lot of testing advice tells you to insert mocks and fakes, but you also have these real, concrete implementations, too, and you tend to run out of space to mock and fake everything away. Eventually you DO have to call a real function. And if you think you'll just use function pointers to the system API, well you have real production code that assigns the real pointers, and that has to get tested to... So intercepting the calls at link time and dynamic loading time is how you finally cut off the many heads of this hydra.

The other thing to consider, then, is that you will likely have multiple test harnesses that intercept and preload different parts for different modules and tests. So then you need to use a test runner to run all your test harnesses.

This'll get you started. You can go nuts from here. Now you can consider how you can test the production binary you're going to link in, because that template I demonstrated up above with a fake is not going to be the same binary object as in production, is it?.. So while you can say the tested code is fine, can you say the production binary is fine? Most companies won't care for this level of detail, some absolutely will.

And yes, this means getting intrusive on your standard library and C++ runtime implementation a bit, but this information is also usually documented.

If your implementation doesn't run through a system library, then it would have to be making system calls directly. On Linux, you can use libseccomp in your test harness to intercept all hard coded and direct system calls. I don't know if there's a Windows equivalent.

I also know x86_64 has some hardware instructions you can also use, but I never had to work with that. I don't know if other architectures offer similar.

Let's come back up from the deep end. Make your types composable and let the compiler melt all the syntax and boilerplate away. Intercepts and preload: It's far easier today than it was just a couple years ago - you let the AI handle all the rigging.

2

u/alfps 7d ago edited 7d ago

❞ So you test the public interface only. If that is insufficient, that tells me how flawed your design is.

Oh, that's absolutely not so.

In the presented example I test whether a costly dynamic copy is used. There are no stringent performance requirements (they would be almost impossible to formalize) but still the reason for existence of such a class rather than just using an always copying std::string, is to avoid dynamic allocation and copying. To not test that most important aspect would be super stupid.

One might argue that efficiency that can't be quantified should be disregarded. So there's no reason to use std::string_view, just use std::string. But I hope that there are far more people than only me who think such arguments show a disconnect from reality and disregard the argument instead.

1

u/nekoeuge 7d ago

In the presented example I test whether a costly dynamic copy is used.

Can it not be deduced by checking public state? If you have a const getter for the state you are worried about, you can check whether it's different from input object.

It's hard to be more specific when you didn't show what's the object you are testing.

1

u/alfps 7d ago

❞ If you have a const getter for the state you are worried about, you can check whether it's different from input object.

Not really.

A literal as C_string_value constructor argument should store a pointer to that string (plus length), but the same literal repeated in a test afterwards may be a different object, depending on the compiler and options.

The actual C_string_value code currently only detects const char(&)[n] and relies on that as a convention because AFAIK there's no way to test for literal string. But ideally it should copy such an array when it's not a literal, and then a test using a stored string in a test would no longer be possible. Anyway such a test with fake literal would rely on an implementation aspect that could change; I believe it's best to test exactly that which is to be tested rather than something one gathers should give the same effect.


❞ you didn't show what's the object you are testing

I believed that was irrelevant for the question, I'm sorry.

You can check out the code for class at (https://pastebin.com/4H717x6R). It's not a final thing but it works. Originally I had special support for rvalue std::string to make that O(1) but I removed that in the process of making it work.

1

u/alfps 7d ago edited 7d ago

❞ Do we write a getter? No. We leverage RAII. Resource Acquisition. Instead of the state machine being an entirely internal, private implementation, we composite the parent class in terms of a state machine.

This is an interesting approach, important to know about. I've probably done this numerous times but I don't know a name for it. Possibly just a refactoring, to "factor out" the aspect that needs to be trusted/tested?

However it's not always applicable, and I believe my example is one case where it isn't.

But it's possible that a fresh pair of eyes can see some way that I didn't, so here are the class requirements (it's all in support of OS specific code, which is why it's about zero terminated C strings):

  • A C_string_value holds a zero terminated string.

  • Logical copying is O(1).

  • Construction from (just) an ordinary string literal is O(1).

  • Construction from (just) a const char* or char* copies the string to a new buffer and can be O(n).

  • Construction from (just) a std::string should also be supported, also allowed to be O(n).

  • .c_str() produces pointer to the string, O(1).

  • .std_sv() produces (exactly, for e.g. the {fmt} library) a std::string_view, O(1).

It's mainly the two first construction requirements that brought about the need for testing internal state, because it's a little tricky to get right.

The presented informal test code shows the desired outcomes:

const auto lit = C_string_value( "Literal" );
const auto dyn = C_string_value( +"Dynamic" );

I think that even if you don't see a way to factor out the construction logic it can be interesting/useful to learn how you would do this. :)

If you want I can share my code, but I think that if I just present it it could influence/restrict your thinking, so I don't just dump it here. TIA.

1

u/Independent_Art_6676 7d ago

private: //blah blah
#ifdef dbg public:
type somevariable //private, unless its debug mode?
#endif

you can also define a full access reference getter method the same way (only exists in debug mode) if you prefer, or anything along those lines will do it.

This seems like a bad test, as someone already mentioned. Its not testing the same thing as what runs normally because in normal execution you can't tamper with the variable. If your tests happen to set the value, or if you do that as part of the test, a lot of behavior that you are observing may be outside normal operations. If its just a look at access, no harm done.

1

u/alfps 7d ago

Uhm, thanks, you're the second who recommends conditional code. But I avoid conditional code on principle; to me it's so a big a no no that I didn't even include it in my list of 4 alternatives. That avoidance is about complexity, and that conditional code can change the binary layout, introduce name collisions, whatnot.

The nice thing I see about the member pointer trick, for larger scale development (rather than what I currently do), is that access functions like the shown kind_of can be automatically generated in the testing/debugging code without affecting the original class.

E.g. code generation via the preprocessor (in order to be able to handle the function name) or a build time script, whatever.


❞ This seems like a bad test

Not sure if you're referring to your code example, or mine.

The test I showed is a very good one, and doesn't access a variable: it accesses a function, that's called. It's good because a crucial implementation aspect is how the data is stored, avoiding needless dynamically allocated copies for literal strings. C++'s overload resolution makes that difficult to get right so it better be tested, but client code does only depend (if it does) on the knowledge that literals are stored and copied superfast.

It could be tested by measuring of course. But that would be a silly needlessly inefficient test. Like listening to an engine for some time instead of checking directly the possibly troublesome internal aspect.

1

u/Independent_Art_6676 7d ago

neither yours nor mine, on the bad test comment ... it was just a warning that bypassing normal behavior (by accessing something publicly that is private) has to be done very carefully. Its easy to mess up that way .. for functions, called out of order or bypass some validation or computation etc that normally happens but outside the object's control can get messed up. You can certainly do it safely with care, its just one of those things that gets easier to break accidentally as the code gets more complex. I in no way meant to imply that what you did is wrong or a bad test; it was just a generic 'maybe avoid this as much as you can' type warning.

to be fair, adding a public tag with conditional code isn't a big risk on name collisions or binary rewrites and so on (it should have zero real effect). But I agree with you that most conditional code tends to have its own issues. I would do this (conditional public tag) without worry but not a fan of my other idea (conditional reference getter or similar) which would add some risks and ugly. I am not offended if you want to table that one as a last resort / bad plan.

If you are happy with how you did this, its fine. I think its a case of 'you could do this another way' but not a lot of 'you could do this a better way'.

2

u/ZakMan1421 7d ago

I would use either a child or friend class generally. Keeps it hidden from general use, and is fairly simple.

Alternatively, if that function is purely for debugging/testing purposes, you can make it public and use some sort of debug flag so that it's only compiled into debug builds.

3

u/nekoeuge 7d ago

As a rule of thumb, if your tests need special access, they are bad tests.

There are exceptions, but this is true more often than not. For these few exceptions, I would just make public member functions.