Conversation
ajm188
left a comment
ajm188
left a comment
There was a problem hiding this comment.
Tests can still be improved a bit. Other than that just minor nit
spec/yelp/error_spec.rb
Outdated
|
|
||
| it 'should expose the field parameter' do | ||
| begin | ||
| expect{ |
spec/yelp/error_spec.rb
Outdated
| # verifies that we can get access to the specific field that was invalid | ||
| expect(e.field).to eq('oauth_token') | ||
| end | ||
| }.to raise_error { |error| |
There was a problem hiding this comment.
expect { ... }.to raise_error(Yelp::Error::InvalidParameter, 'One or more ...')
spec/yelp/error_spec.rb
Outdated
| expect { | ||
| Yelp::Error.check_for_error(bad_response) | ||
| }.to raise_error { |error| | ||
| error.message.should eq('One or more parameters are invalid in request: limit. Description: Limit maximum is 20') |
|
@ajm188 statistic preferences are hard to guess (block vs. non-block) :/ The non-block style will not allow the validation of Otherwise, it would make sense to have ` Right? |
* Prefer param vs block * Remove assertion for field
I totally feel you on this, but I still want that space in there! :)
We're still implicitly testing it, because the field name appears in the error message. But I will defer to @tomelm and @mittonk here, as they have more historical context than I do. |
| error.message.should eq('One or more parameters are invalid in request: oauth_token') | ||
| error.field.should eq('oauth_token') | ||
| } | ||
| }.to raise_error ('One or more parameters are invalid in request: oauth_token') |
There was a problem hiding this comment.
Still missing: expect { }.to raise_error(ErrorClass, error_message)
2 participants
Including description of the error in the exception message. This is only included if provided back form the API.
I also standardized the error_spec to have all examples use raise_error.