Update Action Cable connection testing.

* Don't reimplement assert_raises

  Also test what happens in case there's no explicit rejection.

* Avoid OpenStruct. Remove space beneath private.

* Simplify verification methods for code under test.

* Match documentation with other Rails docs.

  Also remove mention of the custom path argument for now.
  Unsure how useful that really is.

2 files changed, 57 insertions, 57 deletions

diff --git a/actioncable/lib/action_cable/connection/test_case.rb b/actioncable/lib/action_cable/connection/test_case.rb
index ac3bf32df5..233fd837e0 100644
--- a/actioncable/lib/action_cable/connection/test_case.rb
+++ b/actioncable/lib/action_cable/connection/test_case.rb
@@ -23,15 +23,7 @@ module ActionCable
       #   # Asserts that connection without user_id fails
       #   assert_reject_connection { connect params: { user_id: '' } }
       def assert_reject_connection(&block)
-        res =
-          begin
-            block.call
-            false
-          rescue ActionCable::Connection::Authorization::UnauthorizedError
-            true
-          end
-
-        assert res, "Expected to reject connection but no rejection were made"
+        assert_raises(Authorization::UnauthorizedError, "Expected to reject connection but no rejection was made", &block)
       end
     end
 
@@ -66,61 +58,64 @@ module ActionCable
       end
     end
 
-    # Superclass for Action Cable connection unit tests.
+    # Unit test Action Cable connections.
+    #
+    # Useful to check whether a connection's +identified_by+ gets assigned properly
+    # and that any improper connection requests are rejected.
     #
     # == Basic example
     #
     # Unit tests are written as follows:
-    # 1. First, one uses the +connect+ method to simulate connection.
-    # 2. Then, one asserts whether the current state is as expected (e.g. identifiers).
-    #
-    # For example:
-    #
-    #   module ApplicationCable
-    #     class ConnectionTest < ActionCable::Connection::TestCase
-    #       def test_connects_with_cookies
-    #         cookies["user_id"] = users[:john].id
-    #         # Simulate a connection
-    #         connect
-    #
-    #         # Asserts that the connection identifier is correct
-    #         assert_equal "John", connection.user.name
-    #       end
-    #
-    #       def test_does_not_connect_without_user
-    #         assert_reject_connection do
-    #           connect
-    #         end
-    #       end
+    #
+    # 1. Simulate a connection attempt by calling +connect+.
+    # 2. Assert state, e.g. identifiers, has been assigned.
+    #
+    #
+    #   class ApplicationCable::ConnectionTest < ActionCable::Connection::TestCase
+    #     def test_connects_with_proper_cookie
+    #       # Simulate the connection request with a cookie.
+    #       cookies["user_id"] = users(:john).id
+
+    #       connect
+    #
+    #       # Assert the connection identifier matches the fixture.
+    #       assert_equal users(:john).id, connection.user.id
+    #     end
+    #
+    #     def test_rejects_connection_without_proper_cookie
+    #       assert_reject_connection { connect }
     #     end
     #   end
     #
-    # You can also provide additional information about underlying HTTP request
-    # (params, headers, session and Rack env):
+    # +connect+ accepts additional information the HTTP request with the
+    # +params+, +headers+, +session+ and Rack +env+ options.
     #
     #   def test_connect_with_headers_and_query_string
-    #     connect "/cable?user_id=1", headers: { "X-API-TOKEN" => 'secret-my' }
+    #     connect params: { user_id: 1 }, headers: { "X-API-TOKEN" => "secret-my" }
     #
-    #     assert_equal connection.user_id, "1"
+    #     assert_equal "1", connection.user.id
+    #     assert_equal "secret-my", connection.token
     #   end
     #
     #   def test_connect_with_params
     #     connect params: { user_id: 1 }
     #
-    #     assert_equal connection.user_id, "1"
+    #     assert_equal "1", connection.user.id
     #   end
     #
-    # You can also manage request cookies:
+    # You can also setup the correct cookies before the connection request:
     #
     #   def test_connect_with_cookies
-    #     # plain cookies
+    #     # Plain cookies:
     #     cookies["user_id"] = 1
-    #     # or signed/encrypted
+    #
+    #     # Or signed/encrypted:
     #     # cookies.signed["user_id"] = 1
+    #     # cookies.encrypted["user_id"] = 1
     #
     #     connect
     #
-    #     assert_equal connection.user_id, "1"
+    #     assert_equal "1", connection.user_id
     #   end
     #
     # == Connection is automatically inferred
@@ -179,13 +174,14 @@ module ActionCable
           end
         end
 
-        # Performs connection attempt (i.e. calls #connect method).
+        # Performs connection attempt to exert #connect on the connection under test.
         #
         # Accepts request path as the first argument and the following request options:
+        #
         # - params – url parameters (Hash)
         # - headers – request headers (Hash)
         # - session – session data (Hash)
-        # - env – addittional Rack env configuration (Hash)
+        # - env – additional Rack env configuration (Hash)
         def connect(path = ActionCable.server.config.mount_path, **request_params)
           path ||= DEFAULT_PATH
 
@@ -198,7 +194,7 @@ module ActionCable
           @connection = connection
         end
 
-        # Disconnect the connection under test (i.e. calls #disconnect)
+        # Exert #disconnect on the connection under test.
         def disconnect
           raise "Must be connected!" if connection.nil?
 
@@ -211,7 +207,6 @@ module ActionCable
         end
 
         private
-
           def build_test_request(path, params: nil, headers: {}, session: {}, env: {})
             wrapped_headers = ActionDispatch::Http::Headers.from_hash(headers)
 
diff --git a/actioncable/test/connection/test_case_test.rb b/actioncable/test/connection/test_case_test.rb
index 76cfb2c07c..3b19465d7b 100644
--- a/actioncable/test/connection/test_case_test.rb
+++ b/actioncable/test/connection/test_case_test.rb
@@ -75,10 +75,8 @@ class Connection < ActionCable::Connection::Base
   end
 
   private
-
     def verify_user
-      reject_unauthorized_connection unless cookies.signed[:user_id].present?
-      cookies.signed[:user_id]
+      cookies.signed[:user_id].presence || reject_unauthorized_connection
     end
 end
 
@@ -101,6 +99,14 @@ class ConnectionTest < ActionCable::Connection::TestCase
   def test_connection_rejected
     assert_reject_connection { connect }
   end
+
+  def test_connection_rejected_assertion_message
+    error = assert_raises Minitest::Assertion do
+      assert_reject_connection { "Intentionally doesn't connect." }
+    end
+
+    assert_match(/Expected to reject connection/, error.message)
+  end
 end
 
 class EncryptedCookiesConnection < ActionCable::Connection::Base
@@ -111,10 +117,8 @@ class EncryptedCookiesConnection < ActionCable::Connection::Base
   end
 
   private
-
     def verify_user
-      reject_unauthorized_connection unless cookies.encrypted[:user_id].present?
-      cookies.encrypted[:user_id]
+      cookies.encrypted[:user_id].presence || reject_unauthorized_connection
     end
 end
 
@@ -142,10 +146,8 @@ class SessionConnection < ActionCable::Connection::Base
   end
 
   private
-
     def verify_user
-      reject_unauthorized_connection unless request.session[:user_id].present?
-      request.session[:user_id]
+      request.session[:user_id].presence || reject_unauthorized_connection
     end
 end
 
@@ -170,11 +172,9 @@ class EnvConnection < ActionCable::Connection::Base
   end
 
   private
-
     def verify_user
       # Warden-like authentication
-      reject_unauthorized_connection unless env["authenticator"]&.user.present?
-      env["authenticator"].user
+      env["authenticator"]&.user || reject_unauthorized_connection
     end
 end
 
@@ -182,7 +182,12 @@ class EnvConnectionTest < ActionCable::Connection::TestCase
   tests EnvConnection
 
   def test_connected_with_env
-    connect env: { "authenticator" => OpenStruct.new(user: "David") }
+    authenticator = Class.new do
+      def user; "David"; end
+    end
+
+    connect env: { "authenticator" => authenticator.new }
+
     assert_equal "David", connection.user
   end