-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
Changes from 1 commit
ce559fb
008b185
a39cdfc
2d8c3fb
ab2e384
b351432
887980d
7d4bba9
5f47a25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
|
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would check for |
||
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| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you make this There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
This is the output with the custom failure message, much more useful in figuring out why it failed.
|
||
expect(client).to have_received(:message).with(channel: channel, text: exp).once | ||
end | ||
true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
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 |
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 |
There was a problem hiding this comment.
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 "Addedrespond_with_slack_messages
RSpec expectation".You need a period at the end of this for Danger to be happy ;)