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

capybara-screenshot does not work with Rails 5.1, RSpec 3.7 and system tests #225

Open
dzirtusss opened this issue Mar 8, 2018 · 13 comments
Labels

Comments

@dzirtusss
Copy link

dzirtusss commented Mar 8, 2018

Problem is that capybara-screenshot capture screenshot on failure hook is positioned after rails system's test after_teardown method which destroys Capybara session.

Currently, I made a quick workaround for myself (which is not a proper way of fixing the problem of cause) as following:

  config.before(:all, type: :system) do
    # HACK: move capybara-screenshot hook on top of rspec after hooks to make it working in RSpec 3.7 system tests,
    # otherwise ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown.after_teardown destroys Capybara.session
    # before screenshot can be captured
    rspec_after_hooks = self.class.
                        instance_variable_get(:@hooks).
                        instance_variable_get(:@after_example_hooks).
                        instance_variable_get(:@items_and_filters)

    screenshot_hook_index = rspec_after_hooks.index { |i| i.first.block.to_s =~ %r{/capybara-screenshot/} }
    rspec_after_hooks.unshift(rspec_after_hooks.delete_at(screenshot_hook_index)) if screenshot_hook_index > 0
  end

Can you please think of a possible proper solution and add it to the gem?

@dzirtusss dzirtusss changed the title capybara-screenshor does not work with Rails 5.1, RSpec 3.7 and system tests capybara-screenshot does not work with Rails 5.1, RSpec 3.7 and system tests Mar 8, 2018
@Systho
Copy link
Contributor

Systho commented Mar 13, 2018

Personally I have used another workaround : disabling rails default system test screenshot using this snippet in rails_helper:

require "action_dispatch/system_testing/test_helpers/setup_and_teardown"
::ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown.module_eval do
  def before_setup
    super
  end

  def after_teardown
    super
  end
end

But I agree that an official solution would be much better :)

Also keep the good work ! Your gem is currently a thousand times better than the Rails machanism

@mattheworiordan
Copy link
Owner

@dzirtusss thanks for alerting me to this. I was not aware of the problem as I am mostly not working in Rails these days.

@Systho I was not aware that Rails has introduced default screenshots now. Given that's something that could be disabled, it feels like that's the thing to focus on i.e. if capybara-screnshot is required, then it disables the new Rails system screenshot.

I fear that given I am flat out on non-Rails things these days, it may take me a long time to implement a solution here. Would either of you be willing to help resolve this and I will do what I can to assist?

@deivid-rodriguez
Copy link

Thanks @Systho. your monkeypatch worked like a charm, although I also needed to add the snippet below to my rspec config

config.after :each, type: :system do |example|
  Capybara::Screenshot::RSpec.after_failed_example(example)
end

Maybe it's also worth mentioning here that I tried to add support for "HTML screenshots" to Rails mechanism in this PR a while ago, but it looks like there's no interest on it. On top of that, this gem has some extra features like automatic maintainance of the screenshots folder, so it is still very valuable for some of us! :)

@mattheworiordan
Copy link
Owner

so it is still very valuable for some of us! :)

Glad to hear :)

Any chance you have a little bandwidth to help find a more permanent fix?

@deivid-rodriguez
Copy link

Ei @mattheworiordan. We migrated to capybara-screenshot briefly using the patches I mentioned previously, but we had to revert it due to some weird eager loading issues. If I can get it fully working I'll gladly try to contribute back my findings to the project 👍.

@deivid-rodriguez
Copy link

My problems were not due to any eager loading at all, actually.

@Systho Did you get your monkeypatch fully working? I found that the way you're doing it does not reset the capybara session, so I get flakies. But if I keep the original Capybara.reset_sessions! in the Rails code, I can't get capybara-screenshot working because by the time it tries to take the screenshot, the session has already been reset. Did you not notice any issues?

@Systho
Copy link
Contributor

Systho commented Jul 30, 2018

I have no such problem. I'm relying on this code of the file capybara/rspec :

  # The before and after blocks must run instantaneously, because Capybara
  # might not actually be used in all examples where it's included.
  config.after do
    if self.class.include?(Capybara::DSL)
      Capybara.reset_sessions!
      Capybara.use_default_driver
    end
  end

@deivid-rodriguez
Copy link

Indeed, I saw that code too. But it seemed to not happen at the right time... I'll try get a reproducible example so we can keep investigating. :)

@mattheworiordan
Copy link
Owner

@deivid-rodriguez or anyone else here, if someone can create a reproducible example for me to work with, I will find the time on a weekend to look into this issue and fix it. I am sorry I don't have much time right now, but I realise this is a blocker now so will try and help!

@deivid-rodriguez
Copy link

deivid-rodriguez commented Aug 1, 2018

@mattheworiordan We are using a little gem that monkeypatches Rails mechanism to also take HTML screenshots. That's good enough for us for now, so no blockers on our side! 😉

In any case, I managed to figure out what was going on... 🎉. The bad news is that I don't know what the best solution for this is, or where it should live (capybara? rails? rspec-rails? capybara-screenshot?). Current RSpec integration with Rails apps is very hard and hacky due to the fact that Rails does not use RSpec internally but Minitest, so a lot of hacks need to be done for things to work.

I'll now try to explain my research. @Systho Are you using by any chance database_cleaner instead of Rails builtin use_transactional_fixtures = true? I think that might explain why you're not having issues.

According to my research, the problem was that with @Systho's monkeypatch, transactional fixtures were being rolledback before capybara sessions were reset, so any asyncronous requests that started before the rollback would see the database in an inconsistent state. Capybara.reset_sessions! waits for every pending request to finish, so by calling that before resetting transactional fixtures we avoid that problem. However, with the monkeypatch, the correct order no longer happens.

A sketch of the final super-hacky solution that I found would be something like:

#
# Make sure Rails hooks are available
#
require "action_dispatch/system_testing/test_helpers/setup_and_teardown"

#
# Monkeypatch after_teardown to just call "super". No screenshot taken or capybara sessions being reset. That super there is what rolls back transactional fixtures after each test.
#
::ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown.module_eval do
  def after_teardown
    super
  end
end

#
# Save the previously monkeypatched method so we can call it when we need to.
#
unbounded_after_teardown = ::ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown.instance_method(:after_teardown)

#
# Fully disable the after_teardown method now, make it do nothing
#
::ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown.module_eval do
  def after_teardown; end
end

#
# Finally, require rspec-rails, knowing that no teardown stuff will happen without our control.
#
require "rspec/rails"

RSpec.configure do |config|
  config.use_transactional_fixtures = true

  #
  # Do the following after each test:
  #
  # * First, take a screenshot if necessary.
  # * Second, reset sessions.
  # * Third, rollback transactional fixtures.
  #
  config.after :each, type: :system do |example|
    begin
      Capybara::Screenshot::RSpec.after_failed_example(example)
      Capybara.reset_sessions!
    ensure
      unbounded_after_teardown.bind(self).call
    end
  end

@szTheory
Copy link
Contributor

szTheory commented Sep 9, 2019

I had to change both capybara-screenshot and rspec-rails to get it working in Rails 6. See:

@stoplion
Copy link

Any way (without monkey patching) to make this work with Rails 6.0, and Rspec-Rails 4.0.1. ?

@timdiggins
Copy link

timdiggins commented May 5, 2022

There is a PR in progress on rspec-rails, which works for rails 6.1+. You could use this with:

group :development, :test do
  gem "rspec-rails", github: "timdiggins/rspec-rails", branch: "after-teardown-in-system-specs"
end

NB: this will still captures the screenshot png via both the capybara-screenshot and rails built-in mechanism. You could monkey patch the rails built-in mechanism via the following (this could be added to capybara-screenshot once the rspec-rails PR is merged)

require "action_dispatch/system_testing/test_helpers/screenshot_helper"
module ActionDispatch
  module SystemTesting
    module TestHelpers
      module ScreenshotHelper
        def take_failed_screenshot
        end
      end
    end
  end
end

For 5.2 and 6.0 there is a branch that works:

group :development, :test do
  gem "rspec-rails", github: "timdiggins/rspec-rails", branch: "after-teardown-in-system-specs-allowing-rails52"
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants