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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Action Cable testing #2113

Merged
merged 3 commits into from May 15, 2019

Conversation

Projects
None yet
4 participants
@palkan
Copy link

commented Apr 26, 2019

This PR migrates action-cable-testing gem RSpec functionality into rspec-rails (since core functionality has been merged into Rails 6).

You can check the existing gem documentation (RSpec-like) to see what's coming: https://relishapp.com/palkan/action-cable-testing/docs.

What's inside:

Closes #1606

@palkan palkan marked this pull request as ready for review Apr 26, 2019

@palkan palkan changed the title Add Action Cable testing Add Action Cable testing [WIP] Apr 26, 2019

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch 3 times, most recently from 5a943a0 to f645d3a Apr 26, 2019

@palkan palkan changed the title Add Action Cable testing [WIP] Add Action Cable testing Apr 28, 2019

@palkan

This comment has been minimized.

Copy link
Author

commented Apr 28, 2019

@benoittgt @samphippen Hey folks, what should I do to make Travis show me the failures?)

@benoittgt

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

Hello @palkan

Sorry busy weekend. Maybe you can comment those lines?
https://github.com/rspec/rspec-rails/blob/master/.travis.yml#L210-L213

I will have a look on your PR tomorrow. Thanks for the hard work.

@samphippen

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

@palkan please rebase this on top of the 4-0-dev branch, as this'll have to make it in to 4-0 at this point.

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch from f645d3a to 5814ecf Apr 28, 2019

@palkan

This comment has been minimized.

Copy link
Author

commented Apr 28, 2019

@samphippen

please rebase this on top of the 4-0-dev branch

Done!

@benoittgt
Copy link
Member

left a comment

Very good job. I have few questions related to documentation but also where matcher are defined.

begin
require 'action_cable'
generate('channel chat') if ::Rails::VERSION::STRING.to_f >= 6.0
rescue LoadError

This comment has been minimized.

Copy link
@JonRowe

JonRowe Apr 29, 2019

Member

When would this generate a load error when on Rails 6? Seems like the two gates could be combined?

Show resolved Hide resolved features/channel_specs/channel_spec.feature
Show resolved Hide resolved features/matchers/have_broadcasted_matcher.feature
Show resolved Hide resolved lib/rspec/rails/example/channel_example_group.rb Outdated

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch 8 times, most recently from d6e3034 to 5ac92c3 Apr 30, 2019

@palkan

This comment has been minimized.

Copy link
Author

commented May 1, 2019

@benoittgt @JonRowe thanks for the feedback! All fixed.

@samphippen The build seems green: https://travis-ci.org/rspec/rspec-rails/builds/526676636 (Rails 4.2 specs are failing for non-related to this PR reasons, looks it's no longer supported?).
Also, to make the build pass the RuboCop check I had to freeze the version (~> 0.67.0), the new one breaks it (yeah, as always). That should be fixed in the base branch, I think.

@benoittgt benoittgt force-pushed the rspec:4-0-dev branch from 39cc602 to cbe8e9d May 1, 2019

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch from 5ac92c3 to 640659a May 1, 2019

@JonRowe

JonRowe approved these changes May 3, 2019

@JonRowe

This comment has been minimized.

Copy link
Member

commented May 3, 2019

@samphippen can you give me a final review for this, you're more familiar with action cable!

@samphippen
Copy link
Member

left a comment

This is overall a high quality PR, I had a few nits, but if you can address them, I'll merge this down!

end
end

if ::Rails::VERSION::MAJOR > 5

This comment has been minimized.

Copy link
@samphippen

samphippen May 12, 2019

Member

WDYT about >= 6?

# expect {
# ActionCable.server.broadcast "messages", text: 'Hi!'
# }.to have_broadcasted_to("messages").with(text: 'Hi!')

This comment has been minimized.

Copy link
@samphippen

samphippen May 12, 2019

Member

you need to kill this blankline.

@samphippen

This comment has been minimized.

Copy link
Member

commented May 12, 2019

@palkan BTW you should feel free to DM me on twitter if you need my attention, it is by far and away my fastest response time channel.

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch from 640659a to 44e590a May 13, 2019

@samphippen

This comment has been minimized.

Copy link
Member

commented May 14, 2019

LGTM, will merge on green!

@palkan

This comment has been minimized.

Copy link
Author

commented May 14, 2019

Looks like we're blocked by RuboCop again (0.69 release notes)

image

@benoittgt @samphippen Is anyone going to fix this in 4-0-dev?

@benoittgt

This comment has been minimized.

Copy link
Member

commented May 14, 2019

@palkan We merged #2126 this afternoon sorry. It should be good. 馃槉

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch from 44e590a to aaa97fb May 14, 2019

palkan added some commits Apr 26, 2019

Add have_broadcasted_to matcher
action-cable-testing migration, pt.1

@palkan palkan force-pushed the palkan:feat/action-cable-testing branch from aaa97fb to 024cae9 May 14, 2019

@palkan

This comment has been minimized.

Copy link
Author

commented May 14, 2019

@benoittgt My bad, I should have checked(

Anyway, we're green 馃帀

@benoittgt benoittgt merged commit ac64a6b into rspec:4-0-dev May 15, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@benoittgt

This comment has been minimized.

Copy link
Member

commented May 15, 2019

Thanks a lot @palkan. Awesome job! 馃憦

@palkan palkan referenced this pull request May 15, 2019

Merged

Handle RSpec 4 #54

@palkan palkan deleted the palkan:feat/action-cable-testing branch May 15, 2019

benoittgt added a commit that referenced this pull request May 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.