Can't Verify() arguments that were passed by reference (dangling reference)

[FranckRJ]

The problem

There is a known issue with FakeIt and arguments that were passed by reference, causing potential crash because of dangling references:

struct Interface
{
    virtual void func(const std::string&) = 0;
};

int main()
{
    Mock<Interface> mock;
    Fake(Method(mock, func));
    Interface& i = mock.get();
    i.func("Dangling reference incoming.");
    Verify(Method(mock, func).Using("Dangling reference incoming.")).Once();
    return 0;
}

The Verify() process will likely fails, because FakeIt store a copy of the argument to be able to verify it later, but the copy keep the same exact type (reference to string), so the "copy" will instead be a reference to the original argument. If the referenced object isn't available anymore (because it was a temporary object, like in the example above) then you have a dangling reference and the troubles that come with them.

The fixes (in the library)

There are two ways of fixing the bug, either store a real copy of the argument instead of a reference to it, or dump the Verify() system (assertions after the mocked function have been called) and use an Expected() system instead (requirement written before the mocked function are called, and these requirement are checked at call-time, instead of later).

The first one is probably the easiest to implement, but it can't just be as simple as calling std::remove_reference for the parameters' type, because sometime the user don't want to copy the arguments and instead really want to only keep a reference on them. And it can't be the default behavior as well because it will break too much code. Instead it need to be configurable, maybe by specifying the index of the parameters to copy somewhere, or something like that.

The second one is the best solution as it purely remove the need for storing anything related to the parameters, so it'll work for every kind of parameters (including temporary references to non-copyable types). But it's a bigger change that'll require more time to implement.

Maybe both can be implemented, the first one in a future 2.X, and the second one in a 3.0 release. Nothing is planned yet.

The workarounds (in user code)

There are several workarounds if you need to check the arguments that will be passed by reference to your mocked functions.

Mock only for the values you want

Fake(Method(mock, func).Using("Dangling reference ignored."));
i.func("Dangling reference ignored.");
Verify(Method(mock, func)).Once();

If you only mock your function for the values you expect then when the function is called you know it got the right arguments, so you don't need to check them.

Store the value somewhere

std::vector<std::string> args;
When(Method(mock, func)).AlwaysDo([&](const auto& arg) { args.push_back(arg); });
i.func("Hello");
i.func("world");
i.func("!");
assert(args == std::vector<std::string>{"Hello", "world", "!"});

You can store yourself all the arguments somewhere and check them later, it's a bit cumbersome but at least you have the full control on how to check the arguments.

[malcolmdavey]

Just some rough ideas on implementing a system to allow this, but also allow existing behaviour (by default I assume)

Wondering what options there are to vary the behaviour - e.g. allow copying sometimes.

• have a different Mock class which will copy args(, or move for RRefs)
• have a templated function that can be overloaded.
• Have some configuration - somewhere (per mock, or global?)
• have a different function/method when setting the Fake (e.g. FakeAndStore(Method(mock, func)); )

[fsj]

I fear the 'act first, then verify' approach is a design flaw in mocking frameworks since it requires making a (deep!) copy of the invocation arguments. Not only fakeit is affected by this, but also other frameworks like Mockito have a similar problem, for which the verifications are incorrect when objects involved in the check are modified between invocation and verification (hence the requirement for a deep copy).

The workaround of storing a copy will not be able to circumvent this general limitation since there is no standard way to create deep copies of objects (only shallow copies), and in C++ objects may not even be copyable in the first place.

A real solution that works is to verify the .Using()/.Matching() at the time of the call, when the references are valid and mutable objects have not yet been modified again. This would, of course, require test code to set expectations before executing the test code. This 'expect first, then act' is what googletest does with EXPECT_CALL()s).