Num_calls increments when called function returns

[guess-burger]

It appears that the value returned by meck:num_calls(...) only updates after the called function has returned. This makes verifying behaviour around long running functions difficult.

Reproduction Steps

Here's a passing eunit test to recreate the scenario I have.

num_calls_test() ->
  meck:new(dummy, [non_strict]),
  meck:expect(dummy, test, fun
                             (short) -> ok;
                             (long) -> receive stop -> ok end
                           end),

  ?assertEqual(ok, dummy:test(short)),
  ?assertEqual(1, meck:num_calls(dummy, test, [ short ])),

  Pid = spawn(fun() -> dummy:test(long) end),
  timer:sleep(100),
  ?assertEqual(0, meck:num_calls(dummy, test, [ long ])),
  Pid ! stop,
  timer:sleep(5),
  ?assertEqual(1, meck:num_calls(dummy, test, [ long ])),

  meck:unload(dummy).

Expected behavior

I would expect that meck:num_calls would update as soon as the function is entered and the runtime of that function to have no effect. I would exepect the first call to ?assertEqual(0, meck:num_calls(dummy, test, [ long ])), to fail with a value 1 being returned instead

Versions

Meck version: 0.8.4
Erlang version: 18.2.1

[eproxus]

Thanks for the report, I'll look into changing this.

[eproxus]

Initially a bit trickier, because the count is based on the history, and the history is only recorded when the call has finished (including the result of the call). To migrate to some structure where calls are recorded separately from results is a bigger change. 😞

[guess-burger]

My schedule is a little crazy at the moment but I'm happy to help out where I can.
Out of curiosity, is there anyway we can still use history as it too is a valuable function? I know it's contents contain the return type but is there anyway it could hold some sort of "pending return" value. Obviously, this affects current users and it might break the API so needs thinking through.

[eproxus]

So, currently the history looks like this:

[
...
call   --> .
           .
           .
return <-- {Pid, {Mod, Func, Args}, Result},
...
call   --> .
           .
           .
           .
           .
return <-- {Pid, {Mod, Func, Args}, Result},
...
]

Options:

1. num_calls just goes through that history list and sums up all the calls it finds that match the requested signature. For it to be able to sum up calls that haven't returned yet, we would have to store the initiated calls into the history. The problem with that is later updating those calls with their return values (or exceptions) to keep the current API consistent. For example, we could put {Pid, {Mod, Func, Args}, {'$not_returned_yet', Ref}} or something in the history, and later updated it. Updating it is problematic because we would have to search through the whole history and find the same ref later (it is doable though, just not simple).
2. When I was thinking of this fix, I thought it would be nice if the history instead was of the form [{Pid, {call, {Mod, Func, Args}}}, ..., {Pid, {result, Result}}] but that would break backwards compatibility.
3. Another alternative is to keep a call count state inside the Meck process, but that process is already full of stuff and it wouldn't exactly be simpler with even more state. 😄

[guess-burger]

Sorry for the extended absence.

I've been thinking about solution number 2. Would you want that form to become the result of meck:history in future? I'm wondering if meck_hisory and meck_proc could use that form while meck does the conversion to satisfy the current API. This seems more worthwhile if ultimately it becomes the result of meck:history. However, I appreciate that might not be the case.

I'm interested to see how it would work out but if it's a complete nonstarter then something more option 3 is fine for my case.

[eproxus]

Ideally we would go for option 2. A first step could be an internal refactoring, with an implementation that allows for num_calls to be fixed. Then we could release a 0.9 (or 1.0 or whatever) and document the API change.

We could also just work on master, upgrading the history format and API, and let people who want stability stay on 0.8.