-
Notifications
You must be signed in to change notification settings - Fork 616
docs: update testing docs for vrt #6136
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
Conversation
|
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.
Pull Request Overview
This PR updates the testing documentation to clarify that Storybook must be running before Visual Regression Tests and removes the outdated Storybook tests guide.
- Adds a note in testing.md to ensure Storybook is running locally before tests
- Deletes the old
storybook-tests.md
file
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
contributor-docs/testing.md | Clarify requirement for Storybook to be running |
contributor-docs/storybook-tests.md | Remove deprecated Storybook tests documentation |
Comments suppressed due to low confidence (2)
contributor-docs/testing.md:113
- The markdown bullet is missing a space after the dash and "setup" should be split into the verb form "set up". Consider changing to
- section to set up your machine. You also need Storybook to be running locally before running any tests.
-section to setup your machine. You also need Storybook to be running locally before running any tests.
contributor-docs/storybook-tests.md:1
- Ensure that all links or references to
contributor-docs/storybook-tests.md
have been updated or removed elsewhere in the docs to prevent broken links.
entire file removal
size-limit report 📦
|
@@ -111,7 +111,8 @@ You can run these tests using Playwright locally or you can see the results of | |||
these tests on GitHub through the CI workflow. | |||
|
|||
To get started locally, make sure to follow the [Prerequisites](#prerequisites) | |||
section to setup your machine. If you're looking for a quick overview of the commands | |||
section to setup your machine. You also need Storybook to be running locally |
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.
did running script/test-e2e work for you @joshblack I was runnign into errors when i tried despite running docker and storybook
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.
@francinelucca I opened up: #6142 for this to fix it, seems like a change to vitest caused it to no longer work 🤔
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.
perfect! thanksss
Closes https://github.com/github/primer/issues/5249
Changelog
New
Changed
Removed
Rollout strategy
This is a change to contributor docs