From 7dd905de08b20af7d91481031cf9d702dfe7cba1 Mon Sep 17 00:00:00 2001 From: Dylan Chong Date: Wed, 27 Jul 2022 22:17:35 +1200 Subject: [PATCH 1/6] fix: Copy fails with umbrella mix test stale --- lib/mimic.ex | 38 +++++++++++++++----------------------- lib/mimic/server.ex | 15 ++++++++++++++- test/mimic_test.exs | 22 +++++++++++++++++----- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/lib/mimic.ex b/lib/mimic.ex index 3fc7842..ebc7970 100644 --- a/lib/mimic.ex +++ b/lib/mimic.ex @@ -344,6 +344,9 @@ defmodule Mimic do @doc """ Prepare `module` for mocking. + Ideally, don't call this function twice for the same module, but in case you do, this function + is idempotent. It will not delete any `stub`s or `expect`s that you've set up. + ## Arguments: * `module` - the name of the module to copy. @@ -351,11 +354,15 @@ defmodule Mimic do """ @spec copy(module()) :: :ok | no_return def copy(module) do - with {:module, module} <- Code.ensure_compiled(module), + with :ok <- ensure_module_not_copied(module), + {:module, module} <- Code.ensure_compiled(module), :ok <- Mimic.Server.mark_to_copy(module) do ExUnit.after_suite(fn _ -> Mimic.Server.reset(module) end) :ok else + {:error, :module_already_copied} -> + :ok + {:error, reason} when reason in [:embedded, :badfile, :nofile, :on_load_failure, :unavailable] -> raise ArgumentError, "Module #{inspect(module)} is not available" @@ -363,23 +370,6 @@ defmodule Mimic do error -> validate_server_response(error, :copy) end - - # case Code.ensure_compiled(module) do - # {:error, _} -> - # raise ArgumentError, - # "Module #{inspect(module)} is not available" - - # {:module, module} -> - # case Mimic.Server.mark_to_copy(module) do - # :ok -> - # ExUnit.after_suite(fn _ -> Mimic.Server.reset(module) end) - - # error -> - # validate_server_response(error, :copy) - # end - - # :ok - # end end @doc """ @@ -465,6 +455,13 @@ defmodule Mimic do Server.get_mode() end + defp ensure_module_not_copied(module) do + case Server.marked_to_copy?(module) do + false -> :ok + true -> {:error, :module_already_copied} + end + end + defp raise_if_not_exported_function!(module, fn_name, arity) do unless function_exported?(module, fn_name, arity) do raise ArgumentError, "Function #{fn_name}/#{arity} not defined for #{inspect(module)}" @@ -502,11 +499,6 @@ defmodule Mimic do "Module #{inspect(module)} has not been copied. See docs for Mimic.copy/1" end - defp validate_server_response({:error, {:module_already_copied, module}}, :copy) do - raise ArgumentError, - "Module #{inspect(module)} has already been copied. See docs for Mimic.copy/1" - end - defp validate_server_response(_, :copy) do raise ArgumentError, "Failed to copy module. See docs for Mimic.copy/1" diff --git a/lib/mimic/server.ex b/lib/mimic/server.ex index 4c90930..990aa96 100644 --- a/lib/mimic/server.ex +++ b/lib/mimic/server.ex @@ -95,6 +95,11 @@ defmodule Mimic.Server do GenServer.call(__MODULE__, {:mark_to_copy, module}, @long_timeout) end + @spec mark_to_copy(module) :: :boolean + def marked_to_copy?(module) do + GenServer.call(__MODULE__, {:marked_to_copy?, module}, @long_timeout) + end + def apply(module, fn_name, args) do arity = Enum.count(args) original_module = Mimic.Module.original(module) @@ -453,8 +458,12 @@ defmodule Mimic.Server do end end + def handle_call({:marked_to_copy?, module}, _from, state) do + {:reply, marked_to_copy?(module, state), state} + end + def handle_call({:mark_to_copy, module}, _from, state) do - if MapSet.member?(state.modules_to_be_copied, module) do + if marked_to_copy?(module, state) do {:reply, {:error, {:module_already_copied, module}}, state} else # If cover is enabled call ensure_module_copied now @@ -475,6 +484,10 @@ defmodule Mimic.Server do end end + defp marked_to_copy?(module, state) do + MapSet.member?(state.modules_to_be_copied, module) + end + defp do_reset(module, state) do case state.modules_beam[module] do {beam, coverdata} -> Cover.clear_module_and_import_coverdata!(module, beam, coverdata) diff --git a/test/mimic_test.exs b/test/mimic_test.exs index ff46d5d..6ff3cb1 100644 --- a/test/mimic_test.exs +++ b/test/mimic_test.exs @@ -908,11 +908,23 @@ defmodule Mimic.Test do end end - describe "copy/1" do - test "copying the same module raises" do - assert_raise ArgumentError, - "Module Calculator has already been copied. See docs for Mimic.copy/1", - fn -> Mimic.copy(Calculator) end + describe "copy/1 with duplicates does nothing" do + test "stubs still stub" do + parent_pid = self() + + Mimic.copy(Calculator) + Mimic.copy(Calculator) + + Calculator + |> stub(:add, fn x, y -> + send(parent_pid, {:add, x, y}) + :stubbed + end) + + Mimic.copy(Calculator) + + assert Calculator.add(1, 2) == :stubbed + assert_receive {:add, 1, 2} end end end From 42f376c06afa20ca2599ffb53e6bc41101b8b369 Mon Sep 17 00:00:00 2001 From: Dylan Chong Date: Wed, 27 Jul 2022 22:30:57 +1200 Subject: [PATCH 2/6] Retry CI From ae8ca3c001f9ce0232e0677c5e2469a61ce838f2 Mon Sep 17 00:00:00 2001 From: Dylan Chong Date: Wed, 27 Jul 2022 22:51:18 +1200 Subject: [PATCH 3/6] Fix typespec --- lib/mimic/server.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mimic/server.ex b/lib/mimic/server.ex index 990aa96..f961729 100644 --- a/lib/mimic/server.ex +++ b/lib/mimic/server.ex @@ -95,7 +95,7 @@ defmodule Mimic.Server do GenServer.call(__MODULE__, {:mark_to_copy, module}, @long_timeout) end - @spec mark_to_copy(module) :: :boolean + @spec marked_to_copy?(module) :: boolean def marked_to_copy?(module) do GenServer.call(__MODULE__, {:marked_to_copy?, module}, @long_timeout) end From 696438b64f517b814a47ea70adc6a94a63decb5c Mon Sep 17 00:00:00 2001 From: Dylan Chong Date: Wed, 27 Jul 2022 22:53:05 +1200 Subject: [PATCH 4/6] Fix test --- test/mimic_test.exs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/mimic_test.exs b/test/mimic_test.exs index 6ff3cb1..5ffaa0e 100644 --- a/test/mimic_test.exs +++ b/test/mimic_test.exs @@ -909,6 +909,8 @@ defmodule Mimic.Test do end describe "copy/1 with duplicates does nothing" do + setup :set_mimic_private + test "stubs still stub" do parent_pid = self() From f9e17cd1ddf0a404b644da586753beb4f45e4a72 Mon Sep 17 00:00:00 2001 From: Dylan Chong Date: Wed, 27 Jul 2022 23:07:36 +1200 Subject: [PATCH 5/6] CI test --- test/mimic_test.exs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/test/mimic_test.exs b/test/mimic_test.exs index 5ffaa0e..9bb33d4 100644 --- a/test/mimic_test.exs +++ b/test/mimic_test.exs @@ -911,22 +911,24 @@ defmodule Mimic.Test do describe "copy/1 with duplicates does nothing" do setup :set_mimic_private - test "stubs still stub" do - parent_pid = self() + for i <- 1..10_000 do + test "stubs still stub #{i}" do + parent_pid = self() - Mimic.copy(Calculator) - Mimic.copy(Calculator) + Mimic.copy(Calculator) + Mimic.copy(Calculator) - Calculator - |> stub(:add, fn x, y -> - send(parent_pid, {:add, x, y}) - :stubbed - end) + Calculator + |> stub(:add, fn x, y -> + send(parent_pid, {:add, x, y}) + :stubbed + end) - Mimic.copy(Calculator) + Mimic.copy(Calculator) - assert Calculator.add(1, 2) == :stubbed - assert_receive {:add, 1, 2} + assert Calculator.add(1, 2) == :stubbed + assert_receive {:add, 1, 2} + end end end end From 3b2e13ddee3079b982c8e298243938f423a910a5 Mon Sep 17 00:00:00 2001 From: Dylan Chong Date: Wed, 27 Jul 2022 23:20:21 +1200 Subject: [PATCH 6/6] Remove CI test --- test/mimic_test.exs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/test/mimic_test.exs b/test/mimic_test.exs index 9bb33d4..5ffaa0e 100644 --- a/test/mimic_test.exs +++ b/test/mimic_test.exs @@ -911,24 +911,22 @@ defmodule Mimic.Test do describe "copy/1 with duplicates does nothing" do setup :set_mimic_private - for i <- 1..10_000 do - test "stubs still stub #{i}" do - parent_pid = self() + test "stubs still stub" do + parent_pid = self() - Mimic.copy(Calculator) - Mimic.copy(Calculator) + Mimic.copy(Calculator) + Mimic.copy(Calculator) - Calculator - |> stub(:add, fn x, y -> - send(parent_pid, {:add, x, y}) - :stubbed - end) + Calculator + |> stub(:add, fn x, y -> + send(parent_pid, {:add, x, y}) + :stubbed + end) - Mimic.copy(Calculator) + Mimic.copy(Calculator) - assert Calculator.add(1, 2) == :stubbed - assert_receive {:add, 1, 2} - end + assert Calculator.add(1, 2) == :stubbed + assert_receive {:add, 1, 2} end end end