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

Move screenshot clearing to be before error raise #90

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

TaylorBrysonRouse
Copy link
Contributor

When a spec or test would fail from a difference, the following specs/tests would fail with a generic “no base image” error with the stacktrace containing an error stating that the previous test base screenshot was not present:

First test failure: 

Minitest::Assertion:
  Screenshot does not match for 'features-email_notification_preferences_spec/email_preferences_with__review_requests_should_allow_rescan_customers_to_view_rescan_preference/00_rescan_preference_page_visited' ({"area_size":263590.0,"region":[256.0,19.0,1482.0,234.0],"difference_level":0.006523589743589743})
  /home/circleci/project/spec/screenshots/linux/features-email_notification_preferences_spec/email_preferences_with__review_requests_should_allow_rescan_customers_to_view_rescan_preference/00_rescan_preference_page_visited.png
  /home/circleci/project/spec/screenshots/linux/features-email_notification_preferences_spec/email_preferences_with__review_requests_should_allow_rescan_customers_to_view_rescan_preference/00_rescan_preference_page_visited.base.diff.png
  /home/circleci/project/spec/screenshots/linux/features-email_notification_preferences_spec/email_preferences_with__review_requests_should_allow_rescan_customers_to_view_rescan_preference/00_rescan_preference_page_visited.diff.png
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:457:in `instance_exec'......

Second test failure:
Failure/Error: raise "There is no original (base) screenshot version to compare, located: #{@base_image_path}" unless @base_image_path.exist?

RuntimeError:
  There is no original (base) screenshot version to compare, located: /home/circleci/project/spec/screenshots/linux/features-email_notification_preferences_spec/email_preferences_with__review_requests_should_allow_rescan_customers_to_view_rescan_preference/00_rescan_preference_page_visited.base.png
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff/image_compare.rb:106:in `_different?'
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff/image_compare.rb:67:in `different?'
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff/test_methods.rb:92:in `assert_image_not_changed'
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff.rb:95:in `block in track_failures'
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff.rb:94:in `map'
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff.rb:94:in `track_failures'
/home/circleci/project/vendor/bundle/ruby/3.1.0/gems/capybara-screenshot-diff-1.8.0/lib/capybara/screenshot/diff.rb:83:in `block in included'....

This bug was caused by the fact that in diff.rb the tested screenshots are cleared out after track_failures. This is a problem because track_failures raises an error if the screenshots are different; causing the code to break and not return to the clearing of the testing screenshots. In this PR, I have moved the clearing of the testing screenshots to happen inside of track_failures after the errors have been collected from the test screenshots.

@pftg pftg merged commit 7a81562 into snap-diff:master Nov 30, 2023
6 checks passed
@pftg
Copy link
Collaborator

pftg commented Nov 30, 2023

@TaylorBrysonRouse, nice catch. 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