Merge pull request rails#49628 from Shopify/enable-minitest-assert-predicate-rubocop-rule

[Tests only] Enable `Minitest/AssertPredicate` rule

Author: rafaelfranca

.rubocop.yml

@@ -353,6 +353,9 @@ Performance/RedundantStringChars:
 Performance/StringInclude:
   Enabled: true
 
+Minitest/AssertPredicate:
+  Enabled: true
+
 Minitest/AssertRaisesWithRegexpArgument:
   Enabled: true
 

actioncable/test/channel/test_case_test.rb

@@ -44,7 +44,7 @@ def test_no_subscribe
   def test_subscribe
     subscribe
 
-    assert subscription.confirmed?
+    assert_predicate subscription, :confirmed?
     assert_not subscription.rejected?
     assert_equal 1, connection.transmissions.size
     assert_equal ActionCable::INTERNAL[:message_types][:confirmation],
@@ -77,7 +77,7 @@ def test_rejection
     subscribe
 
     assert_not subscription.confirmed?
-    assert subscription.rejected?
+    assert_predicate subscription, :rejected?
     assert_equal 1, connection.transmissions.size
     assert_equal ActionCable::INTERNAL[:message_types][:rejection],
                  connection.transmissions.last["type"]

actioncable/test/client_test.rb

@@ -325,7 +325,7 @@ def test_remote_disconnect_client
       assert_equal({ "type" => "disconnect", "reason" => "remote", "reconnect" => true }, c.read_message)
 
       c.wait_for_close
-      assert(c.closed?)
+      assert_predicate(c, :closed?)
     end
   end
 
@@ -342,7 +342,7 @@ def test_remote_disconnect_client_with_reconnect
       assert_equal({ "type" => "disconnect", "reason" => "remote", "reconnect" => false }, c.read_message)
 
       c.wait_for_close
-      assert(c.closed?)
+      assert_predicate(c, :closed?)
     end
   end
 

actioncable/test/subscription_adapter/postgresql_test.rb

@@ -62,7 +62,7 @@ def active?
 
     ActiveRecord::Base.connection_handler.clear_reloadable_connections!
 
-    assert adapter.active?
+    assert_predicate adapter, :active?
   end
 
   def test_default_subscription_connection_identifier

actionmailbox/test/unit/mailbox/bouncing_test.rb

@@ -31,7 +31,7 @@ class ActionMailbox::Base::BouncingTest < ActiveSupport::TestCase
       BouncingWithReplyMailbox.receive @inbound_email
     end
 
-    assert @inbound_email.bounced?
+    assert_predicate @inbound_email, :bounced?
     assert_emails 1
 
     mail = ActionMailer::Base.deliveries.last
@@ -44,7 +44,7 @@ class ActionMailbox::Base::BouncingTest < ActiveSupport::TestCase
       BouncingWithImmediateReplyMailbox.receive @inbound_email
     end
 
-    assert @inbound_email.bounced?
+    assert_predicate @inbound_email, :bounced?
     assert_emails 1
 
     mail = ActionMailer::Base.deliveries.last

actionmailbox/test/unit/mailbox/callbacks_test.rb

@@ -61,15 +61,15 @@ class ActionMailbox::Base::CallbacksTest < ActiveSupport::TestCase
 
   test "bouncing in a callback terminates processing" do
     BouncingCallbackMailbox.receive @inbound_email
-    assert @inbound_email.bounced?
+    assert_predicate @inbound_email, :bounced?
     assert_equal [ "Pre-bounce", "Bounce" ], $before_processing
     assert_not $processed
     assert_not $after_processing
   end
 
   test "marking the inbound email as delivered in a callback terminates processing" do
     DiscardingCallbackMailbox.receive @inbound_email
-    assert @inbound_email.delivered?
+    assert_predicate @inbound_email, :delivered?
     assert_equal [ "Pre-discard", "Discard" ], $before_processing
     assert_not $processed
     assert_not $after_processing

actionmailbox/test/unit/mailbox/state_test.rb

@@ -33,19 +33,19 @@ class ActionMailbox::Base::StateTest < ActiveSupport::TestCase
 
   test "successful mailbox processing leaves inbound email in delivered state" do
     SuccessfulMailbox.receive @inbound_email
-    assert @inbound_email.delivered?
+    assert_predicate @inbound_email, :delivered?
     assert_equal "I was processed", $processed
   end
 
   test "unsuccessful mailbox processing leaves inbound email in failed state" do
     UnsuccessfulMailbox.receive @inbound_email
-    assert @inbound_email.failed?
+    assert_predicate @inbound_email, :failed?
     assert_equal :failure, $processed
   end
 
   test "bounced inbound emails are not delivered" do
     BouncingMailbox.receive @inbound_email
-    assert @inbound_email.bounced?
+    assert_predicate @inbound_email, :bounced?
     assert_equal :bounced, $processed
   end
 end

actionmailbox/test/unit/relayer_test.rb

@@ -19,7 +19,7 @@ class RelayerTest < ActiveSupport::TestCase
       result = @relayer.relay(file_fixture("welcome.eml").read)
       assert_equal "2.0.0", result.status_code
       assert_equal "Successfully relayed message to ingress", result.message
-      assert result.success?
+      assert_predicate result, :success?
       assert_not result.failure?
 
       assert_requested :post, URL, body: file_fixture("welcome.eml").read,
@@ -34,7 +34,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.7.0", result.status_code
       assert_equal "Invalid credentials for ingress", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
 
     test "unsuccessfully relaying due to an unspecified server error" do
@@ -44,7 +44,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.0.0", result.status_code
       assert_equal "HTTP 500", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
 
     test "unsuccessfully relaying due to a gateway timeout" do
@@ -54,7 +54,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.0.0", result.status_code
       assert_equal "HTTP 504", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
 
     test "unsuccessfully relaying due to ECONNRESET" do
@@ -64,7 +64,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.4.2", result.status_code
       assert_equal "Network error relaying to ingress: Connection reset by peer", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
 
     test "unsuccessfully relaying due to connection failure" do
@@ -74,7 +74,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.4.2", result.status_code
       assert_equal "Network error relaying to ingress: Failed to open TCP connection to example.com:443", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
 
     test "unsuccessfully relaying due to client-side timeout" do
@@ -84,7 +84,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.4.2", result.status_code
       assert_equal "Timed out relaying to ingress", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
 
     test "unsuccessfully relaying due to an unhandled exception" do
@@ -94,7 +94,7 @@ class RelayerTest < ActiveSupport::TestCase
       assert_equal "4.0.0", result.status_code
       assert_equal "Error relaying to ingress: Something went wrong", result.message
       assert_not result.success?
-      assert result.failure?
+      assert_predicate result, :failure?
     end
   end
 end

actionmailbox/test/unit/router_test.rb

@@ -136,7 +136,7 @@ class RouterTest < ActiveSupport::TestCase
       assert_raises(ActionMailbox::Router::RoutingError) do
         @router.route inbound_email
       end
-      assert inbound_email.bounced?
+      assert_predicate inbound_email, :bounced?
     end
 
     test "invalid address" do

actionpack/test/controller/live_stream_test.rb

@@ -324,9 +324,9 @@ def ignore_client_disconnect
     tests TestController
 
     def assert_stream_closed
-      assert response.stream.closed?, "stream should be closed"
-      assert response.committed?,     "response should be committed"
-      assert response.sent?,          "response should be sent"
+      assert_predicate response.stream, :closed?, "stream should be closed"
+      assert_predicate response, :committed?,     "response should be committed"
+      assert_predicate response, :sent?,          "response should be sent"
     end
 
     def capture_log_output

actionpack/test/controller/new_base/bare_metal_test.rb

@@ -47,7 +47,7 @@ class BareTest < ActiveSupport::TestCase
       controller.set_response!(BareController.make_response!(controller.request))
       controller.index
 
-      assert controller.performed?
+      assert_predicate controller, :performed?
       assert_equal ["Hello world"], controller.response_body
     end
 
@@ -56,7 +56,7 @@ class BareTest < ActiveSupport::TestCase
       controller.set_request!(ActionDispatch::Request.empty)
       controller.assign_response_array
 
-      assert controller.performed?
+      assert_predicate controller, :performed?
       assert_equal true, controller.response_body
       assert_equal 200, controller.response[0]
       assert_equal "text/html", controller.response[1]["content-type"]
@@ -67,7 +67,7 @@ class BareTest < ActiveSupport::TestCase
       controller.set_request!(ActionDispatch::Request.empty)
       controller.assign_response_object
 
-      assert controller.performed?
+      assert_predicate controller, :performed?
       assert_equal true, controller.response_body
       assert_equal 200, controller.response.status
       assert_equal "text/html", controller.response.headers["content-type"]
@@ -79,10 +79,10 @@ class BareTest < ActiveSupport::TestCase
       controller.set_response!(BareController.make_response!(controller.request))
       controller.assign_response_body_proc
 
-      assert controller.performed?
+      assert_predicate controller, :performed?
       assert controller.response_body.is_a?(Proc)
       assert_equal 200, controller.response.status
-      assert controller.response.headers.empty?
+      assert_predicate controller.response.headers, :empty?
     end
 
     test "connect a request to controller instance without dispatch" do

actionpack/test/controller/parameters/accessors_test.rb

@@ -55,7 +55,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
 
   test "each carries permitted status" do
     @params.permit!
-    @params.each { |key, value| assert(value.permitted?) if key == "person" }
+    @params.each { |key, value| assert_predicate(value, :permitted?) if key == "person" }
   end
 
   test "each carries unpermitted status" do
@@ -77,7 +77,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
 
   test "each_pair carries permitted status" do
     @params.permit!
-    @params.each_pair { |key, value| assert(value.permitted?) if key == "person" }
+    @params.each_pair { |key, value| assert_predicate(value, :permitted?) if key == "person" }
   end
 
   test "each_pair carries unpermitted status" do

actionpack/test/dispatch/request_test.rb

@@ -933,7 +933,7 @@ class RequestFormat < BaseRequestTest
       "action_dispatch.logger" => Logger.new(output = StringIO.new)
     )
     assert request.formats
-    assert request.format.html?
+    assert_predicate request.format, :html?
 
     output.rewind && (err = output.read)
     assert_match(/Error occurred while parsing request parameters/, err)

actionpack/test/dispatch/routing/route_set_test.rb

@@ -20,7 +20,7 @@ def call(env)
       end
 
       test "not being empty when route is added" do
-        assert empty?
+        assert_predicate self, :empty?
 
         draw do
           get "foo", to: SimpleApp.new("foo#index")

actionpack/test/dispatch/routing_test.rb

@@ -3509,16 +3509,16 @@ def test_trailing_slash
     end
 
     get "/streams"
-    assert @response.ok?, "route without trailing slash should work"
+    assert_predicate @response, :ok?, "route without trailing slash should work"
 
     get "/streams/"
-    assert @response.ok?, "route with trailing slash should work"
+    assert_predicate @response, :ok?, "route with trailing slash should work"
 
     get "/streams?foobar"
-    assert @response.ok?, "route without trailing slash and with QUERY_STRING should work"
+    assert_predicate @response, :ok?, "route without trailing slash and with QUERY_STRING should work"
 
     get "/streams/?foobar"
-    assert @response.ok?, "route with trailing slash and with QUERY_STRING should work"
+    assert_predicate @response, :ok?, "route with trailing slash and with QUERY_STRING should work"
   end
 
   def test_route_with_dashes_in_path

actionpack/test/dispatch/uploaded_file_test.rb

@@ -86,14 +86,14 @@ def test_delegates_close_to_tempfile
       tf = Tempfile.new
       uf = Http::UploadedFile.new(tempfile: tf)
       uf.close
-      assert tf.closed?
+      assert_predicate tf, :closed?
     end
 
     def test_close_accepts_parameter
       tf = Tempfile.new
       uf = Http::UploadedFile.new(tempfile: tf)
       uf.close(true)
-      assert tf.closed?
+      assert_predicate tf, :closed?
       assert_nil tf.path
     end
 

actiontext/test/unit/model_test.rb

@@ -17,18 +17,18 @@ class ActionText::ModelTest < ActiveSupport::TestCase
 
   test "without content" do
     message = Message.create!(subject: "Greetings")
-    assert message.content.nil?
-    assert message.content.blank?
-    assert message.content.empty?
+    assert_predicate message.content, :nil?
+    assert_predicate message.content, :blank?
+    assert_predicate message.content, :empty?
     assert_not message.content?
     assert_not message.content.present?
   end
 
   test "with blank content" do
     message = Message.create!(subject: "Greetings", content: "")
     assert_not message.content.nil?
-    assert message.content.blank?
-    assert message.content.empty?
+    assert_predicate message.content, :blank?
+    assert_predicate message.content, :empty?
     assert_not message.content?
     assert_not message.content.present?
   end

actionview/test/template/partial_iteration_test.rb

@@ -12,7 +12,7 @@ def test_has_size_and_index
 
   def test_first_is_true_when_current_is_at_the_first_index
     iteration = ActionView::PartialIteration.new 3
-    assert iteration.first?, "first when current is 0"
+    assert_predicate iteration, :first?, "first when current is 0"
   end
 
   def test_first_is_false_unless_current_is_at_the_first_index
@@ -25,7 +25,7 @@ def test_last_is_true_when_current_is_at_the_last_index
     iteration = ActionView::PartialIteration.new 3
     iteration.iterate!
     iteration.iterate!
-    assert iteration.last?, "last when current is 2"
+    assert_predicate iteration, :last?, "last when current is 2"
   end
 
   def test_last_is_false_unless_current_is_at_the_last_index

actionview/test/template/tag_helper_test.rb

@@ -135,7 +135,7 @@ def test_tag_builder_do_not_modify_html_safe_options
     html_safe_str = '"'.html_safe
     assert_equal "<p value=\"&quot;\" />", tag("p", value: html_safe_str)
     assert_equal '"', html_safe_str
-    assert html_safe_str.html_safe?
+    assert_predicate html_safe_str, :html_safe?
   end
 
   def test_tag_with_dangerous_name