Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support env named parameter to import SERVER_PORT header. #954

Closed
zw963 opened this issue Aug 9, 2016 · 11 comments
Closed

Support env named parameter to import SERVER_PORT header. #954

zw963 opened this issue Aug 9, 2016 · 11 comments

Comments

@zw963
Copy link

zw963 commented Aug 9, 2016

Rails 5 could not get @request object in ActionDispatch::IntegrationTest,
But it support a env name args to support import SERVER_PORT header.

get api_v1_test_hello_url,
      params: { name: 'new_name' },
      env: { 'SERVER_PORT' => '8629' }

How to do this in should-matcher ?

Thanks.

@zw963
Copy link
Author

zw963 commented Aug 9, 2016

I have test like this, it told me no should method exist.

class RoutesTest < ActionController::TestCase
  include Shoulda::Matchers::ActionController
  setup { request.env['SERVER_PORT'] = '8629' }
  test 'test case' do
    should route(:get, 'v1/test/hello').to('api/v1/test#hello')
  end
end

If change to this:

class RoutesTest < ActionController::TestCase
  include Shoulda::Matchers::ActionController
  setup { request.env['SERVER_PORT'] = '8629' }
  should route(:get, 'v1/test/hello').to('api/v1/test#hello')
end

should exist, but request.env is not valid.

@mcmire
Copy link
Collaborator

mcmire commented Aug 9, 2016

It looks like the env argument that get, post, etc. takes refers to the Rack environment hash. Setting SERVER_PORT doesn't actually change the Rack env, however -- it gets used to build the URL.

That said -- route doesn't actually hit a URL, it only checks to make sure that a combination of request method and corresponds to a controller/action pair. So I don't see why you need to set SERVER_PORT here.

@zw963
Copy link
Author

zw963 commented Aug 9, 2016

because i need test route which use constraint to limit only special port can be access。

@mcmire
Copy link
Collaborator

mcmire commented Aug 9, 2016

Oh, interesting. I didn't know you could do that.

Under the hood the route matcher actually uses Rails' assert_routing test helper. There doesn't seem to be a way to specify a port with that, either. One would think it would be possible to specify a full URL (assert_routing("http://foo.com:1234/my/path", ...)), but I'm not sure if that works or not.

If you can figure out a way to get assert_routing to do this then perhaps we can get route to do this, too.

@zw963
Copy link
Author

zw963 commented Aug 11, 2016

I just tried it. It worked!

class RoutesTest < ActionDispatch::IntegrationTest
  test 'route' do
   # asdfasdf can be replace to any char.
    assert_routing('http://asdfasdf.com:8629/v1/test/hello', controller: 'api/v1/test', action: 'hello')
  end
end

@zw963
Copy link
Author

zw963 commented Aug 11, 2016

And, following is worked too.

class RoutesTest < ActionController::TestCase
  should route(:get, 'http://example.com:8629/v1/test/hello').to('api/v1/test#hello')
end

Thanks.

@zw963 zw963 closed this as completed Aug 11, 2016
@zw963 zw963 reopened this Aug 11, 2016
@zw963
Copy link
Author

zw963 commented Aug 11, 2016

Maybe should close by you.

Rails 5 current exist headers: {...} and env: {...} named argument
which I don't know support by shoulda-matcher.

Anywhy, port problem is resolved.

@mcmire
Copy link
Collaborator

mcmire commented Aug 16, 2016

Glad you got it working!

I'm not sure what all of the possible values of env can be, so perhaps it's worth it to only support SERVER_PORT. Then again, the route matcher isn't intended to completely emulate get, post, etc., so perhaps we just add a port option, such as:

should route(:get, '/v1/test/hello', port: 8629).to('api/v1/test#hello')

We'll have to come to some kind of decision on this.

@zw963
Copy link
Author

zw963 commented Aug 20, 2016

Cool, maybe format and port is the most useful arguments for route test.

for me, I only occur those two case in recent project.

Thanks.

@mcmire
Copy link
Collaborator

mcmire commented Jan 26, 2018

Okay, this has been added like so:

it { should route(:get, '/my/path', port: 1234).to('controller#action')

@zw963
Copy link
Author

zw963 commented Jan 26, 2018

Cool, will try it later.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants