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

build: implement visual regression testing with @web/test-runner #2624

Merged
merged 55 commits into from
May 15, 2024

Conversation

kyubisation
Copy link
Contributor

@kyubisation kyubisation commented Apr 26, 2024

This PR sets up the basic configuration for visual regression testing with @web/test-runner.

web-test-runner.config.js contains a new test group, which is only activated when explicitly called with yarn test:visual-regression. This test group introduces a new file pattern *.snapshot.spec.ts, which contains the visual regression tests.
Additionally, there is a new GitHub Actions workflow, which runs the visual regression tests and saves failed screenshots in an artifact. This artifact is then downloaded in the secure workflow, which adds an in_progress check, if any failed screenshots exist.

We do not want to save the snapshot png in the repository, as this would bloat it considerably. Due to this, we use containers as an external storage of the png files. The process works as follows:
Each continuous integration run on our main branch creates/updates a container baseline (https://github.com/lyne-design-system/lyne-components/pkgs/container/lyne-components%2Fvisual-regression). For any other CI run (in the secure context) this container is used as a service and the png files are downloaded when needed.
Additionally, the baseline image is hosted at https://lyne-visual-regression-baseline.app.sbb.ch/ in order to enable local visual regression testing.

As for local visual regression testing; To avoid diffs between operating system browser differences, visual regression testing is only directly run in a Linux environment. If it is executed in any other OS, it will try to run it in a container to prevent OS differences showing up in the snapshots.

@kyubisation kyubisation requested a review from TomMenga April 26, 2024 15:43
@kyubisation kyubisation force-pushed the build-visual-regression-container-setup branch from 47cb60a to 0ac1261 Compare April 26, 2024 15:44
Copy link
Contributor

@TomMenga TomMenga left a comment

Choose a reason for hiding this comment

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

Some elements are still obscure but I have a general idea of the implementation.
I'm not sure how the remote baseline is used and why we need to use a container to run the diff locally (but I'm sure there is a reason)

Some discussion topics:

  • I'm a bit scared of the exponential nature of the number of cases (P1 * P2 * ... * Viewport * browsers * 3 (default, focus & hover))
    • Pro: covers almost every use case
    • Con: grows very quickly and requires an "implementation" for each component
  • Alternatively, we could reuse the stories template?
    • Pro: we would avoid most of the .snapshot.spec code
    • Con: not exhaustive like combining each case

@github-actions github-actions bot requested a deployment to preview-pr2624 May 2, 2024 12:59 In progress
@codecov-commenter
Copy link

codecov-commenter commented May 2, 2024

Codecov Report

Attention: Patch coverage is 41.17647% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 93.13%. Comparing base (e6397df) to head (f5256ad).
Report is 91 commits behind head on main.

❗ Current head f5256ad differs from pull request most recent head 42cf6c2. Consider uploading reports for the commit 42cf6c2 to get more accurate results

Files Patch % Lines
src/components/core/testing/test-setup-ssr.ts 9.09% 20 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2624      +/-   ##
==========================================
- Coverage   93.18%   93.13%   -0.06%     
==========================================
  Files         316      315       -1     
  Lines       25389    25323      -66     
  Branches     2063     2071       +8     
==========================================
- Hits        23660    23585      -75     
- Misses       1696     1705       +9     
  Partials       33       33              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot requested a deployment to preview-pr2624 May 2, 2024 13:52 In progress
@github-actions github-actions bot requested a deployment to preview-pr2624 May 2, 2024 14:42 In progress
@github-actions github-actions bot requested a deployment to preview-pr2624 May 2, 2024 15:40 In progress
@github-actions github-actions bot temporarily deployed to preview-pr2624 May 6, 2024 10:21 Inactive
@kyubisation kyubisation force-pushed the build-visual-regression-container-setup branch from f5256ad to 42cf6c2 Compare May 14, 2024 08:13
@github-actions github-actions bot temporarily deployed to preview-pr2624 May 14, 2024 08:26 Inactive
…r-setup' into build-visual-regression-container-setup

# Conflicts:
#	package.json
#	src/components/button/button/button.snapshot.spec.ts
#	src/components/core/testing/private/describe-each.ts
#	src/components/core/testing/private/describe-viewports.ts
#	src/components/core/testing/private/visual-regression-snapshot.ts
#	src/components/core/testing/test-setup.ts
#	yarn.lock
Signed-off-by: Jeremias Peier <[email protected]>
Signed-off-by: Jeremias Peier <[email protected]>
Signed-off-by: Jeremias Peier <[email protected]>
@github-actions github-actions bot temporarily deployed to preview-pr2624 May 14, 2024 08:55 Inactive
@github-actions github-actions bot temporarily deployed to preview-pr2624 May 14, 2024 15:21 Inactive
@kyubisation
Copy link
Contributor Author

Some elements are still obscure but I have a general idea of the implementation. I'm not sure how the remote baseline is used and why we need to use a container to run the diff locally (but I'm sure there is a reason)

Some discussion topics:

  • I'm a bit scared of the exponential nature of the number of cases (P1 * P2 * ... * Viewport * browsers * 3 (default, focus & hover))

    • Pro: covers almost every use case
    • Con: grows very quickly and requires an "implementation" for each component
  • Alternatively, we could reuse the stories template?

    • Pro: we would avoid most of the .snapshot.spec code
    • Con: not exhaustive like combining each case

Thanks for the review and feedback!
We have reduced the amount of snapshots by being a bit more selective.
Additionally I updated the PR comment to better describe the container use cases.
I think it makes sense to reuse story templates where it is reasonable, but I assume it will depend on the case.

@kyubisation kyubisation marked this pull request as ready for review May 15, 2024 08:41
@github-actions github-actions bot added the pr: peer review required A peer review is required for this pull request label May 15, 2024
@jeripeierSBB jeripeierSBB merged commit ec4fdc8 into main May 15, 2024
22 checks passed
@jeripeierSBB jeripeierSBB deleted the build-visual-regression-container-setup branch May 15, 2024 09:46
@github-actions github-actions bot added pr: peer review required A peer review is required for this pull request and removed pr: peer review required A peer review is required for this pull request labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: peer review required A peer review is required for this pull request preview-available
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants