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

Mocking a method that has a lambda parameter is being stored, keeping the captured objects alive. #229

Closed bakobence closed 3 years ago

bakobence commented 3 years ago

In the scenario below, we can see that T is being captured by a lambda, and that lambda is passed to the mocked method. In my expectations here, the object held by the shared pointer should die when the control flow reaches the end of the scope. However, it only dies when the entire executable stops (at return 0). I suspect fakeit is storing the lambda for some reason, and since the lambda is alive, the captured objects also kept alive.

This also happens if I use AlwaysDo, Do, Fake. Is this an intended behavior?

class Object {
public:
    Object() { std::cout << "[Captured object] Constructor\n";}
    ~Object() { std::cout << "[Captured object] Destructor\n";}
};

class Interface {
public:
    virtual void someMethod(std::function<bool()> callback) = 0;
};

auto main() -> int {
    Mock<Interface> mock;
    When(Method(mock, someMethod)).Do([](auto callback) {
        std::cout << "Mocked | someMethod\n";
        callback();
        });

    {
        // Test suite.
        auto T = std::make_shared<Object>();
        auto callback = [T]() {
            // Use T for something
            return true;
        };

        mock.get().someMethod(callback);
        std::cout << "End of scope\n";
    }

    std::cout << "About to quit\n";
    return 0;
}

Output of this code:

[Captured object] Constructor
Mocked | someMethod
End of scope
About to quit
[Captured object] Destructor

Ofcourse If I reset the mock before leaving the inner scope this issue gets resolved, however I don't think thats the correct way here.

FranckRJ commented 3 years ago

It's the intended behavior because you can later verify which value was passed as parameter for your mocked function.

Verify(Method(mock, func)).Using("val").Once();

bakobence commented 3 years ago

I completely forgot about this. I guess I will reset the mock after every test case. Thank you

FranckRJ commented 3 years ago

I don't know what you use for your tests, personally I use Catch2 and it re-execute everything for each test, so I'm sure I'm always in a clean state without having to manually write tear down functions.

FranckRJ commented 3 years ago

And maybe you don't have to clear all the mock, maybe a Verify().atLeastOnce() is enough to clear the saved patameters, I don't know.

bakobence commented 3 years ago

I use QTest so I have a init() / cleanup() function running before and after every test case, so resetting the mock in the cleanup function is completely justified