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

Expect function call while still calling original function #85

Open
DReigada opened this issue Nov 27, 2024 · 7 comments
Open

Expect function call while still calling original function #85

DReigada opened this issue Nov 27, 2024 · 7 comments

Comments

@DReigada
Copy link

We've recently started using Mimic (migrating from Mock) and there's one pattern that we frequently use: assert that a function call happened but not changing the original behaviour of the function (calling the original)

This is what we end up doing most of the times:

SomeModule
|> expect(:some_function, fn %{type: "some_type"} = arg1, _ = arg2 ->
  call_original(SomeModule, :some_function, [arg1, arg2])
end)

This can become quite verbose if the function has multiple arguments that or if we want to expect multiple function calls on the same module.

We've decided to implement a macro to simplify this to the following (implementation below):

SomeModule
|> expect_passthrough(:some_function, [%{type: "some_type"}, :_])
Macro Implementation
  defmodule MimicHelper do
    @doc """
    This is an alternative to `expect` that allows you to pass through the call to the original function while still verifying that the call was made.
    Note: This is doable with expect, it just requires more boilerplate.

    ## Example

    Converts the following call:

    ```
    SomeModule
    |> expect_passthrough(:some_function, [%{type: "some_type"}, :_]
    ```

    Into the following:

    ```
    SomeModule
    |> expect(:some_function, fn %{type: "some_type"} = arg1, _ = arg2 ->
      call_original(SomeModule, :some_function, [arg1, arg2])
    end)
    """
    defmacro expect_passthrough(module, fn_name, num_calls \\ 1, args) do
      # generate arguments to pass to the original function
      original_call_args = Macro.generate_unique_arguments(Enum.count(args), __MODULE__)

      func_args =
        args
        # convert the wildcard :_ to {:_, [], nil} to match any argument.
        # {:_, [], nil} is the representation of `_` in the AST
        |> Enum.map(fn
          :_ -> {:_, [], nil}
          arg -> arg
        end)
        # assign the call_args to the arguments
        # eg: [%{key: 12}, "Asdf", :_] will be fn %{key: 12} = arg1, "Asdf" = arg2, _ = arg3 -> ... end
        |> Enum.zip_with(original_call_args, fn a, b -> {:=, [], [a, b]} end)

      func =
        quote do
          fn unquote_splicing(func_args) ->
            call_original(module, fn_name, unquote(original_call_args))
          end
        end

      quote do
        # Assign to variables to avoid re-evaluating the bindings
        module = unquote(module)
        fn_name = unquote(fn_name)
        num_calls = unquote(num_calls)

        expect(module, fn_name, num_calls, unquote(func))
      end
    end
  end

The macro fits our use case and simplifies the test code a lot, but I feel that it's a bit too complex and has some caveats (pinning variables for pattern matching can become cumbersome).
I wonder if you have any suggestion of how to make this better, or if this is even a feature that would make sense to add to Mimic.

@edgurgel
Copy link
Owner

edgurgel commented Dec 8, 2024

Hi @DReigada,

Thanks for such a well detailed issue report.

I think the use case is super valid. Would you mind if I spend some time thinking about this before I reply?

I want to experiment with this macro a bit before committing to something but in principle I like the idea 👍

Hoping to get some time before the end of the year

Thanks!

@DReigada
Copy link
Author

@edgurgel take your time, the macro is working for us so this is not urgent at all.

Looking forward to knowing your thoughts on this. Let me know if you need to discuss this further.

@edgurgel
Copy link
Owner

@DReigada hey after playing with it I think it's a good addition to the Mimic.DSL module maybe renaming it to expect_and_call_original (which is similar how RSpec does it).

WDYT?

@DReigada
Copy link
Author

DReigada commented Jan 6, 2025

@edgurgel that sounds great! Should I open a PR?

@edgurgel
Copy link
Owner

edgurgel commented Jan 7, 2025

Yes, please! 🙏

@DReigada
Copy link
Author

DReigada commented Jan 8, 2025

@edgurgel I didn't know about the Mimic.DSL syntax before starting to look into the implementation of the macro there.
How would you see the usage of expect_and_call_original if it belonged to Mimic.DSL?
I'm asking this because our implementation uses the "default" Mimic syntax. Would it make sense to have the new macro in the two "syntaxes"?

Default syntax

SomeModule |> expect_and_call_original(:some_function, [%{type: "some_type"}, :_])

Mimic.DSL

expect_and_call_original SomeModule.some_function(%{type: "some_type"}, :_)

@edgurgel
Copy link
Owner

edgurgel commented Jan 9, 2025

I was thinking initially on using the existing Mimic.DSL style which hopefully you could use _ instead of having to make this an atom.

expect_and_call_original SomeModule.some_function(%{type: "some_type"}, _)

But we could support both if we really want to. WDYT?

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

No branches or pull requests

2 participants