Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mrnovalles
Copy link

This PR is a working implementation to fix an issue with the args_matcher. I iniitally tried in #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

  • 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
Copy link
Owner

eproxus commented Mar 15, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants