Use Playwright with Storybook for component tests in fast-foundation #6189
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
packages/web-components/fast-foundation/src/anchor/anchor.pw.spec.ts
Outdated
Show resolved
Hide resolved
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
7ebf587 to
3923480
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
3923480 to
1cbff46
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
1cbff46 to
2ea7fc2
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
2ea7fc2 to
bd1448e
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
bd1448e to
1e844bd
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
2 times, most recently
from
65e504a to
b7a6e12
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
b7a6e12 to
7f10a75
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
1 similar comment
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
| ): string { | ||
| const params: Record<string, any> = { id }; | ||
| if (args) { | ||
| params.args = qs |
There was a problem hiding this comment.
I don't know if I should comment here, but I tried something similar in my project. I think you could add a validation for the args in this method as Storybook does not allow specific characters in it's url. See here
This could help some developers who are going to use any of those characters in their arguments as they get a better error response.
Also, I just wanted to thank you for the great work on this PR. Really helped me a lot!
There was a problem hiding this comment.
Hi @sonntag-philipp, comments and contributions are always welcome!
I dug into the source for Storybook args a few days ago and unfortunately they don't have a function that we could import to validate the args I did some more searching and found the args validator, linked below. They do at least use qs, and they also apply some extra formatting in both the encoding and decoding of story args. Our method can definitely be improved to more closely match the expected behavior, which is handled in these files:
- The args query string parser: https://github.com/storybookjs/storybook/blob/v6.5.9/lib/preview-web/src/parseArgsParam.ts
- The story args store validator: https://github.com/storybookjs/storybook/blob/v6.5.9/lib/store/src/args.ts#L74
There was a problem hiding this comment.
On further reading, it looks like the second link might not be as useful as I thought, since it requires us to pass the argTypes to validate that the args are correct.
There was a problem hiding this comment.
Alright, thanks for the input on this. I think I'll just omit the validation within my own project as it's not going to be public.
Asking Storybook to expose a validation method would be the most future proof way I think - but you may already thought that too.
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
7f10a75 to
445da19
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
445da19 to
1c110b3
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
1c110b3 to
11fc46a
Compare
📊 Tachometer Benchmark ResultsSummaryclickTrigger10x
create10k
createDelete5x
runFile1k
update10th
usedJSHeapSize
Resultsobservable-runFile1k
runFile1k
usedJSHeapSize
render-create10k
create10k
usedJSHeapSize
render-createDelete5x
createDelete5x
usedJSHeapSize
render-update10th
update10th
usedJSHeapSize
repeat-basic-splice-itemCount=1000&deleteCount=20&addCount=20
clickTrigger10x
usedJSHeapSize
repeat-basic-splice-loopCount=1000&itemCount=1000&deleteCount=10&addCount=10
clickTrigger10x
usedJSHeapSize
repeat-nested-push-itemCount=100&addCount=20
clickTrigger10x
usedJSHeapSize
repeat-nested-push-loopCount=200&itemCount=200
clickTrigger10x
usedJSHeapSize
repeat-nested-reverse-itemCount=100
clickTrigger10x
usedJSHeapSize
repeat-nested-reverse-loopCount=200&itemCount=200
clickTrigger10x
usedJSHeapSize
repeat-nested-shift-itemCount=100
clickTrigger10x
usedJSHeapSize
repeat-nested-shift-loopCount=200&itemCount=200
clickTrigger10x
usedJSHeapSize
repeat-nested-unshift-itemCount=100&addCount=20
clickTrigger10x
usedJSHeapSize
repeat-nested-unshift-loopCount=200&itemCount=200&addCount=1
clickTrigger10x
usedJSHeapSize
|
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
11fc46a to
e80f9a6
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
radium-v
force-pushed
the
users/jokreitl/foundation-playwright
branch
from
e80f9a6 to
4c146ff
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
chrisdholt
approved these changes
Sep 16, 2022
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
1 similar comment
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-6189.centralus.azurestaticapps.net |
janechu
pushed a commit
that referenced
this pull request
Jun 10, 2024
…6189) * add playwright and remove karma from fast-foundation * make ElementsFilter type arguments optional * convert utilities tests to playwright - Simplify whitespaceFilter and getDirection functions - Add getDirection tests * convert accordion tests to playwright * convert accordion-item tests to playwright * convert anchor tests to playwright * convert anchored-region tests to playwright * add slottedBreadcrumbItemFilter - Add prev and next to slottedBreadcrumbItemsChanged method - Add isBreadcrumbItem filter function * convert breadcrumb tests to playwright * convert breadcrumb-item tests to playwright * mark formnovalidate as boolean attribute in button template * convert breadcrumb-item tests to playwright * convert button tests to playwright * clean up extra spacing in checkbox template * convert checkbox tests to playwright * reset combobox value if no first selected option * convert combobox tests to playwright * allow cell templates to be SyntheticViewTemplate or string * use lookup for data-grid-cell template role and class * convert data-grid tests to playwright * convert data-grid-row tests to playwright * convert data-grid-cell tests to playwright * convert dialog tests to playwright * convert disclosure tests to playwright * convert divider tests to playwright * convert flipper tests to playwright * convert horizontal-scroll tests to playwright * convert listbox-option tests to playwright * convert listbox tests to playwright * menu should not navigate to hidden items when set after connection * convert menu tests to playwright * convert menu-item tests to playwright * convert number-field tests to playwright * convert progress tests to playwright * convert progress-ring tests to playwright * convert radiogroup tests to playwright * convert radio tests to playwright * convert search tests to playwright * convert select tests to playwright * ensure slider value is within min and max limits * convert slider tests to playwright * convert slider-label tests to playwright * use whitespaceFilter with switch template * convert switch tests to playwright * convert tab tests to playwright * convert tab-panel tests to playwright * convert tabs tests to playwright * convert text-area tests to playwright * convert text-field tests to playwright * convert toolbar tests to playwright * convert tooltip tests to playwright * add ARIA attributes to tree-item * convert tree-item tests to playwright * convert tree-view tests to playwright * update api-report.md * update lockfile * Change files * wip * update playwright * update api-report.md * update imports * menu tests * tooltip tests * text-area tests * progress tests * horizontal-scroll tests * toolbar tests * make dialog tests syncronous * finish converting all tests to be synchronous * remove attribute enumeration * remove positioningRegion from radio-group * fix flaky tree-item test * ensure aria-expanded is only set when the tree-item has children * simplify rewritten combobox form reset fix * revert breadcrumb and breadcrumb item changes * revert checkbox template changes * use regex for toHaveClass assertions * revert unrelated number-field changes * remove unused import from radio-group template * revert unrelated search template changes * revert unrelated switch template changes * revert unrelated text-area template changes * revert unrelated text-field class and template changes * revert unrelated tree-item and tree-view changes * make tree-view tests synchronous * revert unrelated radio-group changes * revert unrelated search template changes * revert unrelated text-field template changes * update api-report and READMEs * revert unrelated combobox changes * revert unrelated data-grid-row template changes * revert unrelated menu-item template changes
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.
Pull Request
📖 Description
Converts the test suite in
fast-foundationfrom Karma to Playwright.🎫 Issues
Resolves #5805
👩💻 Reviewer Notes
The original approach was to make all tests async, and load individual pages for each test. Unfortunately, this ended up being extremely slow on the GH Actions pipeline, taking over 30 minutes to run tests in all three configured browsers. The tests are now rewritten in a synchronous way, which is more performant for CI environments (workers=1).
📑 Test Plan
All tests should pass. Some tests report as flaky, which should be tracked as future issues. Additionally, these files have not yet been converted due to the Playwright environment not being able to handle decorators:
src/design-token/core/design-token-node.spec.tssrc/design-token/event-strategy.spec.tssrc/design-token/fast-design-token.spec.tssrc/directives/reflect-attributes.spec.tssrc/form-associated/form-associated.spec.ts✅ Checklist
General
$ yarn changeComponent-specific
⏭ Next Steps