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.

Sign up for GitHub

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

Reset CurrentAttributes on each rails example #2752

Merged
merged 9 commits into from
Apr 10, 2024
Merged

Reset CurrentAttributes on each rails example #2752

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0e72a05
Reset CurrentAttributes on each rails example
javierjulio Apr 6, 2024
d829a02
Match formatting for require statement
javierjulio Apr 6, 2024
9089639
Merge branch 'rspec:main' into patch-1
javierjulio Apr 8, 2024
91167a0
Add spec
javierjulio Apr 9, 2024
9e35e00
Add spec
javierjulio Apr 9, 2024
808fef7
Include RailsExampleGroup in spec
javierjulio Apr 9, 2024
b27a5b9
Address spec failures
javierjulio Apr 9, 2024
d1a4c0d
Remove trailing spaces
javierjulio Apr 9, 2024
3013459
Refactor spec for current attributes
JonRowe Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/rspec/rails/example/rails_example_group.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'rspec/rails/matchers'

if ::Rails::VERSION::MAJOR >= 7
require 'active_support/current_attributes/test_helper'
require 'active_support/execution_context/test_helper'
end

Expand All @@ -18,6 +19,7 @@ module RailsExampleGroup
include RSpec::Rails::FixtureSupport
if ::Rails::VERSION::MAJOR >= 7
include RSpec::Rails::TaggedLoggingAdapter
include ActiveSupport::CurrentAttributes::TestHelper
include ActiveSupport::ExecutionContext::TestHelper
end
end
Expand Down
17 changes: 17 additions & 0 deletions spec/rspec/rails/example/rails_example_group_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
module RSpec::Rails
RSpec.describe RailsExampleGroup do
if ::Rails::VERSION::MAJOR >= 7
class CurrentSample < ActiveSupport::CurrentAttributes
Copy link
Member

Choose a reason for hiding this comment

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

What fails exactly without this?
At a glance it may looks that it’s the following example that check for tagged logger, but I guess it’s not, is it?

Copy link
Member

Choose a reason for hiding this comment

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

Its just setup...

attribute :request_id
end

it 'supports tagged_logger' do
expect(described_class.private_instance_methods).to include(:tagged_logger)
end
Expand Down Expand Up @@ -32,5 +36,18 @@ module RSpec::Rails

expect(results).to all be true
end

describe 'CurrentAttributes', order: :defined, if: ::Rails::VERSION::MAJOR >= 7 do
Copy link
Member

Choose a reason for hiding this comment

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

The concept is hard to absorb at first, but here there are two types of groups, the first that is part of our spec suite, that is first evaluated when you run rspec over rspec-rails specs, but before running any specs. The second is defined during specs runtime, like RSpec.describe you can see above. I wouldn’t mix in RailsExampleGroup into the former, but it can be freely done with the latter. See example above for the reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pirj I didn't get to this in time. Seeing Jon's update, I'm just not familiar with how specs are written here and struggled a bit. I'll know how to better approach it in the future. Thanks!

include RSpec::Rails::RailsExampleGroup

it 'sets a current attribute' do
CurrentSample.request_id = '123'
expect(CurrentSample.request_id).to eq('123')
end

it 'does not leak current attributes' do
expect(CurrentSample.request_id).to eq(nil)
end
end
end
end