Repository: edgurgel/mimic (elixir)

Issue:
Mimic.copy could be idempotent
When running `mix test--stale` from the top level of an umbrella app, and you have `Mimic.copy(M)` in two different app test_helper files, you'll get this error upon rerunning the tests with `--stale` (dunno why it doesnt happen without stale).

```
** (ArgumentError) Module M has already been copied.  See docs for Mimic.copy/1
    (mimic 1.7.2) lib/mimic.ex:506: Mimic.validate_server_response/2
    test/test_helper.exs:1: (file)
    (elixir 1.13.1) lib/code.ex:1183: Code.require_file/2
```

Unsure what the underlying issue is, but if Mimic ignored double `copy`s then we'd *maybe* be ok.


Pull request:
fix: Copy fails with umbrella mix test stale (make Mimic.copy idempotent)
Fixes #44 

Have tested in an umbrella app and works great!


Patch:
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..f961729 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 marked_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)


Test patch:
diff --git a/test/mimic_test.exs b/test/mimic_test.exs
index ff46d5d..5ffaa0e 100644
--- a/test/mimic_test.exs
+++ b/test/mimic_test.exs
@@ -908,11 +908,25 @@ 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
+    setup :set_mimic_private
+
+    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