From 3631d7eee4bd034f2eefe1b9892d5fcd565579ac Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Mon, 14 Jan 2019 00:26:27 +0100 Subject: 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. --- .../lib/action_cable/connection/test_case.rb | 83 ++++++++++------------ actioncable/test/connection/test_case_test.rb | 31 ++++---- 2 files changed, 57 insertions(+), 57 deletions(-) (limited to 'actioncable') 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 -- cgit v1.2.3