Warn on duplicate IDs and fail test, but don't prevent debugging

Relates to: #3560

lib/phoenix_live_view/test/client_proxy.ex

@@ -82,12 +82,13 @@ defmodule Phoenix.LiveViewTest.ClientProxy do
       router: router,
       session: session,
       url: url,
-      test_supervisor: test_supervisor
+      test_supervisor: test_supervisor,
+      duplicate_id_target: duplicate_id_target
     } = opts
 
     # We can assume there is at least one LiveView
     # because the live_module assign was set.
-    root_html = DOM.parse(response_html)
+    root_html = DOM.parse(response_html, duplicate_id_target)
 
     {id, session_token, static_token, redirect_url} =
       case Map.fetch(opts, :live_redirect) do

lib/phoenix_live_view/test/dom.ex

@@ -18,46 +18,43 @@ defmodule Phoenix.LiveViewTest.DOM do
     end
   end
 
-  @spec parse(binary) :: [
+  @spec parse(html :: binary(), duplicate_id_target :: nil | {pid(), reference()}) :: [
           {:comment, binary}
           | {:pi | binary, binary | list, list}
           | {:doctype, binary, binary, binary}
         ]
-  def parse(html) do
+  def parse(html, duplicate_id_target \\ nil) do
     {:ok, parsed} = Floki.parse_document(html)
-    detect_duplicate_ids(parsed)
+
+    if duplicate_id_target do
+      detect_duplicate_ids(parsed, duplicate_id_target)
+    end
 
     parsed
   end
 
-  defp detect_duplicate_ids(tree), do: detect_duplicate_ids(tree, MapSet.new())
+  defp detect_duplicate_ids(tree, target), do: detect_duplicate_ids(tree, MapSet.new(), target)
 
-  defp detect_duplicate_ids([node | rest], ids) do
-    ids = detect_duplicate_ids(node, ids)
-    detect_duplicate_ids(rest, ids)
+  defp detect_duplicate_ids([node | rest], ids, target) do
+    ids = detect_duplicate_ids(node, ids, target)
+    detect_duplicate_ids(rest, ids, target)
   end
 
-  defp detect_duplicate_ids({_tag_name, _attrs, children} = node, ids) do
+  defp detect_duplicate_ids({_tag_name, _attrs, children} = node, ids, {pid, ref} = target) do
     case Floki.attribute(node, "id") do
       [id] ->
         if MapSet.member?(ids, id) do
-          raise """
-          Duplicate id found: #{id}
-
-          LiveView requires that all elements have unique ids, duplicate IDs will cause
-          undefined behavior at runtime, as DOM patching will not be able to target the correct
-          elements.
-          """
-        else
-          detect_duplicate_ids(children, MapSet.put(ids, id))
+          send(pid, {:duplicate_id, ref, id, node})
         end
 
+        detect_duplicate_ids(children, MapSet.put(ids, id), target)
+
       _ ->
-        detect_duplicate_ids(children, ids)
+        detect_duplicate_ids(children, ids, target)
     end
   end
 
-  defp detect_duplicate_ids(_non_tag, seen_ids), do: seen_ids
+  defp detect_duplicate_ids(_non_tag, seen_ids, _target), do: seen_ids
 
   def all(html_tree, selector), do: Floki.find(html_tree, selector)
 

lib/phoenix_live_view/test/live_view_test.ex

@@ -371,6 +371,7 @@ defmodule Phoenix.LiveViewTest do
 
   defp start_proxy(path, %{} = opts) do
     ref = make_ref()
+    warn_pid = start_duplicate_id_check(ref)
 
     opts =
       Map.merge(opts, %{
@@ -382,7 +383,8 @@ defmodule Phoenix.LiveViewTest do
         endpoint: opts.endpoint,
         session: opts.session,
         url: opts.url,
-        test_supervisor: fetch_test_supervisor!()
+        test_supervisor: fetch_test_supervisor!(),
+        duplicate_id_target: {warn_pid, ref}
       })
 
     case ClientProxy.start_link(opts) do
@@ -401,6 +403,48 @@ defmodule Phoenix.LiveViewTest do
     end
   end
 
+  defp start_duplicate_id_check(ref) do
+    pid =
+      spawn(fn ->
+        receive do
+          {:collect, pid} ->
+            warn_duplicate_ids_loop(ref, pid, false)
+        end
+      end)
+
+    ExUnit.Callbacks.on_exit(fn ->
+      send(pid, {:collect, self()})
+
+      receive do
+        {^ref, :ok} -> :ok
+        {^ref, :raise} -> raise "Detected duplicate IDs! Check the warnings logged for details."
+      end
+    end)
+
+    pid
+  end
+
+  defp warn_duplicate_ids_loop(ref, pid, warned?) do
+    receive do
+      {:duplicate_id, ^ref, id, node} ->
+        IO.warn("""
+        Duplicate id found while testing LiveView: #{id}
+
+        #{DOM.inspect_html(node)}
+
+        LiveView requires that all elements have unique ids, duplicate IDs will cause
+        undefined behavior at runtime, as DOM patching will not be able to target the correct
+        elements.
+        """)
+
+        # there could be multiple messages waiting for us
+        warn_duplicate_ids_loop(ref, pid, true)
+    after
+      0 ->
+        send(pid, {ref, (warned? && :raise) || :ok})
+    end
+  end
+
   defp fetch_test_supervisor!() do
     case ExUnit.fetch_test_supervisor() do
       {:ok, sup} -> sup

test/phoenix_live_view/integrations/nested_test.exs

@@ -147,7 +147,7 @@ defmodule Phoenix.LiveView.NestedTest do
     :ok = GenServer.call(view.pid, {:dynamic_child, :static})
 
     assert Exception.format(:exit, catch_exit(render(view))) =~
-             "Duplicate id found: static"
+             "expected selector \"#static\" to return a single element, but got 2"
   end
 
   describe "navigation helpers" do