[WIP] Allow tests to specify a 'packageDir', add storybook 7 tests #137
Conversation
tests/storybook.ts
Outdated
| dir: 'storybook7', | ||
| packageDir: 'react-vite/default-js/after-storybook', | ||
| branch: 'next', | ||
| build: ['yarn install', 'build-storybook'], |
There was a problem hiding this comment.
ecosystem ci calls install, why is it separate here?
There was a problem hiding this comment.
Maybe it's not needed, I'll try removing it.
tests/storybook.ts
Outdated
| build: ['yarn install', 'build-storybook'], | ||
| test: [ | ||
| 'yarn add --dev @storybook/test-runner http-server concurrently', | ||
| 'npx concurrently -k "http-server storybook-static --port 6006 --silent" "npx wait-on http://localhost:6006 && npx test-storybook"' |
There was a problem hiding this comment.
is it possible to not use concurrently here? ecosystem ci calls this in a subprocess but not sure it behaves when cancelled
There was a problem hiding this comment.
I need to be able to start up a web server, wait for it to start, run my tests, and then tear everything down when the tests are finished. Do you know of a better way to accomplish that?
There was a problem hiding this comment.
no clue with storybook's own testrunner, sorry. vitest and playwright have utilities for that
| branch: 'next', | ||
| build: ['yarn install', 'build-storybook'], | ||
| test: [ | ||
| 'yarn add --dev @storybook/test-runner http-server concurrently', |
There was a problem hiding this comment.
there is beforeTest but ideally those are installed in the repo already
There was a problem hiding this comment.
Unfortunately they're not. We're using this repo here in a bit of an unintended manner. It's used in storybook's CI process, which doesn't need these to be installed in the projects themselves.
There was a problem hiding this comment.
can't you use storybook repo with the subdir option? thought that's what that was about
There was a problem hiding this comment.
I can use the storybook repo, yes. But that doesn't have these dependencies, which are being added here just for this vite CI job.
| branch: 'main', | ||
| build: 'prepublish', | ||
| test: ['build-examples', 'test-ci'] | ||
| repo: 'storybookjs/repro-templates-temp', |
There was a problem hiding this comment.
temp? we need a repo that allows us to reproduce the testrun. if you only push new versions it may be ok, but how large is it?
There was a problem hiding this comment.
@shilman, @ndelangen, is this the final name of the repo?
And yes, it's "regenerated" nightly. We clear out the node_modules and yarn.lock now, so the size of the repo is not as large as it was. Do you only check out the HEAD commit, or all git history?
There was a problem hiding this comment.
head commit for test repos for now but may use full checkout for bisect in the future.
how can you trace this repos commit to storybook source on fails?
There was a problem hiding this comment.
That's a good point. We used to save the node_modules that was installed from the nightly build, but now it's not saved off. So, now the best we can do is run against the latest release of storybook. It's not ideal, but at least it's better than nothing. We can run unreleased vite against released storybook, and unreleased storybook against released vite. I don't think we have a good way to run the latest and greatest of both against each other.
There was a problem hiding this comment.
Definitely not the final name of the repo. We can try to get that cleaned up cc @yannbf
There was a problem hiding this comment.
Hey @IanVS we're changing the repo name, right now it's been renamed to repro-sandboxes but possibly will be come just sandboxes, that's something we'll decide soon
There was a problem hiding this comment.
Alright, final name is https://github.com/storybookjs/sandboxes
There was a problem hiding this comment.
Thanks for the heads-up, Yann. I'll try to get back to this soon.
|
is there an update on this? i understand storybooks infra is built around testing releases on the registry, but would love to see this work out so we can get feedback from it before releasing a new vite version |
|
I rebased and pushed up the name change from |
|
if you installed pnpm via ran it locally and get an error during /home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook $> nr build-storybook
> before-storybook@0.0.0 build-storybook /home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook
> storybook build
@storybook/cli v7.0.0-beta.16
info => Cleaning outputDir: /storybook-static
(node:7362) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
info => Loading presets
info => Building manager..
info => Manager built (97 ms)
attention => Storybook now collects completely anonymous telemetry regarding usage.
This information is used to shape Storybook's roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://storybook.js.org/telemetry
vite v4.0.3 building for production...
transforming...
✓ 43 modules transformed.
[vite]: Rollup failed to resolve import "prop-types" from "/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/src/stories/Button.jsx".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
ERR! Error: [vite]: Rollup failed to resolve import "prop-types" from "/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/src/stories/Button.jsx".
ERR! This is most likely unintended because it can break your application at runtime.
ERR! If you do want to externalize this module explicitly add it to
ERR! `build.rollupOptions.external`
ERR! at onRollupWarning (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/file+..+..+..+..+..+vite+packages+vite/node_modules/vite/dist/node/chunks/dep-38aa69fc.js:44627:19)
ERR! at onwarn (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/file+..+..+..+..+..+vite+packages+vite/node_modules/vite/dist/node/chunks/dep-38aa69fc.js:44398:13)
ERR! at Object.onwarn (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/rollup@3.8.0/node_modules/rollup/dist/es/shared/rollup.js:24290:13)
ERR! at ModuleLoader.handleInvalidResolvedId (file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/rollup@3.8.0/node_modules/rollup/dist/es/shared/rollup.js:22927:26)
ERR! at file:///home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/node_modules/.pnpm/rollup@3.8.0/node_modules/rollup/dist/es/shared/rollup.js:22888:26
ERR! at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
ERR! Error: [vite]: Rollup failed to resolve import "prop-types" from "/home/dominikg/develop/vitejs/vite-ecosystem-ci/workspace/storybook/storybook7/react-vite/default-js/after-storybook/src/stories/Button.jsx".
ERR! This is most likely unintended because it can break your application at runtime.
ERR! If you do want to externalize this module explicitly add it to
ERR! `build.rollupOptions.external`personal sidenote: I did not consent to telemetry and believe it should not be enabled by default. |
|
also saw that the test is using react, so you might want to / have to override vite-plugin-react in addition to vite, a recent feature in vite-ecosystem-ci allows you to add it via config: https://github.com/vitejs/vite-ecosystem-ci/blob/main/tests/vite-plugin-ssr.ts#L9 This only works if the project under test is in the same monorepo structure, if you build a completely separate project, that would still use vite and vite-plugin-react from the npm registry instead |
|
Thanks for testing it out. npm itself is not working in node 18. If/when I get that working I'll come back to this. Edit: it's working today, I think the npm registry was just struggling last night. |
If which project is in the same structure as what else? |
overrides are set in |
|
Ah ok thanks, there is no package.json there in my case, so I may need to adjust that to work in testSubdirectory (if I didn't already, I can't remember what I did here). |
|
This already exposed a bug in storybook, which will be fixed by storybookjs/storybook#20449 when it is released. |
This is an attempt to get storybook back into vite-ecosystem-ci. For now, this just adds storybook 7, though 6.5 could likely be added as well.
In order to do so, I had to add support for specifying a directory containing a
package.jsonfile, other than the repo root. I did this by adding apackageDirparameter to test option objects.This checks out storybook's repro repository, which contains the latest storybook versions and bootstrapped projects with various frameworks, all regenerated nightly to make sure the latest versions are used.
For now, I've just added a test for storybook and react, though theoretically others could be used as well I think.
I can't get this to pass locally, but I'm not sure what's going on. The
node_modules/vite/node_modulesin my project seems to be full of broken symlinks.I'm opening this up as a WIP to see what happens in CI, and potentially get feedback from @dominikg whether this seems like a decent approach in the first place.