Prefer assert_raise instead of flunk + rescue to test for exceptions

Change most tests to make use of assert_raise returning the raised
exception rather than relying on a combination of flunk + rescue to
check for exception types/messages.

11 files changed, 52 insertions, 84 deletions

diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb
index ba4cffcd3e..b6b5a218cc 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -444,22 +444,18 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase
 
   def test_assert_response_uses_exception_message
     @controller = AssertResponseWithUnexpectedErrorController.new
-    get :index
+    e = assert_raise RuntimeError, 'Expected non-success response' do
+      get :index
+    end
     assert_response :success
-    flunk 'Expected non-success response'
-  rescue RuntimeError => e
-    assert e.message.include?('FAIL')
+    assert_includes 'FAIL', e.message
   end
 
   def test_assert_response_failure_response_with_no_exception
     @controller = AssertResponseWithUnexpectedErrorController.new
     get :show
-    assert_response :success
-    flunk 'Expected non-success response'
-  rescue ActiveSupport::TestCase::Assertion
-    # success
-  rescue
-    flunk "assert_response failed to handle failure response with missing, but optional, exception."
+    assert_response 500
+    assert_equal 'Boom', response.body
   end
 end
 
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index 2c84e95c6e..df453a0251 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -1833,11 +1833,11 @@ class RackMountIntegrationTests < ActiveSupport::TestCase
     assert_equal({:controller => 'foo', :action => 'id_default', :id => 1 }, @routes.recognize_path('/id_default'))
     assert_equal({:controller => 'foo', :action => 'get_or_post'}, @routes.recognize_path('/get_or_post', :method => :get))
     assert_equal({:controller => 'foo', :action => 'get_or_post'}, @routes.recognize_path('/get_or_post', :method => :post))
-    assert_raise(ActionController::ActionControllerError) { @routes.recognize_path('/get_or_post', :method => :put) }
-    assert_raise(ActionController::ActionControllerError) { @routes.recognize_path('/get_or_post', :method => :delete) }
+    assert_raise(ActionController::RoutingError) { @routes.recognize_path('/get_or_post', :method => :put) }
+    assert_raise(ActionController::RoutingError) { @routes.recognize_path('/get_or_post', :method => :delete) }
 
     assert_equal({:controller => 'posts', :action => 'index', :optional => 'bar'}, @routes.recognize_path('/optional/bar'))
-    assert_raise(ActionController::ActionControllerError) { @routes.recognize_path('/optional') }
+    assert_raise(ActionController::RoutingError) { @routes.recognize_path('/optional') }
 
     assert_equal({:controller => 'posts', :action => 'show', :id => '1', :ws => true}, @routes.recognize_path('/ws/posts/show/1', :method => :get))
     assert_equal({:controller => 'posts', :action => 'list', :ws => true}, @routes.recognize_path('/ws/posts/list', :method => :get))
@@ -1916,11 +1916,4 @@ class RackMountIntegrationTests < ActiveSupport::TestCase
       end
       extras
     end
-
-    def assert_raise(e)
-      result = yield
-      flunk "Did not raise #{e}, but returned #{result.inspect}"
-    rescue e
-      assert true
-    end
 end
diff --git a/actionview/test/actionpack/abstract/helper_test.rb b/actionview/test/actionpack/abstract/helper_test.rb
index 89c4567715..7d346e917d 100644
--- a/actionview/test/actionpack/abstract/helper_test.rb
+++ b/actionview/test/actionpack/abstract/helper_test.rb
@@ -78,9 +78,9 @@ module AbstractController
       end
 
       def test_declare_missing_helper
-        AbstractHelpers.helper :missing
-        flunk "should have raised an exception"
-      rescue LoadError => e
+        e = assert_raise AbstractController::Helpers::MissingHelperError do
+          AbstractHelpers.helper :missing
+        end
         assert_equal "helpers/missing_helper.rb", e.path
       end
 
diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb
index 7e28ceb9d8..7e594d5030 100644
--- a/actionview/test/actionpack/controller/render_test.rb
+++ b/actionview/test/actionpack/controller/render_test.rb
@@ -693,9 +693,9 @@ class RenderTest < ActionController::TestCase
   end
 
   def test_line_offset
-    get :render_line_offset
-    flunk "the action should have raised an exception"
-  rescue StandardError => exc
+    exc = assert_raises ActionView::Template::Error do
+      get :render_line_offset
+    end
     line = exc.backtrace.first
     assert(line =~ %r{:(\d+):})
     assert_equal "1", $1,
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index bfb80afa61..c6291e8aa4 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -318,9 +318,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_belongs_to_sanity
     c = Client.new
-    assert_nil c.firm
-
-    flunk "belongs_to failed if check" if c.firm
+    assert_nil c.firm, "belongs_to failed sanity check on new object"
   end
 
   def test_find_ids
@@ -1781,12 +1779,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
     assert_equal [original_child], car.reload.failed_bulbs
   end
-  
+
   test 'updates counter cache when default scope is given' do
     topic = DefaultRejectedTopic.create approved: true
 
     assert_difference "topic.reload.replies_count", 1 do
       topic.approved_replies.create!
     end
-  end 
+  end
 end
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index 5f55696c1d..de618902aa 100644
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -56,13 +56,11 @@ class ValidationsTest < ActiveRecord::TestCase
     assert_raise(ActiveRecord::RecordInvalid) { WrongReply.create! }
     assert_raise(ActiveRecord::RecordInvalid) { WrongReply.new.save! }
 
-    begin
-      r = WrongReply.new
+    r = WrongReply.new
+    invalid = assert_raise ActiveRecord::RecordInvalid do
       r.save!
-      flunk
-    rescue ActiveRecord::RecordInvalid => invalid
-      assert_equal r, invalid.record
     end
+    assert_equal r, invalid.record
   end
 
   def test_exception_on_create_bang_many
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index 8eae8c832c..28ba33331e 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -53,12 +53,10 @@ class DurationTest < ActiveSupport::TestCase
   end
 
   def test_argument_error
-    1.second.ago('')
-    flunk("no exception was raised")
-  rescue ArgumentError => e
+    e = assert_raise ArgumentError do
+      1.second.ago('')
+    end
     assert_equal 'expected a time or date, got ""', e.message, "ensure ArgumentError is not being raised by dependencies.rb"
-  rescue Exception => e
-    flunk("ArgumentError should be raised, but we got #{e.class} instead")
   end
 
   def test_fractional_weeks
diff --git a/activesupport/test/core_ext/name_error_test.rb b/activesupport/test/core_ext/name_error_test.rb
index 03ce09f22a..7525f80cf0 100644
--- a/activesupport/test/core_ext/name_error_test.rb
+++ b/activesupport/test/core_ext/name_error_test.rb
@@ -3,18 +3,18 @@ require 'active_support/core_ext/name_error'
 
 class NameErrorTest < ActiveSupport::TestCase
   def test_name_error_should_set_missing_name
-    SomeNameThatNobodyWillUse____Really ? 1 : 0
-    flunk "?!?!"
-  rescue NameError => exc
+    exc = assert_raise NameError do
+      SomeNameThatNobodyWillUse____Really ? 1 : 0
+    end
     assert_equal "NameErrorTest::SomeNameThatNobodyWillUse____Really", exc.missing_name
     assert exc.missing_name?(:SomeNameThatNobodyWillUse____Really)
     assert exc.missing_name?("NameErrorTest::SomeNameThatNobodyWillUse____Really")
   end
 
   def test_missing_method_should_ignore_missing_name
-    some_method_that_does_not_exist
-    flunk "?!?!"
-  rescue NameError => exc
+    exc = assert_raise NameError do
+      some_method_that_does_not_exist
+    end
     assert !exc.missing_name?(:Foo)
     assert_nil exc.missing_name
   end
diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb
index 0380ea3c95..00bec5bd9d 100644
--- a/activesupport/test/dependencies_test.rb
+++ b/activesupport/test/dependencies_test.rb
@@ -73,12 +73,11 @@ class DependenciesTest < ActiveSupport::TestCase
       $raises_exception_load_count = 0
 
       5.times do |count|
-        begin
+        e = assert_raise Exception, 'should have loaded dependencies/raises_exception which raises an exception' do
           require_dependency filename
-          flunk 'should have loaded dependencies/raises_exception which raises an exception'
-        rescue Exception => e
-          assert_equal 'Loading me failed, so do not add to loaded or history.', e.message
         end
+
+        assert_equal 'Loading me failed, so do not add to loaded or history.', e.message
         assert_equal count + 1, $raises_exception_load_count
 
         assert !ActiveSupport::Dependencies.loaded.include?(filename)
@@ -366,26 +365,19 @@ class DependenciesTest < ActiveSupport::TestCase
 
   def test_non_existing_const_raises_name_error_with_fully_qualified_name
     with_autoloading_fixtures do
-      begin
-        A::DoesNotExist.nil?
-        flunk "No raise!!"
-      rescue NameError => e
-        assert_equal "uninitialized constant A::DoesNotExist", e.message
-      end
-      begin
-        A::B::DoesNotExist.nil?
-        flunk "No raise!!"
-      rescue NameError => e
-        assert_equal "uninitialized constant A::B::DoesNotExist", e.message
-      end
+      e = assert_raise(NameError) { A::DoesNotExist.nil? }
+      assert_equal "uninitialized constant A::DoesNotExist", e.message
+
+      e = assert_raise(NameError) { A::B::DoesNotExist.nil? }
+      assert_equal "uninitialized constant A::B::DoesNotExist", e.message
     end
   end
 
   def test_smart_name_error_strings
-    Object.module_eval "ImaginaryObject"
-    flunk "No raise!!"
-  rescue NameError => e
-    assert e.message.include?("uninitialized constant ImaginaryObject")
+    e = assert_raise NameError do
+      Object.module_eval "ImaginaryObject"
+    end
+    assert_includes "uninitialized constant ImaginaryObject", e.message
   end
 
   def test_loadable_constants_for_path_should_handle_empty_autoloads
@@ -553,12 +545,10 @@ class DependenciesTest < ActiveSupport::TestCase
       c = ServiceOne
       ActiveSupport::Dependencies.clear
       assert ! defined?(ServiceOne)
-      begin
+      e = assert_raise ArgumentError do
         ActiveSupport::Dependencies.load_missing_constant(c, :FakeMissing)
-        flunk "Expected exception"
-      rescue ArgumentError => e
-        assert_match %r{ServiceOne has been removed from the module tree}i, e.message
       end
+      assert_match %r{ServiceOne has been removed from the module tree}i, e.message
     end
   end
 
@@ -897,12 +887,10 @@ class DependenciesTest < ActiveSupport::TestCase
     with_autoloading_fixtures do
       Object.send(:remove_const, :RaisesNameError) if defined?(::RaisesNameError)
       2.times do
-        begin
+        e = assert_raise NameError do
           ::RaisesNameError::FooBarBaz.object_id
-          flunk 'should have raised NameError when autoloaded file referenced FooBarBaz'
-        rescue NameError => e
-          assert_equal 'uninitialized constant RaisesNameError::FooBarBaz', e.message
         end
+        assert_equal 'uninitialized constant RaisesNameError::FooBarBaz', e.message
         assert !defined?(::RaisesNameError), "::RaisesNameError is defined but it should have failed!"
       end
 
diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb
index d118b0ddbf..ee1c69502e 100644
--- a/activesupport/test/deprecation_test.rb
+++ b/activesupport/test/deprecation_test.rb
@@ -104,14 +104,11 @@ class DeprecationTest < ActiveSupport::TestCase
     message   = 'Revise this deprecated stuff now!'
     callstack = %w(foo bar baz)
 
-    begin
+    e = assert_raise ActiveSupport::DeprecationException do
       ActiveSupport::Deprecation.behavior.first.call(message, callstack)
-    rescue ActiveSupport::DeprecationException => e
-      assert_equal message, e.message
-      assert_equal callstack, e.backtrace
-    else
-      flunk 'the :raise deprecation behaviour should raise the expected exception'
     end
+    assert_equal message, e.message
+    assert_equal callstack, e.backtrace
   end
 
   def test_default_stderr_behavior
diff --git a/activesupport/test/multibyte_chars_test.rb b/activesupport/test/multibyte_chars_test.rb
index 2bf73291a2..659fceb852 100644
--- a/activesupport/test/multibyte_chars_test.rb
+++ b/activesupport/test/multibyte_chars_test.rb
@@ -221,9 +221,9 @@ class MultibyteCharsUTF8BehaviourTest < ActiveSupport::TestCase
   end
 
   def test_include_raises_when_nil_is_passed
-    @chars.include?(nil)
-    flunk "Expected chars.include?(nil) to raise TypeError or NoMethodError"
-  rescue Exception
+    assert_raises TypeError, NoMethodError, "Expected chars.include?(nil) to raise TypeError or NoMethodError" do
+      @chars.include?(nil)
+    end
   end
 
   def test_index_should_return_character_offset