Merge pull request #35688 from jhawthorn/render_file_rfc

RFC: Introduce Template::File

5 files changed, 72 insertions, 27 deletions

diff --git a/actionview/test/actionpack/abstract/render_test.rb b/actionview/test/actionpack/abstract/render_test.rb
index d863548a5c..e4e8ac93b2 100644
--- a/actionview/test/actionpack/abstract/render_test.rb
+++ b/actionview/test/actionpack/abstract/render_test.rb
@@ -26,7 +26,7 @@ module AbstractController
       end
 
       def file
-        render file: "some/file"
+        ActiveSupport::Deprecation.silence { render file: "some/file" }
       end
 
       def inline
diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb
index 52c3c54d96..c8ce7366d1 100644
--- a/actionview/test/actionpack/controller/render_test.rb
+++ b/actionview/test/actionpack/controller/render_test.rb
@@ -872,48 +872,64 @@ class RenderTest < ActionController::TestCase
 
   # :ported:
   def test_render_file_with_instance_variables
-    get :render_file_with_instance_variables
+    assert_deprecated do
+      get :render_file_with_instance_variables
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   def test_render_file
-    get :hello_world_file
+    assert_deprecated do
+      get :hello_world_file
+    end
     assert_equal "Hello world!", @response.body
   end
 
   # :ported:
   def test_render_file_not_using_full_path
-    get :render_file_not_using_full_path
+    assert_deprecated do
+      get :render_file_not_using_full_path
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_not_using_full_path_with_dot_in_path
-    get :render_file_not_using_full_path_with_dot_in_path
+    assert_deprecated do
+      get :render_file_not_using_full_path_with_dot_in_path
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_using_pathname
-    get :render_file_using_pathname
+    assert_deprecated do
+      get :render_file_using_pathname
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_with_locals
-    get :render_file_with_locals
+    assert_deprecated do
+      get :render_file_with_locals
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :ported:
   def test_render_file_as_string_with_locals
-    get :render_file_as_string_with_locals
+    assert_deprecated do
+      get :render_file_as_string_with_locals
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
   # :assessed:
   def test_render_file_from_template
-    get :render_file_from_template
+    assert_deprecated do
+      get :render_file_from_template
+    end
     assert_equal "The secret is in the sauce\n", @response.body
   end
 
@@ -1133,11 +1149,19 @@ class RenderTest < ActionController::TestCase
   end
 
   def test_bad_render_to_string_still_throws_exception
-    assert_raise(ActionView::MissingTemplate) { get :render_to_string_with_exception }
+    assert_deprecated do
+      assert_raise(ActionView::MissingTemplate) do
+        get :render_to_string_with_exception
+      end
+    end
   end
 
   def test_render_to_string_that_throws_caught_exception_doesnt_break_assigns
-    assert_nothing_raised { get :render_to_string_with_caught_exception }
+    assert_deprecated do
+      assert_nothing_raised do
+        get :render_to_string_with_caught_exception
+      end
+    end
     assert_equal "i'm before the render", @controller.instance_variable_get(:@before)
     assert_equal "i'm after the render", @controller.instance_variable_get(:@after)
   end
diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb
index 85735139c1..8b160a7336 100644
--- a/actionview/test/template/log_subscriber_test.rb
+++ b/actionview/test/template/log_subscriber_test.rb
@@ -51,9 +51,20 @@ class AVLogSubscriberTest < ActiveSupport::TestCase
     def @view.combined_fragment_cache_key(*); "ahoy `controller` dependency"; end
   end
 
+  def test_render_template_template
+    Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do
+      @view.render(template: "test/hello_world")
+      wait
+
+      assert_equal 2, @logger.logged(:info).size
+      assert_match(/Rendering test\/hello_world\.erb/, @logger.logged(:info).first)
+      assert_match(/Rendered test\/hello_world\.erb/, @logger.logged(:info).last)
+    end
+  end
+
   def test_render_file_template
     Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do
-      @view.render(file: "test/hello_world")
+      @view.render(file: "#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
       wait
 
       assert_equal 2, @logger.logged(:info).size
diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index baff7cd83f..15c41051de 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -53,15 +53,20 @@ module RenderTestCases
     assert_match(/You invoked render but did not give any of (.+) option\./, e.message)
   end
 
+  def test_render_template
+    assert_equal "Hello world!", @view.render(template: "test/hello_world")
+  end
+
+
   def test_render_file
-    assert_equal "Hello world!", @view.render(file: "test/hello_world")
+    assert_equal "Hello world!", assert_deprecated { @view.render(file: "test/hello_world") }
   end
 
   # Test if :formats, :locale etc. options are passed correctly to the resolvers.
   def test_render_file_with_format
-    assert_match "<h1>No Comment</h1>", @view.render(file: "comments/empty", formats: [:html])
-    assert_match "<error>No Comment</error>", @view.render(file: "comments/empty", formats: [:xml])
-    assert_match "<error>No Comment</error>", @view.render(file: "comments/empty", formats: :xml)
+    assert_match "<h1>No Comment</h1>", assert_deprecated { @view.render(file: "comments/empty", formats: [:html]) }
+    assert_match "<error>No Comment</error>", assert_deprecated { @view.render(file: "comments/empty", formats: [:xml]) }
+    assert_match "<error>No Comment</error>", assert_deprecated { @view.render(file: "comments/empty", formats: :xml) }
   end
 
   def test_render_template_with_format
@@ -94,8 +99,8 @@ module RenderTestCases
   end
 
   def test_render_file_with_locale
-    assert_equal "<h1>Kein Kommentar</h1>", @view.render(file: "comments/empty", locale: [:de])
-    assert_equal "<h1>Kein Kommentar</h1>", @view.render(file: "comments/empty", locale: :de)
+    assert_equal "<h1>Kein Kommentar</h1>", assert_deprecated { @view.render(file: "comments/empty", locale: [:de]) }
+    assert_equal "<h1>Kein Kommentar</h1>", assert_deprecated { @view.render(file: "comments/empty", locale: :de) }
   end
 
   def test_render_template_with_locale
@@ -107,8 +112,8 @@ module RenderTestCases
   end
 
   def test_render_file_with_handlers
-    assert_equal "<h1>No Comment</h1>\n", @view.render(file: "comments/empty", handlers: [:builder])
-    assert_equal "<h1>No Comment</h1>\n", @view.render(file: "comments/empty", handlers: :builder)
+    assert_equal "<h1>No Comment</h1>\n", assert_deprecated { @view.render(file: "comments/empty", handlers: [:builder]) }
+    assert_equal "<h1>No Comment</h1>\n", assert_deprecated { @view.render(file: "comments/empty", handlers: :builder) }
   end
 
   def test_render_template_with_handlers
@@ -156,22 +161,27 @@ module RenderTestCases
     assert_equal "Elastica", @view.render(template: "/shared")
   end
 
-  def test_render_file_with_full_path
+  def test_render_file_with_full_path_no_extension
     template_path = File.expand_path("../fixtures/test/hello_world", __dir__)
+    assert_equal "Hello world!", assert_deprecated { @view.render(file: template_path) }
+  end
+
+  def test_render_file_with_full_path
+    template_path = File.expand_path("../fixtures/test/hello_world.erb", __dir__)
     assert_equal "Hello world!", @view.render(file: template_path)
   end
 
   def test_render_file_with_instance_variables
-    assert_equal "The secret is in the sauce\n", @view.render(file: "test/render_file_with_ivar")
+    assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/render_file_with_ivar") }
   end
 
   def test_render_file_with_locals
     locals = { secret: "in the sauce" }
-    assert_equal "The secret is in the sauce\n", @view.render(file: "test/render_file_with_locals", locals: locals)
+    assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/render_file_with_locals", locals: locals) }
   end
 
   def test_render_file_not_using_full_path_with_dot_in_path
-    assert_equal "The secret is in the sauce\n", @view.render(file: "test/dot.directory/render_file_with_ivar")
+    assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/dot.directory/render_file_with_ivar") }
   end
 
   def test_render_partial_from_default
@@ -292,7 +302,7 @@ module RenderTestCases
   end
 
   def test_render_file_with_errors
-    e = assert_raises(ActionView::Template::Error) { @view.render(file: File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) }
+    e = assert_raises(ActionView::Template::Error) { assert_deprecated { @view.render(file: File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) } }
     assert_match %r!method.*doesnt_exist!, e.message
     assert_equal "", e.sub_template_message
     assert_equal "1", e.line_number
diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb
index a5b59a700e..a5e673e71e 100644
--- a/actionview/test/template/streaming_render_test.rb
+++ b/actionview/test/template/streaming_render_test.rb
@@ -47,12 +47,12 @@ class FiberedTest < SetupFiberedBase
   end
 
   def test_render_file
-    assert_equal "Hello world!", buffered_render(file: "test/hello_world")
+    assert_equal "Hello world!", assert_deprecated { buffered_render(file: "test/hello_world") }
   end
 
   def test_render_file_with_locals
     locals = { secret: "in the sauce" }
-    assert_equal "The secret is in the sauce\n", buffered_render(file: "test/render_file_with_locals", locals: locals)
+    assert_equal "The secret is in the sauce\n", assert_deprecated { buffered_render(file: "test/render_file_with_locals", locals: locals) }
   end
 
   def test_render_partial