Merge pull request rails#53139 from mjcheetham/db-transactions

Add support for disabling transactions per database in `ActiveRecord`

Author: matthewd

activerecord/CHANGELOG.md

@@ -1,3 +1,18 @@
+*   Add support for enabling or disabling transactional tests per database.
+
+    A test class can now override the default `use_transactional_tests` setting
+    for individual databases, which can be useful if some databases need their
+    current state to be accessible to an external process while tests are running.
+
+    ```ruby
+    class MostlyTransactionalTest < ActiveSupport::TestCase
+      self.use_transactional_tests = true
+      skip_transactional_tests_for_database :shared
+    end
+    ```
+
+    *Matthew Cheetham*, *Morgan Mareve*
+
 *   Cast `query_cache` value when using URL configuration.
 
     *zzak*

activerecord/lib/active_record/test_fixtures.rb

@@ -36,11 +36,26 @@ def after_teardown # :nodoc:
       class_attribute :pre_loaded_fixtures, default: false
       class_attribute :lock_threads, default: true
       class_attribute :fixture_sets, default: {}
+      class_attribute :database_transactions_config, default: {}
 
       ActiveSupport.run_load_hooks(:active_record_fixtures, self)
     end
 
     module ClassMethods
+      # Do not use transactional tests for the given database. This overrides
+      # the default setting as defined by `use_transactional_tests`, which
+      # applies to all database connection pools not explicitly configured here.
+      def skip_transactional_tests_for_database(database_name)
+        use_transactional_tests_for_database(database_name, false)
+      end
+
+      # Enable or disable transactions per database. This overrides the default
+      # setting as defined by `use_transactional_tests`, which applies to all
+      # database connection pools not explicitly configured here.
+      def use_transactional_tests_for_database(database_name, enabled = true)
+        self.database_transactions_config = database_transactions_config.merge(database_name => enabled)
+      end
+
       # Sets the model class for a fixture when the class name cannot be inferred from the fixture name.
       #
       # Examples:
@@ -106,7 +121,8 @@ def fixture(fixture_set_name, *fixture_names)
 
     private
       def run_in_transaction?
-        use_transactional_tests &&
+        has_explicit_config = database_transactions_config.any? { |_, enabled| enabled }
+        (use_transactional_tests || has_explicit_config) &&
           !self.class.uses_transaction?(name)
       end
 
@@ -169,11 +185,19 @@ def invalidate_already_loaded_fixtures
         @@already_loaded_fixtures.clear
       end
 
+      def transactional_tests_for_pool?(pool)
+        database_transactions_config.fetch(pool.db_config.name.to_sym, use_transactional_tests)
+      end
+
       def setup_transactional_fixtures
         setup_shared_connection_pool
 
         # Begin transactions for connections already established
         @fixture_connection_pools = ActiveRecord::Base.connection_handler.connection_pool_list(:writing)
+
+        # Filter to pools that want to use transactions
+        @fixture_connection_pools.select! { |pool| transactional_tests_for_pool?(pool) }
+
         @fixture_connection_pools.each do |pool|
           pool.pin_connection!(lock_threads)
           pool.lease_connection
@@ -189,7 +213,8 @@ def setup_transactional_fixtures
             if pool
               setup_shared_connection_pool
 
-              unless @fixture_connection_pools.include?(pool)
+              # Don't begin a transaction if we've already done so, or are not using them for this pool
+              if !@fixture_connection_pools.include?(pool) && transactional_tests_for_pool?(pool)
                 pool.pin_connection!(lock_threads)
                 pool.lease_connection
                 @fixture_connection_pools << pool

activerecord/test/cases/fixtures_test.rb

@@ -1080,13 +1080,20 @@ def connect!; end
     end.new
 
     pool = connection.pool = Class.new do
+      attr_accessor :db_config
+
       def initialize(connection); @connection = connection; end
       def lease_connection; @connection; end
       def release_connection; end
       def pin_connection!(_); end
       def unpin_connection!; @connection.rollback_transaction; true; end
     end.new(connection)
 
+    connection.pool.db_config = Class.new do
+      attr_accessor :name
+      def initialize(name); @name = name; end
+    end.new("database_name")
+
     assert_called_with(pool, :pin_connection!, [true]) do
       fire_connection_notification(connection.pool)
     end
@@ -1107,13 +1114,20 @@ def connect!; end
     end.new
 
     connection.pool = Class.new do
+      attr_accessor :db_config
+
       def initialize(connection); @connection = connection; end
       def lease_connection; @connection; end
       def release_connection; end
       def pin_connection!(_); end
       def unpin_connection!; @connection.rollback_transaction; true; end
     end.new(connection)
 
+    connection.pool.db_config = Class.new do
+      attr_accessor :name
+      def initialize(name); @name = name; end
+    end.new("database_name")
+
     fire_connection_notification(connection.pool)
     teardown_fixtures
 
@@ -1131,13 +1145,20 @@ def connect!; end
     end.new
 
     connection.pool = Class.new do
+      attr_accessor :db_config
+
       def initialize(connection); @connection = connection; end
       def lease_connection; @connection; end
       def release_connection; end
       def pin_connection!(_); end
       def unpin_connection!; @connection.rollback_transaction; true; end
     end.new(connection)
 
+    connection.pool.db_config = Class.new do
+      attr_accessor :name
+      def initialize(name); @name = name; end
+    end.new("database_name")
+
     assert_called_with(connection.pool, :pin_connection!, [true]) do
       fire_connection_notification(connection.pool, shard: :shard_two)
     end

activerecord/test/cases/test_fixtures_test.rb

@@ -70,5 +70,133 @@ def test_run_successfully
       clean_up_connection_handler
       FileUtils.rm_r(tmp_dir)
     end
+
+    def test_transactional_tests_per_db_explicitly_disabled
+      tmp_dir = Dir.mktmpdir
+      File.write(File.join(tmp_dir, "zines.yml"), <<~YML)
+      going_out:
+        title: Hello
+      YML
+
+      klass = Class.new(Minitest::Test) do
+        include ActiveRecord::TestFixtures
+
+        self.fixture_paths = [tmp_dir]
+        self.use_transactional_tests = true
+        self.skip_transactional_tests_for_database :primary
+
+        fixtures :all
+
+        def test_run_successfully
+          assert_equal("Hello", Zine.first.title)
+          assert_equal("Hello", zines(:going_out).title)
+          # Change the data in the primary connection
+          Zine.first.update!(title: "Goodbye")
+        end
+      end
+
+      test_result = klass.new("test_run_successfully").run
+      assert_predicate(test_result, :passed?)
+      # Ensure that the primary connection was NOT rolled back
+      assert_equal("Goodbye", Zine.first.title)
+    ensure
+      FileUtils.rm_r(tmp_dir)
+    end
+
+    def test_transactional_tests_per_db_explicitly_enabled
+      tmp_dir = Dir.mktmpdir
+      File.write(File.join(tmp_dir, "zines.yml"), <<~YML)
+      going_out:
+        title: Hello
+      YML
+
+      klass = Class.new(Minitest::Test) do
+        include ActiveRecord::TestFixtures
+
+        self.fixture_paths = [tmp_dir]
+        self.use_transactional_tests = false
+        self.use_transactional_tests_for_database :primary
+
+        fixtures :all
+
+        def test_run_successfully
+          assert_equal("Hello", Zine.first.title)
+          assert_equal("Hello", zines(:going_out).title)
+          # Change the data in the primary connection
+          Zine.first.update!(title: "Goodbye")
+        end
+      end
+
+      test_result = klass.new("test_run_successfully").run
+      assert_predicate(test_result, :passed?)
+      # Ensure that the primary connection WAS rolled back
+      assert_equal("Hello", Zine.first.title)
+    ensure
+      FileUtils.rm_r(tmp_dir)
+    end
+
+    def test_transactional_tests_per_db_default_enabled
+      tmp_dir = Dir.mktmpdir
+      File.write(File.join(tmp_dir, "zines.yml"), <<~YML)
+      going_out:
+        title: Hello
+      YML
+
+      klass = Class.new(Minitest::Test) do
+        include ActiveRecord::TestFixtures
+
+        self.fixture_paths = [tmp_dir]
+        self.use_transactional_tests = true
+        self.skip_transactional_tests_for_database :unrelated
+
+        fixtures :all
+
+        def test_run_successfully
+          assert_equal("Hello", Zine.first.title)
+          assert_equal("Hello", zines(:going_out).title)
+          # Change the data in the primary connection
+          Zine.first.update!(title: "Goodbye")
+        end
+      end
+
+      test_result = klass.new("test_run_successfully").run
+      assert_predicate(test_result, :passed?)
+      # Ensure that the primary connection WAS rolled back
+      assert_equal("Hello", Zine.first.title)
+    ensure
+      FileUtils.rm_r(tmp_dir)
+    end
+
+    def test_transactional_tests_per_db_default_disabled
+      tmp_dir = Dir.mktmpdir
+      File.write(File.join(tmp_dir, "zines.yml"), <<~YML)
+      going_out:
+        title: Hello
+      YML
+
+      klass = Class.new(Minitest::Test) do
+        include ActiveRecord::TestFixtures
+
+        self.fixture_paths = [tmp_dir]
+        self.use_transactional_tests = false
+        self.use_transactional_tests_for_database :unrelated
+
+        fixtures :all
+
+        def test_run_successfully
+          assert_equal("Hello", Zine.first.title)
+          assert_equal("Hello", zines(:going_out).title)
+          # Change the data in the primary connection
+          Zine.first.update!(title: "Goodbye")
+        end
+      end
+
+      test_result = klass.new("test_run_successfully").run
+      assert_predicate(test_result, :passed?)
+      # Ensure that the primary connection was NOT rolled back
+      assert_equal("Goodbye", Zine.first.title)
+    ensure
+      FileUtils.rm_r(tmp_dir)
+    end
   end
 end