1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

ActionController::TestCase: reset instance variables after each request

`ActionController::TestCase` keeps a `@controller` instance variable, which represents the controller being tested. At the end of each request inside a test, its [params and format](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/metal/testing.rb) are reset. But any other instance variables set in the test aren't reset. This creates a problem if you do something like this:

```ruby
class UserController
  def show
    @user ||= User.find(params[:id])
    render plain: @user.name
  end
end
```

```ruby

test "gets the user" do
  get :show, params: { id: users(:one).id }
  assert "one", response.body

  get :show, params: { id: users(:two).id }
  assert "two", response.body
end
```

The second assertion will fail, because `@user` won't be re-assigned in the second test (due to `||=`). This example is a bit contrived, but it shows how instance variables persisting between requests can lead to surprising outcomes.

This PR fixes this by clearing all instance variables that were created on the controller while a request was processed.

It explicitly excludes instance variables that were created *before* any requests were run. And it leaves the instance variable around until the *next* request in the test. This means that you can still do this:

```ruby

test "gets the user" do
  @controller.user = users(:one) # assuming `Controller#user` users an ivar internally, you can set the ivar here...

  get :show_current
  assert "one", response.body

  assert_equal users(:one), @controller.user # and you can read the ivar here
end
```
This commit is contained in:
Alex Ghiculescu 2021-11-26 17:53:42 -06:00
parent bcc1c0e4af
commit 054fa96bac
4 changed files with 74 additions and 0 deletions

View file

@ -1,3 +1,8 @@
* Instance variables set in requests in a `ActionController::TestCase` are now cleared before the next request
This means if you make multiple requests in the same test, instance variables set in the first request will
not persist into the second one. (It's not recommended to make multiple requests in the same test.)
*Alex Ghiculescu*
Please check [7-0-stable](https://github.com/rails/rails/blob/7-0-stable/actionpack/CHANGELOG.md) for previous changes.

View file

@ -4,6 +4,15 @@ module ActionController
module Testing
# Behavior specific to functional tests
module Functional # :nodoc:
def clear_instance_variables_between_requests
if @_ivars
new_ivars = instance_variables - @_ivars
new_ivars.each { |ivar| remove_instance_variable(ivar) }
end
@_ivars = instance_variables
end
def recycle!
@_url_options = nil
self.formats = nil

View file

@ -465,9 +465,15 @@ module ActionController
# prefer using #get, #post, #patch, #put, #delete and #head methods
# respectively which will make tests more expressive.
#
# It's not recommended to make more than one request in the same test. Instance
# variables that are set in one request will not persist to the next request,
# but it's not guaranteed that all Rails internal state will be reset. Prefer
# ActionDispatch::IntegrationTest for making multiple requests in the same test.
#
# Note that the request method is not verified.
def process(action, method: "GET", params: nil, session: nil, body: nil, flash: {}, format: nil, xhr: false, as: nil)
check_required_ivars
@controller.clear_instance_variables_between_requests
action = +action.to_s
http_method = method.to_s.upcase

View file

@ -165,6 +165,12 @@ XML
raise "boom!"
end
def increment_count
@counter ||= 0
@counter += 1
render plain: @counter
end
private
def generate_url(opts)
url_for(opts.merge(action: "test_uri"))
@ -988,6 +994,54 @@ XML
post :render_json, body: { foo: "heyo" }.to_json, as: :json
assert_equal({ "foo" => "heyo" }, response.parsed_body)
end
def test_reset_instance_variables_after_each_request
get :increment_count
assert_equal "1", response.body
get :increment_count
assert_equal "1", response.body
end
def test_can_read_instance_variables_before_or_after_request
assert_nil @controller.instance_variable_get(:@counter)
get :increment_count
assert_equal "1", response.body
assert_equal 1, @controller.instance_variable_get(:@counter)
get :increment_count
assert_equal "1", response.body
assert_equal 1, @controller.instance_variable_get(:@counter)
end
def test_ivars_are_not_reset_if_they_are_given_a_value_before_any_requests
@controller.instance_variable_set(:@counter, 3)
get :increment_count
assert_equal "4", response.body
assert_equal 4, @controller.instance_variable_get(:@counter)
get :increment_count
assert_equal "5", response.body
assert_equal 5, @controller.instance_variable_get(:@counter)
get :increment_count
assert_equal "6", response.body
assert_equal 6, @controller.instance_variable_get(:@counter)
end
def test_ivars_are_reset_if_they_are_given_a_value_after_some_requests
get :increment_count
assert_equal "1", response.body
assert_equal 1, @controller.instance_variable_get(:@counter)
@controller.instance_variable_set(:@counter, 3)
get :increment_count
assert_equal "1", response.body
assert_equal 1, @controller.instance_variable_get(:@counter)
end
end
class ResponseDefaultHeadersTest < ActionController::TestCase