Merge pull request #30104 from trev/deprecate-actiondispatch-testresponse-alias

Deprecate ActionDispatch::TestResponse response aliases

3 files changed, 36 insertions, 3 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 1bc26cd5e1..938b24a6b9 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Deprecate `ActionDispatch::TestResponse` response aliases
+
+    `#success?`, `#missing?` & `#error?` are not supported by the actual
+    `ActionDispatch::Response` object and can produce false-positives. Instead,
+    use the response helpers provided by `Rack::Response`.
+
+    *Trevor Wistaff*
+
 *   Protect from forgery by default
 
     Rather than protecting from forgery in the generated `ApplicationController`,
diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb
index 0b7e9ca945..b23ea7479c 100644
--- a/actionpack/lib/action_dispatch/testing/test_response.rb
+++ b/actionpack/lib/action_dispatch/testing/test_response.rb
@@ -20,13 +20,31 @@ module ActionDispatch
     end
 
     # Was the response successful?
-    alias_method :success?, :successful?
+    def success?
+      ActiveSupport::Deprecation.warn(<<-MSG.squish)
+       The success? predicate is deprecated and will be removed in Rails 6.0.
+       Please use successful? as provided by Rack::Response::Helpers.
+      MSG
+      successful?
+    end
 
     # Was the URL not found?
-    alias_method :missing?, :not_found?
+    def missing?
+      ActiveSupport::Deprecation.warn(<<-MSG.squish)
+       The missing? predicate is deprecated and will be removed in Rails 6.0.
+       Please use not_found? as provided by Rack::Response::Helpers.
+      MSG
+      not_found?
+    end
 
     # Was there a server-side error?
-    alias_method :error?, :server_error?
+    def error?
+      ActiveSupport::Deprecation.warn(<<-MSG.squish)
+       The error? predicate is deprecated and will be removed in Rails 6.0.
+       Please use server_error? as provided by Rack::Response::Helpers.
+      MSG
+      server_error?
+    end
 
     def parsed_body
       @parsed_body ||= @response_parser.call(body)
diff --git a/actionpack/test/dispatch/test_response_test.rb b/actionpack/test/dispatch/test_response_test.rb
index 2629a61057..f0b8f7785d 100644
--- a/actionpack/test/dispatch/test_response_test.rb
+++ b/actionpack/test/dispatch/test_response_test.rb
@@ -27,4 +27,11 @@ class TestResponseTest < ActiveSupport::TestCase
     response = ActionDispatch::TestResponse.create(200, { "Content-Type" => "application/json" }, '{ "foo": "fighters" }')
     assert_equal({ "foo" => "fighters" }, response.parsed_body)
   end
+
+  test "response status aliases deprecated" do
+    response = ActionDispatch::TestResponse.create
+    assert_deprecated { response.success? }
+    assert_deprecated { response.missing? }
+    assert_deprecated { response.error? }
+  end
 end