From 947e1d5e85fa87128b7c5e21da55b92826cd7801 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Sat, 5 Jan 2013 17:59:36 +0100 Subject: [PATCH] deprecate `assert_blank` and `assert_present`. They don't add any benefits over `assert object.blank?` and `assert object.present?` --- .../controller/action_pack_assertions_test.rb | 2 +- .../test/controller/output_escaping_test.rb | 2 +- actionpack/test/controller/render_test.rb | 22 ++++++------ .../request_forgery_protection_test.rb | 2 +- actionpack/test/controller/test_case_test.rb | 2 +- .../test/template/atom_feed_helper_test.rb | 2 +- .../cases/associations/join_model_test.rb | 4 +-- .../cases/deprecated_dynamic_methods_test.rb | 4 +-- .../test/cases/relation_scoping_test.rb | 2 +- activerecord/test/cases/relations_test.rb | 4 +-- activesupport/CHANGELOG.md | 5 +++ .../lib/active_support/testing/assertions.rb | 2 ++ activesupport/test/caching_test.rb | 6 ++-- activesupport/test/deprecation_test.rb | 7 ++-- activesupport/test/test_test.rb | 36 +++++++++++-------- guides/source/4_0_release_notes.md | 1 + 16 files changed, 60 insertions(+), 43 deletions(-) diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index b94f45bfe7a36..5d727b3811ab6 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -259,7 +259,7 @@ def test_empty_flash def test_flash_exist process :flash_me assert flash.any? - assert_present flash['hello'] + assert flash['hello'].present? end def test_flash_does_not_exist diff --git a/actionpack/test/controller/output_escaping_test.rb b/actionpack/test/controller/output_escaping_test.rb index f6913a21389bd..43a8c05cdaf84 100644 --- a/actionpack/test/controller/output_escaping_test.rb +++ b/actionpack/test/controller/output_escaping_test.rb @@ -3,7 +3,7 @@ class OutputEscapingTest < ActiveSupport::TestCase test "escape_html shouldn't die when passed nil" do - assert_blank ERB::Util.h(nil) + assert ERB::Util.h(nil).blank? end test "escapeHTML should escape strings" do diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index c7a66f42989e8..84a173980cb8a 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1221,27 +1221,27 @@ def test_overwritting_rendering_relative_file_with_extension def test_head_created post :head_created - assert_blank @response.body + assert @response.body.blank? assert_response :created end def test_head_created_with_application_json_content_type post :head_created_with_application_json_content_type - assert_blank @response.body + assert @response.body.blank? assert_equal "application/json", @response.header["Content-Type"] assert_response :created end def test_head_ok_with_image_png_content_type post :head_ok_with_image_png_content_type - assert_blank @response.body + assert @response.body.blank? assert_equal "image/png", @response.header["Content-Type"] assert_response :ok end def test_head_with_location_header get :head_with_location_header - assert_blank @response.body + assert @response.body.blank? assert_equal "/foo", @response.headers["Location"] assert_response :ok end @@ -1254,7 +1254,7 @@ def test_head_with_location_object end get :head_with_location_object - assert_blank @response.body + assert @response.body.blank? assert_equal "http://www.nextangle.com/customers/1", @response.headers["Location"] assert_response :ok end @@ -1262,14 +1262,14 @@ def test_head_with_location_object def test_head_with_custom_header get :head_with_custom_header - assert_blank @response.body + assert @response.body.blank? assert_equal "something", @response.headers["X-Custom-Header"] assert_response :ok end def test_head_with_www_authenticate_header get :head_with_www_authenticate_header - assert_blank @response.body + assert @response.body.blank? assert_equal "something", @response.headers["WWW-Authenticate"] assert_response :ok end @@ -1603,7 +1603,7 @@ def test_request_not_modified @request.if_modified_since = @last_modified get :conditional_hello assert_equal 304, @response.status.to_i - assert_blank @response.body + assert @response.body.blank? assert_equal @last_modified, @response.headers['Last-Modified'] end @@ -1618,7 +1618,7 @@ def test_request_modified @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' get :conditional_hello assert_equal 200, @response.status.to_i - assert_present @response.body + assert @response.body.present? assert_equal @last_modified, @response.headers['Last-Modified'] end @@ -1632,7 +1632,7 @@ def test_request_not_modified_with_record @request.if_modified_since = @last_modified get :conditional_hello_with_record assert_equal 304, @response.status.to_i - assert_blank @response.body + assert @response.body.blank? assert_equal @last_modified, @response.headers['Last-Modified'] end @@ -1647,7 +1647,7 @@ def test_request_modified_with_record @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' get :conditional_hello_with_record assert_equal 200, @response.status.to_i - assert_present @response.body + assert @response.body.present? assert_equal @last_modified, @response.headers['Last-Modified'] end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 1f637eb7919fc..523a8d0572175 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -320,7 +320,7 @@ def test_should_allow_all_methods_without_token test 'should not emit a csrf-token meta tag' do get :meta - assert_blank @response.body + assert @response.body.blank? end end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index bdca1d4d778ef..e4d78d58b936a 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -692,7 +692,7 @@ def test_params_reset_after_post_request assert_equal "bar", @request.params[:foo] @request.recycle! post :no_op - assert_blank @request.params[:foo] + assert @request.params[:foo].blank? end def test_symbolized_path_params_reset_after_request diff --git a/actionpack/test/template/atom_feed_helper_test.rb b/actionpack/test/template/atom_feed_helper_test.rb index 89aae4ac56b4d..63b5ac0fab7c8 100644 --- a/actionpack/test/template/atom_feed_helper_test.rb +++ b/actionpack/test/template/atom_feed_helper_test.rb @@ -238,7 +238,7 @@ def test_providing_builder_to_atom_feed get :index, :id=>"provide_builder" # because we pass in the non-default builder, the content generated by the # helper should go 'nowhere'. Leaving the response body blank. - assert_blank @response.body + assert @response.body.blank? end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 9b00c21b5229b..10ec33be75e4f 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -443,8 +443,8 @@ def test_add_to_self_referential_has_many_through def test_has_many_through_uses_conditions_specified_on_the_has_many_association author = Author.first - assert_present author.comments - assert_blank author.nonexistant_comments + assert author.comments.present? + assert author.nonexistant_comments.blank? end def test_has_many_through_uses_correct_attributes diff --git a/activerecord/test/cases/deprecated_dynamic_methods_test.rb b/activerecord/test/cases/deprecated_dynamic_methods_test.rb index dde36e7f7269d..32eb87d5228bc 100644 --- a/activerecord/test/cases/deprecated_dynamic_methods_test.rb +++ b/activerecord/test/cases/deprecated_dynamic_methods_test.rb @@ -568,9 +568,9 @@ def test_dynamic_scope end def test_dynamic_scope_should_create_methods_after_hitting_method_missing - assert_blank @test_klass.methods.grep(/scoped_by_type/) + assert @test_klass.methods.grep(/scoped_by_type/).blank? @test_klass.scoped_by_type(nil) - assert_present @test_klass.methods.grep(/scoped_by_type/) + assert @test_klass.methods.grep(/scoped_by_type/).present? end def test_dynamic_scope_with_less_number_of_arguments diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index d318dab1e13db..78fb91d321396 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -227,7 +227,7 @@ def test_nested_scoped_create def test_nested_exclusive_scope_for_create comment = Comment.create_with(:body => "Hey guys, nested scopes are broken. Please fix!").scoping do Comment.unscoped.create_with(:post_id => 1).scoping do - assert_blank Comment.new.body + assert Comment.new.body.blank? Comment.create :body => "Hey guys" end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 0cd838c0b0c92..3a499a2025249 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -509,7 +509,7 @@ def test_find_ids def test_find_in_empty_array authors = Author.all.where(:id => []) - assert_blank authors.to_a + assert authors.to_a.blank? end def test_where_with_ar_object @@ -723,7 +723,7 @@ def test_relation_merging_with_eager_load def test_relation_merging_with_locks devs = Developer.lock.where("salary >= 80000").order("id DESC").merge(Developer.limit(2)) - assert_present devs.locked + assert devs.locked.present? end def test_relation_merging_with_preload diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index cc23ceb02ce11..08bec2f4aebcc 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Deprecate `assert_present` and `assert_blank` in favor of + `assert object.blank?` and `assert object.present?` + + *Yves Senn* + * Change `String#to_date` to use `Date.parse`. This gives more consistent error messages and allows the use of partial dates. diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index 88aebba5c5b49..175f7ffe5aa28 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -103,6 +103,7 @@ def assert_no_difference(expression, message = nil, &block) # # assert_blank [], 'this should be blank' def assert_blank(object, message=nil) + ActiveSupport::Deprecation.warn('"assert_blank" is deprecated. Please use "assert object.blank?" instead') message ||= "#{object.inspect} is not blank" assert object.blank?, message end @@ -117,6 +118,7 @@ def assert_blank(object, message=nil) # # assert_present({ data: 'x' }, 'this should not be blank') def assert_present(object, message=nil) + ActiveSupport::Deprecation.warn('"assert_present" is deprecated. Please use "assert object.present?" instead') message ||= "#{object.inspect} is blank" assert object.present?, message end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index a2e2c51283805..5158bbc196f7b 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -675,7 +675,7 @@ def test_delete_matched_when_cache_directory_does_not_exist def test_log_exception_when_cache_read_fails File.expects(:exist?).raises(StandardError, "failed") @cache.send(:read_entry, "winston", {}) - assert_present @buffer.string + assert @buffer.string.present? end end @@ -897,12 +897,12 @@ def setup def test_logging @cache.fetch('foo') { 'bar' } - assert_present @buffer.string + assert @buffer.string.present? end def test_mute_logging @cache.mute { @cache.fetch('foo') { 'bar' } } - assert_blank @buffer.string + assert @buffer.string.blank? end end diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index 332100f5a126b..c1a468ec86143 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -119,9 +119,10 @@ def test_default_silence_behavior ActiveSupport::Deprecation.behavior = :silence behavior = ActiveSupport::Deprecation.behavior.first - assert_blank capture(:stderr) { + stderr_output = capture(:stderr) { assert_nil behavior.call('Some error!', ['call stack!']) } + assert stderr_output.blank? end def test_deprecated_instance_variable_proxy @@ -254,10 +255,10 @@ def test_deprecated_constant_with_deprecator_given klass::OLD.to_s end end - + def test_deprecated_instance_variable_with_instance_deprecator deprecator = deprecator_with_messages - + klass = Class.new() do def initialize(deprecator) @request = ActiveSupport::Deprecation::DeprecatedInstanceVariableProxy.new(self, :request, :@request, deprecator) diff --git a/activesupport/test/test_test.rb b/activesupport/test/test_test.rb index 0f93c8b59eb35..3e6ac811a481c 100644 --- a/activesupport/test/test_test.rb +++ b/activesupport/test/test_test.rb @@ -92,17 +92,21 @@ class AssertBlankTest < ActiveSupport::TestCase NOT_BLANK = [ EmptyFalse.new, Object.new, true, 0, 1, 'x', [nil], { nil => 0 } ] def test_assert_blank_true - BLANK.each { |v| assert_blank v } + BLANK.each { |value| + assert_deprecated { assert_blank value } + } end def test_assert_blank_false NOT_BLANK.each { |v| - begin - assert_blank v - fail 'should not get to here' - rescue Exception => e - assert_match(/is not blank/, e.message) - end + assert_deprecated { + begin + assert_blank v + fail 'should not get to here' + rescue Exception => e + assert_match(/is not blank/, e.message) + end + } } end end @@ -112,17 +116,21 @@ class AssertPresentTest < ActiveSupport::TestCase NOT_BLANK = [ EmptyFalse.new, Object.new, true, 0, 1, 'x', [nil], { nil => 0 } ] def test_assert_present_true - NOT_BLANK.each { |v| assert_present v } + NOT_BLANK.each { |v| + assert_deprecated { assert_present v } + } end def test_assert_present_false BLANK.each { |v| - begin - assert_present v - fail 'should not get to here' - rescue Exception => e - assert_match(/is blank/, e.message) - end + assert_deprecated { + begin + assert_present v + fail 'should not get to here' + rescue Exception => e + assert_match(/is blank/, e.message) + end + } } end end diff --git a/guides/source/4_0_release_notes.md b/guides/source/4_0_release_notes.md index 34d2c0981205b..80af0c1225584 100644 --- a/guides/source/4_0_release_notes.md +++ b/guides/source/4_0_release_notes.md @@ -142,6 +142,7 @@ Please refer to the [Changelog](https://github.com/rails/rails/blob/master/activ * BufferedLogger is deprecated. Use ActiveSupport::Logger, or the logger from Ruby stdlib. +* Deprecate `assert_present` and `assert_blank` in favor of `assert object.blank?` and `assert object.present?` Action Pack -----------