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

[INFRA] Allow to provide e2e reference screenshots by browser #2838

Open
tbouffard opened this issue Aug 30, 2023 · 0 comments
Open

[INFRA] Allow to provide e2e reference screenshots by browser #2838

tbouffard opened this issue Aug 30, 2023 · 0 comments
Labels
chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...)

Comments

@tbouffard
Copy link
Member

tbouffard commented Aug 30, 2023

Current situation

Currently, for each non regression visual test, we have a single reference screenshot that is use for all browsers on all OSes.
Remember that browsers don't render a page in the same way as the final rendering depends on various OS specifics, available resources (like GPU, pre-installed fonts, ...) and activated options of the browser.

When the BPMN elements in the involved BPMN diagram have no label, it runs quite fine. There are minor discrepancies and we handle slight rendering differences by setting threshold in the diff detection. The threshold is small and only a few tests require specific configuration.

However, when labels are present, the discrepancies can be large due to the fact that a same named font is not rendered across browsers and OSes.
For some Firefox tests, we have been forced to set a threshold larger than 10%, so we are not confident that the test is able to detect unwanted change.

During a recent version bump of playwright, the Mozilla Firefox version changed from 113.0 to 115.0. Despite a failure threshold of 12.78%, a test fails with a difference of 12.84%! For more details, see #2790 (comment)

At that time, I discovered that the word wrapping on Firefox is completely different from the one on Chrome. That explains why we had to configure a so large threshold.

expected received
labels 01 general-expected labels 01 general-received

Proposal

For specific screenshots that require it, we should store a dedicated screenshots for browsers that render a diagram differently from the reference one done with Chrome.
Notice that the default behavior in Playwright tests is to have a dedicated screenshots per browser for each tested OS. See https://playwright.dev/docs/test-snapshots for more details. We probably don't need this (we don't see large discrepancies for the same browser on different OSes) especially as this force to store more space in the repository and use a lot of disk space.

The way we configure thresholds should be reworked to let use a different identifiers in jest-image-snapshot checks when needed.
For instance:

  • default and no specific browser: the same id as tody
  • specific browser: postfix by -browser_name, for example -firefox

Notes

Currently, the thresholds configuration is done by subclassing. We may think about switching to passing the configuration in an options parameter of the constructor if it simplifies the code.

@tbouffard tbouffard added the chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...) label Aug 30, 2023
@tbouffard tbouffard added this to the 0.38.2 milestone Aug 30, 2023
@csouchet csouchet modified the milestones: 0.38.2, 0.39.1 Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...)
Projects
None yet
Development

No branches or pull requests

2 participants