Closed ben-axnick closed 9 years ago
@bentheax thanks for the pull requests!
@bentheax the tests fail after merging this request...
........F........................
Failures:
1) App Updating Quick API not found doesn't raise a generic NoMethodError
Failure/Error: expect {
ArgumentError:
`expect { }.not_to raise_error(SpecificErrorClass)` is not valid, use `expect { }.not_to raise_error` (with no args) instead
# ./spec/market_bot/android/app_spec.rb:191:in `block (4 levels) in <top (required)>'
I don't have time to dig into the problem at the moment. Do you have time to send me another pull request to resolve the issue?
@chadrem Sure. I know what the error is, I'm not sure why I didn't encounter it in my own test run :blush:
That's the test I originally wrote in TDD fashion, it's superseded by the "raises a ResponseError"
test.The fix here is just to remove it.
@chadrem The issue is due to a deprecation in rspec 3. Since I ran with bundle exec rspec
, I got the 2.8 version from Gemfile.lock
and it all ran fine.
Since the test adds no value, it can be removed. See: https://github.com/chadrem/market_bot/pull/35
@bentheax Thanks again for the pull requests and fix. I've just released v0.14.0 with your changes.
Exception structure styled under the advice found here: https://stackoverflow.com/questions/5200842/where-to-define-custom-error-types-in-ruby-and-or-rails
Currently, a request for an invalid package raises a
NoMethodError
on nil. This is pretty hard tocatch
without potentially catching a bunch of other things.This PR represents a small departure from the existing style in the spec-file. I'm happy to work to make it uniform with your standards if this is something you'd like for the gem.