Fixed expectation failures when client.say is passed extra arguments #160
Conversation
…such as as_user: true Closes slack-ruby#159
…r rtm.connect since either method could be chosen
| @@ -28,3 +28,31 @@ http_interactions: | |||
| string: '{"ok":false,"error":"unknown"}' | |||
| http_version: | |||
| recorded_at: Tue, 28 Apr 2015 12:55:22 GMT | |||
| - request: | |||
There was a problem hiding this comment.
You have a spec that's making 3 requests somehow, this shouldn't be here, especially that the last one is not responding with migration_in_progress.
There was a problem hiding this comment.
This isn't my spec at all. None of my code changes deal with this part of the code whatsoever. This spec was failing on my last PR #157 because the slack-ruby-client was selecting rtm.connect where it used to call rtm.start and there were no fixtures for that method. I updated the fixture file in the last PR, but the test failed again on this PR, but only on the build server. Worked on my machine (I know), so I surmised that the client had tried to use rtm.start on the build machine, as it has the ability to choose either now. So to handle that I simply duplicated the fixture responses to handle either rtm.start or rtm.connect.
If that's not the approach you want to take, I'm open to suggestions.
There was a problem hiding this comment.
That's fine, but the last request that doesn't return migration in progress shouldn't be there then. Which spec fails with that?
|
This is great, see my comment re yml fix and it can be merged. As a followup item, maybe we should add support for taking a hash in |
| string: '{"ok":false,"error":"migration_in_progress"}' | ||
| http_version: | ||
| recorded_at: Tue, 28 Apr 2015 12:55:22 GMT | ||
| - request: |
There was a problem hiding this comment.
This one should not be necessary.
There was a problem hiding this comment.
If the '{"ok":false,"error":"unknown"}' responses are removed, the migration_in_progress spec fails. It's part of the expectations:
1) SlackRubyBot::Server retries on rtm.start errors migration_in_progress
Failure/Error:
expect do
subject.run
end.to raise_error Slack::Web::Api::Error, 'unknown'
expected Slack::Web::Api::Errors::SlackError with "unknown" got #<VCR::Errors::UnhandledHTTPRequestError:
|
I was thinking that I'd add extra expectations that would allow for parameter matching, so you could have a spec that makes sure a command is using as_user: true, or whatever other parameters are supported. |
Fixed expectation failures when client.say is passed extra arguments such as as_user: true
Added test cases for this situation as well.
Closes #159