mattheworiordan / capybara-screenshot

Automatically save screen shots when a Capybara scenario fails
MIT License
1.02k stars 168 forks source link

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

Open dzirtusss opened 6 years ago

dzirtusss commented 6 years ago

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?

Systho commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 :+1:.

deivid-rodriguez commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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! :wink:

In any case, I managed to figure out what was going on... :tada:. 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 commented 5 years ago

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

stoplion commented 4 years ago

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

timdiggins commented 2 years ago

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