Merge pull request rails#51021 from Shopify/test-tweaks

Make various minor fixes to the Active Record test suite

Author: byroot

activerecord/lib/active_record/test_fixtures.rb

@@ -110,64 +110,64 @@ def uses_transaction?(method)
       end
     end
 
-    def fixture_path # :nodoc:
-      ActiveRecord.deprecator.warn(<<~WARNING)
-        TestFixtures#fixture_path is deprecated and will be removed in Rails 7.2. Use #fixture_paths instead.
-        If multiple fixture paths have been configured with #fixture_paths, then #fixture_path will just return
-        the first path.
-      WARNING
-      fixture_paths.first
-    end
-
-    def run_in_transaction?
-      use_transactional_tests &&
-        !self.class.uses_transaction?(name)
-    end
+    private
+      def fixture_path
+        ActiveRecord.deprecator.warn(<<~WARNING)
+          TestFixtures#fixture_path is deprecated and will be removed in Rails 7.2. Use #fixture_paths instead.
+          If multiple fixture paths have been configured with #fixture_paths, then #fixture_path will just return
+          the first path.
+        WARNING
+        fixture_paths.first
+      end
 
-    def setup_fixtures(config = ActiveRecord::Base)
-      if pre_loaded_fixtures && !use_transactional_tests
-        raise RuntimeError, "pre_loaded_fixtures requires use_transactional_tests"
+      def run_in_transaction?
+        use_transactional_tests &&
+          !self.class.uses_transaction?(name)
       end
 
-      @fixture_cache = {}
-      @fixture_connections = {}
-      @fixture_cache_key = [self.class.fixture_table_names.dup, self.class.fixture_paths.dup, self.class.fixture_class_names.dup]
-      @@already_loaded_fixtures ||= {}
-      @connection_subscriber = nil
-      @saved_pool_configs = Hash.new { |hash, key| hash[key] = {} }
-
-      if run_in_transaction?
-        # Load fixtures once and begin transaction.
-        @loaded_fixtures = @@already_loaded_fixtures[@fixture_cache_key]
-        unless @loaded_fixtures
-          @@already_loaded_fixtures.clear
-          @loaded_fixtures = @@already_loaded_fixtures[@fixture_cache_key] = load_fixtures(config)
+      def setup_fixtures(config = ActiveRecord::Base)
+        if pre_loaded_fixtures && !use_transactional_tests
+          raise RuntimeError, "pre_loaded_fixtures requires use_transactional_tests"
         end
 
-        setup_transactional_fixtures
-      else
-        # Load fixtures for every test.
-        ActiveRecord::FixtureSet.reset_cache
-        invalidate_already_loaded_fixtures
-        @loaded_fixtures = load_fixtures(config)
-      end
+        @fixture_cache = {}
+        @fixture_connections = {}
+        @fixture_cache_key = [self.class.fixture_table_names.dup, self.class.fixture_paths.dup, self.class.fixture_class_names.dup]
+        @@already_loaded_fixtures ||= {}
+        @connection_subscriber = nil
+        @saved_pool_configs = Hash.new { |hash, key| hash[key] = {} }
+
+        if run_in_transaction?
+          # Load fixtures once and begin transaction.
+          @loaded_fixtures = @@already_loaded_fixtures[@fixture_cache_key]
+          unless @loaded_fixtures
+            @@already_loaded_fixtures.clear
+            @loaded_fixtures = @@already_loaded_fixtures[@fixture_cache_key] = load_fixtures(config)
+          end
 
-      # Instantiate fixtures for every test if requested.
-      instantiate_fixtures if use_instantiated_fixtures
-    end
+          setup_transactional_fixtures
+        else
+          # Load fixtures for every test.
+          ActiveRecord::FixtureSet.reset_cache
+          invalidate_already_loaded_fixtures
+          @loaded_fixtures = load_fixtures(config)
+        end
 
-    def teardown_fixtures
-      # Rollback changes if a transaction is active.
-      if run_in_transaction?
-        teardown_transactional_fixtures
-      else
-        ActiveRecord::FixtureSet.reset_cache
+        # Instantiate fixtures for every test if requested.
+        instantiate_fixtures if use_instantiated_fixtures
       end
 
-      ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
-    end
+      def teardown_fixtures
+        # Rollback changes if a transaction is active.
+        if run_in_transaction?
+          teardown_transactional_fixtures
+        else
+          ActiveRecord::FixtureSet.reset_cache
+        end
+
+        ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
+      end
 
-    private
       def setup_transactional_fixtures
         setup_shared_connection_pool
 

activerecord/test/cases/adapter_test.rb

@@ -820,19 +820,21 @@ class AdapterThreadSafetyTest < ActiveRecord::TestCase
       @threads.each(&:kill)
     end
 
-    test "#active? is synchronized" do
-      threads(2, 25) { @connection.select_all("SELECT 1") }
-      threads(2, 25) { @connection.verify! }
-      threads(2, 25) { @connection.disconnect! }
+    unless in_memory_db?
+      test "#active? is synchronized" do
+        threads(2, 25) { @connection.select_all("SELECT 1") }
+        threads(2, 25) { @connection.verify! }
+        threads(2, 25) { @connection.disconnect! }
 
-      join
-    end
+        join
+      end
 
-    test "#verify! is synchronized" do
-      threads(2, 25) { @connection.verify! }
-      threads(2, 25) { @connection.disconnect! }
+      test "#verify! is synchronized" do
+        threads(2, 25) { @connection.verify! }
+        threads(2, 25) { @connection.disconnect! }
 
-      join
+        join
+      end
     end
 
     private

activerecord/test/cases/base_test.rb

@@ -1555,6 +1555,7 @@ def test_marshal_between_processes
         post.comments.build
         wr.write Marshal.dump(post)
         wr.close
+        exit!(0)
       end
 
       wr.close

activerecord/test/cases/connection_pool_test.rb

@@ -6,6 +6,11 @@
 module ActiveRecord
   module ConnectionAdapters
     module ConnectionPoolTests
+      def self.included(test)
+        super
+        test.use_transactional_tests = false
+      end
+
       attr_reader :pool
 
       def setup

activerecord/test/cases/migration/foreign_key_test.rb

@@ -674,20 +674,30 @@ def change
         end
 
         def test_add_foreign_key_is_reversible
+          @connection.drop_table("cities", if_exists: true)
+          @connection.drop_table("houses", if_exists: true)
+
           migration = CreateCitiesAndHousesMigration.new
           silence_stream($stdout) { migration.migrate(:up) }
           assert_equal 1, @connection.foreign_keys("houses").size
-        ensure
           silence_stream($stdout) { migration.migrate(:down) }
+        ensure
+          @connection.drop_table("cities", if_exists: true)
+          @connection.drop_table("houses", if_exists: true)
         end
 
         def test_foreign_key_constraint_is_not_cached_incorrectly
+          @connection.drop_table("cities", if_exists: true)
+          @connection.drop_table("houses", if_exists: true)
+
           migration = CreateCitiesAndHousesMigration.new
           silence_stream($stdout) { migration.migrate(:up) }
           output = dump_table_schema "houses"
           assert_match %r{\s+add_foreign_key "houses",.+on_delete: :cascade$}, output
-        ensure
           silence_stream($stdout) { migration.migrate(:down) }
+        ensure
+          @connection.drop_table("cities", if_exists: true)
+          @connection.drop_table("houses", if_exists: true)
         end
 
         class CreateSchoolsAndClassesMigration < ActiveRecord::Migration::Current

activerecord/test/cases/test_fixtures_test.rb

@@ -6,6 +6,8 @@
 require "models/zine"
 
 class TestFixturesTest < ActiveRecord::TestCase
+  self.use_transactional_tests = false
+
   setup do
     @klass = Class.new
     @klass.include(ActiveRecord::TestFixtures)