ActionCable channels unit-testing#27191

Closed

palkan wants to merge 1 commit intorails:masterfrom

palkan:ac-channels-testing

Contributor

palkan commented Nov 27, 2016

Extracted from #23211.

Example:

class ChatChannelTest < ActionCable::Channel::TestCase
  def test_subscribed_with_room_number
    # Simulate a subscription creation
    subscribe room_number: 1

    # Asserts that the subscription was successfully created
    assert subscription.confirmed?

    # Asserts that the channel subscribes connection to a stream
    assert "chat_1", streams.last
  end
  
  def test_subscribed_without_room_number
     subscribe

     # Asserts that the subscription was rejected
     assert subscription.rejected?
  end
  
  def test_perform_speak
    subscribe room_number: 1

    perform :speak, message: "Hello, Rails!"
    
    assert_equal "Hello, Rails!", transmissions.last["message"]["text"]
  end
end

rails-bot assigned chancancode

rails-bot commented Nov 27, 2016

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @chancancode (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

palkan changed the title ~~ActionCable channel unit-testing~~ ActionCable channels unit-testing

palkan force-pushed the ac-channels-testing branch from c4a2df1 to 16120f7 Compare

November 27, 2016 20:33

Contributor Author

palkan commented Nov 27, 2016

@kaspth
So, I've got rid of explicit identifier construction and removed assertions. I hope it looks much more clear now)

Contributor

kaspth commented Nov 27, 2016

Thanks, I'll take a look tomorrow 😄

maclover7 added the actioncable label

vipulnsward reviewed

View reviewed changes

actioncable/lib/action_cable/channel/test_case.rb Outdated

Member

vipulnsward Nov 28, 2016

definition.

actioncable/lib/action_cable/channel/test_case.rb Outdated

Member

vipulnsward Nov 28, 2016

channel

actioncable/lib/action_cable/channel/test_case.rb Outdated

Member

vipulnsward Nov 28, 2016

test_does_not_subscribe_without_room_number

kaspth reviewed

View reviewed changes

Contributor

kaspth left a comment

All right, this is about what I made it to tonight. I'll look more in the coming days.

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

Let's stick the broadcasts naming: transmissions -> broadcasts

Contributor Author

palkan Dec 4, 2016

There can be a confusion: transmissions list here contains messages transmitted to the socket directly, not through broadcasting.

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

Won't the logger need to support ActiveSupport::TaggedLogging?

Contributor

kaspth Nov 28, 2016

Or maybe we should reuse new_tagged_logger from the actual Connection.

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

Why can't we reuse this?

rails/actioncable/lib/action_cable/connection/subscriptions.rb

Lines 56 to 58 in 06d2049

    
           def identifiers 
        
             subscriptions.keys 
        
           end

Contributor Author

palkan Dec 4, 2016

These are different identifiers: here we accessing the method argument (within a channel instance).

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

Why prepend the underscore here?

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

Why do we need this setup? Isn't it more likely users will need to make their own connection_stub with the needed identifiers?

Contributor Author

palkan Nov 29, 2016

Agree, the case of connection w/o identifiers is very unlikely.

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

new line before

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

Is there precedent for adding this in the Behavior module? I'd rather we just force people to call self.channel_class = :some_channel_class and then move the lookup behavior there.

Contributor Author

palkan Nov 29, 2016

I've been using ActionMailer::TestCase and ActionController::TestCase as examples.
So, I've tried to preserve kind of consistency)

The same goes for #27191 (comment) and #27191 (comment) .

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

This isn't something people are likely to call on their own. So why don't we just let the nil through and let it be caught in tests?

actioncable/test/channel/test_case_test.rb Outdated

Contributor

kaspth Nov 28, 2016

@dhh should people have access to the streams here? Or is it better that they test what get's streamed?

actioncable/lib/action_cable/channel/test_case.rb Outdated

Contributor

kaspth Nov 28, 2016

This seems like it's overly reaching into how identifiers work. (And doing it improperly since it seems to use attr_accessor not singleton methods).

Contributor Author

palkan Dec 4, 2016

I don't think we should care here about how identifiers really work. We only want to know that identifier has a value and can be accessed by its name. This is exactly what a channel knows about the connection identifiers.

And this makes our ConnectionStub more abstract. It's just quacking like a connection.

palkan force-pushed the ac-channels-testing branch from 16120f7 to 9ff9f06 Compare

December 4, 2016 14:53

Contributor Author

palkan commented Dec 4, 2016

@vipulnsward @kaspth Thanks for the reviews! I've updated the PR.

Contributor Author

palkan commented Dec 20, 2016

@kaspth Ping! Anything I can do here to get it merged AFAP? And that one #23211 too, btw.

palkan mentioned this pull request

Decouple ActionCable application logic from low-level logic #27648

Closed

Undo1 commented Feb 14, 2017

It'd be awesome to have this implemented. Has there been any progress?

Contributor Author

palkan commented Feb 16, 2017

@Undo1 nothing from my side, waiting for the core team feedback( /cc @kaspth

palkan closed this

palkan reopened this

maclover7 added the needs feedback label


          Add ActionCable::Channel::TestCase

9e40836

palkan force-pushed the ac-channels-testing branch from 9ff9f06 to 9e40836 Compare

July 6, 2017 14:39

palkan mentioned this pull request

ActionCable testing #23211

Closed

palkan mentioned this pull request

Action cable testing #33659

Merged

6 tasks

Contributor Author

palkan commented Aug 19, 2018

Closed in favour of #33659

palkan closed this

palkan deleted the ac-channels-testing branch

August 19, 2018 23:36

palkan mentioned this pull request

Add ActionCable::Channel::TestCase #33969

Merged

palkan added a commit to palkan/rails that referenced this pull request


          Add ActionCable::Channel::TestCase

16e821a

ActionCable::Channel::TestCase provides an ability
to unit-test channel classes.

There are several reasons to write unit/functional cable tests:
- Access control (who has access to the channel? who can perform action and with which argument?
- Frontend-less applications have no system tests at all–and we still need a way to test channels logic.

See also rails#27191

jeremy pushed a commit that referenced this pull request


          Add ActionCable::Channel::TestCase

ActionCable::Channel::TestCase provides an ability
to unit-test channel classes.

There are several reasons to write unit/functional cable tests:
- Access control (who has access to the channel? who can perform action and with which argument?
- Frontend-less applications have no system tests at all–and we still need a way to test channels logic.

See also #27191

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

Labels

actioncable needs feedback