-
Notifications
You must be signed in to change notification settings - Fork 2.9k
refactor: migrate VR tests StoryWright steps from decorators to params API #35346
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
refactor: migrate VR tests StoryWright steps from decorators to params API #35346
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
I think CSF2 should be also supported, and the problem is that some stories didn't have StoryWright steps configured at all, but now every story should have them like in here c537392 |
AFAIR that's not a requirement to have the lets proceed without enabling the |
| "clean": "just-scripts clean", | ||
| "format": "prettier . -w --ignore-path ../../.prettierignore", | ||
| "lint": "just-scripts lint", | ||
| "start": "storybook dev", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Avatar Converged 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Avatar Converged.badgeMask.normal.chromium.png | 5 | Changed |
| vr-tests-react-components/Avatar Converged.badgeMask - RTL.normal.chromium.png | 7 | Changed |
vr-tests-react-components/Charts-DonutChart 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Charts-DonutChart.Dynamic - RTL.default.chromium.png | 30793 | Changed |
| vr-tests-react-components/Charts-DonutChart.Dynamic - Dark Mode.default.chromium.png | 12635 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 160 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 142 | Changed |
vr-tests-react-components/Skeleton converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Skeleton converged.Opaque Skeleton with rectangle - Dark Mode.default.chromium.png | 2 | Changed |
vr-tests-react-components/TagPicker 4 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - Dark Mode.disabled input hover.chromium.png | 659 | Changed |
| vr-tests-react-components/TagPicker.disabled - RTL.disabled input hover.chromium.png | 635 | Changed |
| vr-tests-react-components/TagPicker.disabled - High Contrast.disabled input hover.chromium.png | 1321 | Changed |
| vr-tests-react-components/TagPicker.disabled.chromium.png | 678 | Changed |
vr-tests-web-components/Avatar 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Avatar. - Dark Mode.normal.chromium_1.png | 298 | Changed |
| vr-tests-web-components/Avatar. - Dark Mode.normal.chromium.png | 10381 | Changed |
vr-tests-web-components/MenuList 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/MenuList. - RTL.2nd selected.chromium.png | 17 | Changed |
| vr-tests-web-components/MenuList. - RTL.normal.chromium_1.png | 39082 | Changed |
vr-tests/Callout 11 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/Callout.Bottom center.default.chromium.png | 2116 | Changed |
| vr-tests/Callout.Bottom auto edge.default.chromium.png | 2181 | Changed |
| vr-tests/Callout.Gap space 25.default.chromium.png | 2181 | Changed |
| vr-tests/Callout.Left center.default.chromium.png | 2544 | Changed |
| vr-tests/Callout.Rendering callout attached to a rectangle.default.chromium.png | 1832 | Changed |
| vr-tests/Callout.Left top edge.default.chromium.png | 2168 | Changed |
| vr-tests/Callout.No beak.default.chromium.png | 2177 | Changed |
| vr-tests/Callout.Right center.default.chromium.png | 2080 | Changed |
| vr-tests/Callout.Right bottom edge.default.chromium.png | 3037 | Changed |
| vr-tests/Callout.Top right edge.default.chromium.png | 1134 | Changed |
| vr-tests/Callout.Top left edge.default.chromium.png | 2196 | Changed |
vr-tests/DatePicker 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/DatePicker.Underlined and Required.hover datepicker.chromium.png | 73 | Changed |
| vr-tests/DatePicker.Underlined and Required.default.chromium.png | 1825955 | Changed |
| vr-tests/DatePicker.Underlined and Required.hover month.chromium.png | 190 | Changed |
vr-tests/DatePicker - Disabled 7 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/DatePicker - Disabled.Without Label.hover datepicker.chromium.png | 38 | Changed |
| vr-tests/DatePicker - Disabled.Without Value.hover datepicker.chromium.png | 38 | Changed |
| vr-tests/DatePicker - Disabled.With Label.default.chromium.png | 1816883 | Changed |
| vr-tests/DatePicker - Disabled.Without Label.click.chromium.png | 38 | Changed |
| vr-tests/DatePicker - Disabled.Without Label.default.chromium.png | 1825915 | Changed |
| vr-tests/DatePicker - Disabled.Without Value.default.chromium.png | 1816752 | Changed |
| vr-tests/DatePicker - Disabled.Without Value.click.chromium.png | 38 | Changed |
vr-tests/Keytip 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/Keytip.Disabled.default.chromium.png | 24 | Changed |
| vr-tests/Keytip.Offset.default.chromium.png | 86 | Changed |
| vr-tests/Keytip.Root.default.chromium.png | 6 | Changed |
vr-tests/ScrollablePane 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/ScrollablePane.Default ScrollablePane Example.default.chromium.png | 35304 | Changed |
| vr-tests/ScrollablePane.Default ScrollablePane Example.scrolled.chromium.png | 26468 | Changed |
vr-tests/ScrollablePane Grouped Details List 9 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.If groups are expanded, on horizontal scroll, over scroll should not happen for content container.chromium.png | 2866 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.If groups are expanded, when header becomes sticky, horizontal scrollbar should be visible.chromium.png | 6188 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.On expanding a group, horizontal scrollbar should be visible.chromium.png | 5816 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.Scrollbars visibility after scrolling down to the bottom with non-zero scrollLeft.chromium.png | 5378 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.Scrollbars visibility after scrolling down to the bottom.chromium.png | 5120 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.Scrollbars visibility when header is sticky and scrollLeft is non-zero.chromium.png | 5877 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.Scrollbars visibility when header is sticky.chromium.png | 5620 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.default- scrollbars should be visible.chromium.png | 5598 | Changed |
| vr-tests/ScrollablePane Grouped Details List.ScrollablePane scrollbars visibility.Scrollbars visibilty after scrolling left.chromium.png | 5130 | Changed |
vr-tests/SearchBox 4 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/SearchBox.Root - RTL.default.chromium.png | 143 | Changed |
| vr-tests/SearchBox.Root - RTL.default.chromium_1.png | 91 | Changed |
| vr-tests/SearchBox.Root.default.chromium.png | 147 | Changed |
| vr-tests/SearchBox.Root.default.chromium_1.png | 95 | Changed |
vr-tests/react-charting-AreaChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-AreaChart.Custom Accessibility.default.chromium.png | 11 | Changed |
vr-tests/react-charting-MultiStackBarChart 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-MultiStackBarChart.Basic_Absolute.default.chromium.png | 359 | Changed |
| vr-tests/react-charting-MultiStackBarChart.Basic_Absolute - RTL.default.chromium.png | 343 | Changed |
| vr-tests/react-charting-MultiStackBarChart.Basic_PartToWhole.default.chromium.png | 359 | Changed |
There were 12 duplicate changes discarded. Check the build logs for more information.
I've thought that enabling the |
b67cf5c to
7c14ada
Compare
7c14ada to
9debb82
Compare
| "test": "just-scripts test", | ||
| "type-check": "tsc -p . --noEmit --baseUrl .", | ||
| "test-vr": "storywright --browsers chromium --url dist/storybook --destpath dist/screenshots --waitTimeScreenshot 500 --concurrency 4 --headless true --bailOnStoriesError" | ||
| "test-vr": "storywright --browsers chromium --url dist/storybook --destpath dist/screenshots --waitTimeScreenshot 500 --concurrency 4 --headless true --stepsApi parameters" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is crucial --bailOnStoriesError, without we would never catch that something is wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets also add it to other vr apps in our repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SW patch doesn't fix all the issues, as there are still some problems with the VR tests themselves. If I enable --bailOnStoriesError, the pipeline will fail and I'll have to resolve all the failed VR tests. My intention with this PR was mainly to migrate the stories steps API, not to change or fix them.
I brought back the --bailOnStoriesError for vr-tests-react-components as it was there before, and created a separate issue #35430 to fix the failed stories and enable the --bailOnStoriesError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that works . ty!
| TestWrapperDecoratorTallFixedWidth, | ||
| } from './TestWrapperDecorator'; | ||
| export { DARK_MODE, HIGH_CONTRAST, RTL, getStoryVariant } from './getStoryVariant'; | ||
| export { withStoryWrightSteps } from './withStoryWrightSteps'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 nice
| "test-vr": "storywright --browsers chromium --url dist/storybook --destpath dist/screenshots --waitTimeScreenshot 500 --concurrency 4 --headless true --stepsApi parameters" | ||
| }, | ||
| "dependencies": { | ||
| "react": "18.3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with new platform agnostic api from storywright WC will no longer need react for authoring stories @chrisdholt @radium-v ;)
using that argument enforces Storybook story retrieval
|
…tests-react-components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will be removed after microsoft/storywright#77 is merged
Hotell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM



Previous Behavior
VR tests are not compatible with SB8 #35279 (comment)
New Behavior
0.0.27-storybook7.14as the new--stepsAPI parametersis introduced there that should add support for Storybook 8vr-tests,vr-tests-react-componentsandvr-tests-web-componentsto use the SBparametersAPI instead ofdecorators--stepsAPI parametersflag to StoryWright scriptsRelated Issue(s)