Fix Phoenix.ChannelTest sockets violating Phoenix.Socket.t (#6870)

* Type topic and channel_pid as nilable in Phoenix.Socket.t

Both fields are nil until a channel is joined: Socket.user_connect
builds the socket that Channel.Server.join/4 receives with neither set.
The spec on Server.join/4 required a binary topic and a pid, so any
caller passing an unjoined socket broke the contract.

* Keep the ChannelTest supervisor in socket.private

Phoenix.ChannelTest stored its test supervisor in the transport field
as {Phoenix.ChannelTest, sup}, but Phoenix.Socket.t types transport as
an atom. Dialyzer then reported every test join as never returning.

The transport is now Phoenix.ChannelTest and the supervisor is kept in
socket.private, which channel join preserves, so sockets returned from
join/4 can still be used to join again.

Closes #5509.

Author: osazemeu

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog for v1.9
 
+## Bug fixes
+
+  * [Phoenix.ChannelTest] Fix test sockets violating the Phoenix.Socket.t
+    typespec, which made Dialyzer treat every test join as never returning
+    ([#5509](https://github.com/phoenixframework/phoenix/issues/5509))
+  * [Phoenix.Socket] Type `topic` and `channel_pid` as possibly `nil` in
+    `Phoenix.Socket.t`, as they are before a channel is joined
+
 ## Enhancements
 
   * The longpoll session token is now sent in a `x-phoenix-longpoll-token`

lib/phoenix/socket.ex

@@ -275,7 +275,7 @@ defmodule Phoenix.Socket do
   @type t :: %Socket{
           assigns: map,
           channel: atom,
-          channel_pid: pid,
+          channel_pid: pid | nil,
           endpoint: atom,
           handler: atom,
           id: String.t() | nil,
@@ -284,7 +284,7 @@ defmodule Phoenix.Socket do
           private: map,
           pubsub_server: atom,
           serializer: atom,
-          topic: String.t(),
+          topic: String.t() | nil,
           transport: atom,
           transport_pid: pid
         }

lib/phoenix/test/channel_test.ex

@@ -261,7 +261,8 @@ defmodule Phoenix.ChannelTest do
       id: id,
       pubsub_server: endpoint.config(:pubsub_server),
       serializer: NoopSerializer,
-      transport: {__MODULE__, fetch_test_supervisor!(options)},
+      private: %{phoenix_test_supervisor: fetch_test_supervisor!(options)},
+      transport: __MODULE__,
       transport_pid: self()
     }
   end
@@ -335,17 +336,19 @@ defmodule Phoenix.ChannelTest do
         Keyword.pop(options, :connect_info, %{})
       end
 
+    sup = fetch_test_supervisor!(options)
+
     map = %{
       endpoint: endpoint,
-      transport: {__MODULE__, fetch_test_supervisor!(options)},
+      transport: __MODULE__,
       options: [serializer: [{NoopSerializer, "~> 1.0.0"}]],
       params: __stringify__(params),
       connect_info: connect_info
     }
 
     with {:ok, state} <- handler.connect(map),
          {:ok, {_, socket}} = handler.init(state),
-         do: {:ok, socket}
+         do: {:ok, %{socket | private: Map.put(socket.private, :phoenix_test_supervisor, sup)}}
   end
 
   @doc "See `subscribe_and_join!/4`."
@@ -441,7 +444,7 @@ defmodule Phoenix.ChannelTest do
         match_topic_to_channel!(socket, topic)
       end
 
-    %Socket{transport: {__MODULE__, sup}} = socket
+    %Socket{private: %{phoenix_test_supervisor: sup}} = socket
 
     starter =
       fn _, _, spec ->

test/phoenix/test/channel_test.exs

@@ -234,11 +234,12 @@ defmodule Phoenix.Test.ChannelTest do
     assert socket.endpoint == @endpoint
     assert socket.pubsub_server == Phoenix.Test.ChannelTest.PubSub
     assert socket.topic == "foo:socket"
-    assert {Phoenix.ChannelTest, _} = socket.transport
+    assert socket.transport == Phoenix.ChannelTest
     assert socket.transport_pid == self()
     assert socket.serializer == Phoenix.ChannelTest.NoopSerializer
     assert socket.assigns == %{hello: :world, original: :assign}
     assert %{socket | joined: true} == client
+    assert {:ok, _, _} = join(client, Channel, "foo:socket")
 
     {:links, links} = Process.info(self(), :links)
     assert client.channel_pid in links
@@ -366,6 +367,7 @@ defmodule Phoenix.Test.ChannelTest do
   test "connects and joins topics directly" do
     :error = connect(UserSocket, %{"reject" => true})
     {:ok, socket} = connect(UserSocket, %{})
+    assert socket.transport == Phoenix.ChannelTest
     socket = subscribe_and_join!(socket, "foo:ok")
     push(socket, "broadcast", %{"foo" => "bar"})
     assert socket.id == "123"