Skip to content

Add ActionCable::Connection::TestCase#34845

Merged
kaspth merged 1 commit intorails:masterfrom
palkan:feature/action-cable-connection-testing
Jan 13, 2019
Merged

Add ActionCable::Connection::TestCase#34845
kaspth merged 1 commit intorails:masterfrom
palkan:feature/action-cable-connection-testing

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.

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.

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

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?

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.

5 participants