eproxus / meck

A mocking library for Erlang
http://eproxus.github.io/meck
Apache License 2.0
811 stars 231 forks source link

Add a mechanism to check for map arguments in the args_matcher #244

Open mrnovalles opened 7 months ago

mrnovalles commented 7 months ago

This PR is a working implementation to fix an issue with the args_matcher. I iniitally tried in https://github.com/eproxus/meck/pull/242 but never succeeded there.

When asserting on calls to functions were the arguments are maps, the matching function returns a false positive, ie. matching on a 3 items map, even when the called function had 2.

Current issue

On master, the issue is in the MatchSpec that we send over to ets:match_spec_run:

MatchSpecItem = meck_util:match_spec_item({[#{a => 1,b => 2}]}),
CompMatchSpec = ets:match_spec_compile([MatchSpecItem]),
Args = [#{a => 1,b => 2,c => 3}],
% No match expected
[] = ets:match_spec_run([{Args}], CompMatchSpec).
** exception error: no match of right hand side value [{[#{c => 3,a => 1,b => 2}]}]

Changes

This PR adds a new matching function meck_util:match_spec_item() that builds an erlang Match Specification in the case that any of the arguments given to the function are a map.

I understand the complexity of match_spec_item is now way higher than it was before, opening this PR to hear your thoughts on it.

eproxus commented 6 months ago

Hi! Nice catch. I think the current solution has a problem in that it is only checking map size, but not the actual values. E.g.:

1> AM = meck_args_matcher:new([1, #{a => 1, b => 2}]).
{args_matcher,[1,#{a => 1,b => 2}],
              #Ref<0.2522732729.1137311749.246851>,false}
2> meck_args_matcher:match([1, #{a => 1, b => 2}], AM).
true
3> meck_args_matcher:match([1, #{a => 1, b => 3}], AM).
true
4> meck_args_matcher:match([1, #{a => 1, b => 1}], AM).
true

If the guard is created like so, it works:

[{'==', IndexVar, Arg} | Acc];

Which would simply be equivalent to Fun(Var) when Var == #{...} ->. Now that I think about it, a simplification could be to generate guards like that for all arguments, regardless if they are maps or not... That would simplify the logic a lot. What do you think?

In that case using =:= would be best (doesn't matter for maps, but for ints vs floats it does).