Skip to content
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

palkan
Copy link
Contributor

@palkan 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.

Copy link
Contributor

@sponomarev sponomarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish that could be included to 5.2 branch.

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

@sponomarev sponomarev Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: '' } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@palkan palkan force-pushed the feature/action-cable-connection-testing branch from 641d1c8 to 2111bc2 Compare January 2, 2019 22:49
@palkan palkan force-pushed the feature/action-cable-connection-testing branch from 2111bc2 to 9029667 Compare January 3, 2019 00:47
Copy link
Contributor

@sponomarev sponomarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️❤️❤️

@rmacklin
Copy link
Contributor

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

@palkan
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
Copy link
Contributor

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
@kaspth
Copy link
Contributor

kaspth commented Jan 13, 2019

There we go, thanks @palkan!

@palkan palkan mentioned this pull request Apr 6, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants