Skip to content

Proposal: clearer calls assertions with pattern matching #116

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

Open
eteeselink opened this issue Mar 30, 2020 · 2 comments
Open

Proposal: clearer calls assertions with pattern matching #116

eteeselink opened this issue Mar 30, 2020 · 2 comments

Comments

@eteeselink
Copy link

Hi there,

First off, thanks for Mock, it's super nice. Now, I read #109 but I think I have a better solution. I wanted to discuss the API a bit before I turn it into a PR.

Basically, besides the lack of pattern matching there's something more that bothers me about assert_called: it asserts that the function has been called at least once. That means you don't catch bugs where a function is called too often. I'd prefer to be able to verify that a function is called exactly as many times as I expected, with exactly the parameters I care about.

The good news is, I think I wrote a macro that solves this:

  defmacro calls({{:., _, [mod, fun]}, _, args}) do
    quote do
      for {_pid, {_mod, unquote(fun), unquote(args) = params}, _return} <-
            :meck.history(unquote(mod)) do
        params
      end
    end
  end

It's my first Elixir macro so let me know if I did something stupid :-)

Call it like this:

# similar to `called`, except the parameters can be a pattern
calls = calls(MyModule.somefunc(:insert, %{type: "user"}))

This returns a list of all calls that were made to MyModule.somefunc with parameters that match the parameters shown. Eg if the software under test calls this:

MyModule.somefunc(:insert, %{type: "user", name: "George", age: 7})
MyModule.somefunc(:insert, %{type: "group", topic: "pimples"})
MyModule.somefunc(:update, %{type: "user", name: "Hans", age: 5})
MyModule.somefunc(:insert, %{type: "user", name: "Pete", age: 8})

then the calls call above returns this list, the parameters for each call:

[
   [:insert, %{type: "user", name: "George", age: 7}],
   [:insert, %{type: "user", name: "Pete", age: 8}]
]

Now, that's a lovely list for a bog standard assert, which allows pattern-matched assignments. Eg:

# this verifies that exactly 2 calls were made
assert [george, pete] = calls(MyModule.somefunc(:insert, %{type: "user"}))

# this verifies that the calls happened in the expected order and with the right data
assert [_, %{name: "George"}] = george
assert [_, %{name: "Pete"}] = pete

What do you think about this? I'll be happy to prepare a PR.

Note: personally I think I'd want to ship a second overload for the macro that accepts each part separately:

calls(MyModule, :somefunc, [:insert, %{type: "user"}])

This lets you specify a pattern for the function name too, eg calls(MyModule, _, _) gets all calls of any arity to (mocked) functions in MyModule.

If you like this approach I'll code that overload up as well and put them in the same PR. I'm also not 100% sure about the name calls. I considered get_calls too, and call_history, but they feel needlessly long, plus you got call_history in master already (even though I feel like this macro would essentially supersede it).

The final thing I'm not sure about is that this macro returns the parameter list, but not the return values. I don't think return values are often useful when asserting mock behavior, but maybe sometimes they are (esp when using :passthrough). Maybe it should return a defstruct [:params, :return_value] instead of just the params.

Curious about your thoughts!

@eteeselink
Copy link
Author

@Olshansk any thoughts? I'd be happy to prepare a PR (incl proper documentation etc) but I prefer not to do the work if you don't like the API.

@Olshansk
Copy link
Collaborator

Olshansk commented May 3, 2020

Hey, sorry for the delay on this...

Overall, I like the proposal and think we should move forward with it.

That means you don't catch bugs where a function is called too often

Just noteworthy: Erlang's meck (what this uses) has a num_calls function. In a separate PR, we could build a wrapper around it to expose it in an elixir native way.

If you like this approach I'll code that overload up as well and put them in the same PR. I'm also not 100% sure about the name calls. I considered get_calls too, and call_history, but they feel needlessly long, plus you got call_history in master already (even though I feel like this macro would essentially supersede it).

Let's go with matching_calls. What do you think?

The final thing I'm not sure about is that this macro returns the parameter list, but not the return values

Let's avoid looking at the return values for the purpose of this feature.

Olshansk pushed a commit that referenced this issue Aug 29, 2020
I've added the assert_called_exactly assertation which is related to what was proposed here #116 inspired by RSpec Mocks.

* implement assert_called_exactly
* removing unused mock function on mock test
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

No branches or pull requests

2 participants