eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.24k stars 170 forks source link

Unable to verify function calls with move-only (rvalue) arguments #110

Open djleach-belcan opened 6 years ago

djleach-belcan commented 6 years ago

I'm experiencing two issues involving a move-only (rvalue) function argument. I'm using the Catch framework. Given an interface:

struct IFoo
{
    virtual ~IFoo() = default;
    virtual void SetName(std::string&& name) = 0;
};

And a mock with the method faked:

Mock<IFoo> mockFoo{};
Fake(Method(mockFoo, SetName));

When I try to verify the following:

mockFoo.get().SetName("Bar");
Verify(Method(mockFoo, SetName).Using("Bar"));

The verification fails with this output:

Verification error
  Expected pattern: mockFoo.SetName(Bar)
  Expected matches: at least 1
  Actual matches  : 0
  Actual sequence : total of 1 actual invocations:
    mockFoo.SetName(SetName)

However if I verify after manually creating a std::string: Verify(Method(mockFoo, SetName).Using(std::string{"Bar"}));

The verification is successful.

Finally, if I try to verify there's no other invocations:

Mock<IFoo> mockFoo{};
Fake(Method(mockFoo, SetName));
mockFoo.get().SetName("Bar");
// Verify(Method(mockFoo, SetName).Using("Bar"));
VerifyNoOtherInvocations(mockFoo);

FakeIt attempts to print the moved string which no longer exists:

Verification error
  Expected no more invocations!! But the following unverified invocations were found:
    mockFoo.SetName(
[...hundreds of blank lines...]
... message truncated due to excessive size

This sometimes results in a SIGABRT.

I expected FakeIt to:

  1. Correctly verify the method call when using a string literal, and
  2. Copy the string literal arguments into a std::string (since that's the actual type of the argument) rather than (apparently) holding on to the character pointer. This would avoid crashing when printing the string later.
djleach-belcan commented 6 years ago

Hmm, this problem is much worse than I thought. It turns out the following verification passes!

mockFoo.get().SetName("Bar");
Verify(Method(mockFoo, SetName).Using("Qux"s));

This is worse than an issue with move-only types; it's a false positive!

djleach-belcan commented 6 years ago

The following appears to successfully work around the issue (with GCC 4.9.3 anyway):

std::string name{"42"};
mockFoo.get().SetName(std::move(name));
Verify(Method(mockFoo, SetName).Matching([](auto&& string) { return string == "42"; }));
djleach-belcan commented 6 years ago

Unfortunately my last comment is incorrect and seems to be working only by luck. The following code results in a verification error with the actual sequence being reported as "mockFoo.SetName()" with an empty string. It would seem FakeIt is not retaining a copy of move-only arguments.

std::string name{"42"};
mockFoo.get().SetName(std::move(name));
name.clear();
Verify(Method(mockFoo, SetName).Matching([](auto&& string) { return string == "42"; }));
f1xpl commented 6 years ago

I think lack verification of arguments passed by (const) reference/rvalue should be mentioned somewhere in Wiki/README.

For many potential users this is crucial feature that can make huge difference during choosing of mocking framework.

djleach-belcan commented 6 years ago

Arguments passed by const reference aren't validated correctly either? That's definitely not what I expected. Is there more information about that somewhere?

jadams760 commented 6 years ago

Is there a plan to fix this? The majority of my test cases require verifying arguments that are passed by const reference.

aminroosta commented 6 years ago

Same issue here

  #include "fakeit.hpp"
  using arg_t = std::string&&; // fake it is happy with T but not T&&
  struct interface {
      virtual int method(arg_t) = 0;
      virtual ~interface() = default;
  };

  void fakeit_is_not_happy() {
      fakeit::Mock<interface> mock;
      std::function<int(arg_t)> fn = [](arg_t) { return 0; };

      // error: no viable conversion from 'function<int (std::string &&)>' to
      //'function<int (typename fakeit::test_arg<basic_string<char> &&>::type)>'
✗     fakeit::When(Method(mock, method)).AlwaysDo(fn);
  }
aminroosta commented 6 years ago

Alright so to mock a member function taking T&& one should pass a lambda taking T& ... this makes sense because fakeit stores the T&& parameter into a tuple and passes a T& from there.

struct interface {
    virtual int method(std::string&&) = 0;
    virtual ~interface() = default;
};

void fakeit_is_happy() {
    fakeit::Mock<interface> mock;
    std::function<int(std::string&)> fn = [](std::string&) { return 0; };

    fakeit::When(Method(mock, method)).AlwaysDo(fn);
}

I wish this was documented.

AlexKven commented 5 years ago

I'm having this issue too. I'm trying to verify a function with a byte& parameter (which is typedefed as a char), and passing Any<byte&>() into Using does not compile, while passing Any<byte>() results in a segfault.