Add visual regression tests with Playwright #345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eric-Arellano
force-pushed
the
playwright
branch
2 times, most recently
from
f97142f
to
80fd17d
Compare
Eric-Arellano
changed the title
Eric-Arellano
commented
May 26, 2023
.github/workflows/main.yml
Outdated
| with: | ||
| name: html_docs | ||
| path: artifacts | ||
|
|
||
| - name: Run JavaScript and snapshot tests | ||
| run: npm run _run-tests |
There was a problem hiding this comment.
Note that this isn't the standard npm test, which will use Docker. I optimized npm test to be the ideal experience for local developers.
Eric-Arellano
requested review from
javabster,
coruscating,
HuangJunye and
1ucian0
Eric-Arellano
force-pushed
the
playwright
branch
2 times, most recently
from
146f6b7
to
1839db1
Compare
Eric-Arellano
added a commit
that referenced
this pull request
Jun 5, 2023
When we added our tests for web components, we only had Jest/JavaScript test infrastructure. No Pytest/Python test infrastructure. So, I wrote the tests in JavaScript. But the tests can easily be written in Python instead. Why make this change? If we land #345, then it will require now using Docker to run the tests locally (but you can use CI to get the updated snapshot files). So, I want to have our JavaScript tests only be things that truly require JavaScript: our visual regression tests, and possibly in the future tests of our JavaScript code. If we decide to not land #345, then we may want to remove this JavaScript test infrastructure entirely.
vabarbosa
reviewed
Jun 6, 2023
javabster
reviewed
Jun 7, 2023
javabster
reviewed
Jun 7, 2023
CONTRIBUTING.md
Outdated
|
|
||
| To run these tests, you first need to install Node.js on your machine. If you expect to use JavaScript in other projects, we recommend using [NVM](https://github.com/nvm-sh/nvm). Otherwise, consider using [Homebrew](https://formulae.brew.sh/formula/node) or installing [Node.js directly](https://nodejs.org/en). | ||
| 1. Find the "actual" snapshot for the failing test, such as `footer-snapshot-has-not-changed-1-actual.png`. | ||
| 2. Copy that snapshot into the folder `tests/js/snapshots.test.js-snapshots`. Rename the `-actual.png` file ending to be `-linux.png` and overwrite the prior file. |
There was a problem hiding this comment.
why do we need to rename the file from actual to linux?
There was a problem hiding this comment.
Because snapshot_results will call it -actual.png, but when it's in the folder snapshots.test.js-snapshots, Playwright will expect it to end in -linux.png.
This is how Playwright works. Not something we can adjust.
javabster
reviewed
Jun 7, 2023
javabster
reviewed
Jun 7, 2023
javabster
requested changes
Jun 7, 2023
javabster
reviewed
Jun 9, 2023
javabster
reviewed
Jun 9, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #208.
How snapshots work
Playwright snapshot tests take screenshots of certain portions of our site, such as the footer. They save that screenshot to Git, so that we all have the exact copy. Then, for every PR, our tests will retake a new screenshot and make sure it's the exact same as before. That way, we always know when we make a change.
When there was a change, Playwright will output this:
As stated there, Playwright will generate a before, after, and diff photo. In CI, we upload those as GitHub artifacts.
If the change was intentional, then the author copies the updated screenshot to overwrite the old file.
Uses Docker for consistency
Websites render slightly differently between operating systems, e.g. Linux and macOS. Naively, this would cause visual regression tests to fail when running the tests locally on a mac.
So, we use Docker to make sure we always use the same environment.
If users don't want to run Docker locally, they can rely on CI to get the updated snapshots via GitHub Actions Artifacts.
More advanced users can also run the tests locally. They need to install Docker and have the Docker daemon running. But otherwise, our scripts automate everything. They don't need to know how to use Docker. They only need to run
npm test.How to run locally
(Reminder that contributors can rely on CI for visual regression testing.)
The user has to first build the docs with
THEME=_qiskit_furo tox -e docs. Then, they only runnpm test(after originally runningnpm install& starting Docker).Playwright will start up a server for us.
If the user changed the theme, they must rebuild the docs with Tox.
Switches from Jest to Playwright
I originally tried doing this change via Jest, a common JavaScript test runner. But I had major issues getting the browser automation library Puppeteer #341 to work properly on my M1.
So, this changes our test runner to Playwright, a test runner from Microsoft optimized for visual regression testing like we're doing. It can do things like click buttons, e.g. our Translations menu.
Only Furo
To keep things simple, this only adds infrastructure to snapshot test Furo. Given that our goal is to migrate ASAP, this seemed okay to me.