From a351149e805910cd980bee1558e56e61c4a82db2 Mon Sep 17 00:00:00 2001 From: Tony Wooster Date: Tue, 18 Feb 2014 14:12:11 -0800 Subject: Fix controller test not resetting @_url_options Commit 4f2cd3e9 introduced a bug by reordering the call to `@controller.recycle!` above the call to `build_request_uri`. The impact of this was that the `@_url_options` cache ends up not being reset between building a request URI (occurring within the test controller) and the firing of the actual request. We encountered this bug because we had the following setup: class MinimumReproducibleController < ActionController::Base before_filter { @param = 'param' } def index render text: url_for(params) end def default_url_options { custom_opt: @param } end end def test_index get :index # builds url, then fires actual request end The first step in `get :index` in the test suite would populate the @_url_options cache. The subsequent call to `url_for` inside of the controller action would then utilize the uncleared cache, thus never calling the now-updated default_url_options. This commit fixes this bug calling recycle! twice, and removes a call to set response_body, which should no longer be needed since we're recycling the request object explicitly. --- actionpack/CHANGELOG.md | 5 +++++ actionpack/lib/action_controller/metal/testing.rb | 1 - actionpack/lib/action_controller/test_case.rb | 1 + actionpack/test/controller/test_case_test.rb | 23 +++++++++++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 68b5213bfc..66cef08b1b 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1 +1,6 @@ +* Fix URL generation in controller tests with request-dependent + `default_url_options` methods. + + *Tony Wooster* + Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/lib/action_controller/metal/testing.rb b/actionpack/lib/action_controller/metal/testing.rb index 0377b8c4cf..dd8da4b5dc 100644 --- a/actionpack/lib/action_controller/metal/testing.rb +++ b/actionpack/lib/action_controller/metal/testing.rb @@ -17,7 +17,6 @@ module ActionController def recycle! @_url_options = nil - self.response_body = nil self.formats = nil self.params = nil end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index cf11ce1a9b..8650b75400 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -568,6 +568,7 @@ module ActionController name = @request.parameters[:action] + @controller.recycle! @controller.process(name) if cookies = @request.env['action_dispatch.cookies'] diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 5ff4a383ec..fbc10baf21 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -163,6 +163,29 @@ XML end end + class DefaultUrlOptionsCachingController < ActionController::Base + before_filter { @dynamic_opt = 'opt' } + + def test_url_options_reset + render text: url_for(params) + end + + def default_url_options + if defined?(@dynamic_opt) + super.merge dynamic_opt: @dynamic_opt + else + super + end + end + end + + def test_url_options_reset + @controller = DefaultUrlOptionsCachingController.new + get :test_url_options_reset + assert_nil @request.params['dynamic_opt'] + assert_match(/dynamic_opt=opt/, @response.body) + end + def test_raw_post_handling params = Hash[:page, {:name => 'page name'}, 'some key', 123] post :render_raw_post, params.dup -- cgit v1.2.3