Merge pull request #44547 from skipkayhil/fix-incorrect-assertions

fix remaining asserts that should be assert_equal

.rubocop.yml

@@ -327,5 +327,8 @@ Performance/StringInclude:
 Minitest/AssertRaisesWithRegexpArgument:
   Enabled: true
 
+Minitest/AssertWithExpectedArgument:
+  Enabled: true
+
 Minitest/UnreachableAssertion:
   Enabled: true

actionpack/test/dispatch/request_test.rb

@@ -1329,7 +1329,7 @@ class RequestEtag < BaseRequestTest
     assert_equal header, request.if_none_match
     assert_equal expected, request.if_none_match_etags
     expected.each do |etag|
-      assert request.etag_matches?(etag), etag
+      assert request.etag_matches?(etag), "Etag #{etag} did not match HTTP_IF_NONE_MATCH values"
     end
   end
 end

activemodel/test/cases/validations/length_validation_test.rb

@@ -324,13 +324,13 @@ def test_optionally_validates_length_of_using_within_utf8
     Topic.validates_length_of :title, within: 3..5, allow_nil: true
 
     t = Topic.new(title: "一二三四五")
-    assert t.valid?, t.errors.inspect
+    assert_predicate t, :valid?
 
     t = Topic.new(title: "一二三")
-    assert t.valid?, t.errors.inspect
+    assert_predicate t, :valid?
 
     t.title = nil
-    assert t.valid?, t.errors.inspect
+    assert_predicate t, :valid?
   end
 
   def test_validates_length_of_using_is_utf8

activemodel/test/cases/validations/presence_validation_test.rb

@@ -75,33 +75,33 @@ def test_validates_presence_of_with_allow_nil_option
     Topic.validates_presence_of(:title, allow_nil: true)
 
     t = Topic.new(title: "something")
-    assert t.valid?, t.errors.full_messages
+    assert_predicate t, :valid?
 
     t.title = ""
     assert_predicate t, :invalid?
     assert_equal ["can't be blank"], t.errors[:title]
 
     t.title = "  "
-    assert t.invalid?, t.errors.full_messages
+    assert_predicate t, :invalid?
     assert_equal ["can't be blank"], t.errors[:title]
 
     t.title = nil
-    assert t.valid?, t.errors.full_messages
+    assert_predicate t, :valid?
   end
 
   def test_validates_presence_of_with_allow_blank_option
     Topic.validates_presence_of(:title, allow_blank: true)
 
     t = Topic.new(title: "something")
-    assert t.valid?, t.errors.full_messages
+    assert_predicate t, :valid?
 
     t.title = ""
-    assert t.valid?, t.errors.full_messages
+    assert_predicate t, :valid?
 
     t.title = "  "
-    assert t.valid?, t.errors.full_messages
+    assert_predicate t, :valid?
 
     t.title = nil
-    assert t.valid?, t.errors.full_messages
+    assert_predicate t, :valid?
   end
 end

activerecord/test/cases/adapters/postgresql/enum_test.rb

@@ -103,12 +103,12 @@ def test_schema_dump
 
     output = dump_all_table_schema
 
-    assert output.include?("# Note that some types may not work with other database engines. Be careful if changing database."), output
+    assert_includes output, "# Note that some types may not work with other database engines. Be careful if changing database."
 
-    assert output.include?('create_enum "mood", ["sad", "ok", "happy"]'), output
+    assert_includes output, 'create_enum "mood", ["sad", "ok", "happy"]'
 
-    assert output.include?('t.enum "current_mood", enum_type: "mood"'), output
-    assert output.include?('t.enum "good_mood", default: "happy", null: false, enum_type: "mood"'), output
+    assert_includes output, 't.enum "current_mood", enum_type: "mood"'
+    assert_includes output, 't.enum "good_mood", default: "happy", null: false, enum_type: "mood"'
   end
 
   def test_schema_load
@@ -198,9 +198,9 @@ def test_schema_dump_scoped_to_schemas
 
       output = dump_all_table_schema
 
-      assert output.include?('create_enum "public.mood", ["sad", "ok", "happy"]'), output
-      assert output.include?('create_enum "mood_in_test_schema", ["sad", "ok", "happy"]'), output
-      assert output.include?('t.enum "current_mood", enum_type: "mood_in_test_schema"'), output
+      assert_includes output, 'create_enum "public.mood", ["sad", "ok", "happy"]'
+      assert_includes output, 'create_enum "mood_in_test_schema", ["sad", "ok", "happy"]'
+      assert_includes output, 't.enum "current_mood", enum_type: "mood_in_test_schema"'
     end
   end
 

activerecord/test/cases/arel/nodes/node_test.rb

@@ -15,7 +15,7 @@ def test_all_nodes_are_nodes
         next if Nodes::SqlLiteral == klass
         next if Nodes::BindParam == klass
         next if /^Arel::Nodes::(?:Test|.*Test$)/.match?(klass.name)
-        assert klass.ancestors.include?(Nodes::Node), klass.name
+        assert_includes klass.ancestors, Nodes::Node
       end
     end
   end

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

@@ -992,7 +992,7 @@ def test_preloaded_associations_size
         projects.
         detect { |p| p.id == first_project.id }
 
-    assert preloaded_first_project.salaried_developers.loaded?, true
+    assert_predicate preloaded_first_project.salaried_developers, :loaded?
     assert_equal first_project.salaried_developers.size, preloaded_first_project.salaried_developers.size
   end
 

activerecord/test/cases/encryption/encryptable_record_api_test.rb

@@ -96,15 +96,15 @@ class ActiveRecord::Encryption::EncryptableRecordApiTest < ActiveRecord::Encrypt
 
     book = ActiveRecord::Encryption.without_encryption { EncryptedBook.create!(name: "Dune".encode("US-ASCII")) }
     book.encrypt
-    assert Encoding::UTF_8, book.reload.name.encoding
+    assert_equal Encoding::UTF_8, book.reload.name.encoding
   end
 
   test "encrypt won't force encoding for deterministic attributes when option is nil" do
     ActiveRecord::Encryption.config.forced_encoding_for_deterministic_encryption = nil
 
     book = ActiveRecord::Encryption.without_encryption { EncryptedBook.create!(name: "Dune".encode("US-ASCII")) }
     book.encrypt
-    assert Encoding::US_ASCII, book.reload.name.encoding
+    assert_equal Encoding::US_ASCII, book.reload.name.encoding
   end
 
   test "encrypt will preserve case when :ignore_case option is used" do

activerecord/test/cases/encryption/read_only_null_encryptor_test.rb

@@ -8,7 +8,7 @@ class ActiveRecord::Encryption::ReadOnlyNullEncryptorTest < ActiveRecord::Encryp
   end
 
   test "decrypt returns the encrypted message" do
-    assert "some text", @encryptor.decrypt("some text")
+    assert_equal "some text", @encryptor.decrypt("some text")
   end
 
   test "encrypt raises an Encryption" do

activerecord/test/cases/multi_db_migrator_test.rb

@@ -73,8 +73,8 @@ def puts(*)
   def test_schema_migration_is_different_for_different_connections
     assert_not_equal @schema_migration_a, @schema_migration_b
     assert_not_equal @schema_migration_a.connection, @schema_migration_b.connection
-    assert "ActiveRecord::Base", @schema_migration_a.connection.pool.pool_config.connection_name
-    assert "Arunit2", @schema_migration_b.connection.pool.pool_config.connection_name
+    assert_equal "ActiveRecord::Base", @schema_migration_a.connection.pool.pool_config.connection_name
+    assert_equal "ARUnit2Model", @schema_migration_b.connection.pool.pool_config.connection_name
   end
 
   def test_finds_migrations

activerecord/test/cases/test_case.rb

@@ -68,16 +68,13 @@ def assert_no_queries(options = {}, &block)
     end
 
     def assert_column(model, column_name, msg = nil)
-      assert has_column?(model, column_name), msg
+      model.reset_column_information
+      assert_includes model.column_names, column_name.to_s, msg
     end
 
     def assert_no_column(model, column_name, msg = nil)
-      assert_not has_column?(model, column_name), msg
-    end
-
-    def has_column?(model, column_name)
       model.reset_column_information
-      model.column_names.include?(column_name.to_s)
+      assert_not_includes model.column_names, column_name.to_s, msg
     end
 
     def with_has_many_inversing(model = ActiveRecord::Base)

activerecord/test/support/async_helper.rb

@@ -4,7 +4,7 @@ module AsyncHelper
   private
     def assert_async_equal(expected, async_result)
       message = "Expected to return an ActiveRecord::Promise, got: #{async_result.inspect}"
-      assert ActiveRecord::Promise === async_result, message
+      assert_equal(true, ActiveRecord::Promise === async_result, message)
 
       if expected.nil?
         assert_nil async_result.value

activesupport/test/test_case_test.rb

@@ -366,7 +366,7 @@ def test_warning_is_logged_if_caught_internally
       If you expected this exception, use `assert_raises` as near to the code that raises as possible.
       Other block based assertions (e.g. `assert_no_changes`) can be used, as long as `assert_raises` is inside their block.
     MSG
-    assert @out.string.include?(expected), @out.string
+    assert_includes @out.string, expected
   end
 
   def test_warning_is_not_logged_if_caught_correctly_by_user
@@ -391,7 +391,7 @@ def test_fails_and_warning_is_logged_if_wrong_error_caught
       If you expected this exception, use `assert_raises` as near to the code that raises as possible.
       Other block based assertions (e.g. `assert_no_changes`) can be used, as long as `assert_raises` is inside their block.
     MSG
-    assert @out.string.include?(expected), @out.string
+    assert_includes @out.string, expected
     assert error.message.include?("ArgumentError: ArgumentError")
     assert error.message.include?("in `block (2 levels) in run_test_that_should_fail_confusingly'")
   end

railties/test/application/mailer_previews_test.rb

@@ -263,7 +263,7 @@ def foo
       prev = ActionMailer::Base.preview_paths
       ActionMailer::Base.preview_paths = ["#{app_path}/lib/mailer/previews"]
       assert_deprecated do
-        assert "#{app_path}/lib/mailer/previews", ActionMailer::Base.preview_path
+        assert_equal "#{app_path}/lib/mailer/previews", ActionMailer::Base.preview_path
       end
     ensure
       ActionMailer::Base.preview_paths = prev
@@ -274,7 +274,7 @@ def foo
       assert_deprecated do
         ActionMailer::Base.preview_path = "#{app_path}/lib/mailer/previews"
       end
-      assert ["#{app_path}/lib/mailer/previews"], ActionMailer::Base.preview_paths
+      assert_equal ["#{app_path}/lib/mailer/previews"], ActionMailer::Base.preview_paths
     ensure
       ActionMailer::Base.preview_paths = prev
     end

railties/test/railties/engine_test.rb

@@ -1688,8 +1688,7 @@ def index
       RUBY
 
       Dir.chdir(@plugin.path) do
-        output = `bundle exec rake app:active_storage:install`
-        assert $?.success?, output
+        assert_command_succeeds("bundle exec rake app:active_storage:install")
 
         active_storage_migration = migrations.detect { |migration| migration.name == "CreateActiveStorageTables" }
         assert active_storage_migration
@@ -1703,8 +1702,7 @@ def index
       RUBY
 
       Dir.chdir(@plugin.path) do
-        output = `bundle exec rake app:active_storage:update`
-        assert $?.success?, output
+        assert_command_succeeds("bundle exec rake app:active_storage:update")
 
         assert migrations.detect { |migration| migration.name == "AddServiceNameToActiveStorageBlobs" }
         assert migrations.detect { |migration| migration.name == "CreateActiveStorageVariantRecords" }
@@ -1738,5 +1736,10 @@ def restrict_frameworks
       environment = File.read("#{app_path}/config/application.rb")
       File.open("#{app_path}/config/application.rb", "w") { |f| f.puts frameworks + "\n" + environment }
     end
+
+    def assert_command_succeeds(command)
+      output = `#{command}`
+      assert_predicate $?, :success?, "Command did not succeed: #{command}\n#{output}"
+    end
   end
 end