Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call Executor#wrap around each test #43550

Merged
merged 1 commit into from Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion actionview/lib/action_view/cache_expiry.rb
Expand Up @@ -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
Expand Down
Expand Up @@ -7,6 +7,9 @@ class << self
def active?
true
end

def finalize
end
end
end

Expand Down
13 changes: 0 additions & 13 deletions activerecord/lib/active_record/query_logs/test_helper.rb

This file was deleted.

5 changes: 0 additions & 5 deletions activerecord/lib/active_record/railtie.rb
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions activerecord/test/cases/query_logs_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 9 additions & 1 deletion activesupport/CHANGELOG.md
@@ -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.
Expand All @@ -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`.
Expand Down
20 changes: 19 additions & 1 deletion activesupport/lib/active_support/execution_wrapper.rb
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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 ||= {}
Expand Down
7 changes: 7 additions & 0 deletions activesupport/lib/active_support/executor/test_helper.rb
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module ActiveSupport::Executor::TestHelper # :nodoc:
def run(...)
Rails.application.executor.perform { super }
end
end
11 changes: 9 additions & 2 deletions activesupport/lib/active_support/railtie.rb
Expand Up @@ -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

Expand Down
12 changes: 9 additions & 3 deletions activesupport/test/current_attributes_test.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)

Expand Down
1 change: 1 addition & 0 deletions railties/lib/rails/application/configuration.rb
Expand Up @@ -214,6 +214,7 @@ def load_defaults(target_version)
active_support.remove_deprecated_time_with_zone_name = true
active_support.cache_format_version = 7.0
active_support.use_rfc4122_namespaced_uuids = true
active_support.executor_around_test_case = true
end

if respond_to?(:action_mailer)
Expand Down
Expand Up @@ -45,6 +45,12 @@
# and you have no plans to rollback.
# Rails.application.config.active_support.cache_format_version = 7.0

# Calls `Rails.application.executor.wrap` around test cases.
# This makes test cases behave closer to an actual request or job, several
# features that are normally disabled in test, such as Active Record query cache
# and asynchronous queries will then be enabled.
# Rails.application.config.active_support.executor_around_test_case = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is missing updating configuring.md to add to document this new options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fbd11ab


# Set both the `:open_timeout` and `:read_timeout` values for `:smtp` delivery method.
# Rails.application.config.action_mailer.smtp_timeout = 5

Expand Down