From 65e42c91f5850edd0e5b55a409c2336fff9057f3 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 27 Aug 2020 13:45:15 -0700 Subject: [PATCH 1/2] Use unique controller per ActionView::TestCase Previously the same class, ActionView::TestCase::TestController, was used to build a controller for every ActionView::TestCase class. This caused issues when helpers/helper methods were set directly on the controller (which from one test we seem to want to support). This commit solves this by creating a new controller class for every test case, which gives the controller a unique set of helpers to work with. Co-authored-by: John Crepezzi --- actionpack/lib/action_controller/metal.rb | 2 +- actionview/lib/action_view/test_case.rb | 7 ++++--- actionview/test/template/test_case_test.rb | 7 +++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 038beea32e..e1812a5c89 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -126,7 +126,7 @@ module ActionController # ==== Returns # * string def self.controller_name - @controller_name ||= name.demodulize.delete_suffix("Controller").underscore + @controller_name ||= (name.demodulize.delete_suffix("Controller").underscore unless anonymous?) end def self.make_response!(request) diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 9f1b36174a..3aa6af0893 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -16,11 +16,12 @@ module ActionView attr_accessor :request, :response, :params class << self - attr_writer :controller_path + # Overrides AbstractController::Base#controller_path + attr_accessor :controller_path end def controller_path=(path) - self.class.controller_path = (path) + self.class.controller_path = path end def initialize @@ -101,7 +102,7 @@ module ActionView end def setup_with_controller - @controller = ActionView::TestCase::TestController.new + @controller = Class.new(ActionView::TestCase::TestController).new @request = @controller.request @view_flow = ActionView::OutputFlow.new # empty string ensures buffer has UTF-8 encoding as diff --git a/actionview/test/template/test_case_test.rb b/actionview/test/template/test_case_test.rb index bf93259305..55e89cd3f5 100644 --- a/actionview/test/template/test_case_test.rb +++ b/actionview/test/template/test_case_test.rb @@ -156,6 +156,13 @@ module ActionView assert_equal "controller_helper_method", some_method end + + class AnotherTestClass < ActionView::TestCase + test "doesn't use controller helpers from other tests" do + assert_not_respond_to view, :render_from_helper + assert_not_includes @controller._helpers.instance_methods, :render_from_helper + end + end end class ViewAssignsTest < ActionView::TestCase From 3ec8ddd0cf3851fd0abfc45fd430b0de19981547 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 17 Aug 2020 10:22:57 -0700 Subject: [PATCH 2/2] Avoid module_eval on helpers from AV::TestCase Previously we were using module_eval to manually add some helpers in ActionView::TestCase, which assumes a lot about what _helpers allows. Instead, we can use a standard helper block to define these methods only one time, and add a method to the new uniq controller class to tie back to the test instance. --- actionview/lib/action_view/test_case.rb | 36 ++++++++++--------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 3aa6af0893..c93cf64d6e 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -102,7 +102,8 @@ module ActionView end def setup_with_controller - @controller = Class.new(ActionView::TestCase::TestController).new + controller_class = Class.new(ActionView::TestCase::TestController) + @controller = controller_class.new @request = @controller.request @view_flow = ActionView::OutputFlow.new # empty string ensures buffer has UTF-8 encoding as @@ -110,8 +111,8 @@ module ActionView @output_buffer = ActiveSupport::SafeBuffer.new "" @rendered = +"" - make_test_case_available_to_view! - say_no_to_protect_against_forgery! + test_case_instance = self + controller_class.define_method(:_test_case) { test_case_instance } end def config @@ -161,6 +162,16 @@ module ActionView included do setup :setup_with_controller ActiveSupport.run_load_hooks(:action_view_test_case, self) + + helper do + def protect_against_forgery? + false + end + + def _test_case + controller._test_case + end + end end private @@ -169,25 +180,6 @@ module ActionView Nokogiri::HTML::Document.parse(@rendered.blank? ? @output_buffer : @rendered).root end - def say_no_to_protect_against_forgery! - _helpers.module_eval do - silence_redefinition_of_method :protect_against_forgery? - def protect_against_forgery? - false - end - end - end - - def make_test_case_available_to_view! - test_case_instance = self - _helpers.module_eval do - unless private_method_defined?(:_test_case) - define_method(:_test_case) { test_case_instance } - private :_test_case - end - end - end - module Locals attr_accessor :rendered_views