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

mrnovalles · 2024-02-05T16:15:42Z

This PR is a working implementation to fix an issue with the args_matcher. I iniitally tried in #242 but never succeeded the 8000 re.

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

Add a new matching function in meck_util:match_spec_item in the case that the some of the args to the function is a map
Add tests for maps to test equality of key/values

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 · 2024-03-15T13:12:49Z

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).

mrnovalles added 6 commits November 13, 2023 11:22

Add a test for maps to test equality of key/values

d8bed17

Simplify an exemplify the test better

b210b12

Add a happy path test

7bd594e

Add new match spec item for maps in meck:util

b4f7ed0

Make the args matcher for maps work with any amount of args

72294da

Revert unwanted changes

5a1987f

This was referenced Feb 5, 2024

Add a test for argument assertion using maps mrnovalles/mock#1

Closed

Add a test for argument assertion using maps jjh42/mock#144

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

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

Are you sure you want to change the base?

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

Conversation

Current issue

Changes