Merge pull request #49470 from rails/rm-eager-load-model-schema

rafaelfranca · rafaelfranca · commit 1490788ecd06 · 2023-10-04T09:51:55.000Z
Load the model schema when running test in eager load context
diff --git a/actionmailbox/test/test_helper.rb b/actionmailbox/test/test_helper.rb
@@ -5,6 +5,8 @@
 ENV["RAILS_ENV"] = "test"
 ENV["RAILS_INBOUND_EMAIL_PASSWORD"] = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL"
 
+require_relative "../../tools/test_common"
+
 require_relative "../test/dummy/config/environment"
 ActiveRecord::Migrator.migrations_paths = [File.expand_path("../test/dummy/db/migrate", __dir__)]
 require "rails/test_help"
@@ -53,5 +55,3 @@ def bounce(to:)
     end
   end
 end
-
-require_relative "../../tools/test_common"
diff --git a/activejob/test/helper.rb b/activejob/test/helper.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative "../../tools/test_common"
+
 require "active_support/testing/strict_warnings"
 require "active_job"
 require "support/job_buffer"
@@ -18,8 +20,6 @@
 
 require "active_support/testing/autorun"
 
-require_relative "../../tools/test_common"
-
 def adapter_is?(*adapter_class_symbols)
   adapter_class_symbols.map(&:to_s).include? ActiveJob::Base.queue_adapter_name
 end
diff --git a/activejob/test/support/integration/adapters/backburner.rb b/activejob/test/support/integration/adapters/backburner.rb
@@ -9,7 +9,7 @@ def setup
     end
     unless can_run?
       puts "Cannot run integration tests for backburner. To be able to run integration tests for backburner you need to install and start beanstalkd.\n"
-      status = ENV["CI"] ? false : true
+      status = ENV["BUILDKITE"] ? false : true
       exit status
     end
   end
diff --git a/activejob/test/support/integration/adapters/queue_classic.rb b/activejob/test/support/integration/adapters/queue_classic.rb
@@ -41,7 +41,7 @@ def start_workers
 
   rescue PG::ConnectionBad
     puts "Cannot run integration tests for queue_classic. To be able to run integration tests for queue_classic you need to install and start postgresql.\n"
-    status = ENV["CI"] ? false : true
+    status = ENV["BUILDKITE"] ? false : true
     exit status
   end
 
diff --git a/activejob/test/support/integration/adapters/resque.rb b/activejob/test/support/integration/adapters/resque.rb
@@ -7,7 +7,7 @@ def setup
     Resque.logger = Rails.logger
     unless can_run?
       puts "Cannot run integration tests for resque. To be able to run integration tests for resque you need to install and start redis.\n"
-      status = ENV["CI"] ? false : true
+      status = ENV["BUILDKITE"] ? false : true
       exit status
     end
   end
diff --git a/activejob/test/support/integration/adapters/sidekiq.rb b/activejob/test/support/integration/adapters/sidekiq.rb
@@ -10,7 +10,7 @@ def setup
     ActiveJob::Base.queue_adapter = :sidekiq
     unless can_run?
       puts "Cannot run integration tests for sidekiq. To be able to run integration tests for sidekiq you need to install and start redis.\n"
-      status = ENV["CI"] ? false : true
+      status = ENV["BUILDKITE"] ? false : true
       exit status
     end
   end
diff --git a/activejob/test/support/integration/adapters/sneakers.rb b/activejob/test/support/integration/adapters/sneakers.rb
@@ -18,7 +18,7 @@ def setup
                         log: Rails.root.join("log/sneakers.log").to_s
     unless can_run?
       puts "Cannot run integration tests for sneakers. To be able to run integration tests for sneakers you need to install and start rabbitmq.\n"
-      status = ENV["CI"] ? false : true
+      status = ENV["BUILDKITE"] ? false : true
       exit status
     end
   end
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
@@ -553,6 +553,20 @@ def reset_column_information
         initialize_find_by_cache
       end
 
+      def load_schema # :nodoc:
+        return if schema_loaded?
+        @load_schema_monitor.synchronize do
+          return if @columns_hash
+
+          load_schema!
+
+          @schema_loaded = true
+        rescue
+          reload_schema_from_cache # If the schema loading failed half way through, we must reset the state.
+          raise
+        end
+      end
+
       protected
         def initialize_load_schema_monitor
           @load_schema_monitor = Monitor.new
@@ -594,20 +608,6 @@ def schema_loaded?
           defined?(@schema_loaded) && @schema_loaded
         end
 
-        def load_schema
-          return if schema_loaded?
-          @load_schema_monitor.synchronize do
-            return if @columns_hash
-
-            load_schema!
-
-            @schema_loaded = true
-          rescue
-            reload_schema_from_cache # If the schema loading failed half way through, we must reset the state.
-            raise
-          end
-        end
-
         def load_schema!
           unless table_name
             raise ActiveRecord::TableNotSpecified, "#{self} has no table configured. Set one with #{self}.table_name="
diff --git a/activerecord/test/support/connection.rb b/activerecord/test/support/connection.rb
@@ -22,7 +22,7 @@ def self.connect
     ActiveRecord.async_query_executor = :global_thread_pool
     puts "Using #{connection_name}"
 
-    if ENV["CI"]
+    if ENV["BUILDKITE"]
       ActiveRecord::Base.logger = nil
     else
       ActiveRecord::Base.logger = ActiveSupport::Logger.new("debug.log", 1, 100.megabytes)
diff --git a/activestorage/test/analyzer/image_analyzer/image_magick_test.rb b/activestorage/test/analyzer/image_analyzer/image_magick_test.rb
@@ -65,7 +65,7 @@ def analyze_with_image_magick
 
       yield
     rescue LoadError
-      ENV["CI"] ? raise : skip("Variant processor image_magick is not installed")
+      ENV["BUILDKITE"] ? raise : skip("Variant processor image_magick is not installed")
     ensure
       ActiveStorage.variant_processor = previous_processor
     end
diff --git a/activestorage/test/analyzer/image_analyzer/vips_test.rb b/activestorage/test/analyzer/image_analyzer/vips_test.rb
@@ -65,7 +65,7 @@ def analyze_with_vips
 
       yield
     rescue LoadError
-      ENV["CI"] ? raise : skip("Variant processor vips is not installed")
+      ENV["BUILDKITE"] ? raise : skip("Variant processor vips is not installed")
     ensure
       ActiveStorage.variant_processor = previous_processor
     end
diff --git a/activestorage/test/dummy/config/environments/test.rb b/activestorage/test/dummy/config/environments/test.rb
@@ -45,7 +45,7 @@
   end
   # Azure service tests are currently failing on the main branch.
   # We temporarily disable them while we get things working again.
-  if ENV["CI"]
+  if ENV["BUILDKITE"]
     SERVICE_CONFIGURATIONS.delete(:azure)
     SERVICE_CONFIGURATIONS.delete(:azure_public)
   end
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
@@ -287,7 +287,7 @@ def process_variants_with(processor)
       previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, processor
       yield
     rescue LoadError
-      ENV["CI"] ? raise : skip("Variant processor #{processor.inspect} is not installed")
+      ENV["BUILDKITE"] ? raise : skip("Variant processor #{processor.inspect} is not installed")
     ensure
       ActiveStorage.variant_processor = previous_processor
     end
diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb
@@ -29,7 +29,7 @@ def ensure_connected! # after https://github.com/petergoldstein/dalli/pull/863
     end
   end
 
-  if ENV["CI"]
+  if ENV["BUILDKITE"]
     MEMCACHE_UP = true
   else
     begin
diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb
@@ -22,7 +22,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests
   REDIS_URL = ENV["REDIS_URL"] || "redis://localhost:6379/0"
   REDIS_URLS = ENV["REDIS_URLS"]&.split(",") || %w[ redis://localhost:6379/0 redis://localhost:6379/1 ]
 
-  if ENV["CI"]
+  if ENV["BUILDKITE"]
     REDIS_UP = true
   else
     begin
diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb
@@ -14,14 +14,9 @@
 
 require "active_support/testing/autorun"
 
-if defined?(ActiveRecord::Base)
-  begin
-    ActiveRecord::Migration.maintain_test_schema!
-  rescue ActiveRecord::PendingMigrationError => e
-    puts e.to_s.strip
-    exit 1
-  end
+require "rails/testing/maintain_test_schema"
 
+if defined?(ActiveRecord::Base)
   ActiveSupport.on_load(:active_support_test_case) do
     include ActiveRecord::TestDatabases
     include ActiveRecord::TestFixtures
diff --git a/railties/lib/rails/testing/maintain_test_schema.rb b/railties/lib/rails/testing/maintain_test_schema.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+if defined?(ActiveRecord::Base)
+  begin
+    ActiveRecord::Migration.maintain_test_schema!
+  rescue ActiveRecord::PendingMigrationError => e
+    puts e.to_s.strip
+    exit 1
+  end
+
+  if Rails.configuration.eager_load
+    ActiveRecord::Base.descendants.each do |model|
+      model.load_schema unless model.abstract_class?
+    end
+  end
+end
diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb
@@ -317,6 +317,39 @@ class UserTest < ActiveSupport::TestCase
       assert_not_includes output, "after:"
     end
 
+    test "schema for all the models is loaded when tests are run in eager load context" do
+      output = rails("generate", "model", "user", "name:string")
+      version = output.match(/(\d+)_create_users\.rb/)[1]
+
+      app_file "db/schema.rb", <<-RUBY
+        ActiveRecord::Schema.define(version: #{version}) do
+          create_table :users do |t|
+            t.string :name
+          end
+
+          create_table :action_text_rich_texts
+          create_table :active_storage_variant_records
+          create_table :active_storage_blobs
+          create_table :active_storage_attachments
+          create_table :action_mailbox_inbound_emails do |t|
+            t.integer :status
+          end
+        end
+      RUBY
+
+      app_file "config/initializers/enable_eager_load.rb", <<-RUBY
+        Rails.application.config.eager_load = true
+      RUBY
+
+      app_file "app/models/user.rb", <<-RUBY
+        class User < ApplicationRecord
+          enum :type, [:admin, :user]
+        end
+      RUBY
+
+      assert_unsuccessful_run "models/user_test.rb", "Unknown enum attribute 'type' for User"
+    end
+
     private
       def assert_unsuccessful_run(name, message)
         result = run_test_file(name)
diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb
@@ -24,6 +24,8 @@
   Minitest::Retry.use!(verbose: false, retry_count: 1)
 end
 
+require_relative "../../../tools/test_common"
+
 RAILS_FRAMEWORK_ROOT = File.expand_path("../../..", __dir__)
 
 # These files do not require any others and are needed
diff --git a/railties/test/plugin_helpers.rb b/railties/test/plugin_helpers.rb
@@ -29,7 +29,11 @@ def prepare_plugin(plugin_path)
   def in_plugin_context(plugin_path, &block)
     # Run with `Bundler.with_unbundled_env` so that Bundler uses the plugin's
     # Gemfile instead of this repo's Gemfile.
-    Dir.chdir(plugin_path) { Bundler.with_unbundled_env(&block) }
+    delete_ci_env = -> do
+      ENV.delete("CI") if ENV["BUILDKITE"]
+      block.call
+    end
+    Dir.chdir(plugin_path) { Bundler.with_unbundled_env(&delete_ci_env) }
   end
 
   def with_new_plugin(plugin_path, *args, &block)
diff --git a/tools/test_common.rb b/tools/test_common.rb
@@ -2,6 +2,7 @@
 
 if ENV["BUILDKITE"]
   require "minitest-ci"
+  ENV.delete("CI") # CI has affect on the applications, and we don't want it applied to the apps.
 
   Minitest::Ci.report_dir = File.join(__dir__, "../test-reports/#{ENV['BUILDKITE_JOB_ID']}")
 end
