Proposal: changes to mocking syntax

[Gazler]

I have a couple of suggestions with regards to the mocking section.

When mocking multiple functions on the same module, the syntax currently looks like:

allow(Mockable) |> to_receive(do_something: fn -> :error end)
Mockable |> to_receive(do_something_else: fn -> :success end)

If you attempt the following:

allow(Mockable) |> to_receive(do_something: fn -> :error end)
allow(Mockable) |> to_receive(do_something_else: fn -> :success end)

Then an error will be raised:

** (ErlangError) erlang error: {:already_started, #PID<0.343.0>}
stacktrace:
  src/meck_proc.erl:96: :meck_proc.start(Fixtures.Mockable, [:no_link])

This is because :meck.new assigns the name Elixir.Fixtures.Mockable_meck to the process it starts. Since it is a named process, there is a collision.

I have a potential fix for this issue if you wish to use it then I can raise a pull request. This fix looks like:

    def allow(module, opts \\ [:no_link]) do
      case Process.whereis(String.to_atom("#{to_string(module)}_meck")) do
        nil -> setup_meck(module, opts)
        _   -> module
       end
    end

    defp setup_meck(module, opts) do
      :meck.new(module, opts)
      # Unload the module once the test exits
      on_exit fn ->
        :meck.unload(module)
      end
      module
    end

The second proposal is to allow chaining from to_receive e.g.

    allow(Mockable)
      |> to_receive(do_something: fn -> :error end)
      |> to_receive(do_something_else: fn -> :success end)

A naive implementation of this is to simply return the module instead of the {module, mock, value} tuple that is currently returned.

A more future proof implementation may be to return a struct for the module from both allow and to_receive and change the first argument of to_receive to be the struct and pattern match the module out of it. This means that the other parameters will still be available if any additional functions require them in the future.

[inf0rmer]

Hi! Thanks for contributing :).

I like the readability of your first idea, but it is more chatty, since you need to do allow(Mockable) ... twice. Do you see any reason to not support both styles though? If we were to support the chaining style as well I like your idea of using a Struct instead of a tuple.

[Gazler]

@inf0rmer I am personally in favour of the chaining syntax, it feels more Elixiry. One potential use case for calling allow twice could be:

context "counting" do

  before :each do
    allow(Counter) |> to_receive(:increment).and_return(1)
  end

  it "counts" do
    expect(Counter.increment) |> to_eql(1)
  end

  context "when decrementing" do

    it "counts backwards" do
      allow(Counter) |> to_receive(:decrement).and_return(-1)
      expect(Counter.decrement) |> to_eql(-1)
    end
  end

end

Quite contrived, but hopefully it conveys the idea. Currently the correct way to implement this would be to replace:

allow(Counter) |> to_receive(:decrement).and_return(-1)

with:

Counter |> to_receive(:decrement).and_return(-1)

[inf0rmer]

Yeah, I get what you're saying. I also agree that your example is contrived, and arguably would make the test case harder to read than just having two contexts for incrementing and decrementing. I would say to move forward with implementing the chaining interface then. If you want to take a stab at it open a PR and with a test case and I'll merge it :).

Thanks!

[Gazler]

I also have https://github.com/Gazler/pavlov/commit/ec1a5776a772da7adabcf99394384f005dfe68a1 if you want to allow multiple allow calls, it does depend on the knowledge of how meck names its processes internaly.