Fix Unreliable Hound.Helpers.Session API

lib/hound.ex

@@ -26,7 +26,7 @@ defmodule Hound do
 
   @doc "See `Hound.Helpers.Session.end_session/1`"
   defdelegate end_session,        to: Hound.Helpers.Session
-  defdelegate end_session(pid),   to: Hound.Helpers.Session
+  defdelegate end_session(session_id),   to: Hound.Helpers.Session
 
   @doc false
   defdelegate current_session_id, to: Hound.Helpers.Session

lib/hound/helpers/session.ex

@@ -2,48 +2,28 @@ defmodule Hound.Helpers.Session do
   @moduledoc "Session helpers"
 
   @doc """
-  Switches to another session.
-
-  When you need more than one browser session, use this function switch to another session.
-  If the session doesn't exist it a new one will be created for you.
+  Switches to another session when you need more than one browser session.
   All further commands will then run in the session you switched to.
-
-      # Pass any name to the session to refer to it later.
-      change_session_to("random-session")
-
-  The name can be an atom or a string. The default session created is called `:default`.
+  Exits if session does not exist.
   """
-  def change_session_to(session_name, opts \\ []) do
-    Hound.SessionServer.change_current_session_for_pid(self(), session_name, opts)
-  end
-
-
-  @doc """
-  When running multiple browser sessions, calling this function will switch to the default browser session.
-
-      change_to_default_session
-
-      # is the same as calling
-      change_session_to(:default)
-  """
-  def change_to_default_session do
-    change_session_to(:default)
+  def change_session_to(session_id) do
+    Hound.SessionServer.change_current_session_for_pid(self(), session_id)
   end
 
 
   @doc """
   Execute commands in a separate browser session.
 
-      in_browser_session "another_user", fn ->
+      perform_in_session "another_user", fn ->
         navigate_to "http://example.com"
         click({:id, "announcement"})
       end
   """
-  def in_browser_session(session_name, func) do
-    previous_session_name = current_session_name()
-    change_session_to(session_name)
+  def perform_in_session(session_id, func) do
+    previous_session_id = current_session_id()
+    change_session_to(session_id)
     result = apply(func, [])
-    change_session_to(previous_session_name)
+    change_session_to(previous_session_id)
     result 
   end
 
@@ -94,32 +74,35 @@ defmodule Hound.Helpers.Session do
     * `:driver` - The additional capabilities to be passed directly to the webdriver.
   """
   def start_session(opts \\ []) do
-    Hound.SessionServer.session_for_pid(self(), opts)
+    Hound.SessionServer.start_session_for_pid(self(), opts)
   end
 
 
   @doc """
-  Ends a Hound session that is associated with a pid.
-
-  If you have multiple sessions, all of those sessions are killed.
+  Ends a session that is associated with a session_id.
   """
-  def end_session(pid \\ self()) do
-    Hound.SessionServer.destroy_sessions_for_pid(pid)
+  def end_session(session_id) do
+    Hound.Session.destroy_session(session_id)
   end
 
+  @doc """
+  Ends the current session for the process
+  """
+  def end_session() do
+    Hound.SessionServer.destroy_session_for_pid(self)
+  end
 
   @doc false
-  def current_session_id do
+  def current_session_id() do
     Hound.SessionServer.current_session_id(self()) ||
       raise "could not find a session for process #{inspect self()}"
   end
 
-
-  @doc false
-  def current_session_name do
-      Hound.SessionServer.current_session_name(self()) ||
-        raise "could not find a session for process #{inspect self()}"
-
-
+  @doc """
+  Get list of active sessions
+  """
+  def active_sessions() do
+    Hound.Session.active_sessions()
   end
+
 end

lib/hound/session_server.ex

@@ -8,105 +8,52 @@ defmodule Hound.SessionServer do
     GenServer.start_link(__MODULE__, %{}, name: @name)
   end
 
-
-  def session_for_pid(pid, opts) do
-    current_session_id(pid) ||
-      change_current_session_for_pid(pid, :default, opts)
+  def start_session_for_pid(pid, opts) do
+    GenServer.call(@name, {:start_session, pid, opts}, 60000)
   end
 
-
   def current_session_id(pid) do
-    case :ets.lookup(@name, pid) do
-      [{^pid, _ref, session_id, _all_sessions}] -> session_id
-      [] -> nil
-    end
-  end
-
-  def current_session_name(pid) do
-    case :ets.lookup(@name, pid) do
-      [{^pid, _ref, session_id, all_sessions}] ->
-        Enum.find_value all_sessions, fn
-          {name, id} when id == session_id -> name
-          _ -> nil
-        end
-      [] -> nil
-    end
+    GenServer.call(@name, {:current_session, pid}, 60000)
   end
 
-
-  def change_current_session_for_pid(pid, session_name, opts) do
-    GenServer.call(@name, {:change_session, pid, session_name, opts}, 60000)
+  def change_current_session_for_pid(pid, session_id) do
+    GenServer.call(@name, {:change_session, pid, session_id}, 60000)
   end
 
-
-  def all_sessions_for_pid(pid) do
-    case :ets.lookup(@name, pid) do
-      [{^pid, _ref, _session_id, all_sessions}] -> all_sessions
-      [] -> %{}
-    end
+  def destroy_session_for_pid(pid) do
+    GenServer.call(@name, {:destroy_session, pid}, 60000)
   end
 
 
-  def destroy_sessions_for_pid(pid) do
-    GenServer.call(@name, {:destroy_sessions, pid}, 60000)
-  end
-
   ## Callbacks
 
   def init(state) do
-    :ets.new(@name, [:set, :named_table, :protected, read_concurrency: true])
     {:ok, state}
   end
 
-
-  def handle_call({:change_session, pid, session_name, opts}, _from, state) do
+  def handle_call({:start_session, pid, opts}, _from, state) do
     {:ok, driver_info} = Hound.driver_info
-
-    {ref, sessions} =
-      case :ets.lookup(@name, pid) do
-        [{^pid, ref, _session_id, sessions}] ->
-          {ref, sessions}
-        [] ->
-          {Process.monitor(pid), %{}}
-      end
-
-    {session_id, sessions} =
-      case Map.fetch(sessions, session_name) do
-        {:ok, session_id} ->
-          {session_id, sessions}
-        :error ->
-          session_id = create_session(driver_info, opts)
-          {session_id, Map.put(sessions, session_name, session_id)}
-      end
-
-    :ets.insert(@name, {pid, ref, session_id, sessions})
-    {:reply, session_id, Map.put(state, ref, pid)}
+    {:ok, session_id} = Hound.Session.create_session(driver_info[:browser], opts)
+    state = Map.put(state, pid, session_id)
+    {:reply, session_id, state}
   end
 
-  def handle_call({:destroy_sessions, pid}, _from, state) do
-    destroy_sessions(pid)
-    {:reply, :ok, state}
+  def handle_call({:current_session, pid}, _from, state) do
+    {:reply, state[pid], state}
   end
 
-  def handle_info({:DOWN, ref, _, _, _}, state) do
-    if pid = state[ref] do
-      destroy_sessions(pid)
+  def handle_call({:change_session, pid, session_id}, _from, state) do
+    if (!Enum.any?(Hound.Session.active_sessions(), fn(session) -> session["id"] == session_id end)) do
+      raise "Error: Session id does not exist"
     end
-    {:noreply, state}
+    state = Map.put(state, pid, session_id)
+    {:reply, session_id, state}
   end
 
-  defp create_session(driver_info, opts) do
-    case Hound.Session.create_session(driver_info[:browser], opts) do
-      {:ok, session_id} -> session_id
-      {:error, reason} -> raise "could not create a new session: #{reason}, check webdriver is running"
-    end
+  def handle_call({:destroy_session, pid}, _from, state) do
+    Hound.Session.destroy_session(state[pid])
+    state = Map.put(state, pid, [])
+    {:reply, :ok, state}
   end
 
-  defp destroy_sessions(pid) do
-    sessions = all_sessions_for_pid(pid)
-    :ets.delete(@name, pid)
-    Enum.each sessions, fn({_session_name, session_id})->
-      Hound.Session.destroy_session(session_id)
-    end
-  end
 end

mix.lock

@@ -7,4 +7,4 @@
   "mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [:rebar3], [], "hexpm"},
   "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], [], "hexpm"},
-  "unicode_util_compat": {:hex, :unicode_util_compat, "0.3.1", "a1f612a7b512638634a603c8f401892afbf99b8ce93a45041f8aaca99cadb85e", [:rebar3], [], "hexpm"}}
+  "unicode_util_compat": {:hex, :unicode_util_compat, "0.3.1", "a1f612a7b512638634a603c8f401892afbf99b8ce93a45041f8aaca99cadb85e", [:rebar3], [], "hexpm"}}

test/multiple_browser_session_test.exs

@@ -5,21 +5,22 @@ defmodule MultipleBrowserSessionTest do
   hound_session()
 
   test "should be able to run multiple sessions" do
+    first_session = Hound.start_session
     url1 = "http://localhost:9090/page1.html"
     url2 = "http://localhost:9090/page2.html"
 
     # Navigate to a url
     navigate_to(url1)
 
     # Change to another session
-    change_session_to :another_session
+    Hound.start_session
     # Navigate to a url in the second session
     navigate_to(url2)
     # Then assert url
     assert url2 == current_url()
 
-    # Now go back to the default session
-    change_to_default_session()
+    # Now go back to the original session
+    change_session_to(first_session)
     # Assert if the url is the one we visited
     assert url1 == current_url()
   end
@@ -33,7 +34,8 @@ defmodule MultipleBrowserSessionTest do
     navigate_to(url1)
 
     # In another session...
-    in_browser_session :another_session, fn->
+    another_session = Hound.start_session
+    perform_in_session another_session, fn->
       navigate_to(url2)
       assert url2 == current_url()
     end
@@ -43,6 +45,7 @@ defmodule MultipleBrowserSessionTest do
   end
 
   test "should preserve session after using in_browser_session" do
+    default = Hound.start_session
     url1 = "http://localhost:9090/page1.html"
     url2 = "http://localhost:9090/page2.html"
     url3 = "http://localhost:9090/page3.html"
@@ -51,11 +54,13 @@ defmodule MultipleBrowserSessionTest do
     navigate_to(url1)
 
     # Change to a second session and navigate to url2
-    change_session_to :session_a
+    session_a = Hound.start_session
+    change_session_to session_a
     navigate_to(url2)
 
     # In a third session...
-    in_browser_session :session_b, fn ->
+    session_b = Hound.start_session
+    perform_in_session session_b, fn ->
       navigate_to(url3)
       assert url3 == current_url()
     end
@@ -64,7 +69,7 @@ defmodule MultipleBrowserSessionTest do
     assert url2 == current_url()
 
     # Switch back to the default session
-    change_session_to :default
+    change_session_to default
 
     # Assert the current url is the one we visited in the default session
     assert url1 == current_url()
@@ -74,8 +79,9 @@ defmodule MultipleBrowserSessionTest do
     url1 = "http://localhost:9090/page1.html"
 
     # In another session, navigate to url1 and return the current url
+    another_session = Hound.start_session
     result =
-      in_browser_session :another_session, fn ->
+      perform_in_session another_session, fn ->
         navigate_to(url1)
         current_url()
       end