-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added new ADR for BackstopJS #1297
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks @alexjanousekGSA!
This looks good to me aside from one minor grammatical change. I'll give folks a bit of time to weigh in as well. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. Looking forward to working with BackstopJS!
Co-authored-by: Carlo Costino <ccostino@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @alexjanousekGSA for solutions to testing! I don't have any questions about BackstopJS for now. I am curious by the potential of BackstopJS. I'd like to see BackstopJS as an option for testing until we become more familiar with it and understand its use cases better. We already have many different testing tools on the frontend, and while BackstopJS is not exactly the same, managing and testing with multiple tools can be cumbersome.
That said, I'm curious about the tool and do think it could be helpful to see at a glance which parts of the site have changed. This could be particularly useful if an image goes missing unexpectedly or if there are CSS or layout changes. I'm open to exploring the value and insights it could provide.
It would be helpful to run it quickly, similar to how we use flake8 or isort, but overall, I think we should keep it optional for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing this up, @alexjanousekGSA! 🎉
Once this is merged, we're going to make a few adjustments and additions to it, and flip it to the accepted
state.
ADR outlining proposal for adding backstop JS to admin project to improve QA.
Original Issue including screenshots done during testing can be found here
Details around the deep dive and POC can be found here