mattheworiordan / capybara-screenshot

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

Screenshots are not taken for expectation failures #212

Closed mattbrictson closed 7 years ago

mattbrictson commented 7 years ago

Consider a feature spec with this expectation:

expect(page).to have_css(".foo")

If this expectation fails, I assume that this gem should take a screenshot. But surprisingly it does not.

A screenshot is only taken if I rewrite the spec as:

expect(page.find(".foo")).to be_present

I much prefer the first syntax, and it is also the style suggested by the Capybara docs.

The reason the second version works is because find raises an exception if it fails.

Otherwise, my understanding is that a failed expect (as in the first example) does not set example.exception, and therefore this test in capybara-screenshot evaluates to false, meaning the screenshot will not be taken.

if Capybara.page.current_url != '' && Capybara::Screenshot.autosave_on_failure && example.exception
                                                                                  ^^^^^^^^^^^^^^^^^

What is the reasoning behind limiting screenshots to exceptions only, and can this be changed? Or perhaps made configurable? I would be happy to contribute a PR.

Otherwise there is an entire style of tests that simply won't produce screenshots on failures, like:

expect(page).to have_content "..."
expect(page).to have_css "..."
expect(page).to have_current_path "..."
# etc.

I'm using:

capybara (2.14.4)
capybara-screenshot (1.0.17)
rspec (3.6.0)
rspec-core (3.6.0)
rspec-expectations (3.6.0)
rspec-mocks (3.6.0)
rspec-rails (3.6.0)
rspec-support (3.6.0)
mattheworiordan commented 7 years ago

Hi @mattbrictson. Thanks for looking into this.

I tracked it down and I think the example.exception can be removed.

See https://github.com/mattheworiordan/capybara-screenshot/issues/2 which is why it was added, and the fixing commit https://github.com/mattheworiordan/capybara-screenshot/commit/339430662ba26b6233a594608d740ec7616ea247. See https://github.com/mattheworiordan/capybara-screenshot/commit/4a255610009567115f4deca2bc356716bbc2459c as well, where the type was filtered to ensure screenshots were not captured unless it's a request.

If you remove the example.exception check, then you will need to figure out how to determine if a spec has failed or not, see https://github.com/mattheworiordan/capybara-screenshot/blob/master/lib/capybara-screenshot/rspec.rb#L80-L85. Looking at the RSpec code, I think you may be able to check the status instead, see https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/example.rb#L549.

If the status field is available, then it should be a trivial PR. I look forward to it :) Thanks!