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

what if I need to use shared_ptr or unique_ptr of the mock object in my unittest #60

Closed xqian closed 1 year ago

xqian commented 8 years ago

The code is like this struct Paint { Paint(shared_ptr p_pen) :m_pen(p_pen) {};

void RePain() { m_pen->Do(); }

shared_ptr m_pen; }

struct IPen { virtual Do() =0; }

When I testing Paint, I want to pass a mocked IPen into Paint. But how can I pass the shared_ptr of IPen into Paint?

If I can change the design to make Paint to have reference on IPen might be better. But If there a solution that I can use mock a shared_ptr?

pellet commented 8 years ago

+1 I'm having trouble working out how to create a shared_ptr for a mocked object, I'm looking into creating a copy of the stack allocated mock object to a shared_ptr, but haven't worked out how to do it yet.

pellet commented 8 years ago

I've attempted to create a shared_ptr with the line: shared_ptr<Mock<HttpResponseInterface>> sharedResp = make_shared<Mock<HttpResponseInterface>>();

however I get a compile error: In file included from /Users/belmoj/otherlevels/ios/cppSdk/src/test/TestFoo.cpp:5: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/iostream:38: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/ios:216: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__locale:15: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/string:439: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:628: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:3718:7: error: exception specification of overriding function is more lax than base version class __shared_ptr_emplace ^ /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:4308:26: note: in instantiation of template class 'std::__1::__shared_ptr_emplace<fakeit::Mock<otherlevels::HttpResponseInterface>, std::__1::allocator<fakeit::Mock<otherlevels::HttpResponseInterface> > >' requested here ::new(__hold2.get()) _CntrlBlk(__a2, _VSTD::forward<_Args>(__args)...); ^ /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:4672:29: note: in instantiation of function template specialization 'std::__1::shared_ptr<fakeit::Mock<otherlevels::HttpResponseInterface> >::make_shared<>' requested here return shared_ptr<_Tp>::make_shared(_VSTD::forward<_Args>(__args)...); ^ /Users/belmoj/otherlevels/ios/cppSdk/src/test/TestFoo.cpp:32:62: note: in instantiation of function template specialization 'std::__1::make_shared<fakeit::Mock<otherlevels::HttpResponseInterface>>' requested here shared_ptr<Mock<HttpResponseInterface>> sharedResp = make_shared<Mock<HttpResponseInterface>>(); ^ In file included from /Users/belmoj/otherlevels/ios/cppSdk/src/test/TestFoo.cpp:5: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/iostream:38: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/ios:216: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__locale:15: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/string:439: In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:628: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:3644:13: note: overridden virtual function is here virtual ~__shared_weak_count(); ^ 1 error generated.

tnovotny commented 8 years ago

just fake the destructor and pass the pointer to shared_ptr

Mock<IPen> pen;
Fake(Dtor(pen));

Paint paint( std::shared_ptr<IPen>( &pen.get() ) );
xqian commented 8 years ago

Thanks, I used to use spying

struct SomeImplement
    : ISomeInterface
{
  ...
};
void TestFunc(const std::shared_ptr<const ISomeInterface>& i);
// Test code piece
    {
        // Create test object and pay.
        std::shared_ptr<SomeImplement> obj {new SomeImplement()};
        Mock<SomeImplement> spy(*obj);

        // Override one method bar.
        When(Method(spy, bar)).AlwaysReturn(10); // virtual table got replaced.

        // Production code
        TestFunc(obj);
    }

But apparently, "fake the destructor" is much better.

pellet commented 8 years ago

Thanks @tnovotny if @eranpeer could add this to the documentation I'm sure it would help a lot of people :+1:

pellet commented 8 years ago

I'm still having trouble with the mock being released prematurely in my async test. I think I need to allocate the mock object on the heap rather than the stack, is there any way to do this? I'm having trouble for example with the 'When' macro not working when I do this:

Mock<SettingsInterface> *mockSettings = new Mock<SettingsInterface>();
When(Method(*mockSettings, appKey)).Return("MockAppKey");

I will attempt to use spying for now, thanks @xqian

tnovotny commented 8 years ago

for me it works when I write (*mock) as in

When( Method( (*mockSettings), appKey )).Return( "MockAppKey" );
pellet commented 8 years ago

Thanks @tnovotny . I managed to make a mock allocated on the heap without issues relating to shared pointers by using the following allocation syntax: const auto mockHttpClient = shared_ptr<Mock<HttpClient>>(new Mock<HttpClient>()); rather than const auto mockHttpClient = make_shared<Mock<HttpClient>>(); then faking the destructor with: Fake(Dtor((*mockHttpClient)));

I needed to write the mock object code outside of my lambda which later returns the mock object, otherwise the mocked definitions are prematurely released. e.g.

const auto mockHttpClient = shared_ptr<Mock<HttpClient>>(new Mock<HttpClient>());
When(Method((*mockHttpClient), get)).Do([&](const string & url,
                                            const HttpResponseCompletionHandler & responseCompletionHandler) {
    const auto response = shared_ptr<Mock<HttpResponseInterface>>(new Mock<HttpResponseInterface>());
    When(Method((*response), code)).Return(200);
    const Json jsonResponse = Json::object{ { "value", true } };
    When(Method((*response), body)).Return(jsonResponse.dump());
    Fake(Dtor((*response)));
    responseCompletionHandler( shared_ptr<HttpResponseInterface>(&response->get()) );

});
Fake(Dtor((*mockHttpClient)));

bootstrap.addComponent(typeid(HttpClient), [&]() {

    return static_pointer_cast<void>(shared_ptr<HttpClient>(&mockHttpClient->get()));
});
pellet commented 8 years ago

In the end I ended up taking the spy approach so I don't have to mock every method in my virtual classes(interfaces). I created stubbed classes to mock via spy, if a method is called which isnt mocked it will throw an unmocked function runtime_error.

xqian commented 8 years ago

" I created stubbed classes to mock via spy," can you share more specifically how you do this without mock every method in my virtual classes(interfaces)?

eranpeer commented 8 years ago

@xqian, I added a section called "Faking" to the quickstart that describes in more details the syntax @tnovotny was using to fake the dtor: Fake(Dtor(mock))

Teemu-Santio commented 8 years ago

I think that the Mock interface could benefit having a method that returns std::shared_ptr<C> It could be implemented with custom deleter with code that looks something like this:

std::shared_tr<C> operator()() {
    return std::shared_ptr<C>(get(), [] (C*) { /* No op custom deleter */ });
}
whschultz commented 8 years ago

@Teemu-Santio 's solution is the correct one, so far as I've found, and it's the only one that will prevent the memory from being freed (which, AFAIK a fake dtor won't do). I wound up writing a template function that accepts a Mock and infers the correct parameters to return a shared_ptr from a Mock using the method mentioned above. The same applies to unique_ptr as well (and the boost versions).

zalox commented 8 years ago

In case someone wants to omit the template function: auto mock_ptr=std::shared_ptr<foo>(&mock.get(),[](...){});

helmesjo commented 6 years ago

@eranpeer I've seen this issue mentioned a few times. Could perhaps mock.getSharedPtr() & mock.getUniquePtr() just be added to the API so related issues can be closed?

tnovotny commented 6 years ago

This issue can be closed because it never really was an issue. Personally I would not pollute the API with such trivial methods.

LossMentality commented 5 years ago

In case someone is looking for a complete example of a way to handle objects that require mocking but are managed by a unique_ptr, I thought I'd post how I handled it.

In our code base, a build is set to either include unit test code or not based on if a macro is defined. If this macro is defined, test code is included and a single unit test app is constructed that runs all tests (this is done with the doctest unit testing framework, which is a great framework, so be sure to check it out).

For this example, we'll say the unit test macro is COMPILING_UNIT_TESTS.

// In some header...
#include <memory>

#ifdef COMPILING_UNIT_TESTS

// We're in a test build; define a default constructible functor that does nothing. This will be given to 
// std::unique_ptr as its second template parameter, which defines what a unique_ptr should do on 
// delete.
struct UniquePtrNoOpDeleter
{
    template <typename T>
    void operator() (const T&) const noexcept { /* Don't delete. */ }
};     

// Now, an alias to a specialization of std::unique_ptr with the no-op deleter.     
template <typename T>
using MockableUniquePtr = std::unique_ptr<T, UniquePtrNoOpDeleter>;

#else

// Else, we're in a production build, so we define an alias of the same name, but this time 
// to a std::unique_ptr with its second template parameter defaulted, which gives us the standard 
// unique_ptr delete functionality. 
template <typename T>
using MockableUniquePtr = std::unique_ptr<T>;

#endif

Now, any code that has a unique_ptr to an object that needs to be mocked when testing uses MockableUniquePtr. This code will then get a standard deleting unique_ptr during production builds, but, when pointing to a mocked object during test builds, will instead get a no-delete unique_ptr.

A word of warning: MockableUniquePtr is actually a different type in each case (because the template parameters to unique_ptr are different in a production build as opposed to a test build). This means that if the unique_ptr's type is required at more than just the definition of the pointer, you'll need to use MockableUniquePtr as the type. For instance, you can't instantiate a MockableUniquePtr and then pass it to a function that takes a std::unique_ptr& parameter; instead, you'll have to write the function to take a MockableUniquePtr&. Likewise, you can't have a std::vector<std::unique_ptr> and then move an already existing MockableUniquePtr into it; instead, the vector must be of type std::vector<MockableUniquePtr>.

Finally, note that there are other ways besides a functor to provide a unique_ptr with a custom deleter (e.g. you could use a lambda), but that the functor approach is generally the better way to go. See here for a discussion on this.

tnovotny commented 5 years ago

@LossMentality I don't get it. What problem does this solve that Fake(Dtor(mock)); doesn't?

Vizor commented 5 years ago

@tnovotny Fake(Dtor(mock)); doesn't solve not freeing of particular mock by this shared_ptr. The test, where you do this:

Mock<IPen> pen;
Fake(Dtor(pen));

Paint paint( std::shared_ptr<IPen>( &pen.get() ) );

will fail with

SEH exception with code 0xc0000005 thrown in the test body.

somewhere deep in destructor of Mock<IPen>. Probably because the shared_ptr called delete on the instance returned by get() before.

Iqon commented 4 years ago

I think this information is crucial. I wasted an hour until I found this thread.

The only solution that worked for me was:

fakeit::Mock<Foo> fooStub;
auto pointerToFoo = std::shared_ptr<Foo>(&fooStub(), [](...) {});

I understand that you don't want to pollute the API with methods like this. But this should at least be part of https://github.com/eranpeer/FakeIt/wiki/Quickstart.

Adding a section on how to deal with shared_ptr and unique_ptr would be extremely helpful and won't pollute the API.

TobiSchluter commented 4 years ago

@Iqon thank you! I've been fighting with this issue repeatedly. Your solution works like a charm and I agree that it would be extremely useful to have it in the documentation and/or FAQ.

jessevdp commented 3 years ago

In my case the solution for std::shared_ptr (passing the second argument to assure no deletion) seems to work correctly, while the case for std::unique_ptr seems to only work on Linux/MacOS systems, not on Windows..

FranckRJ commented 2 years ago

Could someone provide a code snippet with the issue? Because I can't reproduce it : https://godbolt.org/z/fTn66GhMK

As I it's supposed to be fixed since 2015 : #10

ryanmda commented 2 years ago

I'm going backwards and forwards trying as many options as I can, but I can't seem to figure out a solution for std::unique_ptr that doesn't segfault if I'm using std::move to add to a std library container in the function that I'm testing. I'm just about to throw up my hands and write my own stub implementing the interface. What am I missing?

Alas, the 'spying' solution doesn't work for us, because the interface is abstract.

FranckRJ commented 2 years ago

Could you provide a code sample ? Because in the example I showed above I replaced std::shared_ptr by std::unique_ptr and everything worked as expected. If you move the std::unique_ptr content somewhere else then it will point to nullptr, and you won't be able to use the pointed object anymore because there won't be any.

ryanmda commented 2 years ago

Yeah... I'm starting to come to the conclusion this is fundamentally incompatible for that reason, but here's some test code anyways that is roughly equivalent to what I'd be testing:

#include <map>
#include <memory>

class ITestInterface
{
    public:
        ITestInterface() {}

        virtual ~ITestInterface() {}

        virtual double getResult() = 0;
};

class TestContainer
{
    public:
        TestContainer() {}

        void addItem(int key, std::unique_ptr<ITestInterface> item)
        {
            lookup.emplace(key, std::move(item));
        }

        double getItemResult(int key)
        {
            return lookup[key]->getResult();
        }

    private:
        std::map<int, std::unique_ptr<ITestInterface>> lookup;
};

There doesn't seem to be any means to construct a unique_ptr to a stubbed ITestInterface that can be used in addItem() without segfaulting or using elaborate workarounds that redefine unique_ptr like LostMentality.

Without any better ideas, I'm planning to just manually implement a fake version of my interface that itself stores a reference to a stub and proxies calls to it.

FranckRJ commented 2 years ago

It doesn't segfault for me : https://godbolt.org/z/obMjnYKv3

Could you provide a code sample that segfault ?

ryanmda commented 2 years ago

It does if you do roughly the same thing in a catch2 unit test.

SCENARIO("Example Unit Test")
{
    GIVEN("TestContainer instance")
    {
        TestContainer testContainer;
        WHEN("addItem() is called to add an item")
        {
            Mock<ITestInterface> mock;
            Fake(Dtor(mock));
            When(Method(mock, getResult)).AlwaysReturn(2.5);

            testContainer.addItem(5, std::unique_ptr<ITestInterface>(&mock.get()));

            THEN("getItemResult on the same item will return 2.5")
            {
                REQUIRE(2.5 == testContainer.getItemResult(5));
            }
        }
    }
}

Here's the complete example, including catch and fakeit headers for local compile. I'm not really much of a compiler explorer user to figure out the correct way to build catch with it. debugcode.zip

My build command was:

g++ debug.cpp catch_amalgamated.cpp --std=c++17 -Wall -o debug
FranckRJ commented 2 years ago

It's because the mock is destroyed before testContainer (and thus before unique_ptr call the destructor of the mock).

Just move the instantiation of the mock higher so its destructor is called after the one of testContainer and it should be fine.

ryanmda commented 2 years ago

🤦 Of course. Thanks for your help. For anyone else who sees this, the declaration order in the same block of code matters too due to stack unroll order. I needed:

Mock<ITestInterface> mock;
TestContainer testContainer;
malcolmdavey commented 2 years ago

There is a problem with unique_ptr and shared_ptr on windows when there is a virtual destructor.

There is a specific workaround for shared_ptr, and a general work around but here is a proposed general fix: https://github.com/eranpeer/FakeIt/pull/289

FranckRJ commented 1 year ago

Since #289 fixed a bug with destructors on MSVC (in FakeIt 2.3.1) I'm confident that this issue is now definitely fixed on every platforms supported by FakeIt, so I'll close it. Don't hesitate to create a new one if you think it's not fixed for your configuration.