Is it possible to create a ReturnCopy, or ReturnRefOfCopy or similar instead of just Return

[malcolmdavey]

Wondering if we could have another method ReturnCopy(), which stored the value, and then used it as the return value, instead of the reference that could be dangling.

Here are cases that work, and also don't work.

class A
{
public:
    virtual const std::string& method() = 0;
};

TEST(fakeit, MockRefReturn_GoodCases)
{
    Mock<A> mockA;

    std::string aLocalString = "blah22";

    When(Method(mockA, method5))
        .Return(aLocalString);

    // only works if the referenced object is still alive
    EXPECT_EQ("blah22", mockA.get().method());
}

TEST(fakeit, MockRefReturn_BadCases)
{
    Mock<A> mockA;

    {
        std::string aLocalString = "blah22";

        When(Method(mockA, method))
            .Return("blah")
            .Return(aLocalString);
    }

    // these both don't work, as the objects have been destroyed already
    EXPECT_NE("blah", mockA.get().method());
    EXPECT_NE("blah22", mockA.get().method());
}

[malcolmdavey]

Actually looks like there is a way to do this already.

        When(Method(mockA, method))
            .Return<std::string&>("blah")
            .Return<std::string&>(aLocalString);

Though for R value ref's I think it should be made to be automatic, since there is really only one option for them. I might see if there is a way to do this ...

[malcolmdavey]

Actually using explicit reference as template parameter only works with an additional change to make the lambda live as long as the mock object. Had forgotten I needed to make that change as well, when I added my ReturnCopy, which forces.

[FranckRJ]

I thought that keeping lambdas alive as long as the mock was a bad idea, but in the end I think you're right, it'll solve several issues (#254 for example), and it's not really a big deal to keep some objects a bit too long because it's not like mock objects were long-lived nor had a big memory footprint.

I'll try to look how to do that in the near future, but if I understand well you already worked on it, do you want any help ? Have you pushed your changes somewhere so I can look at them ?

[malcolmdavey]

Sorry been delayed doing this. Our company has a bit of process around contributing. Just hadn't finished that step yet. And yes, I have a fairly simple change to do it.

[FranckRJ]

Ok take your time no worries. Could your PR only contains changes about the lifetime of lambdas? You can obviously open another PR for ReturnCopy or whatever but if possible I would like to have the changes for lambdas lifetime isolated in a single PR.

[malcolmdavey]

Yeah - will try to split up, and also add unit tests. I've created a lot of my own unit tests in my own project to try to characterise the behaviour of Fakeit fairly completely. Will try to add a lot more to Fakeit. But I think we'll also need to upgrade the unit test library so we don't have the restrictions on number of tests (as was mentioned on another PR https://github.com/eranpeer/FakeIt/pull/266 )

[FranckRJ]

I've created a ticket for it and planned it for the 2.3 (I don't want too much stuff in the 2.2, I want to release it in the coming weeks): #270.

[malcolmdavey]

All good. Just the number of unit tests I would like to add might dwarf what we already have, so good to have it on the road map.

[FranckRJ]

I'll move this to the 2.3, as I would like to release the 2.2 this weekend.

[FranckRJ]

The unit test library is updated in the dev-2.3.0 branch if you need it.

[malcolmdavey]

Have got PR for this now

[FranckRJ]

Added by #332 with the ReturnCapture function, it will be available in the next version of FakeIt.