-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can't Verify() arguments that were passed by reference (dangling reference) #274
Comments
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.
|
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 |
You're right, it should be updated. |
The problem
There is a known issue with FakeIt and arguments that were passed by reference, causing potential crash because of dangling references:
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 anExpected()
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
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
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.
The text was updated successfully, but these errors were encountered: