Skip to content

Adding visual regression testing with BackstopJS#5

Draft
SriHV wants to merge 31 commits into
mainfrom
testing-to-include-backstopjs
Draft

Adding visual regression testing with BackstopJS#5
SriHV wants to merge 31 commits into
mainfrom
testing-to-include-backstopjs

Conversation

@SriHV

@SriHV SriHV commented May 13, 2024

Copy link
Copy Markdown
Contributor

What is the context of this PR?

This PR is raised as as per 3086 in the Design System.

Following things are changed in this repo:

  • Example components in the DS are included to work in flask.
  • Included BackstopJS for visual tests.
  • Changed and added helper files to support visual tests and components pages.

How to review this PR

  • All screenshots added to this PR should be visually reviewed to make sure they all look as expected
  • Follow readme to make sure it is clear enough to follow
  • Test you are able to spin up the app and run the visual tests

@SriHV SriHV requested a review from a team May 13, 2024 10:31
Comment thread Makefile Outdated
Comment thread Makefile Outdated
./scripts/load_release.sh onsdigital/design-system $(DESIGN_SYSTEM_VERSION)

run:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think it would be good to run load-design-system-templates as part of this command so they are always loaded in when you run the application. The docs will need updating for this

Comment thread .gitignore Outdated
Comment thread README.md Outdated

```
pip install Flask
pip install Flask python-frontmatter

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we should be using a python package manager like poetry rather than pip but again probably something for another PR

Comment thread README.md
To run/automate the Visual Tests, BackstopJs is used, comparing screenshots overtime.To install BackstopJs run

```
npm install -g backstopjs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with the DS we should probably use yarn not npm

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in prev tech session, when I try to install yarn its downloading too many packages that I think are not required. Can you check this and tell me if its doing the same for you also

Comment thread README.md Outdated
Comment thread README.md Outdated

`make generate-backstopjs`: This python file generates `backstop.json` file with specified Design System URLs,screen sizes, DOM selectors etc.

`backstop test`: This creates a set of screenshots and compares them with reference screenshots and shows any changes in the visual report.(`backstop_data/html_report`).Note-: Make sure to keep the local server running before executing this step.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it make sense for this command and the approve one to also be added to the makefile

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread scripts/load_release.sh Outdated
Comment on lines +31 to +38
TEMP_FILE=$(mktemp)

sed "/set CDN_version/d" "templates/external-css.html" > temp_file && mv temp_file "templates/external-css.html"
echo "{% set CDN_version = '${TAG_NAME}' %}" | cat - "templates/external-css.html" > "$TEMP_FILE"

mv "$TEMP_FILE" "templates/external-css.html"
rm -rf "${TEMP_FILE}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isnt needed if we use the page template

@rmccar rmccar changed the title Testing to include backstopjs Adding visual regression testing with BackstopJS May 13, 2024
SriHV and others added 11 commits May 13, 2024 16:23
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
@rmccar rmccar force-pushed the testing-to-include-backstopjs branch from 632a7ec to f38b29e Compare May 24, 2024 13:09
@SriHV SriHV marked this pull request as draft September 3, 2024 10:31
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.

Investigate adding snapshot VR tests for jinja2 environments

2 participants