eranpeer / FakeIt

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

String literals not being matched correctly v2.0.7. #232

Closed zfields closed 3 years ago

zfields commented 3 years ago
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
a.out is a Catch v2.13.4 host application.
Run with -? for options

-------------------------------------------------------------------------------
PseudoSensor::temp() generates a `card.temp` request
-------------------------------------------------------------------------------
pseudo-sensor.test.cpp:11
...............................................................................

pseudo-sensor.test.cpp:27: FAILED:
  Verify( mock_notecard.newRequest("card.temp") )
with message:
  pseudo-sensor.test.cpp:27: Verification error
  Expected pattern: mock_notecard.newRequest("card.temp")
  Expected matches: exactly 1
  Actual matches  : 0
  Actual sequence : total of 1 actual invocations:
    mock_notecard.newRequest("card.temp")

===============================================================================
test cases: 1 | 1 failed
assertions: 1 | 1 failed

I ran the verification with increasing requirements to narrow it down, for example...

fakeit::Verify(Method(mock_notecard,newRequest));                           // passes
fakeit::Verify(Method(mock_notecard,newRequest)).Once();                    // passes
fakeit::Verify(Method(mock_notecard,newRequest).Using("card.temp")).Once(); // fails

The call in the implementation under test looks like this:

_notecard.newRequest("card.temp");

The signature of the mocked method is:

virtual J *  newRequest(const char *) = 0;
zfields commented 3 years ago

I ran some quick tests, and it appears to be comparing pointers (==) instead of comparing the strings at the pointers (strcmp).

If I declare a global variable for the string, then it works. Otherwise, whether literals or variables, it fails.

FranckRJ commented 3 years ago

It's the intended behavior. If you don't want to use == for comparison you should use matchers : https://github.com/eranpeer/FakeIt/wiki/Quickstart#invocation-matching, or use a class that correctly implement the comparison semantics you need like std::string_view.

zfields commented 3 years ago

@FranckRJ Thank you for responding so quickly! I read the documentation you referenced, but I don't see how Matchers would be the correct tool for an EXACT comparison of strings. Based on the documentation, it appears Matchers are used for the invocations and not verifications.

Again, the code looks like this...

_notecard.newRequest("card.temp");

The method signature looks like this...

virtual J *  newRequest(const char *) = 0;

The invocation setup looks like this...

fakeit::Fake(Method(mock_notecard,newRequest));

The verification test looks like this...

fakeit::Verify(Method(mock_notecard,newRequest).Using("card.temp")).Once(); // fails

Can you can provide an example of how a Matcher would be used in this arrangement? If this is the preferred mechanism, I think an example will help several others in my predicament.

Also, I'm failing to understand in what circumstance == would be viable for testing strings of type const char *. Could you also explain the reasoning behind this decision?

FranckRJ commented 3 years ago

There is an example in the wiki :

auto argument_a_is_even = [](int a){return a%2==0;};
When(Method(mock,foo).Matching(argument_a_is_even)).Return(1);

For Verify it's the same, in the wiki it's the section just below the one I linked :

Verify(Method(mock,bar).Matching([](int a, int b){return a > b;})).Exactly(Once);

In your specific use case it should look like :

Verify(Method(mock,bar).Matching([](const char* str){return strcmp(str, "card.temp") == 0;})).Exactly(Once);

Also, I'm failing to understand in what circumstance == would be viable for testing strings of type const char *. Could you also explain the reasoning behind this decision?

Probably never, that's why you don't use const char * for representing strings in C++.

zfields commented 3 years ago

Thank you for the example, I will update my test immediately! Also, It would be great if you could clarify the documentation to reflect that it works for verification as well.

While we're on the subject...

Probably never, that's why you don't use const char * for representing strings in C++.

I'm sure you're being cheeky here, but this statement effectively equates to don't use FakeIt if you didn't write the code you're trying to mock/test. Any time a "string literal" is used, it will be converted, by the compiler, to const char *, so it's not a type the developer gets to choose.

Also, string literals are not so uncommon that they should not be natively supported by the framework. It is awesome the framework is so flexible it can accommodate, but since there is "Probably never" a reason to test const char * strings with ==, then it would be nice for FakeIt to overload the .Using() method to convert the type const char * to std::string before testing with ==.

FranckRJ commented 3 years ago

Any time a "string literal" is used, it will be converted, by the compiler, to const char *, so it's not a type the developer gets to choose.

You're choosing the type of the parameters of your functions, nothing force you to use const char *.

it would be nice for FakeIt to overload the .Using() method to convert the type const char * to std::string before testing with ==.

No, because a const char * isn't necessarily a string.

zfields commented 3 years ago

You're choosing the type of the arguments of your functions, nothing force you to use const char *.

Definitely not. I'm adhering to an interface, that I did NOT create. Which is precisely why interfaces exist - not mocking.

No, because a const char * isn't necessarily a string.

Fair point.

FranckRJ commented 3 years ago

You're choosing the type of the arguments of your functions, nothing force you to use const char *.

Definitely not. I'm adhering to an interface, that I did NOT create. Which is precisely why interfaces exist - not mocking.

You're right, then you should use matchers. Maybe I can add a strcmp matcher to ease the use of the lib for this use-case.

FranckRJ commented 3 years ago

I've created an issue for it : #234.

If you think this solution will be good for you, please close this issue.

zfields commented 3 years ago

Thanks! I really appreciate it, the conversation, and the workaround.