Skip to content

Commit

Permalink
Compromise solution to rspec#2773
Browse files Browse the repository at this point in the history
  • Loading branch information
pond committed Jul 4, 2024
1 parent 8c17b4e commit 1b3ac4b
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 1 deletion.
39 changes: 39 additions & 0 deletions lib/rspec/rails/adapters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,45 @@ def method_name
end
end

# @private
#
# ActiveSupport::CurrentAttributes::TestHelper uses both before-setup and
# after-teardown hooks to clear attributes. These are run in an awkward
# order relative to RSpec hooks. Client suite-configured "around each"
# hooks run first, the attribute reset hooks next, and contexts or examples
# (along with whatever their defined hooks might be) run last. Consequently
# a suite which sets up around-each-example attributes could fail, because
# the attributes would have been reset by time tests actually run.
#
# Reworking with "before" rather than "around" hooks sidesteps the problem.
# Unfortunately, "executes via a block to clean up after itself" cases such
# as with "ActsAsTenant.without_tenant ..." make such a rework either hard
# or impossible. Besides, even realising that you may need to do this can
# take some significant debugging effort and insight.
#
# The compromise is to use "half" of what ActiveSupport does - just add the
# after-hook. Attributes are now only reset after examples run, but still
# before a suite-wide "around" hook's call to "example.run()" returns. If a
# test suite depended upon current attribute data still being set *after*
# examples within its suite-wide "around" hook, it would fail. This is
# unlikely; it's overwhelmingly common to just *set up* data before a test,
# and usually the only things you're doing afterwards are cleanup.
#
# Include this module to reset under the above compromise, instead of
# including ActiveSupport::CurrentAttributes::TestHelper.
#
module ActiveSupportCurrentAttributesAdapter
extend ActiveSupport::Concern

included do
def after_teardown
puts "AFTER TEARDOWN"
super
ActiveSupport::CurrentAttributes.reset_all
end
end
end

# @private
module MinitestAssertionAdapter
extend ActiveSupport::Concern
Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/rails/example/rails_example_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module RailsExampleGroup
include RSpec::Rails::FixtureSupport
if ::Rails::VERSION::MAJOR >= 7
include RSpec::Rails::TaggedLoggingAdapter
include ActiveSupport::CurrentAttributes::TestHelper
include RSpec::Rails::ActiveSupportCurrentAttributesAdapter
include ActiveSupport::ExecutionContext::TestHelper
end
end
Expand Down
80 changes: 80 additions & 0 deletions spec/rspec/rails/example/rails_example_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,85 @@ class CurrentSample < ActiveSupport::CurrentAttributes
group.run(failure_reporter) ? true : failure_reporter.exceptions
).to be true
end

context 'with suite-level around-example hooks configured', if: ::Rails::VERSION::MAJOR >= 7 do
let(:uniquely_identifiable_metadata) do
{ configured_around_example_hook: true }
end

class CurrentAttrsBetweenHooks < ActiveSupport::CurrentAttributes
attribute :request_id
end

# We have to modify the suite's around-each in RSpec.config, but don't
# want to pollute other tests with this (whether or not it is harmless
# to do so). There being no public API to read or remove hooks, instead
# it's necessary use some private APIs to be able to delete the added
# hook via 'ensure'.
#
around :each do | example |

# Client code might legitimately want to wrap examples to ensure
# all-conditions tidy-up, e.g. "ActsAsTenant.without_tenant do...",
# wherein an "around" hook is the only available solution, often used
# in the overall suite via "config.around". Tests would not expect
# anything set in CurrentAttributes here to suddenly be reset by the
# time their actual tests, or their test hooks ran.
#
RSpec.configure do | config |
config.around(:each, uniquely_identifiable_metadata()) do | example |
CurrentAttrsBetweenHooks.request_id = '123'
example.run()
end
end

example.run()

ensure
around_example_repository = RSpec.configuration.hooks.send(:hooks_for, :around, :example)
item_we_added = around_example_repository.items_for(uniquely_identifiable_metadata()).first
around_example_repository.delete(item_we_added, uniquely_identifiable_metadata())
end

it 'does not reset ActiveSupport::CurrentAttributes before examples' do
group =
RSpec::Core::ExampleGroup.describe('A group', uniquely_identifiable_metadata()) do
include RSpec::Rails::RailsExampleGroup

it 'runs normally' do
expect(CurrentAttrsBetweenHooks.request_id).to eq('123')
end
end

expect(
group.run(failure_reporter) ? true : failure_reporter.exceptions
).to be true
end

it 'does not reset ActiveSupport::CurrentAttributes before before-each hooks' do
group =
RSpec::Core::ExampleGroup.describe('A group', uniquely_identifiable_metadata()) do
include RSpec::Rails::RailsExampleGroup

# Client code will often have test setup blocks within "*_spec.rb"
# files that might set up data or other environmental factors for a
# group of tests in e.g. a "before" hook, but would reasonably expect
# suite-wide 'around' settings to remain intact and not be reset.
#
before :each do | example |
expect(CurrentAttrsBetweenHooks.request_id).to eq('123')
CurrentAttrsBetweenHooks.request_id = '234'
end

it 'runs normally' do
expect(CurrentAttrsBetweenHooks.request_id).to eq('234')
end
end

expect(
group.run(failure_reporter) ? true : failure_reporter.exceptions
).to be true
end
end # "context 'with suite-level around-example hooks configured' ..."
end
end

0 comments on commit 1b3ac4b

Please sign in to comment.