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

Teardown hooks are called in the wrong order, resulting in a blank screenshot in Rails 6. #2153

Closed
alexsanderson opened this issue Jul 30, 2019 · 8 comments
Assignees

Comments

@alexsanderson
Copy link

What Ruby, Rails and RSpec versions are you using?

Ruby version: ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-darwin18]
Rails version: 6.0.0.rc1
RSpec version: 3.8

  • rspec-core 3.8.2
  • rspec-expectations 3.8.4
  • rspec-mocks 3.8.1
  • rspec-rails 3.8.2
  • rspec-support 3.8.2

Observed behaviour

after_teardown is run before before_teardown

Which results in Capybara.reset_sessions! being called before take_failed_screenshot and that results in a blank screenshot due to Capybara.reset_sessions! navigating to about:blank.

Expected behaviour

before_teardown is run before after_teardown

Can you provide an example app?

spec/system/teardown.rb

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe "teardown", type: :system, js: true do
  it "fails" do
    visit "/"
    expect(1).to eq(0)
  end
end

rails_helper.rb

# standard stuff, plus:
config.before(:each, type: :system, js: true) do
    driven_by :selenium_chrome
}

That spec will fail (correctly) and produce a screenshot, which is blank.

Here's byebug (positioned before the expect):

    1: # frozen_string_literal: true
    2: 
    3: require 'rails_helper'
    4: 
    5: RSpec.describe "teardown", type: :system, js: true do
    6:   it "fails" do
    7:     visit "/"
    8:     byebug
=>  9:     expect(1).to eq(0)
   10:   end
   11: end
(byebug) b ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown#before_teardown
Created breakpoint 1 at ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown:before_teardown
(byebug) b ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown#after_teardown
Created breakpoint 2 at ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown:after_teardown
(byebug) c
Stopped by breakpoint 2 at /Users/Alex/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/actionpack-6.0.0.rc1/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb:26

[21, 30] in /Users/Alex/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/actionpack-6.0.0.rc1/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
   21:         ensure
   22:           super
   23:         end
   24: 
   25:         def after_teardown
=> 26:           Capybara.reset_sessions!
   27:         ensure
   28:           super
   29:         end
   30:       end
(byebug) c
Stopped by breakpoint 1 at /Users/Alex/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/actionpack-6.0.0.rc1/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb:20

[15, 24] in /Users/Alex/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/actionpack-6.0.0.rc1/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
   15:           host! DEFAULT_HOST
   16:           super
   17:         end
   18: 
   19:         def before_teardown
=> 20:           take_failed_screenshot
   21:         ensure
   22:           super
   23:         end
   24: 
(byebug) c

Investigation

Looking at Rails 5.2 https://github.com/rails/rails/blob/f64de36c29b4dee412dd7fa8359849dabdcc54b9/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb#L19-L27 there is no before_teardown. Both take_failed_screenshot and Capybara.reset_sessions! are called in the same method in the right order.

This bug appears to have been introduced with a change in Rails 6.0 where that single method is split up into two: https://github.com/rails/rails/blob/23cdeceef6373614e1a1c6fa0c5d687d04bfab92/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb#L19-L29

@JonRowe
Copy link
Member

JonRowe commented Aug 1, 2019

Have you tried this on the 4-0-dev branch? That contains Rails 6 support. /cc @samphippen

@alexsanderson
Copy link
Author

I've updated to 4-0-dev@9c6d8f3 with no change -- the failure screenshot is blank with byebug showing the same call order (after_teardown before before_teardown).

@benoittgt
Copy link
Member

Thanks for the detailed report @alexsanderson. Do you think you can provide a fresh repo with a rails app with a reproduction case?

Cheers

@benoittgt benoittgt self-assigned this Aug 21, 2019
@alexsanderson
Copy link
Author

Sure thing, here we are @benoittgt -> https://github.com/alexsanderson/rspec-teardown

I've added how to reproduce in the README, but it's essentially: clone, bundle and run rspec.

@JonRowe
Copy link
Member

JonRowe commented Aug 23, 2019

@alexsanderson can you see if #2164 fixes your issue?

@alexsanderson
Copy link
Author

Yes, it works now @JonRowe. Perfect!

@JonRowe
Copy link
Member

JonRowe commented Aug 27, 2019

Closed in 4-0-dev

@JonRowe JonRowe closed this as completed Aug 27, 2019
@alexsanderson
Copy link
Author

Thank you! @JonRowe

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

3 participants