Feature/capture

[horkhe]

Introduces capability to capture an argument a mocked function was called with. It is particularly useful to obtain references to internal funs passed as callbacks to mocked functions.

meck:expect(test, foo, 3, ok),
test:foo(preved, fun() -> bar end, medved),
Capture = meck:captrue(last, test, foo, ['_', '_', '_'], 2),
bar = Capture().

[eproxus]

I propose we use the matcher structure for this, as a query language. I'd rather see we build a history query API that we then can build these functions upon.

For example, using the ETS match specification "language", the capture function could be implemented as follows (given an imaginary meck_history:query):

capture(Pos, {Mod, Func, Arity}, Args) ->  % <- Note: Args instead of Arg
    Results = meck_history:query(Mod, {Func, Args}),
    case Pos of
        last -> lists:last(Results)
        ...
    end.

Where query(Mod, Pattern) would use ets:match_spec_run/2) to search through the history list.

Also, please do different pull requests for code cleanup stuff. This pull request should not have to be dependent on #96?

[horkhe]

This is an excellent idea! Since it is meck_proc that keeps history of a mocked module, then it should probably provide query_history function to retrieve only those history pieces that are needed, to avoid sending entire module history between processes.

[horkhe]

@eproxus I broaden the notion of args_spec() to make it possible to be defined as arity, When an args_spec() is given as arity then it is equivalent to a pattern of wildcards e.g. 3 is equivalent to ['_', '_', '_']. That in turn made it possible to use args_spec() throughout Meck in every place where somehow an argument pattern needs to be specified: expect/3(4), num_calls/3(4), called/3(4), and in newly added capture/5(6). I hope that this uniformity of interface will be appreciated by users.

Besides I changed the originally proposed interface of capture/5(6) to make it more aligned with other history digging functions. Changes in arg_spec() allowed to make capture into a much more powerful feature. Consider this:

%% Given
meck:new(test, [non_strict]),
meck:expect(test, foo, 2, ok),
meck:expect(test, foo, 3, ok),
meck:expect(test, foo, 4, ok),
meck:expect(test, bar, 3, ok),
test:foo(1001, 2001, 3001, 4001),
test:bar(1002, 2002, 3002),
test:foo(1003, 2003, 3003),
test:bar(1004, 2004, 3004),
test:foo(1005, 2005),
test:foo(1006, 2006, 3006),
test:bar(1007, 2007, 3007),
test:foo(1008, 2008, 3008),
%% When/Then
?assertMatch(2001, meck:capture(first, test, foo, '_', 2)),
?assertMatch(2003, meck:capture(first, test, foo, 3, 2)),
?assertMatch(2005, meck:capture(first, test, foo, ['_', '_'], 2)),
?assertMatch(2006, meck:capture(first, test, foo, [1006, '_', '_'], 2)),
?assertMatch(2008, meck:capture(first, test, foo, ['_', '_', meck:is(hamcrest_matchers:greater_than(3006))], 2)),
%% Clean
meck:unload().

I decided to put history query changes aside for another pull request for this one starting to become rather big.

[horkhe]

By the way, it is one of the functions for history digging mentioned in #85.

[horkhe]

@eproxus Hey Adam, have you had a chance to take a look at this?

[chernser]

Awesome feature. Seems I have to switch to @horkhe 's fork, now.
@eproxus btw, why not transfer meck to @horkhe, as he seems main contributor of meck and you seems have no time even for review?

[eproxus]

@chernser It's true that I don't put as much time as needed on review the incoming changes from @horkhe. My main problem has been the feature vs lines of code ratio which in my opinion is too small, and in many cases I haven't had the time to suggest improvements, because the pull request have most often been huge. That being said, you are free to use @horkhe's fork if you want. This is also a great way for me to determine whether these features are actually needed by users! (So I appreciate the feedback that this is a feature you want).

@horkhe You told me that this feature makes the concept of arg_spec() more general. In what sense, and how does this relate to the code size (almost 300 more lines of code)?

[horkhe]

Before this pull request args_spect() was represented as a list of arguments where an argument can be also a wildcard '_' or a partially defined term where some its pieces are wildcards, or as a matcher. From now on everywhere where an args spec is expected you can specify also just arity which is a shortcut for a list of wildcards. So you can specify arity as arg_spec() throughout entire Meck API in every single method where args_spec() is expected and not just in 'expect/4' how it was before.

Out of 300 lines of code the majority are just comments and unit tests, and also some renaimings to make things allign better with the broader args_spec()

[eproxus]

@horkhe Make sense. :-)

What are your opinions or removing the {ok, ...} wrapper? The alternative would be to raise for example error:{not_found, OptArgsSpec}. This would simplify the API usage, and as the main usage is tests, I think raising an exception would be fine.

When you mentioned earlier that you put history query changes aside, did you mean mimicking the ETS query API when it comes to deciding return values? I think that API is more powerful than the current arity/argument index API.

[horkhe]

Sure we can raise error at the spot without {ok, ...} wrappers. I will change that.

[eproxus]

Get a feel for the API and let me know what you think. I just had the feeling that getting rid of repeated okays all over the code would be nice. :-)

[horkhe]

As for the ETS query the current API uses args_spec() to define arguments of the call to capture so effectively you have the same power as ETS query - you can specify partially defined arguments.

[horkhe]

Is not that what you was asking for?

[eproxus]

I was thinking of defining the return values, like in ets:select/2. Mockup:

[[2006, 3006]] = meck:select(test, foo, [{{1006, '$1', '$2'}, [], [['$1', '$2']]}])

Enhancement with limit (or pos):

[[2006, 3006]] = meck:select(test, foo, [{{1006, '$1', '$2'}, [], [['$1', '$2']]}], 1)
[[2006, 3006]] = meck:select(test, foo, [{{1006, '$1', '$2'}, [], [['$1', '$2']]}], first)

Could also possibly return just the element [2006, 3006] without the outer list, but it's nice to know the function always will return a list of matches. This would also solve not_found, since it would then return an empty list instead.

[horkhe]

Now I get you. The main problem with that is that args_spec() allows specifying matchers as arguments, and the catrue syntax that I proposed allows grabbing arguments that correspond to matchers in the args_spec(). With this pure ETS syntax it wont be possible to capture such arguments.

The main use case for this feature is to capture such things as Pids and functions that you can later on send a message or call to respectively to continue the test flow. So all is needed is the entire value. If other cases called or num_calls should probably be used.

On the other hand it might be useful to capture several arguments with one call. Though I would suggested to make API a little less verbose (after all we do not need everything that comes with ETS. I also would suggest that we relese it in another pull request as another version of the suggested cature function that instead of the number of the argument to capture as the last parameter takes a list of placeholders to return. E.g:

[2006, 3006] = meck:capture(first, test, foo, [blah, '$1', '$2'], ['$1', '$2'])

[horkhe]

@eproxus I added two more commits. Commit d714ef3 fixes minor inconsistency in 9b434d8 that I found yesterday when you draw my attention back to this pull requests. Commit 34de17c makes capture return value itself (without {ok, ...}) and fail if the expected call has not happened.

[horkhe]

I adjusted the pull request description and another my comment to reflect the API change.

[eproxus]

👍