New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ActionCable::Connection::TestCase #34845

Merged
merged 1 commit into from Jan 13, 2019

Conversation

Projects
None yet
5 participants
@palkan
Copy link
Contributor

palkan commented Jan 2, 2019

Summary

Follow-up for #33659.

That's the final part of action-cable-testing merging.

Connection tests are written as follows:

  1. First, one uses the connect method to simulate connection establishment (= connect callback invocation).
  2. Then, one asserts whether the current state is as expected. "State" here means connection identifiers or whether connection has been authorized or not.

For example:

class ApplicationCable::Connection < ActionCable::Connection::Base
  identified_by :user_id

  def connect
    self.user_id = request.params[:user_id] || cookies.signed[:user_id]
    reject_unauthorized_connection if user_id.nil?
  end

  def disconnect
    ActionCable.server.broadcast "users_presence", { id: user_id, event: "left" }
  end
end

class ApplicationCable::ConnectionTest < ActionCable::Connection::TestCase
  def test_connects_with_params
    # Simulate a connection opening
    connect params: { user_id: 42 }

    assert_equal connection.user_id, "42"
  end

  def test_connects_with_cookies
    # the same API as for integrations tests
    cookies.signed[:user_id] = 42

    # just call connect without any arguments
    connect

    assert_equal connection.user_id, "42"
  end

  def test_reject_connection
    assert_reject_connection { connect }
  end
end

You can also specify cookies, headers, Rack–all the options available for integration tests–plus session

Other Information

This PR doesn't contain a changelog entry intentionally (as the previous two): I'll add a change log in another PR, in which I'd like to update a testing guide as well.

@sponomarev
Copy link
Contributor

sponomarev left a comment

I wish that could be included to 5.2 branch.

# end
# end
#
# You can also provide additional information about underlying HTTP request:

This comment has been minimized.

@sponomarev

sponomarev Jan 2, 2019

Contributor

I guess it makes sense to mention the support of signed/encrypted/plain cookies API, headers, env, and session in the documentation.


# Performs connection attempt (i.e. calls #connect method).
#
# Accepts request path as the first argument and cookies and headers as options.

This comment has been minimized.

@sponomarev

sponomarev Jan 2, 2019

Contributor

env and session options are missed here. cookies are no more an option

# class ConnectionTest < ActionCable::Connection::TestCase
# def test_connects_with_cookies
# # Simulate a connection
# connect cookies: { user_id: users[:john].id }

This comment has been minimized.

@sponomarev

sponomarev Jan 2, 2019

Contributor

Am I right that this example is not relevant due to the new cookies API?

# Asserts that the connection is rejected (via +reject_unauthorized_connection+).
#
# # Asserts that connection without user_id fails
# assert_reject_connection { connect cookies: { user_id: '' } }

This comment has been minimized.

@sponomarev

sponomarev Jan 2, 2019

Contributor

Ditto

@palkan palkan force-pushed the palkan:feature/action-cable-connection-testing branch from 641d1c8 to 2111bc2 Jan 2, 2019

@palkan palkan force-pushed the palkan:feature/action-cable-connection-testing branch from 2111bc2 to 9029667 Jan 3, 2019

@sponomarev
Copy link
Contributor

sponomarev left a comment

❤️❤️❤️

@rmacklin

This comment has been minimized.

Copy link
Contributor

rmacklin commented Jan 12, 2019

👍 Great to see action-cable-testing making its way into rails core. Thanks for your continued work @palkan!

@palkan

This comment has been minimized.

Copy link
Contributor Author

palkan commented Jan 12, 2019

@rafaelfranca @kaspth Hey folks! Is there any chance for get this reviewed (and, hopefully, merged for Beta 1) and thus finish the action-cable-testing merging?

@kaspth

This comment has been minimized.

Copy link
Member

kaspth commented Jan 12, 2019

I just saw the other comment and I have added this PR to my list! If all goes well, I'll get to this before beta1 🙏

@kaspth kaspth merged commit 907b528 into rails:master Jan 13, 2019

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kaspth

This comment has been minimized.

Copy link
Member

kaspth commented Jan 13, 2019

There we go, thanks @palkan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment