bploetz / versionist

A plugin for versioning Rails based RESTful APIs.
MIT License
972 stars 51 forks source link

rspec route_to in routing specs #82

Closed chrishough closed 7 years ago

chrishough commented 7 years ago

I have been trying to integrate this SO post https://stackoverflow.com/questions/11436924/rspec-add-some-header-requests-inside-routing-specs/16467507#16467507 but no matter what I try this does not work. Do you have any ideas?

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe 'Status Routes', type: :routing do
  context 'v1' do
    it '/status-api' do
      expect(get('/status-api')).to route_to('v1/monitors#status')
    end
  end

  context 'v2' do
    before do
      ActionDispatch::TestRequest::DEFAULT_ENV.dup
      ActionDispatch::TestRequest::DEFAULT_ENV['X-API-VERSION'] = 2
    end

    it '/status-api' do
      expect(get('/status-api')).to route_to('v2/monitors#status')
    end

    after do
      ActionDispatch::TestRequest::DEFAULT_ENV.delete('X-API-VERSION')
    end
  end
end

  api_version(module: 'v1', header: { name: 'X-API-VERSION', value: '1' }, default: true) do
    get '/status-api', to: 'monitors#status'
  end

  api_version(module: 'v2', header: { name: 'X-API-VERSION', value: '2' }) do
    get '/status-api', to: 'monitors#status'
  end
bploetz commented 7 years ago

@chrishough as you'll note in that example on SO, headers within Rack get translated internally. They get uppercased, underscored, and prefixed with HTTP. So for example in the versionist specs, we test custom headers like so:

https://github.com/bploetz/versionist/blob/master/spec/api_routing_spec.rb#L316 https://github.com/bploetz/versionist/blob/master/spec/api_routing_spec.rb#L338

So try changing your ActionDispatch::TestRequest::DEFAULT_ENV['X-API-VERSION'] to ActionDispatch::TestRequest::DEFAULT_ENV['HTTP_X_API_VERSION'] and see if that does the trick?

chrishough commented 7 years ago

1st, thank you so much for your help on this. I truly appreciate it.

If I try with @headers["HTTP_API_VERSION"] I get...

Failures:

  1) Status Routes v2 /status-api
     Failure/Error: @headers["HTTP_API_VERSION"] = 2

     NoMethodError:
       undefined method `[]=' for nil:NilClass
     # ./spec/routing/status_routes_spec.rb:19:in `block (3 levels) in <top (required)>'

Top 2 slowest examples (0.04982 seconds, 20.2% of total time):
  Status Routes v1 /status-api
    0.03431 seconds ./spec/routing/status_routes_spec.rb:7
  Status Routes v2 /status-api
    0.01551 seconds ./spec/routing/status_routes_spec.rb:18

Finished in 0.24634 seconds (files took 17.69 seconds to load)
2 examples, 1 failure

If I try with ActionDispatch::TestRequest::DEFAULT_ENV['HTTP_X_API_VERSION'] I get...

Failures:

  1) Status Routes v2 /status-api
     Failure/Error: expect(get('/status-api')).to route_to('v2/monitors#status')

       The recognized options <{"controller"=>"v1/monitors", "action"=>"status"}> did not match <{"controller"=>"v2/monitors", "action"=>"status"}>, difference:.
       --- expected
       +++ actual
       @@ -1 +1 @@
       -{"controller"=>"v2/monitors", "action"=>"status"}
       +{"controller"=>"v1/monitors", "action"=>"status"}
     # ./spec/routing/status_routes_spec.rb:19:in `block (3 levels) in <top (required)>'

Thoughts? Could the spec type be a part of this as well?

bploetz commented 7 years ago

@chrishough sorry, didn't mean to confuse the situation by pointing you at the @headers thing (that's a request spec thing I believe), I only wanted to point out the API-VERSION -> HTTP_API_VERSION translation there.

Anyhoo, with the ActionDispatch::TestRequest::DEFAULT_ENV change, it looks like it's returning your default API version, which is why your spec is failing, so clearly that's not working either. Honestly I've never tried to use HTTP headers routing specs before, so I'm not sure how to do this. Some quick googling seems to indicate this may not even be possible?

Worst case, maybe just move your routing specs to request specs where you CAN specify headers?

chrishough commented 7 years ago

not a problem @bploetz, I was wondering if there was a way to make this work. More of an experiment before changing these specific specs to request specs, or skipping the route specs all together in this exercise. thoughts?

bploetz commented 7 years ago

I mean, given the power of Ruby there's GOTTA be a way to make this work in routing specs, but I'd have to spend some time playing with it to find the secret sauce.

Regardless, this doesn't sound like a problem with versionist per se, so I'm going to close this issue, but feel free to keep updating this issue if/when you figure out how to get it working so others may benefit in the future.

chrishough commented 7 years ago

For others coming across this, I moved the specs to request specs and just tested the status codes.

Example...

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe 'Monitors', type: :request do
  before { get '/status-api', headers: { 'X-API-VERSION': '2' } }

  it 'renders status' do
    expect(response.body).to include(JSON.generate(version: '2', status: 'online'))
  end

  it 'schema' do
    expect(response).to match_response_schema('app_monitors_status')
  end

  it 'renders 200 status code' do
    expect(response.status).to be(200)
  end
end