Rename all snapshots to "light"

[obulat]

Fixes

Related to #4305 by @zackkrida

Description

Most of the changes in this PR are renaming of the snapshots: from "...-linux.png" to "...-light.png". In the follow-up PR, the "-dark.png" snapshots will be added.

This PR removes the -linux platform suffix from the snapshot names. This will enable someone on a different system to easily compare the snapshots to the linux ones in the repository, instead of creating new snapshots that cannot be compared. this change was removed to prevent flakiness in the future, as per @sarayourfriend's comment below.

Testing Instructions

The code changes should make sense, and the snapshots should only be renamed, not changed.

Checklist

• My pull request has a descriptive title (not a vague title likeUpdate index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.
• I ran the DAG documentation generator (ov just catalog/generate-docs for catalog
  PRs) or the media properties generator (ov just catalog/generate-docs media-props
  for the catalog or ov just api/generate-docs for the API) where applicable.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[obulat]

By default, this is '{snapshotDir}/{testFileDir}/{testFileName}-snapshots/{arg}{-projectName}{-snapshotSuffix}{ext}'

snapshotSuffix is the platform (linux), project is a logical group of tests running in the same browser (context). Since we only have one project, we don't need this.

https://github.com/microsoft/playwright/blob/a4bd551597f0161a853f7bdb725e738544db2130/packages/playwright/src/common/config.ts#L166

[obulat]

Removed this in 6395b2c

[sarayourfriend]

Add light to the snapshot names makes sense to me 👍

However, I do not believe we should remove the platform indication from the filename. You mention in the PR body that they make it hard to compare snapshots generated across platforms... but that is intentional, it's the reason the platform is indicated. All the major browsers behave differently between platforms, especially and importantly in font rendering. Indicating the platform the snapshot was generated under ensures that when comparisons do happen, there is no possibility for platform-specific variations to cause noise in the diff.

We run playwright tests inside the Docker container for that reason, to eliminate that noise. We really should not compare screenshots from different OSs. If there are devex issues with the Playwright container that make it necessary to run the visual regression tests outside the Playwright container such that the platform indication would not be linux, then we need to fix those issues. Making it easy to do a faulty comparison (screenshots across OSs) is not an appropriate solution to that problem.

This is a blocker because while we can easily revert the change and add the platform name back, doing this at all sets an incorrect precedence about what kinds of comparisons are valid, and could very easily cause significant confusion, and even introduce flakiness in CI. For example, if a screenshot is generated outside the container, with a different platform and browser version, and then were committed, small font rendering and layout variations could cause difficult to understand flakiness in the screenshots.

[sarayourfriend]

Just reviewing the diffs of screenshots. Some storybook ones are actually changed, rather than just renamed. Almost all of these with changes have visible differences, and some seem incorrect, so I wanted to confirm they are intentional changes.

The button screenshots have a variety of differences. Some have significantly different colours, like frontend/test/storybook/visual-regression/v-button.spec.ts-snapshots/filled-pink-8-resting-linux.png:

Some have the same colours, but show ~2px of difference in the width of the button, like frontend/test/storybook/visual-regression/v-button.spec.ts-snapshots/filled-dark-resting-linux.png and frontend/test/storybook/visual-regression/v-button.spec.ts-snapshots/filled-pink-8-hovered-linux.png (although the latter also has the colour difference issue).

By the name frontend/test/storybook/visual-regression/v-button.spec.ts-snapshots/filled-white-hovered-linux.png and frontend/test/storybook/visual-regression/v-button.spec.ts-snapshots/transparent-dark-hovered-linux.png would appear to be white and dark-transparent, but both don't match my expectations of what a white or transparent button would be (including in their versions on main, so something is wrong here overall, maybe).

frontend/test/storybook/visual-regression/v-icon-button.spec.ts-snapshots/v-icon-button-sizes-linux.png is completely broken and doesn't show the components anymore:

[obulat]

Just reviewing the diffs of screenshots. Some storybook ones are actually changed, rather than just renamed. Almost all of these with changes have visible differences, and some seem incorrect, so I wanted to confirm they are intentional changes.

Sorry for clicking "rerequest review" too early. I noticed that the storybook snapshots are off, and I extracted #4919 out of this PR. Since the Storybook tests had maxDiffPixelRatio set, they were not updated when the colors were updated during the dark theme project. It would be good to audit all the colors/focus rings/snapshots, but I think it should be done later. This specific PR is necessary to set up the light/dark tests, which will make fixing that easier, later :)

[zackkrida]

This looks good. Every snapshot is renamed without changes, with the (correct) exception of the additions:

• frontend/test/storybook/visual-regression/v-icon-button.spec.ts-snapshots/v-icon-button-sizes-light-linux.png
• frontend/test/storybook/visual-regression/focus.spec.ts-snapshots/focus-colored-light-linux.png

[sarayourfriend]

Sounds good! LGTM