Do not attempt to return connection with open transaction to pool (#24610)

When the query cache completes, if Active Record is still inside of a
transaction, it is because the transaction is meant to be left open
above this unit of work (such as transactional fixtures in tests). There
were several tests around the behavior of "tests" that were invalid, as
tests are not run through the executor. They have been changed to
reflect the new behavior, which is closer to what actually occurs in
Rails tests.

Fixes #23989
Fixes #24491
Close #24500

2 files changed, 10 insertions, 7 deletions

diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb
index bbbc824b25..ca12a603da 100644
--- a/activerecord/lib/active_record/query_cache.rb
+++ b/activerecord/lib/active_record/query_cache.rb
@@ -44,8 +44,9 @@ module ActiveRecord
       executor.register_hook(self)
 
       executor.to_complete do
-        # FIXME: This should be skipped when env['rack.test']
-        ActiveRecord::Base.clear_active_connections!
+        unless ActiveRecord::Base.connection.transaction_open?
+          ActiveRecord::Base.clear_active_connections!
+        end
       end
     end
   end
diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb
index c4c2c69d1c..1f9b6add7a 100644
--- a/activerecord/test/cases/connection_management_test.rb
+++ b/activerecord/test/cases/connection_management_test.rb
@@ -4,6 +4,8 @@ require "rack"
 module ActiveRecord
   module ConnectionAdapters
     class ConnectionManagementTest < ActiveRecord::TestCase
+      self.use_transactional_tests = false
+
       class App
         attr_reader :calls
         def initialize
@@ -46,8 +48,8 @@ module ActiveRecord
         assert !ActiveRecord::Base.connection_handler.active_connections?
       end
 
-      def test_active_connections_are_not_cleared_on_body_close_during_test
-        executor.wrap do
+      def test_active_connections_are_not_cleared_on_body_close_during_transaction
+        ActiveRecord::Base.transaction do
           _, _, body = @management.call(@env)
           body.close
           assert ActiveRecord::Base.connection_handler.active_connections?
@@ -61,9 +63,9 @@ module ActiveRecord
         assert !ActiveRecord::Base.connection_handler.active_connections?
       end
 
-      def test_connections_not_closed_if_exception_and_test
-        executor.wrap do
-          app               = Class.new(App) { def call(env); raise; end }.new
+      def test_connections_not_closed_if_exception_inside_transaction
+        ActiveRecord::Base.transaction do
+          app               = Class.new(App) { def call(env); raise RuntimeError; end }.new
           explosive         = middleware(app)
           assert_raises(RuntimeError) { explosive.call(@env) }
           assert ActiveRecord::Base.connection_handler.active_connections?