search

westfieldlabs / apivore

Tests your rails API against its Swagger description of end-points, models, and query parameters.
Apache License 2.0
213 stars 66 forks source link

Update to support Rails 5 #96

Closed rdejuana closed 7 years ago

rdejuana commented 8 years ago

Tests in rails 5 currently receive this error

DEPRECATION WARNING: ActionDispatch::IntegrationTest HTTP request methods will accept only
the following keyword arguments in future Rails versions:
params, headers, env, xhr, as

Examples:

get '/profile',
  params: { id: 1 },
  headers: { 'X-Extra-Header' => '123' },
  env: { 'action_dispatch.custom' => 'custom' },
  xhr: true,
  as: :json
 (called from matches? at /home/vagrant/.rvm/gems/ruby-2.3.1/bundler/gems/apivore-275e799ea72c/lib/apivore/validator.rb:21)

Naming the params and headers arguments fixes this.

ashoda commented 7 years ago

@rdejuana thanks for submitting this PR. Once https://github.com/westfieldlabs/apivore/pull/99 is merged to support the actionpack dependency for both rails 4 and 5. We'll re-run the build and evaluate this PR.

As far as I can tell, this will introduce a breaking change for rails <5. We'll need to address this in a backward compatible manner. In any case, we'll keep you posted.

amcaplan commented 7 years ago

FWIW I'm upgrading to Rails 5, noticed https://github.com/thoughtbot/shoulda-matchers/pull/989/files#diff-f46c5b61347eb8f9749762252d06fb52R29 as a way to make the change in a backwards-compatible fashion.

ashoda commented 7 years ago

The shim approach that thoughtbot uses seems pretty sensible. @rdejuana @amcaplan any interest in updating/submitting a PR to address this?

amcaplan commented 7 years ago

I'd be very interested! My specs currently have a wall of deprecations, and I'd be delighted to submit a PR and get this fixed. I'll try to work this out over the next few days.

Also, any thoughts on testing? I suspect it's unnecessary because different Travis builds use different versions of Ruby and, hence, different ActionPack versions, so if all the Travis builds pass, we know it works with both Rails 4 and 5. Is that a fair assessment? (Obviously pending seeing the actual work!)

ashoda commented 7 years ago

Awesome! Thanks @amcaplan. Look forward to reviewing it.

As far as tests, I think it's worth adding unit tests for the shim implementations. The integration should be covered in some of the specs we have.

ashoda commented 7 years ago

closing this in favor of https://github.com/westfieldlabs/apivore/pull/102