From fa27a7f56a32d395d4da6ec45fc414647002133e Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Tue, 18 May 2010 13:07:24 -0400 Subject: [PATCH] Use better assertion methods for testing [#4645 state:resolved] --- actionmailer/test/old_base/asset_host_test.rb | 8 +++--- actionmailer/test/old_base/mail_render_test.rb | 2 +- actionmailer/test/old_base/mail_service_test.rb | 16 +++++----- actionpack/test/controller/filters_test.rb | 14 +++++----- activemodel/test/cases/dirty_test.rb | 4 +- .../serializeration/xml_serialization_test.rb | 2 +- activerecord/test/cases/aggregations_test.rb | 20 +++++++------- .../associations/belongs_to_associations_test.rb | 4 +- activerecord/test/cases/associations/eager_test.rb | 2 +- .../associations/has_many_associations_test.rb | 13 ++------- .../has_many_through_associations_test.rb | 4 +- .../associations/has_one_associations_test.rb | 8 +++--- .../test/cases/associations/join_model_test.rb | 4 +- activerecord/test/cases/attribute_methods_test.rb | 2 +- activerecord/test/cases/base_test.rb | 28 ++++++++++---------- activerecord/test/cases/column_definition_test.rb | 6 ++-- activerecord/test/cases/defaults_test.rb | 4 +- 17 files changed, 67 insertions(+), 74 deletions(-) diff --git a/actionmailer/test/old_base/asset_host_test.rb b/actionmailer/test/old_base/asset_host_test.rb index 0ec0242..cc13c8a 100644 --- a/actionmailer/test/old_base/asset_host_test.rb +++ b/actionmailer/test/old_base/asset_host_test.rb @@ -26,7 +26,7 @@ class AssetHostTest < Test::Unit::TestCase def test_asset_host_as_string mail = AssetHostMailer.email_with_asset - assert_equal "\"Somelogo\"", mail.body.to_s.strip + assert_equal %Q{Somelogo}, mail.body.to_s.strip end def test_asset_host_as_one_arguement_proc @@ -38,7 +38,7 @@ class AssetHostTest < Test::Unit::TestCase end } mail = AssetHostMailer.email_with_asset - assert_equal "\"Somelogo\"", mail.body.to_s.strip + assert_equal %Q{Somelogo}, mail.body.to_s.strip end def test_asset_host_as_two_arguement_proc @@ -51,6 +51,6 @@ class AssetHostTest < Test::Unit::TestCase } mail = nil assert_nothing_raised { mail = AssetHostMailer.email_with_asset } - assert_equal "\"Somelogo\"", mail.body.to_s.strip + assert_equal %Q{Somelogo}, mail.body.to_s.strip end -end \ No newline at end of file +end diff --git a/actionmailer/test/old_base/mail_render_test.rb b/actionmailer/test/old_base/mail_render_test.rb index 405d43d..0895183 100644 --- a/actionmailer/test/old_base/mail_render_test.rb +++ b/actionmailer/test/old_base/mail_render_test.rb @@ -117,7 +117,7 @@ class RenderHelperTest < Test::Unit::TestCase def test_rxml_template mail = RenderMailer.rxml_template.deliver - assert_equal "\n", mail.body.to_s.strip + assert_equal %(\n), mail.body.to_s.strip end def test_included_subtemplate diff --git a/actionmailer/test/old_base/mail_service_test.rb b/actionmailer/test/old_base/mail_service_test.rb index f91e7f8..e967ca1 100644 --- a/actionmailer/test/old_base/mail_service_test.rb +++ b/actionmailer/test/old_base/mail_service_test.rb @@ -281,7 +281,7 @@ class TestMailer < ActionMailer::Base from "One: Two " cc "Three: Four " bcc "Five: Six " - body "testing" + body "testing" end def custom_content_type_attributes @@ -289,7 +289,7 @@ class TestMailer < ActionMailer::Base subject "custom content types" from "some.one@somewhere.test" content_type "text/plain; format=flowed" - body "testing" + body "testing" end def return_path @@ -297,7 +297,7 @@ class TestMailer < ActionMailer::Base subject "return path test" from "some.one@somewhere.test" headers["return-path"] = "another@somewhere.test" - body "testing" + body "testing" end def subject_with_i18n(recipient) @@ -417,7 +417,7 @@ class ActionMailerTest < Test::Unit::TestCase end def test_custom_template - expected = new_mail + expected = new_mail expected.to = @recipient expected.subject = "[Signed up] Welcome #{@recipient}" expected.body = "Hello there, \n\nMr. #{@recipient}" @@ -436,7 +436,7 @@ class ActionMailerTest < Test::Unit::TestCase assert ActionView::Template.template_handler_extensions.include?("haml"), "haml extension was not registered" # N.b., custom_templating_extension.text.plain.haml is expected to be in fixtures/test_mailer directory - expected = new_mail + expected = new_mail expected.to = @recipient expected.subject = "[Signed up] Welcome #{@recipient}" expected.body = "Hello there, \n\nMr. #{@recipient}" @@ -453,7 +453,7 @@ class ActionMailerTest < Test::Unit::TestCase end def test_cancelled_account - expected = new_mail + expected = new_mail expected.to = @recipient expected.subject = "[Cancelled] Goodbye #{@recipient}" expected.body = "Goodbye, Mr. #{@recipient}" @@ -477,7 +477,7 @@ class ActionMailerTest < Test::Unit::TestCase end def test_cc_bcc - expected = new_mail + expected = new_mail expected.to = @recipient expected.subject = "testing bcc/cc" expected.body = "Nothing to see here." @@ -602,7 +602,7 @@ class ActionMailerTest < Test::Unit::TestCase def test_unencoded_subject TestMailer.delivery_method = :test - expected = new_mail + expected = new_mail expected.to = @recipient expected.subject = "testing unencoded subject" expected.body = "Nothing to see here." diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index ea740f7..d5704eb 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -515,7 +515,7 @@ class FilterTest < ActionController::TestCase assert assigns["ran_proc_filter2"] test_process(AnomolousYetValidConditionController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] assert !assigns["ran_class_filter"] assert !assigns["ran_proc_filter1"] assert !assigns["ran_proc_filter2"] @@ -530,16 +530,16 @@ class FilterTest < ActionController::TestCase test_process(ConditionalCollectionFilterController) assert_equal %w( ensure_login ), assigns["ran_filter"] test_process(ConditionalCollectionFilterController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] test_process(ConditionalCollectionFilterController, "another_action") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] end def test_running_only_condition_filters test_process(OnlyConditionSymController) assert_equal %w( ensure_login ), assigns["ran_filter"] test_process(OnlyConditionSymController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] test_process(OnlyConditionProcController) assert assigns["ran_proc_filter"] @@ -556,7 +556,7 @@ class FilterTest < ActionController::TestCase test_process(ExceptConditionSymController) assert_equal %w( ensure_login ), assigns["ran_filter"] test_process(ExceptConditionSymController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] test_process(ExceptConditionProcController) assert assigns["ran_proc_filter"] @@ -573,7 +573,7 @@ class FilterTest < ActionController::TestCase test_process(BeforeAndAfterConditionController) assert_equal %w( ensure_login clean_up_tmp), assigns["ran_filter"] test_process(BeforeAndAfterConditionController, "show_without_filter") - assert_equal nil, assigns["ran_filter"] + assert_nil assigns["ran_filter"] end def test_around_filter @@ -674,7 +674,7 @@ class FilterTest < ActionController::TestCase def test_changing_the_requirements test_process(ChangingTheRequirementsController, "go_wild") - assert_equal nil, assigns['ran_filter'] + assert_nil assigns['ran_filter'] end def test_a_rescuing_around_filter diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index c910cb4..e1a35be 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -22,8 +22,8 @@ class DirtyTest < ActiveModel::TestCase test "changes accessible through both strings and symbols" do model = DirtyModel.new model.name = "David" - assert !model.changes[:name].nil? - assert !model.changes['name'].nil? + assert_not_nil model.changes[:name] + assert_not_nil model.changes['name'] end end diff --git a/activemodel/test/cases/serializeration/xml_serialization_test.rb b/activemodel/test/cases/serializeration/xml_serialization_test.rb index 4e8e4ef..7b9ef3e 100644 --- a/activemodel/test/cases/serializeration/xml_serialization_test.rb +++ b/activemodel/test/cases/serializeration/xml_serialization_test.rb @@ -69,7 +69,7 @@ class XmlSerializationTest < ActiveModel::TestCase test "should allow skipped types" do @xml = @contact.to_xml :skip_types => true - assert %r{25}.match(@xml) + assert_match %r{25}, @xml end test "should include yielded additions" do diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb index 8b6ec04..e5fc1a2 100644 --- a/activerecord/test/cases/aggregations_test.rb +++ b/activerecord/test/cases/aggregations_test.rb @@ -58,36 +58,36 @@ class AggregationsTest < ActiveRecord::TestCase end def test_gps_equality - assert GpsLocation.new('39x110') == GpsLocation.new('39x110') + assert_equal GpsLocation.new('39x110'), GpsLocation.new('39x110') end def test_gps_inequality - assert GpsLocation.new('39x110') != GpsLocation.new('39x111') + assert_not_equal GpsLocation.new('39x110'), GpsLocation.new('39x111') end def test_allow_nil_gps_is_nil - assert_equal nil, customers(:zaphod).gps_location + assert_nil customers(:zaphod).gps_location end def test_allow_nil_gps_set_to_nil customers(:david).gps_location = nil customers(:david).save customers(:david).reload - assert_equal nil, customers(:david).gps_location + assert_nil customers(:david).gps_location end def test_allow_nil_set_address_attributes_to_nil customers(:zaphod).address = nil - assert_equal nil, customers(:zaphod).attributes[:address_street] - assert_equal nil, customers(:zaphod).attributes[:address_city] - assert_equal nil, customers(:zaphod).attributes[:address_country] + assert_nil customers(:zaphod).attributes[:address_street] + assert_nil customers(:zaphod).attributes[:address_city] + assert_nil customers(:zaphod).attributes[:address_country] end def test_allow_nil_address_set_to_nil customers(:zaphod).address = nil customers(:zaphod).save customers(:zaphod).reload - assert_equal nil, customers(:zaphod).address + assert_nil customers(:zaphod).address end def test_nil_raises_error_when_allow_nil_is_false @@ -104,9 +104,9 @@ class AggregationsTest < ActiveRecord::TestCase def test_nil_assignment_results_in_nil customers(:david).gps_location = GpsLocation.new('39x111') - assert_not_equal nil, customers(:david).gps_location + assert_not_nil customers(:david).gps_location customers(:david).gps_location = nil - assert_equal nil, customers(:david).gps_location + assert_nil customers(:david).gps_location end def test_custom_constructor diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index be77ee4..9258c98 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -403,7 +403,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal saved_member.id, sponsor.sponsorable_id sponsor.sponsorable = new_member - assert_equal nil, sponsor.sponsorable_id + assert_nil sponsor.sponsorable_id end def test_polymorphic_assignment_with_primary_key_updates_foreign_id_field_for_new_and_saved_records @@ -415,7 +415,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal saved_writer.name, essay.writer_id essay.writer = new_writer - assert_equal nil, essay.writer_id + assert_nil essay.writer_id end def test_belongs_to_proxy_should_not_respond_to_private_methods diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 79e5ecf..445e688 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -110,7 +110,7 @@ class EagerAssociationTest < ActiveRecord::TestCase author = assert_queries(3) { Author.find(author_id, :include => {:posts_with_comments => :comments}) } # find the author, then find the posts, then find the comments author.posts_with_comments.each do |post_with_comments| assert_equal post_with_comments.comments.length, post_with_comments.comments.count - assert_equal nil, post_with_comments.comments.uniq! + assert_nil post_with_comments.comments.uniq! end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 6e47967..8e5bc56 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -175,14 +175,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase c = Client.new assert_nil c.firm - if c.firm - assert false, "belongs_to failed if check" - end - - unless c.firm - else - assert false, "belongs_to failed unless check" - end + flunk "belongs_to failed if check" if c.firm end def test_find_ids @@ -620,7 +613,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [], Client.destroyed_client_ids[firm.id] # Should be destroyed since the association is exclusively dependent. - assert Client.find_by_id(client_id).nil? + assert_nil Client.find_by_id(client_id) end def test_dependent_association_respects_optional_conditions_on_delete @@ -669,7 +662,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase old_record = firm.clients_using_primary_key_with_delete_all.first firm = Firm.find(:first) firm.destroy - assert Client.find_by_id(old_record.id).nil? + assert_nil Client.find_by_id(old_record.id) end def test_creation_respects_hash_condition diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ff79919..e4dd810 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -63,8 +63,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_queries(1) { posts(:thinking) } assert_queries(0) do - posts(:thinking).people.build(:first_name=>"Bob") - posts(:thinking).people.new(:first_name=>"Ted") + posts(:thinking).people.build(:first_name => "Bob") + posts(:thinking).people.new(:first_name => "Ted") end # Should only need to load the association once diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 8f55409..469a21b 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -149,7 +149,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase num_accounts = Account.count firm = Firm.find(1) - assert !firm.account.nil? + assert_not_nil firm.account account_id = firm.account.id assert_equal [], Account.destroyed_account_ids[firm.id] @@ -162,7 +162,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase num_accounts = Account.count firm = ExclusivelyDependentFirm.find(9) - assert !firm.account.nil? + assert_not_nil firm.account account_id = firm.account.id assert_equal [], Account.destroyed_account_ids[firm.id] @@ -181,7 +181,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm = RestrictedFirm.new(:name => 'restrict') firm.save! account = firm.create_account(:credit_limit => 10) - assert !firm.account.nil? + assert_not_nil firm.account assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } end @@ -246,7 +246,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_dependence_with_missing_association Account.destroy_all firm = Firm.find(1) - assert firm.account.nil? + assert_nil firm.account firm.destroy end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index dca72be..447fe4d 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -280,12 +280,12 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_has_many_find_conditions assert_equal categories(:general), authors(:david).categories.find(:first, :conditions => "categories.name = 'General'") - assert_equal nil, authors(:david).categories.find(:first, :conditions => "categories.name = 'Technology'") + assert_nil authors(:david).categories.find(:first, :conditions => "categories.name = 'Technology'") end def test_has_many_class_methods_called_by_method_missing assert_equal categories(:general), authors(:david).categories.find_all_by_name('General').first - assert_equal nil, authors(:david).categories.find_by_name('Technology') + assert_nil authors(:david).categories.find_by_name('Technology') end def test_has_many_array_methods_called_by_method_missing diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 055590d..d59fa0a 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -310,7 +310,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end assert !@target.instance_method_already_implemented?(:title) topic = @target.new - assert_equal nil, topic.title + assert_nil topic.title Object.send(:undef_method, :title) # remove test method from object end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 7c4d92f..84f57f2 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1369,37 +1369,37 @@ class BasicsTest < ActiveRecord::TestCase end def test_new_record_returns_boolean - assert_equal Topic.new.new_record?, true - assert_equal Topic.find(1).new_record?, false + assert_equal true, Topic.new.new_record? + assert_equal false, Topic.find(1).new_record? end def test_destroyed_returns_boolean developer = Developer.first - assert_equal developer.destroyed?, false + assert_equal false, developer.destroyed? developer.destroy - assert_equal developer.destroyed?, true + assert_equal true, developer.destroyed? developer = Developer.last - assert_equal developer.destroyed?, false + assert_equal false, developer.destroyed? developer.delete - assert_equal developer.destroyed?, true + assert_equal true, developer.destroyed? end def test_persisted_returns_boolean developer = Developer.new(:name => "Jose") - assert_equal developer.persisted?, false + assert_equal false, developer.persisted? developer.save! - assert_equal developer.persisted?, true + assert_equal true, developer.persisted? developer = Developer.first - assert_equal developer.persisted?, true + assert_equal true, developer.persisted? developer.destroy - assert_equal developer.persisted?, false + assert_equal false, developer.persisted? developer = Developer.last - assert_equal developer.persisted?, true + assert_equal true, developer.persisted? developer.delete - assert_equal developer.persisted?, false + assert_equal false, developer.persisted? end def test_clone @@ -1427,7 +1427,7 @@ class BasicsTest < ActiveRecord::TestCase # test if saved clone object differs from original cloned_topic.save assert !cloned_topic.new_record? - assert cloned_topic.id != topic.id + assert_not_equal cloned_topic.id, topic.id end def test_clone_with_aggregate_of_same_name_as_attribute @@ -1447,7 +1447,7 @@ class BasicsTest < ActiveRecord::TestCase assert clone.save assert !clone.new_record? - assert clone.id != dev.id + assert_not_equal clone.id, dev.id end def test_clone_preserves_subtype diff --git a/activerecord/test/cases/column_definition_test.rb b/activerecord/test/cases/column_definition_test.rb index 8b6c019..b576734 100644 --- a/activerecord/test/cases/column_definition_test.rb +++ b/activerecord/test/cases/column_definition_test.rb @@ -71,17 +71,17 @@ class ColumnDefinitionTest < ActiveRecord::TestCase if current_adapter?(:PostgreSQLAdapter) def test_bigint_column_should_map_to_integer bigint_column = ActiveRecord::ConnectionAdapters::PostgreSQLColumn.new('number', nil, "bigint") - assert_equal bigint_column.type, :integer + assert_equal :integer, bigint_column.type end def test_smallint_column_should_map_to_integer smallint_column = ActiveRecord::ConnectionAdapters::PostgreSQLColumn.new('number', nil, "smallint") - assert_equal smallint_column.type, :integer + assert_equal :integer, smallint_column.type end def test_uuid_column_should_map_to_string uuid_column = ActiveRecord::ConnectionAdapters::PostgreSQLColumn.new('unique_id', nil, "uuid") - assert_equal uuid_column.type, :string + assert_equal :string, uuid_column.type end end end diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index bba216a..39aafa1 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -67,8 +67,8 @@ if current_adapter?(:MysqlAdapter) assert_equal '', klass.columns_hash['non_null_blob'].default assert_equal '', klass.columns_hash['non_null_text'].default - assert_equal nil, klass.columns_hash['null_blob'].default - assert_equal nil, klass.columns_hash['null_text'].default + assert_nil klass.columns_hash['null_blob'].default + assert_nil klass.columns_hash['null_text'].default assert_nothing_raised do instance = klass.create! -- 1.6.5.2