Merge pull request rails#43550 from Shopify/as-test-case-executor-fix

Call Executor#wrap around each test

Author: byroot

actionview/lib/action_view/cache_expiry.rb

@@ -4,7 +4,7 @@ module ActionView
   class CacheExpiry
     class Executor
       def initialize(watcher:)
-        @execution_lock = Concurrent::ReadWriteLock.new
+        @execution_lock = Concurrent::ReentrantReadWriteLock.new
         @cache_expiry = ViewModificationWatcher.new(watcher: watcher) do
           clear_cache
         end

activerecord/lib/active_record/asynchronous_queries_tracker.rb

@@ -7,6 +7,9 @@ class << self
         def active?
           true
         end
+
+        def finalize
+        end
       end
     end
 

activerecord/lib/active_record/query_logs/test_helper.rb

@@ -380,11 +380,6 @@ class Railtie < Rails::Railtie # :nodoc:
           app.reloader.before_class_unload { ActiveRecord::QueryLogs.clear_context }
           app.executor.to_run              { ActiveRecord::QueryLogs.clear_context }
           app.executor.to_complete         { ActiveRecord::QueryLogs.clear_context }
-
-          ActiveSupport.on_load(:active_support_test_case) do
-            require "active_record/query_logs/test_helper"
-            include ActiveRecord::QueryLogs::TestHelper
-          end
         end
       end
     end

activerecord/lib/active_record/railtie.rb

@@ -2,19 +2,19 @@
 
 require "cases/helper"
 require "models/dashboard"
-require "active_record/query_logs/test_helper"
 
 class QueryLogsTest < ActiveRecord::TestCase
-  # Automatically included in Rails apps via railtie but that don't run here.
-  include ActiveRecord::QueryLogs::TestHelper
-
   fixtures :dashboards
 
   ActiveRecord::QueryLogs.taggings[:application] = -> {
     "active_record"
   }
 
   def setup
+    # QueryLogs context is automatically reset in Rails app via an executor hooks set in railtie
+    # But not in Active Record's own test suite.
+    ActiveRecord::QueryLogs.clear_context
+
     # Enable the query tags logging
     @original_transformers = ActiveRecord.query_transformers
     @original_prepend = ActiveRecord::QueryLogs.prepend_comment
@@ -28,6 +28,9 @@ def teardown
     ActiveRecord.query_transformers = @original_transformers
     ActiveRecord::QueryLogs.prepend_comment = @original_prepend
     ActiveRecord::QueryLogs.tags = []
+    # QueryLogs context is automatically reset in Rails app via an executor hooks set in railtie
+    # But not in Active Record's own test suite.
+    ActiveRecord::QueryLogs.clear_context
   end
 
   def test_escaping_good_comment

activerecord/test/cases/query_logs_test.rb

@@ -1,3 +1,12 @@
+*   `Rails.application.executor` hooks are now called around every tests.
+
+    This helps to better simulate request or job local state being reset around tests and prevent state
+    to leak from one test to another.
+
+    However it requires the executor hooks executed in the test environment to be re-entrant.
+
+    *Jean Boussier*
+
 *   `ActiveSupport::DescendantsTracker` now mostly delegate to `Class#descendants` on Ruby 3.1
 
     Ruby now provides a fast `Class#descendants` making `ActiveSupport::DescendantsTracker` mostly useless.
@@ -6,7 +15,6 @@
 
       - `ActiveSupport::DescendantsTracker.direct_descendants`
       - `ActiveSupport::DescendantsTracker#direct_descendants`
-
     *Jean Boussier*
 
 *   Fix the `Digest::UUID.uuid_from_hash` behavior for namespace IDs that are different from the ones defined on `Digest::UUID`.

activesupport/CHANGELOG.md

@@ -91,6 +91,16 @@ def self.wrap
       end
     end
 
+    def self.perform # :nodoc:
+      instance = new
+      instance.run
+      begin
+        yield
+      ensure
+        instance.complete
+      end
+    end
+
     class << self # :nodoc:
       attr_accessor :active
     end
@@ -108,6 +118,10 @@ def self.active? # :nodoc:
 
     def run! # :nodoc:
       self.class.active[Thread.current] = true
+      run
+    end
+
+    def run # :nodoc:
       run_callbacks(:run)
     end
 
@@ -116,11 +130,15 @@ def run! # :nodoc:
     #
     # Where possible, prefer +wrap+.
     def complete!
-      run_callbacks(:complete)
+      complete
     ensure
       self.class.active.delete Thread.current
     end
 
+    def complete # :nodoc:
+      run_callbacks(:complete)
+    end
+
     private
       def hook_state
         @_hook_state ||= {}

activesupport/lib/active_support/execution_wrapper.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module ActiveSupport::Executor::TestHelper # :nodoc:
+  def run(...)
+    Rails.application.executor.perform { super }
+  end
+end

activesupport/lib/active_support/executor/test_helper.rb

@@ -28,13 +28,20 @@ class Railtie < Rails::Railtie # :nodoc:
     end
 
     initializer "active_support.reset_all_current_attributes_instances" do |app|
+      executor_around_test_case = app.config.active_support.delete(:executor_around_test_case)
+
       app.reloader.before_class_unload { ActiveSupport::CurrentAttributes.clear_all }
       app.executor.to_run              { ActiveSupport::CurrentAttributes.reset_all }
       app.executor.to_complete         { ActiveSupport::CurrentAttributes.reset_all }
 
       ActiveSupport.on_load(:active_support_test_case) do
-        require "active_support/current_attributes/test_helper"
-        include ActiveSupport::CurrentAttributes::TestHelper
+        if executor_around_test_case
+          require "active_support/executor/test_helper"
+          include ActiveSupport::Executor::TestHelper
+        else
+          require "active_support/current_attributes/test_helper"
+          include ActiveSupport::CurrentAttributes::TestHelper
+        end
       end
     end
 

activesupport/lib/active_support/railtie.rb

@@ -1,11 +1,17 @@
 # frozen_string_literal: true
 
 require_relative "abstract_unit"
-require "active_support/current_attributes/test_helper"
 
 class CurrentAttributesTest < ActiveSupport::TestCase
-  # Automatically included in Rails apps via railtie but that don't run here.
-  include ActiveSupport::CurrentAttributes::TestHelper
+  # CurrentAttributes is automatically reset in Rails app via executor hooks set in railtie
+  # But not in Active Support's own test suite.
+  setup do
+    ActiveSupport::CurrentAttributes.reset_all
+  end
+
+  teardown do
+    ActiveSupport::CurrentAttributes.reset_all
+  end
 
   Person = Struct.new(:id, :name, :time_zone)