Skip to content

Ensure that system test screenshots work for specs that have :aggregate_failures enabled #1907

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

Merged

Conversation

mattbrictson
Copy link
Contributor

Prior to this PR, if RSpec's :aggregate_failures is enabled, a system spec with a failed expectation would not automatically trigger a screenshot as expected.

This is because aggregated failures are not exposed via the typical RSpec.current_example.exception. To correctly detect failures when aggregation is enabled, we have to do more work.

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.

This is the same workaround as used by capybara-screenshot:
mattheworiordan/capybara-screenshot#213

@JonRowe
Copy link
Member

JonRowe commented Nov 15, 2017

Would you mind rebasing this off master to check this build passes now master does?

If RSpec's `:aggregate_failures` is enabled, a system spec with a failed
expectation would not automatically trigger a screenshot as expected.

This is because aggregated failures are not exposed via the typical
`RSpec.current_example.exception`. To correctly detect failures when
aggregation is enabled, we have to do more work.

This commit 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.

This is the same workaround as used by capybara-screenshot:
mattheworiordan/capybara-screenshot#213
@mattbrictson mattbrictson force-pushed the screenshot-support-aggregate-failures branch from 8fe5b58 to c0c5ae0 Compare November 15, 2017 22:56
@mattbrictson
Copy link
Contributor Author

Rebased

@mattbrictson
Copy link
Contributor Author

🎉

@JonRowe JonRowe merged commit 95f4522 into rspec:master Nov 16, 2017
@mattbrictson mattbrictson deleted the screenshot-support-aggregate-failures branch November 16, 2017 01:39
@JonRowe
Copy link
Member

JonRowe commented Nov 16, 2017

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants