Merge pull request #23789 from wisetara/wisetara/deprecate-args-ActiveSupport__TestCase#assert_nothing_raised-for-pr

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

actioncable/test/subscription_adapter/base_test.rb

@@ -43,7 +43,7 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase
 
     assert_respond_to(SuccessAdapter.new(@server), :broadcast)
 
-    assert_nothing_raised NotImplementedError do
+    assert_nothing_raised do
       broadcast
     end
   end
@@ -55,7 +55,7 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase
 
     assert_respond_to(SuccessAdapter.new(@server), :subscribe)
 
-    assert_nothing_raised NotImplementedError do
+    assert_nothing_raised do
       subscribe
     end
   end
@@ -66,7 +66,7 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase
 
     assert_respond_to(SuccessAdapter.new(@server), :unsubscribe)
 
-    assert_nothing_raised NotImplementedError do
+    assert_nothing_raised do
       unsubscribe
     end
   end

actionpack/test/controller/integration_test.rb

@@ -390,7 +390,7 @@ class IntegrationTestUsesCorrectClass < ActionDispatch::IntegrationTest
     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

activemodel/test/cases/validations/format_validation_test.rb

@@ -73,7 +73,7 @@ class PresenceValidationTest < ActiveModel::TestCase
   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

activemodel/test/cases/validations/inclusion_validation_test.rb

@@ -58,9 +58,9 @@ class InclusionValidationTest < ActiveModel::TestCase
     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

activerecord/test/cases/associations/eager_test.rb

@@ -1216,7 +1216,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
   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

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

@@ -938,7 +938,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_with_symbol_class_name
-    assert_nothing_raised NoMethodError do
+    assert_nothing_raised do
       DeveloperWithSymbolClassName.new
     end
   end

activerecord/test/cases/associations/inverse_associations_test.rb

@@ -130,15 +130,15 @@ end
 
 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
@@ -666,7 +666,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
 
   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
@@ -676,7 +676,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
 
   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
@@ -688,7 +688,7 @@ 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
@@ -696,7 +696,7 @@ class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase
   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')

activerecord/test/cases/associations/join_model_test.rb

@@ -88,7 +88,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
 
   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

activerecord/test/cases/calculations_test.rb

@@ -124,7 +124,7 @@ class CalculationsTest < ActiveRecord::TestCase
   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
@@ -742,7 +742,7 @@ class CalculationsTest < ActiveRecord::TestCase
   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

activerecord/test/cases/counter_cache_test.rb

@@ -151,7 +151,7 @@ class CounterCacheTest < ActiveRecord::TestCase
 
   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

activerecord/test/cases/finder_test.rb

@@ -43,7 +43,7 @@ class FinderTest < ActiveRecord::TestCase
     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
@@ -540,7 +540,7 @@ class FinderTest < ActiveRecord::TestCase
     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')
@@ -1086,7 +1086,7 @@ class FinderTest < ActiveRecord::TestCase
   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

activerecord/test/cases/migration_test.rb

@@ -739,7 +739,7 @@ class ExplicitlyNamedIndexMigrationTest < ActiveRecord::TestCase
       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

activerecord/test/cases/modules_test.rb

@@ -63,7 +63,7 @@ class ModulesTest < ActiveRecord::TestCase
   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
@@ -72,7 +72,7 @@ class ModulesTest < ActiveRecord::TestCase
   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

activerecord/test/cases/multiple_db_test.rb

@@ -104,7 +104,7 @@ class MultipleDbTest < ActiveRecord::TestCase
     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

activerecord/test/cases/nested_attributes_test.rb

@@ -76,7 +76,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
 
     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
@@ -180,7 +180,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
 
     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
@@ -511,7 +511,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase
   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
 
@@ -519,7 +519,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase
     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
@@ -536,7 +536,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase
     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
@@ -713,7 +713,7 @@ module NestedAttributesOnACollectionAssociationTests
   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
@@ -748,8 +748,8 @@ module NestedAttributesOnACollectionAssociationTests
   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")
@@ -824,7 +824,7 @@ module NestedAttributesOnACollectionAssociationTests
 
   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

activerecord/test/cases/primary_keys_test.rb

@@ -130,7 +130,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase
   end
 
   def test_supports_primary_key
-    assert_nothing_raised NoMethodError do
+    assert_nothing_raised do
       ActiveRecord::Base.connection.supports_primary_key?
     end
   end

activerecord/test/cases/validations_test.rb

@@ -130,7 +130,7 @@ class ValidationsTest < ActiveRecord::TestCase
   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

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*
@@ -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.

activesupport/lib/active_support/test_case.rb

@@ -75,6 +75,10 @@ module ActiveSupport
     #     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

activesupport/test/caching_test.rb

@@ -836,7 +836,7 @@ class FileStoreTest < ActiveSupport::TestCase
   # 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
@@ -844,7 +844,7 @@ class FileStoreTest < ActiveSupport::TestCase
   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

activesupport/test/core_ext/marshal_test.rb

@@ -96,7 +96,7 @@ class MarshalTest < ActiveSupport::TestCase
         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
 

activesupport/test/core_ext/time_ext_test.rb

@@ -392,7 +392,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase
     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

guides/source/testing.md

@@ -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.|

railties/test/application/initializers/frameworks_test.rb

@@ -16,7 +16,7 @@ module ApplicationTests
 
     # AC & AM
     test "set load paths set only if action controller or action mailer are in use" do
-      assert_nothing_raised NameError do
+      assert_nothing_raised do
         add_to_config <<-RUBY
           config.root = "#{app_path}"
         RUBY

railties/test/application/loading_test.rb

@@ -56,10 +56,10 @@ class LoadingTest < ActiveSupport::TestCase
 
     require "#{rails_root}/config/environment"
 
-    assert_nothing_raised(NameError) { Trackable }
-    assert_nothing_raised(NameError) { EmailLoggable }
-    assert_nothing_raised(NameError) { Orderable }
-    assert_nothing_raised(NameError) { Matchable }
+    assert_nothing_raised { Trackable }
+    assert_nothing_raised { EmailLoggable }
+    assert_nothing_raised { Orderable }
+    assert_nothing_raised { Matchable }
   end
 
   test "models without table do not panic on scope definitions when loaded" do

railties/test/application/middleware/exceptions_test.rb

@@ -85,7 +85,7 @@ module ApplicationTests
     test "unspecified route when action_dispatch.show_exceptions is set shows 404" do
       app.config.action_dispatch.show_exceptions = true
 
-      assert_nothing_raised(ActionController::RoutingError) do
+      assert_nothing_raised do
         get '/foo'
         assert_match "The page you were looking for doesn't exist.", last_response.body
       end
@@ -95,7 +95,7 @@ module ApplicationTests
       app.config.action_dispatch.show_exceptions = true
       app.config.consider_all_requests_local = true
 
-      assert_nothing_raised(ActionController::RoutingError) do
+      assert_nothing_raised do
         get '/foo'
         assert_match "No route matches", last_response.body
       end

railties/test/application/middleware/remote_ip_test.rb

@@ -36,10 +36,10 @@ module ApplicationTests
 
     test "works with both headers individually" do
       make_basic_app
-      assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+      assert_nothing_raised do
         assert_equal "1.1.1.1", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1")
       end
-      assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+      assert_nothing_raised do
         assert_equal "1.1.1.2", remote_ip("HTTP_CLIENT_IP" => "1.1.1.2")
       end
     end
@@ -49,7 +49,7 @@ module ApplicationTests
         app.config.action_dispatch.ip_spoofing_check = false
       end
 
-      assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+      assert_nothing_raised do
         assert_equal "1.1.1.1", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2")
       end
     end