Fix: Do not override internal ref when watching view

What changed?
============

Currently, when we set a timeout for the original view we were watching
(meaning we call `watch_view` with the same view that we pass to
`start_link`), we override the `live_view_ref`. That can cause issues
since we use the `refs` to identify views when a process id DOWN.

This commit fixes that by ensuring we don't monitor a LiveView that
we're already monitoring, and that we don't override the internal
`live_view_ref`. The test peeks into the GenServer's state with `:sys`
commands -- not the prettiest thing. But it didn't quit seem enough to
warrant us having a `Watcher.view_info/1` function that fetched the info
for that view. So, it seems okay for now.

lib/phoenix_test/live_view_watcher.ex

@@ -23,7 +23,7 @@ defmodule PhoenixTest.LiveViewWatcher do
   end
 
   def handle_cast({:watch_view, live_view, timeout}, state) do
-    case monitor_view(live_view, timeout) do
+    case monitor_view(live_view, timeout, state.views) do
       {:ok, view} ->
         views = Map.put(state.views, view.pid, view)
 
@@ -96,21 +96,25 @@ defmodule PhoenixTest.LiveViewWatcher do
     {:noreply, state}
   end
 
-  defp monitor_view(view, timeout) do
+  defp monitor_view(live_view, timeout, watched_views) do
     # Monitor the LiveView for exits and redirects
-    live_view_ref = Process.monitor(view.pid)
+    live_view_ref =
+      case watched_views[live_view.pid] do
+        nil -> Process.monitor(live_view.pid)
+        %{live_view_ref: live_view_ref} -> live_view_ref
+      end
 
     # Set timeout
-    timeout_ref = Process.send_after(self(), {:timeout, view.pid}, timeout)
+    timeout_ref = Process.send_after(self(), {:timeout, live_view.pid}, timeout)
 
     # Monitor all async processes
-    case fetch_async_pids(view) do
+    case fetch_async_pids(live_view) do
       {:ok, pids} ->
         async_refs = Enum.map(pids, &Process.monitor(&1))
 
         view_data =
           %{
-            pid: view.pid,
+            pid: live_view.pid,
             live_view_ref: live_view_ref,
             timeout_ref: timeout_ref,
             async_refs: async_refs

test/phoenix_test/live_view_watcher_test.exs

@@ -88,16 +88,41 @@ defmodule PhoenixTest.LiveViewWatcherTest do
       assert_receive {:watcher, ^view_pid, :async_process_completed}
     end
 
-    test "can override watch settings (e.g. timeout) for original LiveView" do
-      {:ok, view_pid} = start_supervised(DummyLiveView, id: 1)
+    test "can watch original view (e.g. to set a timeout)" do
+      {:ok, view_pid} = start_supervised(DummyLiveView)
+      view = %{pid: view_pid}
+      {:ok, watcher} = start_supervised({LiveViewWatcher, %{caller: self(), view: view}})
+
+      :ok = LiveViewWatcher.watch_view(watcher, view, 0)
+
+      assert_receive {:watcher, ^view_pid, :timeout}
+    end
+
+    test "can override an existing view's timeout" do
+      {:ok, view_pid} = start_supervised(DummyLiveView)
       view = %{pid: view_pid}
       {:ok, watcher} = start_supervised({LiveViewWatcher, %{caller: self(), view: view}})
 
+      :ok = LiveViewWatcher.watch_view(watcher, view, 10_000)
       :ok = LiveViewWatcher.watch_view(watcher, view, 0)
 
       assert_receive {:watcher, ^view_pid, :timeout}
     end
 
+    test "does not overrides an (internal) live_view_ref info" do
+      {:ok, view_pid} = start_supervised(DummyLiveView)
+      view = %{pid: view_pid}
+      {:ok, watcher} = start_supervised({LiveViewWatcher, %{caller: self(), view: view}})
+
+      %{views: views} = :sys.get_state(watcher)
+      %{live_view_ref: live_view_ref} = views[view_pid]
+
+      :ok = LiveViewWatcher.watch_view(watcher, view, 0)
+
+      %{views: views} = :sys.get_state(watcher)
+      assert %{live_view_ref: ^live_view_ref} = views[view_pid]
+    end
+
     test "can watch multiple LiveViews" do
       {:ok, view_pid1} = start_supervised(DummyLiveView, id: 1)
       {:ok, view_pid2} = start_supervised({DummyLiveView, %{redirect_in: 10}}, id: 2)