Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

can't set default_url_options for integration tests #546

Closed
nickurban opened this Issue · 30 comments
@nickurban

When writing integration tests, it would be preferable if url generation automatically made use of default_url_options as specified in ApplicationController. That said, the next best thing would be for default_url_options to be settable in the test environment, but I can't find any way to do that. This makes it impossible (or at least extremely awkward) to run integration tests when using a scope with a parameter in it.

Here's a simplified use case:

  1. in routes.rb:

    scope ':path_prefix' do 
      resources :foos 
    end 
    
  2. in application_controller.rb

    def default_url_options 
      { :path_prefix => 'test' } 
    end 
    

    foos_path will now resolve to /test/foos when referenced within controllers and views.

  3. in test/integration/prefix_test.rb:

    require 'test_helper' 
    class PrefixTest < ActionDispatch::IntegrationTest 
      test "path prefix" do 
        foos_path 
      end 
    end 
    
  4. When running the test (rake test:integration), I get the following error:

    test_path_prefix(PrefixTest): 
    ActionController::RoutingError: No route matches {:controller=>"foos"} 
        /test/integration/prefix_test.rb:5:in `test_path_prefix' 
    

In my real case, path_prefix gets its value from params[:path_prefix] unless no prefix is specified. For example, something like this:

def default_url_options 
  { :path_prefix => params[:path_prefix] || 'test' } 
end 
@josevalim
Owner

I believe you have to define the default_url_options in your test case. Defining it in ActiveSupport::TestCase will probably be fine. The different is that you don't have access to params.

@josevalim josevalim closed this
@nickurban

Yes, but there needs to be a way to set it from params. Otherwise, applications that use scopes with parameters are simply untestable!

@nickurban

Furthermore, I just tried defining it in the test case, and it wasn't used.

@josevalim
Owner

Damn, I see what is happening. You would need to define the default_url_options in the integration session object:

https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/testing/integration.rb#L171

I am reopening this then.

@josevalim josevalim reopened this
@benzittlau

This is a big problem for me. Trying to use a scope for a locale and it works fine in the application but is completely crashing all my cucumber tests.

@nickurban

Yes, it's very annoying. There needs to be a way to conveniently set default_url_options from params.

@benzittlau

I've monkey patched this by adding the following into a cucumber support file:

#monkey patch to fix the fact that ActionDispatch::Integration::Session blows
#away the ApplicationController default_url_options which are necessary
#for the locale scope to work
module ActionDispatch
  module Integration
    class Session
      def default_url_options
        { :host => host, :protocol => https? ? "https" : "http" }.merge!(ApplicationController.new.default_url_options)
      end
    end
  end
end
@nickurban

Thanks for sharing your solution. Unfortunately that doesn't work for me, because I read my option from params, which won't be defined on a new ApplicationController.

@nickurban

I ended up hacking around this by storing my path param in a global variable in my cucumber steps and then monkey patching default_url_options to use that global. It's pretty kludgy, but at least I can run my test suite again.

@keeran

I've been hitting a hurdle trying to upgrade to 3.1 with Rspec, could it be related?

rspec/rspec-rails#395

I was about to put in a monkey patch as per @benzittlau but wanted to find the cause before blindly hacking the framework(s).

@hardipe

@nickurban, can you please give a more precise description of your workaround. I am not using cucumber, just the plain integration testing framework shipped with Rails.

Where do I need to assign the params, so that it's globally accessible, and how do I monkey patch default_url_options to use that?

@nickurban

I made a file in features/support/default_url_options.rb:

def default_url_options(options = {})
    # set site_prefix from global variable. The $site_prefix global is set during various login steps.
    options.merge( :host => host, :protocol => https? ? "https" : "http", :site_prefix => $site_prefix )
end

When my features make use of URL helpers, they find this method and use the prefix.

In my login steps, I have a "When I log into account..." which works entirely through the UI and a "Given I am logged in as
X..." which creates a user directly. Once the user has been created, both of them call a method which sets $site_prefix.

Like I said, it's not the most elegant solution, but after several days of not being able to run tests, it was good enough for me! It might be time for me to revisit this now and redo it properly :)

@bartocc

I've submitted issue #2031 related to default_url_options but with callbacks interaction rather than params.
You might want to have a look at it

@spastorino
Owner

Can you guys provide a patch with a test case or at least just a failing test case?. Thanks.

@hardipe

Here's a fresh rails project to demonstrate that

https://github.com/hardipe/Rails-Issue-546

rake test:functionals passes
rake test:integration fails
they are testing the same thing

@mmack

@nickurban, your monkey patch works. thx. I had the problem with a :locale option in the url.

@logical42

er.. i think the scope method in the router is messed up. i think that whatever name you are trying to pass into the scope you're specifying isn't getting passed.

Using your code example from above:

require 'test_helper'
class PrefixTest < ActionDispatch::IntegrationTest
test "path prefix" do
#foos_path
#instead you replace the line above with this:
foos_path[(:path_prefix)]

end
end

The above will pass.

But you should realize that any parameter sent in there will actually pass. If you go into the browser and try typing any string as your scope-prefix, like '/test/foos' or '/asdf/foos', then the router will route you along.

I suspect the underlying issue is with the router's scope method, since if the scope isn't passing whatever you're 'scoping' (i.e. 'test' or 'scopedpath'), the router can't reasonably be expected to generate a helper method that actually directs to your path.

@logical42

oh yeah and if you delete the method from the application controller, it has no effect on anything whatsoever.

@dyba

I uploaded a sample app to show what @logical42 mentioned when he said there's something wrong with the routing in Rails. See this link. Let me put the important parts here in brief:

# app/controllers/application_controller.rb
class ApplicationController < ActionController::Base
  protect_from_forgery
end
# config/routes.rb
RailsIssue546::Application.routes.draw do
  # The priority is based upon order of creation:
  # first created -> highest priority.

  scope ':path_prefix' do
    resources :foos
  end
end
# rest/integration/prefix_test.rb
require 'test_helper'

class PrefixTest < ActionDispatch::IntegrationTest
  test "path prefix" do
    get 'test/foos'
    assert_response :failure

    get 'asdf/foos'
    assert_response :failure
  end
end

The above two tests don't pass.

@logical42

Awesome! Thanks for doing that Dan!

@dyba dyba referenced this issue from a commit
@dyba dyba Failing test for issue #546 bdebbb7
@dyba

@spastorino, here's the failing test you had asked for. I hope I was able to capture the problem with the test setup I created. I'll welcome any feedback.

@ipoval

@josevalim, @spastorino
The ticket says that should be a way to "set default_url_options for integration tests".

This way looks like something what we are looking for:
app.default_url_options = { :path_prefix => 'ivan_poval' }

Taking into account that we have the routes set like this:
scope ':path_prefix' do
resources :foos
end

The example of usage looks like this:
class TestFlowTest < ActionDispatch::IntegrationTest
fixtures :all

def setup
  app.default_url_options = { :path_prefix => 'ivan_poval' }
end

test "the truth" do          
  puts foos_path
end

end

@josevalim, you mentioned that #default_url_options should be set through the Session object. I would argue that because the routes are global to the application and app.default_url_options = { :path_prefix => 'ivan_poval' } seems to do its job fine.

IMHO: the ticket should be closed without any further code changes, but there is a question about the purpose of these lines:
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/testing/integration.rb#L187
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/testing/integration.rb#L188
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/testing/integration.rb#L189
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/testing/integration.rb#L190

@hardipe

@ipoval
The Integration test should get the default_url_options from ApplicationController
For me, using the 'setup' method in an integration test to set the app.default_url_options errors, because the attribute doesn't exist.

@dyba

@hardpipe
I changed my sample app with the following changes. The error I'm getting has to do with Rails not being able to find the route 'test/foos'. Are you getting the same thing?

require 'test_helper'

class PrefixTest < ActionDispatch::IntegrationTest

  def setup
    app.default_url_options = { :path_prefix => 'test' }
  end

  test "path prefix" do    
    get 'test/foos'
    assert_response :success
    assert_generates "/test/foos", { :controller => "foos", :action => "index" }
  end
end
>_ rake test:recent
Loaded suite /Users/danieldyba/.rvm/gems/ruby-1.9.2-p180@Rails310RC4/gems/rake-0.9.2/lib/rake/rake_test_loader
Started

PrefixTest:
    ERROR path prefix (0.10s) 
          ActionController::RoutingError: No route matches {:controller=>"foos"}
          /Users/danieldyba/.rvm/gems/ruby-1.9.2-p180@Rails310RC4/gems/actionpack-3.1.0.rc4/lib/action_dispatch/routing/route_set.rb:464:in `raise_routing_error'


Finished in 0.100495 seconds.

1 tests, 1 assertions, 0 failures, 1 errors, 0 skips
rake aborted!
Command failed with status (1): [/Users/danieldyba/.rvm/rubies/ruby-1.9.2-p...]
  1 Issue #546: Failing test, no matching route {:controller => "foos"}

Tasks: TOP => test:recent
(See full trace by running task with --trace)
@hardipe

@dyba

No. I don't get that error. Note that @ipoval also mentioned declaring a route. This should remove the error you are getting.

However, I just used the approach of setting the app.default_url_options in the setup method of my integration test. It says default_url_options is not declared.

@thoefer thoefer referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@thoefer
  1. Url helpers in integration tests respect default url options from the 'last used controller' with the above commit:
class OptionsTest < ActionDispatch::IntegrationTest  
  test "default url options" do
    get "/store/index"
    assert_redirected_to root_url # now passes, as :apples => 5 is included
  end
end

This test is from hardipe´s mentioned sample project (https://github.com/hardipe/Rails-Issue-546/). Drawback is that is only works for the last used controller:

2.

class OptionsTest < ActionDispatch::IntegrationTest  
  test "default url options" do
    get "/store/index" 
    # This makes StoreController is our last used controller. Now, imagine there´s an Items- and PeopleController also....
    items_path 
    # should use defaults from ItemsController but uses those from the last used controller. 
    people_path 
    # should use defaults from PeopleController but also uses those from the last used controller. 
  end
end

As far as I can see the behaviour under 2. is the same as in the app´s controllers and functional tests. But I don´t think this is right at all as a call to items_path should include the defaults from the ItemsController and not the one from the calling controller (resp. test), right? Or not?

@hardipe

I don't really know which behaviour is better. The only thing I have used default_url_options for so far is for filling in the :locale in my routes. When I set the default_url_options in the ApplicationController, all the controllers that inherit from it automatically use the same locale everywhere , which is what I want. (doesn't matter if they get the defaults from the previous controller or the controller that's associated with the route helpers (items_path and people_path)).

Would be nice if anyone else could give their opinion on this though.

@thoefer

Can one of the core team members shortly express the desired behaviour?

@josevalim josevalim was assigned
@isaacsanders

Is this still an issue?

@pixeltrix pixeltrix closed this issue from a commit
@pixeltrix pixeltrix Refactor the handling of default_url_options in integration tests
This commit improves the handling of default_url_options in integration
tests by making behave closer to how a real application operates.

Specifically the following issues have been addressed:

* Options specified in routes.rb are used (fixes #546)
* Options specified in controllers are used
* Request parameters are recalled correctly
* Tests can override default_url_options directly
e306ca3
@pixeltrix pixeltrix closed this in e306ca3
@carlosantoniodasilva carlosantoniodasilva referenced this issue from a commit in carlosantoniodasilva/rails
@pixeltrix pixeltrix Refactor the handling of default_url_options in integration tests
This commit improves the handling of default_url_options in integration
tests by making behave closer to how a real application operates.

Specifically the following issues have been addressed:

* Options specified in routes.rb are used (fixes #546)
* Options specified in controllers are used
* Request parameters are recalled correctly
* Tests can override default_url_options directly
7336b33
@arunagw arunagw referenced this issue from a commit in arunagw/rails
@pixeltrix pixeltrix Refactor the handling of default_url_options in integration tests
This commit improves the handling of default_url_options in integration
tests by making behave closer to how a real application operates.

Specifically the following issues have been addressed:

* Options specified in routes.rb are used (fixes #546)
* Options specified in controllers are used
* Request parameters are recalled correctly
* Tests can override default_url_options directly
f3aaac4
@tamird tamird referenced this issue from a commit in square/rails
@pixeltrix pixeltrix Refactor the handling of default_url_options in integration tests
This commit improves the handling of default_url_options in integration
tests by making behave closer to how a real application operates.

Specifically the following issues have been addressed:

* Options specified in routes.rb are used (fixes #546)
* Options specified in controllers are used
* Request parameters are recalled correctly
* Tests can override default_url_options directly
a8e9bbb
@strzalek strzalek referenced this issue from a commit in square/rails
@pixeltrix pixeltrix Refactor the handling of default_url_options in integration tests
This commit improves the handling of default_url_options in integration
tests by making behave closer to how a real application operates.

Specifically the following issues have been addressed:

* Options specified in routes.rb are used (fixes #546)
* Options specified in controllers are used
* Request parameters are recalled correctly
* Tests can override default_url_options directly
48c26c7
@ttosch ttosch referenced this issue from a commit
@pixeltrix pixeltrix Refactor the handling of default_url_options in integration tests
This commit improves the handling of default_url_options in integration
tests by making behave closer to how a real application operates.

Specifically the following issues have been addressed:

* Options specified in routes.rb are used (fixes #546)
* Options specified in controllers are used
* Request parameters are recalled correctly
* Tests can override default_url_options directly
de8428e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.