From beb7fba632b19a6b3c021b616611048bde50f5a4 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Wed, 29 Apr 2020 22:13:52 +0200 Subject: [PATCH] Use the `file_fixture_path` for fixture_file_upload: - We used the `fixture_path` before `file_fixture_path` was a thing, but now that we have the latter we should use it. `fixture_path` is solely used by Active Record so it seems wrong to be using that in ActionPack. --- actionpack/CHANGELOG.md | 15 +++++++++ .../action_dispatch/testing/test_process.rb | 23 +++++++++++++- .../test/controller/integration_test.rb | 2 +- actionpack/test/controller/test_case_test.rb | 31 +++++++++++++++---- 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3b91ed4b8b..74cb16ddca 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,18 @@ +* `fixture_file_upload` now uses path relative to `file_fixture_path` + + Previously the path had to be relative to `fixture_path`. + You can change your existing code as follow: + + ```ruby + # Before + fixture_file_upload('files/dog.png') + + # After + fixture_file_upload('dog.png') + ``` + + *Edouard Chin* + * Remove deprecated `force_ssl` at the controller level. *Rafael Mendonça França* diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index 0b98f27f11..c384b03ee2 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -17,8 +17,29 @@ module ActionDispatch def fixture_file_upload(path, mime_type = nil, binary = false) if self.class.respond_to?(:fixture_path) && self.class.fixture_path && !File.exist?(path) - path = File.join(self.class.fixture_path, path) + original_path = path + path = Pathname.new(self.class.fixture_path).join(path) + + if !self.class.file_fixture_path + ActiveSupport::Deprecation.warn(<<~EOM) + Passing a path to `fixture_file_upload` relative to `fixture_path` is deprecated. + In Rails 6.2, the path needs to be relative to `file_fixture_path` which you + haven't set yet. Set `file_fixture_path` to discard this warning. + EOM + elsif path.exist? + non_deprecated_path = path.relative_path_from(Pathname(self.class.file_fixture_path)) + ActiveSupport::Deprecation.warn(<<~EOM) + Passing a path to `fixture_file_upload` relative to `fixture_path` is deprecated. + In Rails 6.2, the path needs to be relative to `file_fixture_path`. + + Please modify the call from + `fixture_file_upload(#{original_path})` to `fixture_file_upload(#{non_deprecated_path})`. + EOM + end + elsif self.class.file_fixture_path && !File.exist?(path) + path = file_fixture(path) end + Rack::Test::UploadedFile.new(path, mime_type, binary) end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index b8fbefae9b..81027ba00b 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -1205,7 +1205,7 @@ class IntegrationFileUploadTest < ActionDispatch::IntegrationTest self.class end - def self.fixture_path + def self.file_fixture_path File.expand_path("../fixtures/multipart", __dir__) end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 635a91507d..8f1988a7bd 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -7,6 +7,7 @@ require "rails/engine" class TestCaseTest < ActionController::TestCase def self.fixture_path; end + self.file_fixture_path = File.expand_path("../fixtures/multipart", __dir__) class TestController < ActionController::Base def no_op @@ -872,9 +873,13 @@ XML end def test_fixture_path_is_accessed_from_self_instead_of_active_support_test_case - TestCaseTest.stub :fixture_path, FILES_DIR do - uploaded_file = fixture_file_upload("/ruby_on_rails.jpg", "image/png") - assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read + TestCaseTest.stub :fixture_path, File.expand_path("../fixtures", __dir__) do + expected = "`fixture_file_upload(multipart/ruby_on_rails.jpg)` to `fixture_file_upload(ruby_on_rails.jpg)`" + + assert_deprecated(expected) do + uploaded_file = fixture_file_upload("multipart/ruby_on_rails.jpg", "image/png") + assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read + end end end @@ -915,10 +920,24 @@ XML assert_equal "45142", @response.body end + def test_fixture_file_upload_output_deprecation_when_file_fixture_path_is_not_set + TestCaseTest.stub :fixture_path, File.expand_path("../fixtures", __dir__) do + TestCaseTest.stub :file_fixture_path, nil do + assert_deprecated(/In Rails 6.2, the path needs to be relative to `file_fixture_path`/) do + fixture_file_upload("multipart/ruby_on_rails.jpg", "image/jpg") + end + end + end + end + def test_fixture_file_upload_relative_to_fixture_path - TestCaseTest.stub :fixture_path, FILES_DIR do - uploaded_file = fixture_file_upload("ruby_on_rails.jpg", "image/jpg") - assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read + TestCaseTest.stub :fixture_path, File.expand_path("../fixtures", __dir__) do + expected = "`fixture_file_upload(multipart/ruby_on_rails.jpg)` to `fixture_file_upload(ruby_on_rails.jpg)`" + + assert_deprecated(expected) do + uploaded_file = fixture_file_upload("multipart/ruby_on_rails.jpg", "image/jpg") + assert_equal File.open("#{FILES_DIR}/ruby_on_rails.jpg", READ_PLAIN).read, uploaded_file.read + end end end