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

Tests don't run with '.Using(const std::string&)' #92

Closed johannesstricker closed 2 years ago

johannesstricker commented 7 years ago

These are my classes.

class StateFactory
  {
  public:
    // ...
    virtual IState* createState(const std::string& name) const;
  };

class State : public IState
  {
  public:
    State(const StateFactory& factory) : m_factory(factory) {}
  protected:
    const StateFactory& m_factory;
  };

And this is my test.

    Mock<StateFactory> mock;
    Fake(Method(mock, createState));
    const StateFactory& factory = mock.get();

    ScanningState* state = new ScanningState(factory);
    // ...
    Verify(Method(mock, createState).Using("seated"));

When I run the TEST.exe nothing happens, no console output. When I change Verify(Method(mock, createState).Using("seated")); to Verify(Method(mock, createState).Using(_)); The TEST.exe runs and all tests pass. I also tried creating an std::string object and passing that to the .Using method, but that didn't help either.

eranpeer commented 7 years ago

Please send a complete compiling code sample. I can't compile the example nor understand it since the classes IState and ScanningState are missing.

Rogiel commented 7 years ago

I think there is a memory corruption bug somewhere. I am also seeing a similar issue, it appears that the object the reference argument holds to is not copied, but only a pointer to it is kept and if during the tests this object gets destroyed, weird things can happen.

Look at this report from FakeIt-Catch:

  Verification error
  Expected pattern: transport.supports(tcp://localhost:1000)
  Expected matches: exactly 1
  Actual matches  : 0
  Actual sequence : total of 2 actual invocations:
    transport.supports
  (9P=??9P=??9P=?localhost?-
  ?????://localhost:1000)
    transport.connect
  (9P=??9P=??9P=?localhost?-
  ?????://localhost:1000)

Notice the "Actual sequence" where supports and connect have a bunch of gibberish.

Here is the Transport class relevant signatures:

virtual bool supports(const EndpointAddress& endpointAddress) const;
virtual ConnectOperationPromise connect(const EndpointAddress& endpointAddress);

So far I have seen this problem in several places in my code base, but only when using references.

The test works fine in Debug mode, but fails in Release mode (on macOS/Clang). I will try to build a fully working example and see if I can get it into a reproducible test.

Rogiel commented 7 years ago

I got a woking example.

https://github.com/Rogiel/FakeIt-Reference-Capture-Bug

Notice that using "operator new" was the only way I could find to make the compiler DO NOT use a statically allocated string, however the bug should be present for any object which the compiler cannot be smart enough to statically allocate and reuse.

EDIT: I have only tested this on the latest Apple's clang (Apple LLVM version 8.1.0 (clang-802.0.42))

madprog commented 7 years ago

I met a similar problem and reproduced it in this gist, which should be directly usable as a test in the FakeIt project.

It seems that I get a segmentation fault when formatting the arguments tuple to print it, in the Verify() call on line 32.

Just like @johannesstricker, the tests pass when I replace the parameter to Using by _ on line 32:

 Verify(Method(wrapper_mock, open_file).Using(_));

I also can prevent the segmentation fault by removing the Do() specification on line 26:

        When(Method(wrapper_mock, open_file));

In this case, I get an unknown exception type:

[--------------]             
[ RUN          ] MyTests::failure                          
[              ]    exception #1 cause: caught unknown exception type                                                  
[       FAILED ] MyTests::failure                          
[--------------]             
otterovich commented 6 years ago

Please see the latest answer here.

FranckRJ commented 2 years ago

I'll centralize the discussion about this known bug in a new issue: #274