From 892e38c78e03c11afaa5f01d995e3a21bd92b415 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 21 Dec 2018 02:44:01 +0900
Subject: Enable `Style/RedundantBegin` cop to avoid newly adding redundant
 begin block

Currently we sometimes find a redundant begin block in code review
(e.g. https://github.com/rails/rails/pull/33604#discussion_r209784205).

I'd like to enable `Style/RedundantBegin` cop to avoid that, since
rescue/else/ensure are allowed inside do/end blocks in Ruby 2.5
(https://bugs.ruby-lang.org/issues/12906), so we'd probably meets with
that situation than before.
---
 activerecord/test/cases/adapter_test.rb            |  22 ++---
 .../cases/adapters/mysql2/active_schema_test.rb    |  30 +++---
 .../cases/adapters/postgresql/infinity_test.rb     |  18 ++--
 .../test/cases/adapters/postgresql/schema_test.rb  |  20 ++--
 .../adapters/sqlite3/sqlite3_create_folder_test.rb |  18 ++--
 .../has_and_belongs_to_many_associations_test.rb   |  18 ++--
 .../test/cases/associations/required_test.rb       |  50 +++++-----
 activerecord/test/cases/connection_pool_test.rb    |  30 +++---
 activerecord/test/cases/errors_test.rb             |   8 +-
 activerecord/test/cases/filter_attributes_test.rb  |  32 +++---
 activerecord/test/cases/fixtures_test.rb           |  14 ++-
 .../test/cases/habtm_destroy_order_test.rb         |  28 +++---
 .../cases/migration/references_foreign_key_test.rb |  30 +++---
 activerecord/test/cases/migration_test.rb          |  44 ++++-----
 activerecord/test/cases/multiple_db_test.rb        |  12 +--
 activerecord/test/cases/pooled_connections_test.rb |  28 +++---
 activerecord/test/cases/query_cache_test.rb        | 110 ++++++++++-----------
 .../test/cases/relation/update_all_test.rb         |   8 +-
 18 files changed, 234 insertions(+), 286 deletions(-)

(limited to 'activerecord/test')

diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 3d31533ec8..66e594d771 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -132,19 +132,17 @@ module ActiveRecord
       end
 
       def test_not_specifying_database_name_for_cross_database_selects
-        begin
-          assert_nothing_raised do
-            ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations["arunit"].except(:database))
-
-            config = ARTest.connection_config
-            ActiveRecord::Base.connection.execute(
-              "SELECT #{config['arunit']['database']}.pirates.*, #{config['arunit2']['database']}.courses.* " \
-              "FROM #{config['arunit']['database']}.pirates, #{config['arunit2']['database']}.courses"
-            )
-          end
-        ensure
-          ActiveRecord::Base.establish_connection :arunit
+        assert_nothing_raised do
+          ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations["arunit"].except(:database))
+
+          config = ARTest.connection_config
+          ActiveRecord::Base.connection.execute(
+            "SELECT #{config['arunit']['database']}.pirates.*, #{config['arunit2']['database']}.courses.* " \
+            "FROM #{config['arunit']['database']}.pirates, #{config['arunit2']['database']}.courses"
+          )
         end
+      ensure
+        ActiveRecord::Base.establish_connection :arunit
       end
     end
 
diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb
index 261fee13eb..0d5b43afe5 100644
--- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb
@@ -130,29 +130,25 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase
 
   def test_add_timestamps
     with_real_execute do
-      begin
-        ActiveRecord::Base.connection.create_table :delete_me
-        ActiveRecord::Base.connection.add_timestamps :delete_me, null: true
-        assert column_present?("delete_me", "updated_at", "datetime")
-        assert column_present?("delete_me", "created_at", "datetime")
-      ensure
-        ActiveRecord::Base.connection.drop_table :delete_me rescue nil
-      end
+      ActiveRecord::Base.connection.create_table :delete_me
+      ActiveRecord::Base.connection.add_timestamps :delete_me, null: true
+      assert column_present?("delete_me", "updated_at", "datetime")
+      assert column_present?("delete_me", "created_at", "datetime")
+    ensure
+      ActiveRecord::Base.connection.drop_table :delete_me rescue nil
     end
   end
 
   def test_remove_timestamps
     with_real_execute do
-      begin
-        ActiveRecord::Base.connection.create_table :delete_me do |t|
-          t.timestamps null: true
-        end
-        ActiveRecord::Base.connection.remove_timestamps :delete_me, null: true
-        assert_not column_present?("delete_me", "updated_at", "datetime")
-        assert_not column_present?("delete_me", "created_at", "datetime")
-      ensure
-        ActiveRecord::Base.connection.drop_table :delete_me rescue nil
+      ActiveRecord::Base.connection.create_table :delete_me do |t|
+        t.timestamps null: true
       end
+      ActiveRecord::Base.connection.remove_timestamps :delete_me, null: true
+      assert_not column_present?("delete_me", "updated_at", "datetime")
+      assert_not column_present?("delete_me", "created_at", "datetime")
+    ensure
+      ActiveRecord::Base.connection.drop_table :delete_me rescue nil
     end
   end
 
diff --git a/activerecord/test/cases/adapters/postgresql/infinity_test.rb b/activerecord/test/cases/adapters/postgresql/infinity_test.rb
index 5e56ce8427..b1bf06d9e9 100644
--- a/activerecord/test/cases/adapters/postgresql/infinity_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/infinity_test.rb
@@ -71,17 +71,15 @@ class PostgresqlInfinityTest < ActiveRecord::PostgreSQLTestCase
   end
 
   test "assigning 'infinity' on a datetime column with TZ aware attributes" do
-    begin
-      in_time_zone "Pacific Time (US & Canada)" do
-        record = PostgresqlInfinity.create!(datetime: "infinity")
-        assert_equal Float::INFINITY, record.datetime
-        assert_equal record.datetime, record.reload.datetime
-      end
-    ensure
-      # setting time_zone_aware_attributes causes the types to change.
-      # There is no way to do this automatically since it can be set on a superclass
-      PostgresqlInfinity.reset_column_information
+    in_time_zone "Pacific Time (US & Canada)" do
+      record = PostgresqlInfinity.create!(datetime: "infinity")
+      assert_equal Float::INFINITY, record.datetime
+      assert_equal record.datetime, record.reload.datetime
     end
+  ensure
+    # setting time_zone_aware_attributes causes the types to change.
+    # There is no way to do this automatically since it can be set on a superclass
+    PostgresqlInfinity.reset_column_information
   end
 
   test "where clause with infinite range on a datetime column" do
diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb
index 931a9bd0be..336cec30ca 100644
--- a/activerecord/test/cases/adapters/postgresql/schema_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb
@@ -108,23 +108,19 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase
   end
 
   def test_create_schema
-    begin
-      @connection.create_schema "test_schema3"
-      assert @connection.schema_names.include? "test_schema3"
-    ensure
-      @connection.drop_schema "test_schema3"
-    end
+    @connection.create_schema "test_schema3"
+    assert @connection.schema_names.include? "test_schema3"
+  ensure
+    @connection.drop_schema "test_schema3"
   end
 
   def test_raise_create_schema_with_existing_schema
-    begin
+    @connection.create_schema "test_schema3"
+    assert_raises(ActiveRecord::StatementInvalid) do
       @connection.create_schema "test_schema3"
-      assert_raises(ActiveRecord::StatementInvalid) do
-        @connection.create_schema "test_schema3"
-      end
-    ensure
-      @connection.drop_schema "test_schema3"
     end
+  ensure
+    @connection.drop_schema "test_schema3"
   end
 
   def test_drop_schema
diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb
index d70486605f..cfc9853aba 100644
--- a/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_create_folder_test.rb
@@ -8,17 +8,15 @@ module ActiveRecord
     class SQLite3CreateFolder < ActiveRecord::SQLite3TestCase
       def test_sqlite_creates_directory
         Dir.mktmpdir do |dir|
-          begin
-            dir = Pathname.new(dir)
-            @conn = Base.sqlite3_connection database: dir.join("db/foo.sqlite3"),
-                                 adapter: "sqlite3",
-                                 timeout: 100
+          dir = Pathname.new(dir)
+          @conn = Base.sqlite3_connection database: dir.join("db/foo.sqlite3"),
+                               adapter: "sqlite3",
+                               timeout: 100
 
-            assert Dir.exist? dir.join("db")
-            assert File.exist? dir.join("db/foo.sqlite3")
-          ensure
-            @conn.disconnect! if @conn
-          end
+          assert Dir.exist? dir.join("db")
+          assert File.exist? dir.join("db/foo.sqlite3")
+        ensure
+          @conn.disconnect! if @conn
         end
       end
     end
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 515eb65d37..fe8bdd03ba 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -1007,16 +1007,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_has_and_belongs_to_many_while_partial_writes_false
-    begin
-      original_partial_writes = ActiveRecord::Base.partial_writes
-      ActiveRecord::Base.partial_writes = false
-      developer = Developer.new(name: "Mehmet Emin İNAÇ")
-      developer.projects << Project.new(name: "Bounty")
-
-      assert developer.save
-    ensure
-      ActiveRecord::Base.partial_writes = original_partial_writes
-    end
+    original_partial_writes = ActiveRecord::Base.partial_writes
+    ActiveRecord::Base.partial_writes = false
+    developer = Developer.new(name: "Mehmet Emin İNAÇ")
+    developer.projects << Project.new(name: "Bounty")
+
+    assert developer.save
+  ensure
+    ActiveRecord::Base.partial_writes = original_partial_writes
   end
 
   def test_has_and_belongs_to_many_with_belongs_to
diff --git a/activerecord/test/cases/associations/required_test.rb b/activerecord/test/cases/associations/required_test.rb
index 65a3bb5efe..c7a78e6bc4 100644
--- a/activerecord/test/cases/associations/required_test.rb
+++ b/activerecord/test/cases/associations/required_test.rb
@@ -25,20 +25,18 @@ class RequiredAssociationsTest < ActiveRecord::TestCase
   end
 
   test "belongs_to associations can be optional by default" do
-    begin
-      original_value = ActiveRecord::Base.belongs_to_required_by_default
-      ActiveRecord::Base.belongs_to_required_by_default = false
+    original_value = ActiveRecord::Base.belongs_to_required_by_default
+    ActiveRecord::Base.belongs_to_required_by_default = false
 
-      model = subclass_of(Child) do
-        belongs_to :parent, inverse_of: false,
-          class_name: "RequiredAssociationsTest::Parent"
-      end
-
-      assert model.new.save
-      assert model.new(parent: Parent.new).save
-    ensure
-      ActiveRecord::Base.belongs_to_required_by_default = original_value
+    model = subclass_of(Child) do
+      belongs_to :parent, inverse_of: false,
+        class_name: "RequiredAssociationsTest::Parent"
     end
+
+    assert model.new.save
+    assert model.new(parent: Parent.new).save
+  ensure
+    ActiveRecord::Base.belongs_to_required_by_default = original_value
   end
 
   test "required belongs_to associations have presence validated" do
@@ -56,24 +54,22 @@ class RequiredAssociationsTest < ActiveRecord::TestCase
   end
 
   test "belongs_to associations can be required by default" do
-    begin
-      original_value = ActiveRecord::Base.belongs_to_required_by_default
-      ActiveRecord::Base.belongs_to_required_by_default = true
+    original_value = ActiveRecord::Base.belongs_to_required_by_default
+    ActiveRecord::Base.belongs_to_required_by_default = true
 
-      model = subclass_of(Child) do
-        belongs_to :parent, inverse_of: false,
-          class_name: "RequiredAssociationsTest::Parent"
-      end
+    model = subclass_of(Child) do
+      belongs_to :parent, inverse_of: false,
+        class_name: "RequiredAssociationsTest::Parent"
+    end
 
-      record = model.new
-      assert_not record.save
-      assert_equal ["Parent must exist"], record.errors.full_messages
+    record = model.new
+    assert_not record.save
+    assert_equal ["Parent must exist"], record.errors.full_messages
 
-      record.parent = Parent.new
-      assert record.save
-    ensure
-      ActiveRecord::Base.belongs_to_required_by_default = original_value
-    end
+    record.parent = Parent.new
+    assert record.save
+  ensure
+    ActiveRecord::Base.belongs_to_required_by_default = original_value
   end
 
   test "has_one associations are not required by default" do
diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb
index 633d56e479..a15ad9a45b 100644
--- a/activerecord/test/cases/connection_pool_test.rb
+++ b/activerecord/test/cases/connection_pool_test.rb
@@ -567,23 +567,21 @@ module ActiveRecord
 
       def test_disconnect_and_clear_reloadable_connections_attempt_to_wait_for_threads_to_return_their_conns
         [:disconnect, :disconnect!, :clear_reloadable_connections, :clear_reloadable_connections!].each do |group_action_method|
-          begin
-            thread = timed_join_result = nil
-            @pool.with_connection do |connection|
-              thread = Thread.new { @pool.send(group_action_method) }
-
-              # give the other `thread` some time to get stuck in `group_action_method`
-              timed_join_result = thread.join(0.3)
-              # thread.join # => `nil` means the other thread hasn't finished running and is still waiting for us to
-              # release our connection
-              assert_nil timed_join_result
-
-              # assert that since this is within default timeout our connection hasn't been forcefully taken away from us
-              assert_predicate @pool, :active_connection?
-            end
-          ensure
-            thread.join if thread && !timed_join_result # clean up the other thread
+          thread = timed_join_result = nil
+          @pool.with_connection do |connection|
+            thread = Thread.new { @pool.send(group_action_method) }
+
+            # give the other `thread` some time to get stuck in `group_action_method`
+            timed_join_result = thread.join(0.3)
+            # thread.join # => `nil` means the other thread hasn't finished running and is still waiting for us to
+            # release our connection
+            assert_nil timed_join_result
+
+            # assert that since this is within default timeout our connection hasn't been forcefully taken away from us
+            assert_predicate @pool, :active_connection?
           end
+        ensure
+          thread.join if thread && !timed_join_result # clean up the other thread
         end
       end
 
diff --git a/activerecord/test/cases/errors_test.rb b/activerecord/test/cases/errors_test.rb
index b90e6a66c5..0d2be944b5 100644
--- a/activerecord/test/cases/errors_test.rb
+++ b/activerecord/test/cases/errors_test.rb
@@ -8,11 +8,9 @@ class ErrorsTest < ActiveRecord::TestCase
     error_klasses = ObjectSpace.each_object(Class).select { |klass| klass < base }
 
     (error_klasses - [ActiveRecord::AmbiguousSourceReflectionForThroughAssociation]).each do |error_klass|
-      begin
-        error_klass.new.inspect
-      rescue ArgumentError
-        raise "Instance of #{error_klass} can't be initialized with no arguments"
-      end
+      error_klass.new.inspect
+    rescue ArgumentError
+      raise "Instance of #{error_klass} can't be initialized with no arguments"
     end
   end
 end
diff --git a/activerecord/test/cases/filter_attributes_test.rb b/activerecord/test/cases/filter_attributes_test.rb
index 47161a547a..2f4c9b0ef7 100644
--- a/activerecord/test/cases/filter_attributes_test.rb
+++ b/activerecord/test/cases/filter_attributes_test.rb
@@ -90,15 +90,13 @@ class FilterAttributesTest < ActiveRecord::TestCase
   end
 
   test "filter_attributes should handle [FILTERED] value properly" do
-    begin
-      User.filter_attributes = ["auth"]
-      user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]")
+    User.filter_attributes = ["auth"]
+    user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]")
 
-      assert_includes user.inspect, "auth_token: [FILTERED]"
-      assert_includes user.inspect, 'token: "[FILTERED]"'
-    ensure
-      User.remove_instance_variable(:@filter_attributes)
-    end
+    assert_includes user.inspect, "auth_token: [FILTERED]"
+    assert_includes user.inspect, 'token: "[FILTERED]"'
+  ensure
+    User.remove_instance_variable(:@filter_attributes)
   end
 
   test "filter_attributes on pretty_print" do
@@ -121,16 +119,14 @@ class FilterAttributesTest < ActiveRecord::TestCase
   end
 
   test "filter_attributes on pretty_print should handle [FILTERED] value properly" do
-    begin
-      User.filter_attributes = ["auth"]
-      user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]")
-      actual = "".dup
-      PP.pp(user, StringIO.new(actual))
+    User.filter_attributes = ["auth"]
+    user = User.new(token: "[FILTERED]", auth_token: "[FILTERED]")
+    actual = "".dup
+    PP.pp(user, StringIO.new(actual))
 
-      assert_includes actual, "auth_token: [FILTERED]"
-      assert_includes actual, 'token: "[FILTERED]"'
-    ensure
-      User.remove_instance_variable(:@filter_attributes)
-    end
+    assert_includes actual, "auth_token: [FILTERED]"
+    assert_includes actual, 'token: "[FILTERED]"'
+  ensure
+    User.remove_instance_variable(:@filter_attributes)
   end
 end
diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb
index a5592fc86a..fe2f417a04 100644
--- a/activerecord/test/cases/fixtures_test.rb
+++ b/activerecord/test/cases/fixtures_test.rb
@@ -73,14 +73,12 @@ class FixturesTest < ActiveRecord::TestCase
 
   if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
     def test_bulk_insert
-      begin
-        subscriber = InsertQuerySubscriber.new
-        subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber)
-        create_fixtures("bulbs")
-        assert_equal 1, subscriber.events.size, "It takes one INSERT query to insert two fixtures"
-      ensure
-        ActiveSupport::Notifications.unsubscribe(subscription)
-      end
+      subscriber = InsertQuerySubscriber.new
+      subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber)
+      create_fixtures("bulbs")
+      assert_equal 1, subscriber.events.size, "It takes one INSERT query to insert two fixtures"
+    ensure
+      ActiveSupport::Notifications.unsubscribe(subscription)
     end
 
     def test_bulk_insert_multiple_table_with_a_multi_statement_query
diff --git a/activerecord/test/cases/habtm_destroy_order_test.rb b/activerecord/test/cases/habtm_destroy_order_test.rb
index b15e1b48c4..9dbd339fe7 100644
--- a/activerecord/test/cases/habtm_destroy_order_test.rb
+++ b/activerecord/test/cases/habtm_destroy_order_test.rb
@@ -30,23 +30,21 @@ class HabtmDestroyOrderTest < ActiveRecord::TestCase
 
   test "not destroying a student with lessons leaves student<=>lesson association intact" do
     # test a normal before_destroy doesn't destroy the habtm joins
-    begin
-      sicp = Lesson.new(name: "SICP")
-      ben = Student.new(name: "Ben Bitdiddle")
-      # add a before destroy to student
-      Student.class_eval do
-        before_destroy do
-          raise ActiveRecord::Rollback unless lessons.empty?
-        end
+    sicp = Lesson.new(name: "SICP")
+    ben = Student.new(name: "Ben Bitdiddle")
+    # add a before destroy to student
+    Student.class_eval do
+      before_destroy do
+        raise ActiveRecord::Rollback unless lessons.empty?
       end
-      ben.lessons << sicp
-      ben.save!
-      ben.destroy
-      assert_not_empty ben.reload.lessons
-    ensure
-      # get rid of it so Student is still like it was
-      Student.reset_callbacks(:destroy)
     end
+    ben.lessons << sicp
+    ben.save!
+    ben.destroy
+    assert_not_empty ben.reload.lessons
+  ensure
+    # get rid of it so Student is still like it was
+    Student.reset_callbacks(:destroy)
   end
 
   test "not destroying a lesson with students leaves student<=>lesson association intact" do
diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb
index 7a092103c7..620e9ab6ca 100644
--- a/activerecord/test/cases/migration/references_foreign_key_test.rb
+++ b/activerecord/test/cases/migration/references_foreign_key_test.rb
@@ -152,25 +152,23 @@ if ActiveRecord::Base.connection.supports_foreign_keys?
         end
 
         test "foreign key methods respect pluralize_table_names" do
-          begin
-            original_pluralize_table_names = ActiveRecord::Base.pluralize_table_names
-            ActiveRecord::Base.pluralize_table_names = false
-            @connection.create_table :testing
-            @connection.change_table :testing_parents do |t|
-              t.references :testing, foreign_key: true
-            end
+          original_pluralize_table_names = ActiveRecord::Base.pluralize_table_names
+          ActiveRecord::Base.pluralize_table_names = false
+          @connection.create_table :testing
+          @connection.change_table :testing_parents do |t|
+            t.references :testing, foreign_key: true
+          end
 
-            fk = @connection.foreign_keys("testing_parents").first
-            assert_equal "testing_parents", fk.from_table
-            assert_equal "testing", fk.to_table
+          fk = @connection.foreign_keys("testing_parents").first
+          assert_equal "testing_parents", fk.from_table
+          assert_equal "testing", fk.to_table
 
-            assert_difference "@connection.foreign_keys('testing_parents').size", -1 do
-              @connection.remove_reference :testing_parents, :testing, foreign_key: true
-            end
-          ensure
-            ActiveRecord::Base.pluralize_table_names = original_pluralize_table_names
-            @connection.drop_table "testing", if_exists: true
+          assert_difference "@connection.foreign_keys('testing_parents').size", -1 do
+            @connection.remove_reference :testing_parents, :testing, foreign_key: true
           end
+        ensure
+          ActiveRecord::Base.pluralize_table_names = original_pluralize_table_names
+          @connection.drop_table "testing", if_exists: true
         end
 
         class CreateDogsMigration < ActiveRecord::Migration::Current
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 661163b4a1..8b0ecd2516 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -576,29 +576,25 @@ class MigrationTest < ActiveRecord::TestCase
       # table name is 29 chars, the standard sequence name will
       # be 33 chars and should be shortened
       assert_nothing_raised do
-        begin
-          Person.connection.create_table :table_with_name_thats_just_ok do |t|
-            t.column :foo, :string, null: false
-          end
-        ensure
-          Person.connection.drop_table :table_with_name_thats_just_ok rescue nil
+        Person.connection.create_table :table_with_name_thats_just_ok do |t|
+          t.column :foo, :string, null: false
         end
+      ensure
+        Person.connection.drop_table :table_with_name_thats_just_ok rescue nil
       end
 
       # should be all good w/ a custom sequence name
       assert_nothing_raised do
-        begin
-          Person.connection.create_table :table_with_name_thats_just_ok,
-            sequence_name: "suitably_short_seq" do |t|
-            t.column :foo, :string, null: false
-          end
+        Person.connection.create_table :table_with_name_thats_just_ok,
+          sequence_name: "suitably_short_seq" do |t|
+          t.column :foo, :string, null: false
+        end
 
-          Person.connection.execute("select suitably_short_seq.nextval from dual")
+        Person.connection.execute("select suitably_short_seq.nextval from dual")
 
-        ensure
-          Person.connection.drop_table :table_with_name_thats_just_ok,
-            sequence_name: "suitably_short_seq" rescue nil
-        end
+      ensure
+        Person.connection.drop_table :table_with_name_thats_just_ok,
+          sequence_name: "suitably_short_seq" rescue nil
       end
 
       # confirm the custom sequence got dropped
@@ -742,15 +738,13 @@ class MigrationTest < ActiveRecord::TestCase
       test_terminated = Concurrent::CountDownLatch.new
 
       other_process = Thread.new do
-        begin
-          conn = ActiveRecord::Base.connection_pool.checkout
-          conn.get_advisory_lock(lock_id)
-          thread_lock.count_down
-          test_terminated.wait # hold the lock open until we tested everything
-        ensure
-          conn.release_advisory_lock(lock_id)
-          ActiveRecord::Base.connection_pool.checkin(conn)
-        end
+        conn = ActiveRecord::Base.connection_pool.checkout
+        conn.get_advisory_lock(lock_id)
+        thread_lock.count_down
+        test_terminated.wait # hold the lock open until we tested everything
+      ensure
+        conn.release_advisory_lock(lock_id)
+        ActiveRecord::Base.connection_pool.checkin(conn)
       end
 
       thread_lock.wait # wait until the 'other process' has the lock
diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb
index 192d2f5251..f11c441c65 100644
--- a/activerecord/test/cases/multiple_db_test.rb
+++ b/activerecord/test/cases/multiple_db_test.rb
@@ -106,14 +106,12 @@ class MultipleDbTest < ActiveRecord::TestCase
     end
 
     def test_associations_should_work_when_model_has_no_connection
-      begin
-        ActiveRecord::Base.remove_connection
-        assert_nothing_raised do
-          College.first.courses.first
-        end
-      ensure
-        ActiveRecord::Base.establish_connection :arunit
+      ActiveRecord::Base.remove_connection
+      assert_nothing_raised do
+        College.first.courses.first
       end
+    ensure
+      ActiveRecord::Base.establish_connection :arunit
     end
   end
 end
diff --git a/activerecord/test/cases/pooled_connections_test.rb b/activerecord/test/cases/pooled_connections_test.rb
index fa7f759e51..080aeb0989 100644
--- a/activerecord/test/cases/pooled_connections_test.rb
+++ b/activerecord/test/cases/pooled_connections_test.rb
@@ -25,14 +25,12 @@ class PooledConnectionsTest < ActiveRecord::TestCase
     @timed_out = 0
     threads.times do
       Thread.new do
-        begin
-          conn = ActiveRecord::Base.connection_pool.checkout
-          sleep 0.1
-          ActiveRecord::Base.connection_pool.checkin conn
-          @connection_count += 1
-        rescue ActiveRecord::ConnectionTimeoutError
-          @timed_out += 1
-        end
+        conn = ActiveRecord::Base.connection_pool.checkout
+        sleep 0.1
+        ActiveRecord::Base.connection_pool.checkin conn
+        @connection_count += 1
+      rescue ActiveRecord::ConnectionTimeoutError
+        @timed_out += 1
       end.join
     end
   end
@@ -42,14 +40,12 @@ class PooledConnectionsTest < ActiveRecord::TestCase
     @connection_count = 0
     @timed_out = 0
     loops.times do
-      begin
-        conn = ActiveRecord::Base.connection_pool.checkout
-        ActiveRecord::Base.connection_pool.checkin conn
-        @connection_count += 1
-        ActiveRecord::Base.connection.data_sources
-      rescue ActiveRecord::ConnectionTimeoutError
-        @timed_out += 1
-      end
+      conn = ActiveRecord::Base.connection_pool.checkout
+      ActiveRecord::Base.connection_pool.checkin conn
+      @connection_count += 1
+      ActiveRecord::Base.connection.data_sources
+    rescue ActiveRecord::ConnectionTimeoutError
+      @timed_out += 1
     end
   end
 
diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb
index 02ead8d914..2a7a5e736e 100644
--- a/activerecord/test/cases/query_cache_test.rb
+++ b/activerecord/test/cases/query_cache_test.rb
@@ -73,76 +73,74 @@ class QueryCacheTest < ActiveRecord::TestCase
 
   def test_query_cache_across_threads
     with_temporary_connection_pool do
-      begin
-        if in_memory_db?
-          # Separate connections to an in-memory database create an entirely new database,
-          # with an empty schema etc, so we just stub out this schema on the fly.
-          ActiveRecord::Base.connection_pool.with_connection do |connection|
-            connection.create_table :tasks do |t|
-              t.datetime :starting
-              t.datetime :ending
-            end
+      if in_memory_db?
+        # Separate connections to an in-memory database create an entirely new database,
+        # with an empty schema etc, so we just stub out this schema on the fly.
+        ActiveRecord::Base.connection_pool.with_connection do |connection|
+          connection.create_table :tasks do |t|
+            t.datetime :starting
+            t.datetime :ending
           end
-          ActiveRecord::FixtureSet.create_fixtures(self.class.fixture_path, ["tasks"], {}, ActiveRecord::Base)
         end
+        ActiveRecord::FixtureSet.create_fixtures(self.class.fixture_path, ["tasks"], {}, ActiveRecord::Base)
+      end
 
-        ActiveRecord::Base.connection_pool.connections.each do |conn|
-          assert_cache :off, conn
-        end
+      ActiveRecord::Base.connection_pool.connections.each do |conn|
+        assert_cache :off, conn
+      end
 
-        assert_not_predicate ActiveRecord::Base.connection, :nil?
-        assert_cache :off
+      assert_not_predicate ActiveRecord::Base.connection, :nil?
+      assert_cache :off
 
-        middleware {
-          assert_cache :clean
+      middleware {
+        assert_cache :clean
 
-          Task.find 1
-          assert_cache :dirty
+        Task.find 1
+        assert_cache :dirty
 
-          thread_1_connection = ActiveRecord::Base.connection
-          ActiveRecord::Base.clear_active_connections!
-          assert_cache :off, thread_1_connection
+        thread_1_connection = ActiveRecord::Base.connection
+        ActiveRecord::Base.clear_active_connections!
+        assert_cache :off, thread_1_connection
 
-          started = Concurrent::Event.new
-          checked = Concurrent::Event.new
+        started = Concurrent::Event.new
+        checked = Concurrent::Event.new
 
-          thread_2_connection = nil
-          thread = Thread.new {
-            thread_2_connection = ActiveRecord::Base.connection
+        thread_2_connection = nil
+        thread = Thread.new {
+          thread_2_connection = ActiveRecord::Base.connection
 
-            assert_equal thread_2_connection, thread_1_connection
-            assert_cache :off
+          assert_equal thread_2_connection, thread_1_connection
+          assert_cache :off
 
-            middleware {
-              assert_cache :clean
+          middleware {
+            assert_cache :clean
 
-              Task.find 1
-              assert_cache :dirty
+            Task.find 1
+            assert_cache :dirty
 
-              started.set
-              checked.wait
+            started.set
+            checked.wait
 
-              ActiveRecord::Base.clear_active_connections!
-            }.call({})
-          }
+            ActiveRecord::Base.clear_active_connections!
+          }.call({})
+        }
 
-          started.wait
+        started.wait
 
-          thread_1_connection = ActiveRecord::Base.connection
-          assert_not_equal thread_1_connection, thread_2_connection
-          assert_cache :dirty, thread_2_connection
-          checked.set
-          thread.join
+        thread_1_connection = ActiveRecord::Base.connection
+        assert_not_equal thread_1_connection, thread_2_connection
+        assert_cache :dirty, thread_2_connection
+        checked.set
+        thread.join
 
-          assert_cache :off, thread_2_connection
-        }.call({})
+        assert_cache :off, thread_2_connection
+      }.call({})
 
-        ActiveRecord::Base.connection_pool.connections.each do |conn|
-          assert_cache :off, conn
-        end
-      ensure
-        ActiveRecord::Base.connection_pool.disconnect!
+      ActiveRecord::Base.connection_pool.connections.each do |conn|
+        assert_cache :off, conn
       end
+    ensure
+      ActiveRecord::Base.connection_pool.disconnect!
     end
   end
 
@@ -369,12 +367,10 @@ class QueryCacheTest < ActiveRecord::TestCase
       assert_not_predicate Task, :connected?
 
       Task.cache do
-        begin
-          assert_queries(1) { Task.find(1); Task.find(1) }
-        ensure
-          ActiveRecord::Base.connection_handler.remove_connection(Task.connection_specification_name)
-          Task.connection_specification_name = spec_name
-        end
+        assert_queries(1) { Task.find(1); Task.find(1) }
+      ensure
+        ActiveRecord::Base.connection_handler.remove_connection(Task.connection_specification_name)
+        Task.connection_specification_name = spec_name
       end
     end
   end
diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb
index 09c365f31b..bb6912148c 100644
--- a/activerecord/test/cases/relation/update_all_test.rb
+++ b/activerecord/test/cases/relation/update_all_test.rb
@@ -198,11 +198,9 @@ class UpdateAllTest < ActiveRecord::TestCase
     def test_update_all_doesnt_ignore_order
       assert_equal authors(:david).id + 1, authors(:mary).id # make sure there is going to be a duplicate PK error
       test_update_with_order_succeeds = lambda do |order|
-        begin
-          Author.order(order).update_all("id = id + 1")
-        rescue ActiveRecord::ActiveRecordError
-          false
-        end
+        Author.order(order).update_all("id = id + 1")
+      rescue ActiveRecord::ActiveRecordError
+        false
       end
 
       if test_update_with_order_succeeds.call("id DESC")
-- 
cgit v1.2.3