palkan / action-cable-testing

Action Cable testing utils
MIT License
213 stars 17 forks source link

have_broadcasted_to does not seem to work in Rails 5.2 #86

Closed Physium closed 3 years ago

Physium commented 3 years ago

Currently I am on RSpec 4 with Rails 5.2. In order to get action cable test to work, I used the monkey patch as referenced from https://github.com/palkan/action-cable-testing/issues/76.

Channel testing works fine, however I cant seem to get have_broadcasted_to to work successfully. I have checked the logs and its shows that ActionCable is broadcasting to its respective channel with its respective message but the test sees that the broadcast is still 0.

Any idea on this or is this gem simply not compatible with my current setup?

palkan commented 3 years ago

Could you please provide an example of the failing test as long as the exact versions of rspec-rails, rails and action-cable-testing gems?

Physium commented 3 years ago

Versions:

rails (5.2.4.3)
rspec-rails (4.0.0)
action-cable-testing (0.6.1)
      actioncable (>= 5.0)

Controller:

def sample
  room = Room.find_by(name: @room_name)
  RoomChannel.broadcast_to(room, message: 'hello world')

  render json: { hello: 'world' }, status: :ok
end

Spec File:

# assume that I have my test variables setup done
subject { post :sample, params: params }
context 'broadcast test' do
    it do
      expect { subject }.to have_broadcasted_to(room).from_channel(RoomChannel)
    end
 end

Results:

RoomController#sample test is expected to have broadcasted to
     Failure/Error: expect { subject }.to have_broadcasted_to(room).from_channel(RoomChannel)
       expected to broadcast exactly 1 messages to Z2lkOi8vd2ViYXBwL0dhbnRyeVF1ZXVlLzQ1Nw, but broadcast 0

Most of the specs code were reference from the readme.

palkan commented 3 years ago

Could you please do the following modification and show the output:

it do
   expect { subject; puts ActionCable.server.pubsub.send(:channels_data) }.to have_broadcasted_to(room).from_channel(RoomChannel)
end
Physium commented 3 years ago

This is what i get:

{"Z2lkOi8vd2ViYXBwL0dhbnRyeVF1ZXVlLzQ4OA"=>[], "room:Z2lkOi8vd2ViYXBwL0dhbnRyeVF1ZXVlLzQ4OA"=>["{\"message\":\"hello world\",\"timestamp\":1616677886}"]}

I notice that if i do have_broadcasted_to(model), it does not seem to be on the right broadcast channel. As shown in the output it only shows the ref id without the model name prepended. Is this intended? I guessing thats why its not recognising the broadcast.

palkan commented 3 years ago

Yep, without from_channel it doesn't use a channel prefix (that explains the empty array).

Let's do one more experiment: check the value of room.to_gid_param in both the controller and the test; are they equal?

Physium commented 3 years ago

But... i'm using from_channel as shown in the code which is suppose to provide the channel prefix am i right?

Yes both gid are the same after doing a binding.pry to check.

is there a way i can fix this?

palkan commented 3 years ago

Ok, I think, I know the reason.

Could you spot the difference between https://github.com/palkan/action-cable-testing/blob/master/lib/rspec/rails/matchers/action_cable/have_broadcasted_to.rb and https://github.com/rspec/rspec-rails/blob/v4.0.0/lib/rspec/rails/matchers/action_cable/have_broadcasted_to.rb ?

RSpec version of the matcher doesn't support Rails <6: #broadcasting_for method has been changed in 6.0, it's not backward compatible. Thus, the stream generated in the matcher is not equal to the actual stream name.

In the gem, we use a refinement to backport this behavior and be compatible with both Rails 5 and Rails 6.

Thus, I'd suggest either:

# have_broadcasted_to_patch.rb

require "action_cable/testing/rails_six"
using ActionCable::Testing::Rails6

class RSpec::Rails::Matchers::ActionCable::HaveBroadcastedTo
  def stream
    @stream ||= if @target.is_a?(String)
      @target
    else
      check_channel_presence
      @channel.broadcasting_for(@target)
    end
  end
end 

And load it after rspec/rails in your rspec_helper.rb.

Physium commented 3 years ago

Thanks for the super prompt clarification! This made alot of sense now. I also notice from the relish documentation here to do something like this which actually works.

require "rails_helper"

# Activate Rails 6 compatible API (for broadcasting_for)
using ActionCable::Testing::Rails6

RSpec.describe Broadcaster do
  it "matches with stream name" do
    user = User.new(42)
    skip unless ActionCable::Testing::Rails6::SUPPORTED
    expect {
      ChatChannel.broadcast_to(user, text: 'Hi')
    }.to broadcast_to(ChatChannel.broadcasting_for(user))
  end
end

I decided to go with your patch instead as alot cleaner then the one recommended in relish and its also impossible for me to downgrade rspec now. Thank you so much for helping. I think this is worthy to be in the readme for legacy rails users like me. :D