Merge pull request #126 from BKStephens/fix_unstable_async

Fix unstable behavior without `async: false`

README.md

@@ -21,7 +21,7 @@ It's inspired by Ruby's VCR (https://github.com/vcr/vcr), and trying to provide
     - The JSON file can be recorded automatically (vcr_cassettes) or manually updated (custom_cassettes)
 
 ### Notes
-- In case test behaves unstable, please try to specify `use ExUnit.Case, async: false`.
+- ExVCR.Config functions must be called from setup or test. Calls outside of test process, such as in setup_all will not work.
 
 ### Install
 Add `:exvcr` to `deps` section of `mix.exs`.
@@ -50,10 +50,10 @@ Optionally, `preferred_cli_env: [vcr: :test]` can be specified for running `mix
 ##### Example with ibrowse
 ```Elixir
 defmodule ExVCR.Adapter.IBrowseTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock
 
-  setup_all do
+  setup do
     ExVCR.Config.cassette_library_dir("fixture/vcr_cassettes")
     :ok
   end
@@ -79,7 +79,7 @@ end
 ##### Example with hackney
 ```Elixir
 defmodule ExVCR.Adapter.HackneyTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
 
   setup_all do
@@ -97,7 +97,7 @@ end
 ##### Example with httpc
 ```Elixir
 defmodule ExVCR.Adapter.HttpcTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock, adapter: ExVCR.Adapter.Httpc
 
   setup_all do
@@ -121,10 +121,10 @@ You can manually define custom cassette json file for more flexible response con
 
 ```Elixir
 defmodule ExVCR.MockTest do
-  use ExUnit.Case
+  use ExUnit.Case, async: true
   import ExVCR.Mock
 
-  setup_all do
+  setup do
     ExVCR.Config.cassette_library_dir("fixture/vcr_cassettes", "fixture/custom_cassettes")
     :ok
   end
@@ -239,24 +239,6 @@ If `ExVCR.Config.response_headers_blacklist(headers_blacklist)` is specified, th
   end
 ```
 
-#### Clearing Mock After Each Cassette
-By default, mocking through `:meck.expect` is not cleared after each `use_cassette`. It can cause error when mixing actual/mocking accesses. In order to clear mock, please specify `[clear_mock: :true]` option through either of the followings.
-
-```elixir
-# For applying all the tests under the module.
-defmodule ExVCR.Adapter.OptionsTest do
-  use ExVCR.Mock, options: [clear_mock: true]
-  use ExUnit.Case, async: false
-...
-```
-
-```elixir
-# For applying specific test.
-use_cassette "option_clean_each", clear_mock: true do
-  assert HTTPotion.get(@url, []).body == "test_response1"
-end
-```
-
 #### Matching Options
 ##### matching against query params
 By default, query params are not used for matching. In order to include query params, specify `match_requests_on: [:query]` for `use_cassette` call.
@@ -548,4 +530,3 @@ end
 
 ### TODO
 - Improve performance, as it's very slow.
-- Fix unstable behavior without `async: false`.

lib/exvcr/iex.ex

@@ -20,16 +20,12 @@ defmodule ExVCR.IEx do
           recorder = Recorder.start(
             unquote(options) ++ [fixture: "", adapter: unquote(adapter)])
 
-          target_methods = adapter_method().target_methods(recorder)
-          module_name    = adapter_method().module_name
-
-          Enum.each(target_methods, fn({function, callback}) ->
-            :meck.expect(module_name, function, callback)
-          end)
-
           try do
+            ExVCR.Mock.mock_methods(recorder, unquote(adapter))
             unquote(test)
           after
+            :meck.unload(unquote(adapter.module_name))
+            ExVCR.MockLock.release_lock()
             Recorder.get(recorder)
             |> JSX.encode!
             |> JSX.prettify!

lib/exvcr/mock.ex

@@ -33,15 +33,14 @@ defmodule ExVCR.Mock do
       stub = prepare_stub_record(unquote(options), adapter_method())
       recorder = Recorder.start([fixture: stub_fixture, stub: stub, adapter: adapter_method()])
 
-      mock_methods(recorder, adapter_method())
-
       try do
+        mock_methods(recorder, adapter_method())
         [do: return_value] = unquote(test)
         return_value
       after
-        if options_method()[:clear_mock] || unquote(options)[:clear_mock] do
-          :meck.unload(adapter_method().module_name)
-        end
+        module_name = adapter_method().module_name
+        :meck.unload(module_name)
+        ExVCR.MockLock.release_lock()
       end
     end
   end
@@ -61,16 +60,19 @@ defmodule ExVCR.Mock do
       recorder = Recorder.start(
         unquote(options) ++ [fixture: normalize_fixture(unquote(fixture)), adapter: adapter_method()])
 
-      mock_methods(recorder, adapter_method())
 
       try do
+        mock_methods(recorder, adapter_method())
         [do: return_value] = unquote(test)
         return_value
       after
-        if options_method()[:clear_mock] || unquote(options)[:clear_mock] do
-          :meck.unload(adapter_method().module_name)
-        end
-        Recorder.save(recorder)
+        recorder_result = Recorder.save(recorder)
+
+        module_name = adapter_method().module_name
+        :meck.unload(module_name)
+        ExVCR.MockLock.release_lock()
+
+        recorder_result
       end
     end
   end
@@ -91,9 +93,18 @@ defmodule ExVCR.Mock do
     target_methods = adapter.target_methods(recorder)
     module_name    = adapter.module_name
 
-    Enum.each(target_methods, fn({function, callback}) ->
-      :meck.expect(module_name, function, callback)
+    parent_pid = self()
+    Task.async(fn ->
+      ExVCR.MockLock.ensure_started
+      ExVCR.MockLock.request_lock(self(), parent_pid)
+      receive do
+        :lock_granted ->
+          Enum.each(target_methods, fn({function, callback}) ->
+            :meck.expect(module_name, function, callback)
+          end)
+      end
     end)
+    |> Task.await(:infinity)
   end
 
   @doc """

lib/exvcr/mock_lock.ex

@@ -0,0 +1,49 @@
+defmodule ExVCR.MockLock do
+  use ExActor.GenServer, export: :mock_lock
+  @ten_milliseconds 10
+
+  defstart start() do
+    initial_state(%{lock_holder: nil})
+  end
+
+  def ensure_started do
+    unless Process.whereis(:mock_lock) do
+      __MODULE__.start
+    end
+  end
+
+  defcast request_lock(caller_pid, test_pid) do
+    Process.send(self(), {:do_request_lock, caller_pid, test_pid}, [])
+    noreply()
+  end
+
+  defhandleinfo {:do_request_lock, caller_pid, test_pid}, state: state do
+    if Map.get(state, :lock_holder) do
+      Process.send_after(self(), {:do_request_lock, caller_pid, test_pid}, @ten_milliseconds)
+      noreply()
+    else
+      Process.monitor(test_pid)
+      Process.send(caller_pid, :lock_granted, [])
+
+      state
+      |> Map.put(:lock_holder, caller_pid)
+      |> new_state
+    end
+  end
+
+  defhandleinfo {:DOWN, _ref, :process, pid, _}, state: state do
+    if state.lock_holder == pid do
+      state
+      |> Map.put(:lock_holder, nil)
+      |> new_state
+    else
+      noreply()
+    end
+  end
+
+  defcall release_lock(), state: state do
+    state
+    |> Map.put(:lock_holder, nil)
+    |> set_and_reply(:ok)
+  end
+end

lib/exvcr/setting.ex

@@ -3,27 +3,28 @@ defmodule ExVCR.Setting do
   An module to store the configuration settings.
   """
 
-  @ets_table :exvcr_setting
-
-
   def get(key) do
     setup()
-    :ets.lookup(@ets_table, key)[key]
+    :ets.lookup(table(), key)[key]
   end
 
   def set(key, value) do
     setup()
-    :ets.insert(@ets_table, {key, value})
+    :ets.insert(table(), {key, value})
   end
 
   def append(key, value) do
     set(key, [value | ExVCR.Setting.get(key)])
   end
 
   defp setup do
-    if :ets.info(@ets_table) == :undefined do
-      :ets.new(@ets_table, [:set, :public, :named_table])
+    if :ets.info(table()) == :undefined do
+      :ets.new(table(), [:set, :public, :named_table])
       ExVCR.ConfigLoader.load_defaults
     end
   end
+
+  defp table do
+    "exvcr_setting#{inspect self()}" |> String.to_atom
+  end
 end

test/adapter_hackney_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.Adapter.HackneyTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
 
   setup_all do

test/adapter_httpc_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.Adapter.HttpcTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock, adapter: ExVCR.Adapter.Httpc
 
   setup_all do

test/adapter_ibrowse_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.Adapter.IBrowseTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock
 
   setup_all do

test/config_loader_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.ConfigLoaderTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
 
   @dummy_cassette_dir "tmp/vcr_tmp/vcr_cassettes"
   @dummy_custom_dir   "tmp/vcr_tmp/vcr_custom"
@@ -62,4 +62,4 @@ defmodule ExVCR.ConfigLoaderTest do
     Application.get_env(:exvcr, :filter_sensitive_data, filter_sensitive_data)
     Application.get_env(:exvcr, :response_headers_blacklist, response_headers_blacklist)
   end
-end
+end

test/config_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.ConfigTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
 
   @dummy_cassette_dir "tmp/vcr_tmp/vcr_cassettes"
   @dummy_custom_dir   "tmp/vcr_tmp/vcr_custom"

test/filter_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.FilterTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
 
   test "filter_sensitive_data" do
     ExVCR.Config.filter_sensitive_data("<PASSWORD>.+</PASSWORD>", "PLACEHOLDER")

test/handler_custom_mode_test.exs

@@ -1,5 +1,5 @@
 defmodule ExVCR.Adapter.HandlerCustomModeTest do
-  use ExUnit.Case, async: false
+  use ExUnit.Case, async: true
   use ExVCR.Mock
 
   test "query param match succeeds with custom mode" do
@@ -50,4 +50,4 @@ defmodule ExVCR.Adapter.HandlerCustomModeTest do
       end
     end
   end
-end
+end