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

reset_sessions! during system specs (ie. after_teardown) should occur as late as possible #2595

Closed
timdiggins opened this issue Apr 22, 2022 · 1 comment

Comments

@timdiggins
Copy link
Contributor

Is your feature request related to a problem? Please describe.

It's really difficult to integrate the capybara-screenshot gem with rails system specs. (see mattheworiordan/capybara-screenshot#225), but I think there is a generic issue for other libraries and integrations that you want the resetting of capybara sessions to happen as late as possible (and critically, after all RSpec Notifiers have formatted their output).

The minitest after_teardown callback occurs during the after hooks in the lifecycle of system specs, but occurs after all the after hooks (after example.run during the around) in all other specs.

included do |group|
group.before { after_setup }
group.after { before_teardown }
group.around do |example|
before_setup
example.run
after_teardown
end
end

after do
orig_stdout = $stdout
$stdout = StringIO.new
begin
if ::Rails::VERSION::STRING >= '6.0'
original_before_teardown.bind(self).call
end
original_after_teardown.bind(self).call
ensure
myio = $stdout
RSpec.current_example.metadata[:extra_failure_lines] = myio.string
$stdout = orig_stdout
end
end

The after_teardown is where the Capybara.reset_sessions! happens and (as of Rails 6.0) the take_screenshot occurs occurs during before_teardown/

Describe the solution you'd like

I'd like the after_teardown to happen during an around callback.
This wouldn't be possible if rspec-rails was still supporting Rails v5.2

This may change behaviour for downstream users, but should be able to be included in 6 because a major•revision with an appropriate warning in the changelog?

I'll do a possible implementation/illustration in a PR.

Describe alternatives you've considered

I've considered whether this should be an adaptation of rails (I don't think there's any argument for changing rails).

I've considered if this should be an adaptation of downstream libraries such as capybara-screenshot (see mattheworiordan/capybara-screenshot#225, this seems tricky to do -- though this may be down to architectural decisions in capybara-screenshot).

I've also attempted to monkey-patch either the rails ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown class in an ad hoc way in my own rails projects but this is quite difficult to do because it is already heavily monkey-patched by RSpec::Rails::SystemExampleGroup.

Additional context

I will raise this in rspec@googlegroups.com, but I think it's probably easier to see as set out in this issue.

timdiggins added a commit to timdiggins/rspec-rails that referenced this issue Apr 22, 2022
This will only work as expected (print out screenshot location after failed tests)
in >= rails 6.0 but 5.2 is no longer supported in main.

This may possibly change behaviour, but can be included in major
revision?

This should enable easier integration of other libraries such as
capybara-screenshot which require predictable (and late) running of
Capybara.reset_sessions!

This is an illustration (or possible candidate to fix) rspec#2595
@JonRowe
Copy link
Member

JonRowe commented May 5, 2022

Closed in #2596

@JonRowe JonRowe closed this as completed May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants