Storybook: Set up local visual regression testing #43393
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
mirka
force-pushed
the
storybook-vizreg
branch
from
85be6f3
to
9854cbe
Compare
|
Size Change: +336 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
@mirka, I've left a comment on the related issue with my findings from this branch: #43282 (comment) |
mirka
force-pushed
the
storybook-vizreg
branch
2 times, most recently
from
2ff5cb6
to
9c927ca
Compare
|
Thanks for looking into it, @WunderBart! @ciampo I've altered the scaffolding so we can use tailor-made stories and tests. Feel free to play around with it if you have a use case. |
ciampo
reviewed
Aug 24, 2022
test/storybook-playwright/utils.ts
Outdated
| let mutationTimer = setTimeout( resolve, 200 ); | ||
| const observer = new MutationObserver( () => { | ||
| clearTimeout( mutationTimer ); | ||
| mutationTimer = setTimeout( resolve, 200 ); |
There was a problem hiding this comment.
Is 200ms the delay during which the animation should happen? What if the animation takes longer?
There was a problem hiding this comment.
This is the delay between DOM mutations, so I'd say that depends on the animation method. If it's an animation that initiates by setting a classname and lasts longer than 200ms then we might have a falstart as there might not be any other subsequent DOM/attribute mutations to debounce of. But from what I understand FM is doing, those attribute mutations should happen much more frequently:
(...) this is because framer-motion "manually" sets the animated values as inline styles for each frame (instead of relying on native CSS animations and transforms).
– #43282 (comment)
If the above is correct, I think that (for FM animations) the attribute mutation observer could be a reliable solution. Does that make sense?
There was a problem hiding this comment.
I've updated the approach on this one as it looks like we can use explicit waits for the animated elements. Please see this comment for more context :)
waitForSelector implies element is visible
mirka
force-pushed
the
storybook-vizreg
branch
from
e67b953
to
0383efb
Compare
mirka
changed the title
|
I think we can move forward with this setup and see how it goes. This PR is ready for actual review now. |
ciampo
approved these changes
Sep 20, 2022
|
I have tested it and I am happy to tell you that it works fine and passed the test on the Windows host OS 😄 PS D:\_local\WordPress\gutenberg\app\public\wp-content\plugins\gutenberg> npm run test:e2e:storybook
> gutenberg@14.1.0 test:e2e:storybook D:\_local\WordPress\gutenberg\app\public\wp-content\plugins\gutenberg
> playwright test --config test/storybook-playwright/playwright.config.ts
Running 2 tests using 2 workers
2 passed (4s)
To open last HTML report run:
npx playwright show-report test\storybook-playwright\test-results\report
PS D:\_local\WordPress\gutenberg\app\public\wp-content\plugins\gutenberg> |
t-hamano
approved these changes
Sep 20, 2022
bph
added
the
Needs Dev Note
|
This is a great enhancement for developer experience. A stand-alone Dev note on the why and the how to use it would be fabulous. |
|
Hey @bph ! As Lena explained above, the current implementation has a few limitations:
Do you think it would make sense to advertise this even in the current state? |
|
Thanks @ciampo, I leave it in your and @mirka's judgement, of course. Below questions will reveal my limited skill and knowledge. From little, I understand, I feel the Storybook and its tools increase the quality of life for developer using the components package, and visual regression is certainly a next level tool.
.. which in itself might already be helpful, in the sense "better, than not at all".
Is there a way that a developer/contributor can add local reference images instead?
But it can be expanded, even if it's not on the immediate to-do-list? Just because there are limitations doesn't mean there wouldn't be a developer out there who will cherish your work on this and use the examples to create their individual tests. Those warnings can all be made in a dev note. |
Yes, there is. Lena prepared a handy README with instructions.
It can, indeed. At the moment we only have a couple of sample Stories/tests, though.
That's a great point. I guess I was just afraid of announcing something that is still unripe, but as you mention — we add all of those warnings in the dev note. |
ciampo
added
has dev note
|
Hey @bph , I've been discussing with @mirka about having a dev note highlighting the work on VizReg. Given that it's not our intention to make this a proper "framework" that works outside of the Gutenberg repository, we're not sure about how relevant this Dev Note would be for WordPress developers. We were therefore wondering if it's really useful to leave a dev note about VizReg at all. |
|
Thanks for bringing this back for discussion, @ciampo & @mirka I trust your judgement here: let's not make it part of the dev notes. I do think that it would warrant a longer post on the Make Test Blog, sometime after the 6.2 release, as it's another milestone on the playwright testing suite, and it will help guide future contributors. |
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.
What?
Adds a setup to run visual regression tests against Storybook stories.
Some things to note:
packages/components/src/**/e2e.stories.@(js|tsx|mdx)will be included in this special Storybook build.gotoStoryId()util supports decorator options, so we can more easily run matrix tests involving things like LTR/RTL, global CSS, or margins.Why?
We've been wanting a more reliable way of testing visual concerns in components. Some unit tests currently use
.toHaveClass()assertions or snapshot tests for this purpose, but they're not particularly reliable. The snapshot tests are especially not great because the rate of false positives are very high.A vizreg setup like this will help us make CSS changes on complex components with more confidence. In particular, we will be starting cleanup work on the Button component (e.g. #44042), which will benefit from this kind of testing.
Testing instructions
See the included README for instructions.
✍️ Dev Note
In order to have a more reliable way to find visual bugs that wouldn't otherwise be be discovered through other forms of testing, a first iteration of a Visual Regression testing framework has been implemented.
The framework relies on Playwright and Storybook, and it currently has a few limitations:
@wordpress/componentspackage;Furthermore, this testing framework should be considered experimental — its setup may change in future releases of the Gutenberg plugin.
For more information on how to setup and run Visual Regression tests, read the docs