Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wisetara/deprecate args active support test case#assert nothing raised for pr #23789

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions actioncable/test/subscription_adapter/base_test.rb
Expand Up @@ -43,7 +43,7 @@ class BrokenAdapter < ActionCable::SubscriptionAdapter::Base

assert_respond_to(SuccessAdapter.new(@server), :broadcast)

assert_nothing_raised NotImplementedError do
assert_nothing_raised do
broadcast
end
end
Expand All @@ -55,7 +55,7 @@ class BrokenAdapter < ActionCable::SubscriptionAdapter::Base

assert_respond_to(SuccessAdapter.new(@server), :subscribe)

assert_nothing_raised NotImplementedError do
assert_nothing_raised do
subscribe
end
end
Expand All @@ -66,7 +66,7 @@ class BrokenAdapter < ActionCable::SubscriptionAdapter::Base

assert_respond_to(SuccessAdapter.new(@server), :unsubscribe)

assert_nothing_raised NotImplementedError do
assert_nothing_raised do
unsubscribe
end
end
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/controller/integration_test.rb
Expand Up @@ -390,7 +390,7 @@ def test_integration_methods_called
reset!

%w( get post head patch put delete ).each do |verb|
assert_nothing_raised("'#{verb}' should use integration test methods") { __send__(verb, '/') }
assert_nothing_raised { __send__(verb, '/') }
end
end
end
Expand Down
Expand Up @@ -73,7 +73,7 @@ def test_validate_format_of_with_multiline_regexp_should_raise_error
end

def test_validate_format_of_with_multiline_regexp_and_option
assert_nothing_raised(ArgumentError) do
assert_nothing_raised do
Topic.validates_format_of(:title, with: /^Valid Title$/, multiline: true)
end
end
Expand Down
Expand Up @@ -58,9 +58,9 @@ def test_validates_inclusion_of
assert_raise(ArgumentError) { Topic.validates_inclusion_of(:title, in: nil) }
assert_raise(ArgumentError) { Topic.validates_inclusion_of(:title, in: 0) }

assert_nothing_raised(ArgumentError) { Topic.validates_inclusion_of(:title, in: "hi!") }
assert_nothing_raised(ArgumentError) { Topic.validates_inclusion_of(:title, in: {}) }
assert_nothing_raised(ArgumentError) { Topic.validates_inclusion_of(:title, in: []) }
assert_nothing_raised { Topic.validates_inclusion_of(:title, in: "hi!") }
assert_nothing_raised { Topic.validates_inclusion_of(:title, in: {}) }
assert_nothing_raised { Topic.validates_inclusion_of(:title, in: []) }
end

def test_validates_inclusion_of_with_allow_nil
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/associations/eager_test.rb
Expand Up @@ -1216,7 +1216,7 @@ def test_joins_with_includes_should_preload_via_joins
end

def test_join_eager_with_empty_order_should_generate_valid_sql
assert_nothing_raised(ActiveRecord::StatementInvalid) do
assert_nothing_raised do
Post.includes(:comments).order("").where(:comments => {:body => "Thank you for the welcome"}).first
end
end
Expand Down
Expand Up @@ -938,7 +938,7 @@ def test_habtm_with_reflection_using_class_name_and_fixtures
end

def test_with_symbol_class_name
assert_nothing_raised NoMethodError do
assert_nothing_raised do
DeveloperWithSymbolClassName.new
end
end
Expand Down
Expand Up @@ -130,15 +130,15 @@ def test_polymorphic_relationships_should_still_not_have_inverses_when_non_polym

class InverseAssociationTests < ActiveRecord::TestCase
def test_should_allow_for_inverse_of_options_in_associations
assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_many') do
assert_nothing_raised do
Class.new(ActiveRecord::Base).has_many(:wheels, :inverse_of => :car)
end

assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_one') do
assert_nothing_raised do
Class.new(ActiveRecord::Base).has_one(:engine, :inverse_of => :car)
end

assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on belongs_to') do
assert_nothing_raised do
Class.new(ActiveRecord::Base).belongs_to(:car, :inverse_of => :driver)
end
end
Expand Down Expand Up @@ -666,7 +666,7 @@ def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many

def test_trying_to_access_inverses_that_dont_exist_shouldnt_raise_an_error
# Ideally this would, if only for symmetry's sake with other association types
assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.horrible_polymorphic_man }
assert_nothing_raised { Face.first.horrible_polymorphic_man }
end

def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_an_error
Expand All @@ -676,7 +676,7 @@ def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_

def test_trying_to_set_polymorphic_inverses_that_dont_exist_on_the_instance_being_set_should_raise_an_error
# passes because Man does have the correct inverse_of
assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Man.first }
assert_nothing_raised { Face.first.polymorphic_man = Man.first }
# fails because Interest does have the correct inverse_of
assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Interest.first }
end
Expand All @@ -688,15 +688,15 @@ class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase
fixtures :men, :interests, :zines

def test_that_we_can_load_associations_that_have_the_same_reciprocal_name_from_different_models
assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do
assert_nothing_raised do
i = Interest.first
i.zine
i.man
end
end

def test_that_we_can_create_associations_that_have_the_same_reciprocal_name_from_different_models
assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do
assert_nothing_raised do
i = Interest.first
i.build_zine(:title => 'Get Some in Winter! 2008')
i.build_man(:name => 'Gordon')
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/associations/join_model_test.rb
Expand Up @@ -88,7 +88,7 @@ def test_polymorphic_has_many_going_through_join_model_with_include_on_source_re

def test_polymorphic_has_many_going_through_join_model_with_custom_select_and_joins
assert_equal tags(:general), tag = posts(:welcome).tags.add_joins_and_select.first
assert_nothing_raised(NoMethodError) { tag.author_id }
assert_nothing_raised { tag.author_id }
end

def test_polymorphic_has_many_going_through_join_model_with_custom_foreign_key
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/calculations_test.rb
Expand Up @@ -124,7 +124,7 @@ def test_should_group_by_summed_field
end

def test_should_generate_valid_sql_with_joins_and_group
assert_nothing_raised ActiveRecord::StatementInvalid do
assert_nothing_raised do
AuditLog.joins(:developer).group(:id).count
end
end
Expand Down Expand Up @@ -742,7 +742,7 @@ def test_calculation_grouped_by_association_doesnt_error_when_no_records_have_as
end

def test_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association
assert_nothing_raised ActiveRecord::StatementInvalid do
assert_nothing_raised do
developer = Developer.create!(name: 'developer')
developer.ratings.includes(comment: :post).where(posts: { id: 1 }).count
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/counter_cache_test.rb
Expand Up @@ -151,7 +151,7 @@ class ::SpecialReply < ::Reply

test "reset the right counter if two have the same foreign key" do
michael = people(:michael)
assert_nothing_raised(ActiveRecord::StatementInvalid) do
assert_nothing_raised do
Person.reset_counters(michael.id, :friends_too)
end
end
Expand Down
6 changes: 3 additions & 3 deletions activerecord/test/cases/finder_test.rb
Expand Up @@ -43,7 +43,7 @@ def test_find_with_proc_parameter_and_block
end
assert_equal "should happen", exception.message

assert_nothing_raised(RuntimeError) do
assert_nothing_raised do
Topic.all.find(-> { raise "should not happen" }) { |e| e.title == topics(:first).title }
end
end
Expand Down Expand Up @@ -540,7 +540,7 @@ def test_last_with_irreversible_order
assert_deprecated do
Topic.order("coalesce(author_name, title)").last
end
end
end

def test_last_on_relation_with_limit_and_offset
post = posts('sti_comments')
Expand Down Expand Up @@ -1086,7 +1086,7 @@ def test_find_without_primary_key
end

def test_finder_with_offset_string
assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.offset("3").to_a }
assert_nothing_raised { Topic.offset("3").to_a }
end

test "find_by with hash conditions returns the first matching record" do
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/migration_test.rb
Expand Up @@ -738,7 +738,7 @@ def test_drop_index_by_name
t.integer :value
end

assert_nothing_raised ArgumentError do
assert_nothing_raised do
connection.add_index :values, :value, name: 'a_different_name'
connection.remove_index :values, column: :value, name: 'a_different_name'
end
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/modules_test.rb
Expand Up @@ -63,7 +63,7 @@ def test_table_name
def test_assign_ids
firm = MyApplication::Business::Firm.first

assert_nothing_raised NameError, "Should be able to resolve all class constants via reflection" do
assert_nothing_raised do
firm.client_ids = [MyApplication::Business::Client.first.id]
end
end
Expand All @@ -72,7 +72,7 @@ def test_assign_ids
def test_eager_loading_in_modules
clients = []

assert_nothing_raised NameError, "Should be able to resolve all class constants via reflection" do
assert_nothing_raised do
clients << MyApplication::Business::Client.references(:accounts).merge!(:includes => {:firm => :account}, :where => 'accounts.id IS NOT NULL').find(3)
clients << MyApplication::Business::Client.includes(:firm => :account).find(3)
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/multiple_db_test.rb
Expand Up @@ -104,7 +104,7 @@ def test_count_on_custom_connection
def test_associations_should_work_when_model_has_no_connection
begin
ActiveRecord::Base.remove_connection
assert_nothing_raised ActiveRecord::ConnectionNotEstablished do
assert_nothing_raised do
College.first.courses.first
end
ensure
Expand Down
18 changes: 9 additions & 9 deletions activerecord/test/cases/nested_attributes_test.rb
Expand Up @@ -76,7 +76,7 @@ def test_should_disable_allow_destroy_by_default

pirate.update(ship_attributes: { '_destroy' => true, :id => ship.id })

assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.ship.reload }
assert_nothing_raised { pirate.ship.reload }
end

def test_a_model_should_respond_to_underscore_destroy_and_return_if_it_is_marked_for_destruction
Expand Down Expand Up @@ -180,7 +180,7 @@ def test_reject_if_with_blank_nested_attributes_id

pirate = Pirate.new(:catchphrase => "Stop wastin' me time")
pirate.ship_attributes = { :id => "" }
assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.save! }
assert_nothing_raised { pirate.save! }
end

def test_first_and_array_index_zero_methods_return_the_same_value_when_nested_attributes_are_set_to_update_existing_record
Expand Down Expand Up @@ -511,15 +511,15 @@ def test_should_unset_association_when_an_existing_record_is_destroyed
def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy
[nil, '0', 0, 'false', false].each do |not_truth|
@ship.update(pirate_attributes: { id: @ship.pirate.id, _destroy: not_truth })
assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload }
assert_nothing_raised { @ship.pirate.reload }
end
end

def test_should_not_destroy_an_existing_record_if_allow_destroy_is_false
Ship.accepts_nested_attributes_for :pirate, :allow_destroy => false, :reject_if => proc(&:empty?)

@ship.update(pirate_attributes: { id: @ship.pirate.id, _destroy: '1' })
assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload }
assert_nothing_raised { @ship.pirate.reload }
ensure
Ship.accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc(&:empty?)
end
Expand All @@ -536,7 +536,7 @@ def test_should_not_destroy_the_associated_model_until_the_parent_is_saved
pirate = @ship.pirate

@ship.attributes = { :pirate_attributes => { :id => pirate.id, '_destroy' => true } }
assert_nothing_raised(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) }
assert_nothing_raised { Pirate.find(pirate.id) }
@ship.save
assert_raise(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) }
end
Expand Down Expand Up @@ -713,7 +713,7 @@ def test_should_automatically_build_new_associated_models_for_each_entry_in_a_ha
end

def test_should_not_assign_destroy_key_to_a_record
assert_nothing_raised ActiveRecord::UnknownAttributeError do
assert_nothing_raised do
@pirate.send(association_setter, { 'foo' => { '_destroy' => '0' }})
end
end
Expand Down Expand Up @@ -748,8 +748,8 @@ def test_should_sort_the_hash_by_the_keys_before_building_new_associated_models
end

def test_should_raise_an_argument_error_if_something_else_than_a_hash_is_passed
assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, {}) }
assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, Hash.new) }
assert_nothing_raised { @pirate.send(association_setter, {}) }
assert_nothing_raised { @pirate.send(association_setter, Hash.new) }

exception = assert_raise ArgumentError do
@pirate.send(association_setter, "foo")
Expand Down Expand Up @@ -824,7 +824,7 @@ def test_validate_presence_of_parent_works_with_inverse_of

def test_can_use_symbols_as_object_identifier
@pirate.attributes = { :parrots_attributes => { :foo => { :name => 'Lovely Day' }, :bar => { :name => 'Blown Away' } } }
assert_nothing_raised(NoMethodError) { @pirate.save! }
assert_nothing_raised { @pirate.save! }
end

def test_numeric_column_changes_from_zero_to_no_empty_string
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/primary_keys_test.rb
Expand Up @@ -130,7 +130,7 @@ def test_instance_destroy_should_quote_pkey
end

def test_supports_primary_key
assert_nothing_raised NoMethodError do
assert_nothing_raised do
ActiveRecord::Base.connection.supports_primary_key?
end
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/validations_test.rb
Expand Up @@ -130,7 +130,7 @@ def test_save_without_validation
def test_validates_acceptance_of_with_non_existent_table
Object.const_set :IncorporealModel, Class.new(ActiveRecord::Base)

assert_nothing_raised ActiveRecord::StatementInvalid do
assert_nothing_raised do
IncorporealModel.validates_acceptance_of(:incorporeal_column)
end
end
Expand Down
11 changes: 10 additions & 1 deletion activesupport/CHANGELOG.md
@@ -1,3 +1,12 @@
* Deprecate arguments on `assert_nothing_raised`.

`assert_nothing_raised` does not assert the arguments that have been passed
in (usually a specific exception class) since the method only yields the
block. So as not to confuse the users that the arguments have meaning, they
are being deprecated.

*Tara Scherner de la Fuente*

* Make `benchmark('something', silence: true)` actually work

*DHH*
Expand All @@ -6,7 +15,7 @@

`#on_weekday?` returns `true` if the receiving date/time does not fall on a Saturday
or Sunday.

*Vipul A M*

* Add `Array#second_to_last` and `Array#third_to_last` methods.
Expand Down
4 changes: 4 additions & 0 deletions activesupport/lib/active_support/test_case.rb
Expand Up @@ -75,6 +75,10 @@ def test_order
# perform_service(param: 'no_exception')
# end
def assert_nothing_raised(*args)
if args.present?
ActiveSupport::Deprecation.warn("Passing arguments to assert_nothing_raised
is deprecated and will be removed in Rails 5.1.")
end
yield
end
end
Expand Down
4 changes: 2 additions & 2 deletions activesupport/test/caching_test.rb
Expand Up @@ -836,15 +836,15 @@ def test_key_transformation_max_filename_size
# If nothing has been stored in the cache, there is a chance the cache directory does not yet exist
# Ensure delete_matched gracefully handles this case
def test_delete_matched_when_cache_directory_does_not_exist
assert_nothing_raised(Exception) do
assert_nothing_raised do
ActiveSupport::Cache::FileStore.new('/test/cache/directory').delete_matched(/does_not_exist/)
end
end

def test_delete_does_not_delete_empty_parent_dir
sub_cache_dir = File.join(cache_dir, 'subdir/')
sub_cache_store = ActiveSupport::Cache::FileStore.new(sub_cache_dir)
assert_nothing_raised(Exception) do
assert_nothing_raised do
assert sub_cache_store.write('foo', 'bar')
assert sub_cache_store.delete('foo')
end
Expand Down
2 changes: 1 addition & 1 deletion activesupport/test/core_ext/marshal_test.rb
Expand Up @@ -96,7 +96,7 @@ class SomeClass
Marshal.load(dumped)
end

assert_nothing_raised("EM failed to load while we expect only SomeClass to fail loading") do
assert_nothing_raised do
EM.new
end

Expand Down
2 changes: 1 addition & 1 deletion activesupport/test/core_ext/time_ext_test.rb
Expand Up @@ -392,7 +392,7 @@ def test_change
assert_equal Time.local(2005,1,2,11,22,33, 8), Time.local(2005,1,2,11,22,33,44).change(:usec => 8)
assert_equal Time.local(2005,1,2,11,22,33, 8), Time.local(2005,1,2,11,22,33,2).change(:nsec => 8000)
assert_raise(ArgumentError) { Time.local(2005,1,2,11,22,33, 8).change(:usec => 1, :nsec => 1) }
assert_nothing_raised(ArgumentError) { Time.new(2015, 5, 9, 10, 00, 00, '+03:00').change(nsec: 999999999) }
assert_nothing_raised { Time.new(2015, 5, 9, 10, 00, 00, '+03:00').change(nsec: 999999999) }
end

def test_utc_change
Expand Down
2 changes: 1 addition & 1 deletion guides/source/testing.md
Expand Up @@ -287,7 +287,7 @@ specify to make your test failure messages clearer.
| `assert_not_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.|
| `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.|
| `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.|
| `assert_nothing_raised( exception1, exception2, ... ) { block }` | Ensures that the given block doesn't raise one of the given exceptions.|
| `assert_nothing_raised { block }` | Ensures that the given block doesn't raise any exceptions.|
| `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.|
| `assert_not_instance_of( class, obj, [msg] )` | Ensures that `obj` is not an instance of `class`.|
| `assert_kind_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class` or is descending from it.|
Expand Down