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.

1 files changed, 53 insertions, 57 deletions

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