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

Build: Add vite-react to e2e tests #17871

Merged
merged 2 commits into from
Apr 6, 2022
Merged

Build: Add vite-react to e2e tests #17871

merged 2 commits into from
Apr 6, 2022

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Apr 4, 2022

Issue: N/A

What I did

Added vite builder example to E2E, so we guarantee there are no unintended breaking changes 👌

cc @IanVS

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@yannbf yannbf added the maintenance User-facing maintenance tasks label Apr 4, 2022
@nx-cloud
Copy link

nx-cloud bot commented Apr 4, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f4e00ff. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@IanVS
Copy link
Member

IanVS commented Apr 5, 2022

This is great, seems easy! But, is it actually running in CI? I see:

Will run E2E tests for:react, vue3

and don't see vite actually running.

@yannbf
Copy link
Member Author

yannbf commented Apr 5, 2022

The e2e tests under e2e-tests-core happen in parallel so you might have checked the first batch of tests. You can find the one with vite here but it's unfortunately failing with no helpful message. I'll have to check this further!

image

@IanVS
Copy link
Member

IanVS commented Apr 6, 2022

@yann, it seems the error is occurring because @storybook/builder-vite is not in https://github.com/storybookjs/storybook/blob/43be92e13b75f91b99372f0707043739ca5951b9/lib/cli/src/versions.ts. And furthermore, when running yarn local-registry --port 6000 --open --publish, then yarn npm info @storybook/builder-vite --fields version --json errors out because it's looking for a locally published package.

Not Found - GET http://localhost:6000/@storybook%2fbuilder-vite - no such package available

🤦

@IanVS
Copy link
Member

IanVS commented Apr 6, 2022

I've pushed up a fix as a separate PR (#17897)

Maybe you could also add this, so that the interactions are tested?

// addon-interactions.spec.ts

onlyOn('vite_react', () => {
    it('should have interactions', test);
  });

@yannbf
Copy link
Member Author

yannbf commented Apr 6, 2022

I've pushed up a fix as a separate PR (#17897)

Maybe you could also add this, so that the interactions are tested?

// addon-interactions.spec.ts

onlyOn('vite_react', () => {
    it('should have interactions', test);
  });

You're the best! I updated this branch with latest next and included vite_react to the E2E test. 🤞

@shilman shilman changed the title add vite-react to e2e tests Build: Add vite-react to e2e tests Apr 6, 2022
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.

LGTM

@shilman shilman merged commit 645f5ab into next Apr 6, 2022
@shilman shilman deleted the chore/e2e-vite branch April 6, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder-vite maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants