search

Netflix-Skunkworks / Scumblr

Web framework that allows performing periodic syncs of data sources and performing analysis on the identified results
Apache License 2.0
2.64k stars 317 forks source link

Deleted call to #skip as it was removed in Rails 4 #174

Closed ajstiles closed 7 years ago

ajstiles commented 7 years ago

I was running tests locally without Sketchy configured. It looks like #skip was removed from AR in Rails 4, so I removed the method as Scumblr is currently pegged at 4.2.6.

https://github.com/rails/rails/commit/fc57003feb1af98f9b4470c362e05755ce10b618#diff-f44e5c73c8640105ff6fd71298a18302

sbehrens commented 7 years ago

hi @ajstiles,

We're using minitest (which supports skip) and that's what the tests inherit from. You can read about the method here:

https://apidock.com/ruby/MiniTest/Assertions/skip

Thanks, Scott

ajstiles commented 7 years ago

Hi @sbehrens,

I think the problem is that #skip is an instance method, but in results_test.rb, it's being called in the class scope, which is why I see this error when running tests without Sketchy configured.

test/models/results_test.rb:151:in `<class:ResultTest>': undefined method `skip' for ResultTest:Class (NoMethodError)
    from test/models/results_test.rb:3:in `<main>'

One option is to move the "if Rails.configuration.try(:sketchy_url).present?" lines inside their corresponding test cases so we can still call #skip. Want me to redo the PR with that approach?

Adam

sbehrens commented 7 years ago

That makes sense. Sure, feel free to submit a PR with that approach. Thanks!