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

Add Rake task for PR visual regression comparison #459

Closed
wants to merge 2 commits into from
Closed

Add Rake task for PR visual regression comparison #459

wants to merge 2 commits into from

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Aug 22, 2017

Compare production and a review app hosted on Heroku. Pass in a PR number to generate a config and run the test.

eg bundle exec rake wraith:pr[450]

  • Removes unused update_document_types task
  • Refactors wraith tasks to DRY up
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-459 Aug 22, 2017 Inactive
@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Aug 22, 2017

Nice! Should we add something to the Readme so people know they can do this?

@fofr fofr force-pushed the pr-vr branch from 894caae to 086dee5 Aug 22, 2017
@fofr fofr temporarily deployed to government-frontend-pr-459 Aug 22, 2017 Inactive
@fofr
Copy link
Contributor Author

@fofr fofr commented Aug 22, 2017

Updated with documentation and a task for comparing master.

@fofr fofr temporarily deployed to government-frontend-pr-459 Aug 23, 2017 Inactive
@fofr
Copy link
Contributor Author

@fofr fofr commented Aug 23, 2017

Found an issue with Wraith, documented here: bbc/wraith#536
And avoiding the issue with 37d402b

@@ -29,11 +29,13 @@ def build_paths
if @paths.is_a? Hash
@paths.keys.each do |key|
@paths.fetch(key).each_with_index do |path, index|
config_paths[path_index(key, index + 1)] = path
config_paths[path_index(key, index + 1)] = path unless path_blacklisted?(path)

This comment has been minimized.

@nickcolley

nickcolley Aug 29, 2017
Contributor

This currently silently removes config, which could be really confusing. "Why is it not running this path!?" Could we log a message when a path is blacklisted so people know why we've not allowed this path?

This comment has been minimized.

@fofr

fofr Aug 29, 2017
Author Contributor

Good call.

@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-459 Sep 13, 2017 Inactive
fofr added 2 commits Aug 22, 2017
Compare production and a review app hosted on Heroku.

* Pass in a PR number to generate a config and run the test.
* Run against the heroku deployment of master
* Refactors wraith tasks to DRY up
Paths containing "path" in them break Wraith:
bbc/wraith#536

Search API incorrectly returns government-frontend as renderer for
travel advice index, this will always give a difference between live
and a review app.
@tijmenb
Copy link
Contributor

@tijmenb tijmenb commented Nov 2, 2017

Is this PR still relevant now that there's alphagov/govuk-visual-regression#1?

@fofr fofr closed this Nov 9, 2017
@fofr fofr deleted the pr-vr branch Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.