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

Allow testing of commands that return multiple messages with respond_with_slack_message #157

Merged
merged 9 commits into from
Sep 18, 2017
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### 0.10.5 (Next)

* Refactored `SlackRubyBot::MVC::Controller::Base`, consolidated ivar handling, centralized object allocations and DRYed up the code - [@chuckremes](https://github.com/chuckremes).
* [#157](https://github.com/slack-ruby/slack-ruby-bot/pull/157): Added support for checking multiple responses at once via new `respond_with_slack_messages` matcher, and single response out of multiple responses to `response_with_slack_message` matcher - [@gcraig99](https://github.com/gcraig99)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already had respond_with_slack_message (typo btw), so I would just say "Added respond_with_slack_messages RSpec expectation".

You need a period at the end of this for Danger to be happy ;)

* Your contribution here.

### 0.10.4 (07/05/2017)
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ Slack-ruby-bot ships with a number of shared RSpec behaviors that can be used in

* [behaves like a slack bot](lib/slack-ruby-bot/rspec/support/slack-ruby-bot/it_behaves_like_a_slack_bot.rb): A bot quacks like a Slack Ruby bot.
* [respond with slack message](lib/slack-ruby-bot/rspec/support/slack-ruby-bot/respond_with_slack_message.rb): The bot responds with a message.
* [respond with slack messages](lib/slack-ruby-bot/rspec/support/slack-ruby-bot/respond_with_slack_messages.rb): The bot responds with a set of message.
Copy link
Collaborator

Choose a reason for hiding this comment

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

set of messageS or just "multiple messages"

* [respond with error](lib/slack-ruby-bot/rspec/support/slack-ruby-bot/respond_with_error.rb): An exception is raised inside a bot command.

Require `slack-ruby-bot/rspec` in your `spec_helper.rb` along with the following dependencies in Gemfile.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@

allow(Giphy).to receive(:random) if defined?(Giphy)

expect(client).to receive(:message).with(channel: channel, text: expected)
allow(client).to receive(:message)
message_command.call(client, Hashie::Mash.new(text: message, channel: channel, user: user))
expect(client).to have_received(:message).with(channel: channel, text: expected).once
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this changes behavior, ie. it allows the client to receive multiple messages that aren't the one expected plus the one expected. I think we don't want that, we want expectations to be "precise".

true
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
require 'rspec/expectations'
RSpec::Matchers.define :respond_with_slack_messages do |expected|
match do |actual|
raise ArgumentError, 'respond_with_slack_messages expects an array of ordered responses' unless expected.is_a? Array
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would check for Enumerable instead or maybe just that it responds to each? This way I can get the messages from some collection, database, whatever, if needed.

client = if respond_to?(:client)
send(:client)
else
SlackRubyBot::Client.new
end

message_command = SlackRubyBot::Hooks::Message.new
channel, user, message = parse(actual)

allow(Giphy).to receive(:random) if defined?(Giphy)

allow(client).to receive(:message)
message_command.call(client, Hashie::Mash.new(text: message, channel: channel, user: user))
expected.each do |exp|
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you make this expect(...) above you don't need to do this whole saving of @messages and failure_message, no? This seems unnecessarily complicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The expect statements are still there, the issue is that when using the expect statement without the custom failure message and the storage of the received messages, you don't get any useful output.

This is the output without the custom failure message:

  1. RSpec it_fails_on_purpose_to_show_output
    Failure/Error:
    expect(message: "#{SlackRubyBot.config.user} single message")
    .to respond_with_slack_message(/failure message/i)

    expected {:message=>"rubybot single message"} to respond with slack message /failure message/i

  2. RSpec respond_with_failure_to_show_output
    Failure/Error:
    expect(message: "#{SlackRubyBot.config.user} respond 5 times")
    .to respond_with_slack_messages(expected_responses)

    expected {:message=>"rubybot respond 5 times"} to respond with slack messages /response 0/i, /response 1/i, /response 2/i, /response 3/i, /response 4/i, and /response 7/i

This is the output with the custom failure message, much more useful in figuring out why it failed.

  1. RSpec it_fails_on_purpose_to_show_output
    Failure/Error:
    expect(message: "#{SlackRubyBot.config.user} single message")
    .to respond_with_slack_message(/failure message/i)

    `expected to receive message with text: (?i-mx:failure message) once,
     received:[{:text=>"single response", :channel=>"channel"}]`
    
  2. RSpec respond_with_failure_to_show_output
    Failure/Error:
    expect(message: "#{SlackRubyBot.config.user} respond 5 times")
    .to respond_with_slack_messages(expected_responses)

    expected to receive messages in order with text: [/response 0/i, /response 1/i, /response 2/i, /response 3/i, /response 4/i, /response 7/i] received:[{:text=>"response 0", :channel=>"channel"}, {:text=>"response 1", :channel=>"channel"}, {:text=>"response 2", :channel=>"channel"}, {:text=>"response 3", :channel=>"channel"}, {:text=>"response 4", :channel=>"channel"}]

expect(client).to have_received(:message).with(channel: channel, text: exp).once
end
true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, I think we want exact expectations, making sure only the messages expected have been received. This behavior should have tests in both cases.

end

private

def parse(actual)
actual = { message: actual } unless actual.is_a?(Hash)
[actual[:channel] || 'channel', actual[:user] || 'user', actual[:message]]
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's some duplication here with the other expectation. Maybe put the common code in a module and include it?

end
30 changes: 30 additions & 0 deletions spec/slack-ruby-bot/rspec/respond_with_slack_message_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
describe RSpec do
let! :command do
Class.new(SlackRubyBot::Commands::Base) do
command 'single message' do |client, data, _match|
client.say(text: 'single response', channel: data.channel)
end
command 'respond 5 times' do |client, data, _match|
5.times do |i|
client.say(text: "response #{i}", channel: data.channel)
end
end
end
end

def app
SlackRubyBot::App.new
end
it 'respond_with_single_message_using_regex_match' do
expect(message: "#{SlackRubyBot.config.user} single message")
.to respond_with_slack_message(/single response/i)
end
it 'respond_with_single_message_using_string_match' do
expect(message: "#{SlackRubyBot.config.user} single message")
.to respond_with_slack_message('single response')
end
it 'respond_with_multiple_messages_looking_for_single_match' do
expect(message: "#{SlackRubyBot.config.user} respond 5 times")
.to respond_with_slack_message('response 1')
end
end
28 changes: 28 additions & 0 deletions spec/slack-ruby-bot/rspec/respond_with_slack_messages_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
describe RSpec do
let! :command do
Class.new(SlackRubyBot::Commands::Base) do
command 'respond 5 times' do |client, data, _match|
5.times do |i|
client.say(text: "response #{i}", channel: data.channel)
end
end
end
end

def app
SlackRubyBot::App.new
end

it 'respond_with_multiple_messages_using_regex_matches' do
expected_responses = []
5.times { |i| expected_responses.push(/response #{i}/i) }
expect(message: "#{SlackRubyBot.config.user} respond 5 times")
.to respond_with_slack_messages(expected_responses)
end
it 'respond_with_multiple_messages_using_string_matches' do
expected_responses = []
5.times { |i| expected_responses.push("response #{i}") }
expect(message: "#{SlackRubyBot.config.user} respond 5 times")
.to respond_with_slack_messages(expected_responses)
end
end