mattheworiordan / capybara-screenshot

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

Ensure screenshot taken on expect() failures #213

Closed mattbrictson closed 7 years ago

mattbrictson commented 7 years ago

Starting with RSpec 3.3.0, expect() failures do not raise an exception immediately. Instead each failure is recorded in a thread-local "aggregator" and then raised at end of the example.

This interferes with capybara-screenshot because after hooks execute before these aggregated failures are raised. In other words, in the after block example.exception will be nil. This makes capybara-screenshot think that the example hasn't failed, and thus it does not take a screenshot, even though there are actual failures.

This PR works around this problem by using some behind-the-scenes knowledge of how rspec-expectations does the aggregation. Now we reach into the thread-local storage (RSpec::Support.failure_notifier) and discover whether there are in fact failures "queued up". If so, we will consider the example to have failed and take a screenshot.


Fixes #212

This PR is incomplete: I still need to add tests, etc. The PR is ready to merge.

mattheworiordan commented 7 years ago

@mattbrictson I really like where this is going, than you. I think that in order to merge this in, we'd definitely need to modify the Appraisals config to include the latest version of RSpec. See https://github.com/mattheworiordan/capybara-screenshot/blob/master/.travis.yml#L13 and https://github.com/mattheworiordan/capybara-screenshot/blob/master/Appraisals#L1-L3. See https://github.com/thoughtbot/appraisal for more info on the Appraisal gem.

Shout if I can help with anything, but if you can show that this works with 3.0 and 3.3 with separate appraisals, I'd say we're good to go.

Thanks again, 👍

mattbrictson commented 7 years ago

I've added a test and included rspec 3.3 in the Appraisal and Travis configs. So this PR is mostly good to go.

The problem is that Travis fails for Ruby 2.0, due to nokogiri no longer supporting that version. I've tried a couple different approaches to work around the problem (see a146f96 and 51d2ddd) unsuccessfully.

I am not super-familiar with Appraisal so perhaps there is a solution that I am missing. Any suggestions?

mattbrictson commented 7 years ago

Problem solved! See efd81c6.

@mattheworiordan I think this PR is ready for your review when you have a chance. Thanks!

mattbrictson commented 7 years ago

My team has been using this branch for several weeks with no problems. I'd be great to get this merged!

AndreiH commented 7 years ago

Thank you @mattbrictson for this. It looks good. @mattheworiordan can we get this modifications approved ? :D

arkaitzgarro commented 7 years ago

Same here, it works nice for us as well. Thank you @mattbrictson. @mattheworiordan can we make it happen? 🙏

AndreiH commented 7 years ago

@mattheworiordan Any updates on this ? Please please please

mattheworiordan commented 7 years ago

Sorry all, I have been very sh*t of late. I am merging now and bumping the version. Thank you VERY MUCH @mattbrictson for this contribution!

mattheworiordan commented 7 years ago

@mattbrictson https://github.com/mattheworiordan/capybara-screenshot/commit/92aaddcca91cadcb73639b9e54a61f1b642e567c#diff-4ac32a78649ca5bdd8e0ba38b7006a1eR7 🙏