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

Jump to bottom

Add cypress to Storybook CI #8397

Merged
merged 39 commits into from Oct 17, 2019
Merged

Add cypress to Storybook CI #8397

merged 39 commits into from Oct 17, 2019

Conversation

kroeder
Copy link
Member

@kroeder kroeder commented Oct 12, 2019 •
edited

@ndelangen and I paired on integrating cypress in the Storybook repo.

Some notes about what we want to test first

What to e2e test

Run these on the official storybook example

  • Routing & navigation
    • Search navigation item
    • Click on nav item
    • Route changes
  • Ejecting the preview
    • "Url is correct" Knobs + StoryId
  • Keyboard shortcuts (hide/show navigation + hide/show addon panel + fullscreen)
  • Knobs
    • Go to a story with a knob (maybe with a more advanced setup)
    • Check if the changes are applied to the preview

Tasks

  • Integrate cypress so it can run locally
  • Check if it works on Windows machines as well
  • Integrate cypress in the Storybook GitHub CI
  • Add the above mentioned tests

@kroeder kroeder added ci: do not merge maintenance User-facing maintenance tasks labels Oct 12, 2019
@vercel
Copy link

vercel bot commented Oct 12, 2019 •
edited

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/storybook/monorepo/fddvqollm
🌍 Preview: https://monorepo-git-tech-add-cypress.storybook.now.sh

@github-actions
Copy link
Contributor

github-actions bot commented Oct 12, 2019 •
edited

Fails
🚫 PR is marked with "do not merge" label.

Generated by 🚫 dangerJS against 1518b5d

@vercel vercel bot temporarily deployed to staging October 12, 2019 16:56 Inactive
@kroeder
Copy link
Member Author

kroeder commented Oct 12, 2019

@ndelangen basic setup is done. I also added typescript
Can you take a look, please?

kroeder
kroeder commented Oct 14, 2019
View reviewed changes
cypress/helper.ts Outdated Show resolved Hide resolved
@ndelangen
Copy link
Member

I'm missing a cypress run in the CI somewhere?

@kroeder
Copy link
Member Author

kroeder commented Oct 14, 2019

I split it into test:e2e-ci and test:e2e-gui
Doesn't t that fit your needs?

@ndelangen
Copy link
Member

I've added installing and running cypress in the CI & fixed the tests.

I also moved some code into cypress commands. These are unfortunately NOT typescript files. Maybe they can be, if they can be .ts, we should migrate them.

shilman
shilman reviewed Oct 17, 2019
View reviewed changes
package.json Show resolved Hide resolved
shilman
shilman reviewed Oct 17, 2019
View reviewed changes
package.json
"wait-on": "^3.3.0"
},
"optionalDependencies": {
"@cypress/webpack-preprocessor": "^4.1.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think optionalDependencies always get installed by default. Is there a lighter-weight solution?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are they even optional? For not bothering devs when they bootstrap storybook?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in our yarn bootstrap --install we do not install optional dependencies.

@vercel vercel bot temporarily deployed to staging October 17, 2019 10:36 Inactive
@ndelangen
Copy link
Member

I added parallelism to the examples generation script, and running them in 4 batches in circleci now.

shilman
shilman approved these changes Oct 17, 2019
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks GREAT! 💯

@ndelangen ndelangen merged commit a65f3ce into next Oct 17, 2019
@ndelangen ndelangen deleted the tech/add-cypress branch October 17, 2019 17:28
@Hypnosphi
Copy link
Member

Hypnosphi commented Feb 22, 2020 •
edited

@ndelangen How do I run it locally? Can you please update CONTRIBUTING.md with instructions?

@ndelangen
Copy link
Member

https://github.com/storybookjs/storybook/blob/next/CONTRIBUTING.md#2d-run-cypress-tests

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndelangen ndelangen ndelangen approved these changes

@shilman shilman shilman approved these changes

@igor-dv igor-dv Awaiting requested review from igor-dv

@usulpro usulpro Awaiting requested review from usulpro

@danielduan danielduan Awaiting requested review from danielduan

Assignees

@ndelangen ndelangen

@kroeder kroeder

Labels
maintenance User-facing maintenance tasks
Projects
None yet
Milestone
5.3.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kroeder @ndelangen @Hypnosphi @shilman