-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Run storyshot in parallel #7863
Comments
Bump for parallelization. |
Any update on this? We currently have lot of stories in our project and it takes some time to run them on CI now |
I guess it is a prerequisite to enable parallel execution of Storyshot Puppeteer, which might end up in another ticket, correct? |
PR's very welcome! I also want to recommend Chromatic, which has a free plan since Chromatic 2.0 launched earlier this spring. Chromatic includes hyper-parallel, cloud-based visual regression testing that I work on with some of the other core maintainers. We've spent a lot of effort making it considerably faster than anything else out there. |
I hesitate to be this guy @shilman, but after seeing this comment I did try chromatic and it is both surprisingly awesome and exactly what my company needed. A little pricey, but hopefully it will replace our storyshots (which is painful), gemini-based screenshot testing of our stories, and now.sh / vercel automated hosting of our storybooks, and as a bonus has this nice UI review process. Very cool. 👍 |
good @shilman you pointed out Chromatic, which seems to envolved much further the last time I checked. |
@raDiesle re: chromatic. yeah, there's a few seconds latency and then we crunch through snapshots/diffs in small batches. there can be some variation based on the framework you're using and the content of your component library, but our internal storybook contains about 1500 stories and the whole process including image snapshots & diffing takes around 50 seconds end to end. as for parallelizing storyshots, jest itself supports parallelization really well. so i think the thing to do would be to rewrite storyshots to take advantage of jest's structure. if somebody did that it would be a wonderful addition, probably first as a separate package and then maybe as a breaking replacement for storyshots |
We use Chromatic Pro, but we also need to run Storyshots just to record test coverage achieved by Chromatic for Codecov. This step has become the longest now in our entire CI pipeline (7min). I don't see how we could rely on Jests parallization as Jest runs tests in parallel on a file-basis, and storyshots is initialized in a single file (it wouldn't make sense to have multiple files for it). Could you explain a bit more how you imagine this? |
Maybe this would help?
https://jestjs.io/docs/en/api#testconcurrentname-fn-timeout
ср, 23 сент. 2020 г., 15:41 Felix Becker <[email protected]>:
… We use Chromatic, but we also need to run Storyshots just to record test
coverage achieved by Chromatic for Codecov. This step has become the
longest now in our entire CI pipeline (7min).
I don't see how we could rely on Jests parallization as Jest runs tests in
parallel on a file-basis, and storyshots is initialized in a single file
(it wouldn't make sense to have multiple files for it). Could you explain a
bit more how you imagine this?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#7863 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALTBCL7LQL2KRDS6CBADWTSHHUITANCNFSM4IPNX5NA>
.
|
That could indeed work because it looks like it would make an individual test run in parallel. Just need to make sure that Storyshots opens individual tabs for each test (not change the URL of the same tab) |
Working proof of concept: https://twitter.com/tmeasday/status/1316992773493436416?s=20 |
WoW! This is amazing! It works only with the CSF format and React but hey, it's just a first stab and this POC proves that storyshots can do better. Thank you. |
It is also worth to mention a very promising solution: https://github.com/reg-viz/storycap |
For what it's worth, I managed to hack this into something that generated individual snapshots for each story file... and it was quite a bit slower than the single-file-storyshots approach 😞 I'm running a typescript+react project on a 2016 quad core MBP, with 61 story-files containing a total of 133 stories. react-test-renderer is used to generate a snapshot for comparison with previous runs. Before:
After:
I think it's still worth looking into due to the increased parallelism and that jest will be able to just re-run specific stories in response to a change, but it's not the speed-up I hoped it would be. I'm guessing each individual file incurs a big startup cost - eg in this run:
running the actual tests is only taking a few milliseconds. The other 5 seconds is ... requiring dependencies? Compiling typescript? I'm not quite sure. |
I would recommend modeling how performance changes in response to the number of stories. A project with several thousand stories might benefit significantly more from greater parallelization. |
Does anyone have a complete example (git repo) with parallelized tests? I've seen code snippets here and there, but the full picture isn't clear to me. There's a few different tooling options it seems. One is storycap (which only generates the images) combined with reg-suit (which does the diff-ing between images). A repo with any tooling that has paralleled storyshots would be 🔥 |
@jdelStrother that's interesting. For our repo (private chromatic repo), where we only do a smoke test of each story (rather than taking a snapshot), the parallelized approach was night-and-day faster. I don't have numbers to hand. We have ~1.5k stories there. We aren't using typescript on that codebase right now, perhaps that's a factor. |
One thing to do would be to compare on this repo (storybookjs/storybook). |
Actually we only converted our project from js->typescript a month ago. I've rolled back to our pre-typescript codebase, but see similar results. I don't think typescript is having a noticeable impact on the test run times, with or without parallelisation.
I'll see if I can throw something together |
@petermikitsh FWIW, I hacked this up : https://github.com/jdelStrother/storybook/commits/parallel-storyshots Clone it, run That directory has three approaches for generating snapshots. Running
All of these are just using the default storyshots snapshots comparison. Presumably if you're doing proper image screenshot comparisons the performance characteristics are going to be very different. |
@jdelStrother thanks so much for putting that together. I've dug into it enough to confirm your results, but I still want to dig further to figure out why it is so different to what we observed in the Chromatic code base. This is on my list! I think there's something interesting here that needs to be understood. I'm pretty convinced the per-file test approach is better (mainly because it feels like using the tools more like they are intended to be used, with a bunch of benefits resulting), but it would be a big issue if that comes at a performance cost. |
I added one more test style - https://github.com/jdelStrother/storybook/blob/5259806c4f4e6c172687b7532dcd891ccd58874d/examples/cra-kitchen-sink/src/stories/button-simple.test.js - a dumb-as-possible reimplementation of storyshots renderer, just to verify that something in storyshots wasn't causing the slowness. ( It's maybe 10-100ms faster running a single test file (out of approx 3.5 seconds per file), or 0.5 - 1 second faster over the parallel run of 50 files (out of 13 seconds). And it's a lot dumber than the regular storyshots renderer, so I think that overhead is probably fine. (Numbers are very approximate, it seems hard to get reproducible test times.) |
@jdelStrother where did you get https://github.com/storybookjs/storybook/search?q=concurrentJest&type=code |
That was something I hacked together here: jdelStrother@72f58da#diff-34c8b266627e8b81cae2511f5e31f5fd16febdaec479844a263e05ca447c31e5R31-R45 |
Thanks for that 👍 |
@jdelStrother Thank you for sharing your example repo. I decided to further explore Approach 2 -- "one-worker-per-story approach", but for image screenshot comparisons (visual tests). It took me a few (more like 8) hours but I learned a ton. I was able to generate 1,008 images in 85 seconds. (The current sequential implementation, provided by addon-storyshots, takes 525 seconds). By following the 1-task-per-story, there were 503 tasks (test suites, or story files), as opposed to 1 test suite for everything. My findings are here: https://github.com/petermikitsh/storyshots-parallel-demo Summary:
For anyone interested in learning more, these are the key design elements to be aware of:
|
Thanks so much for the update @petermikitsh! We are planning on shipping a test runner in 6.5 that follows this general approach as part of 6.5, and would love your thoughts once we have something worthy of feedback. |
Awesome! Worth mentioning again you may be interested how storycap is doing it: https://github.com/reg-viz/storycap from what you may want to adopt something into. |
Waiting for this as well. |
Not sure if it is solved in new version I am using. Version:
My Test Task: Baseline
Parallelizeuse snapshot.test.js
snapshot2.test.js
Jest command
Parallel Result:
Note:
The test render mostly correct compared to previous snapshot, and tried simple thing to break the view, diffs looks sane, with one exception that my suite has one place where absolute positioned image component has a slight drift, persist even if
If storyshot can support custom filter other than regex, it may be trivial to implement a hash-based filter to approx. evenly distribute shots to number of workers. Edit: Wondering if the failed case is due to testing all resolutions in one worker vs single resolution per worker, previously have seen responsive scaling from some source resolution to target resolution to be different from opening a page directly in target resolution. |
@kftsehk, this is almost what we do here to run storyshots in parallel but we are using a different configPath value for initStoryshots which has differents stories. Also, to get rid of a failure when storyshots found 0 stories, we did this import initStoryshots from '@storybook/addon-storyshots';
import { StoryshotsOptions } from '@storybook/addon-storyshots/dist/ts3.9/api/StoryshotsOptions';
export function initStoryshotsWithNoStoryshotsFoundResilient(options?: StoryshotsOptions): void {
try {
initStoryshots(options);
} catch (e: unknown) {
if (e instanceof Error && e.message === 'storyshots found 0 stories') {
it('No story found', () => { });
} else {
throw e;
}
}
} |
@kftsehk, Note that with version 6.4, I had to do a hack to make it work again. It is so because initStoryshots does not throw anymore a new Error('storyshots found 0 stories'). This is now a rejected promise on require('global').window.__STORYBOOK_STORY_STORE__.initializationPromise Here is my working example now with 6.4. import initStoryshots from '@storybook/addon-storyshots';
import { StoryshotsOptions } from '@storybook/addon-storyshots/dist/ts3.9/api/StoryshotsOptions';
export function initStoryshotsWithNoStoryshotsFoundResilient(options?: StoryshotsOptions): void {
try {
initStoryshots(options);
} catch (e: unknown) {
if (e instanceof Error && e.message === 'storyshots found 0 stories') {
it('No story found', () => { });
} else {
throw e;
}
}
if (require('global').window.__STORYBOOK_STORY_STORE__.initializationPromise.status === 'pending') {
it('No story found', () => { });
}
} |
@vidal7 ended up generate the snapshot.test.js according to *.snapshot.js using
|
The future of storyshots is the test-runner: And to add assertions you can use the play function: |
Can you provide any documentation on how to produce snapshot testing with the new test runner and play function? And at that, how to use it to parallelize the snapshot tests? |
Does this help you @myufa : |
Yes, thank you! |
According to #4996 and #3802 it seems that running storyshots in parallel has been a topic some months ago. I have run image snapshots in CircleCI with various instance sizes and I get roughly the same execution time so I guess it's not run in parallel at the moment
Do you have a plan to support parallelism and is there any way we could assist?
Thanks!
┆Issue is synchronized with this Asana task by Unito
The text was updated successfully, but these errors were encountered: