search

storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.4k stars 9.28k forks source link

Run storyshot in parallel #7863

Closed cgcote closed 1 year ago

cgcote commented 5 years ago

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

SQReder commented 4 years ago

Bump for parallelization. Currently, Loki has some kind of it, as reference for implementation

isokolovskii commented 4 years ago

Any update on this? We currently have lot of stories in our project and it takes some time to run them on CI now

raDiesle commented 4 years ago

I guess it is a prerequisite to enable parallel execution of Storyshot Puppeteer, which might end up in another ticket, correct?

shilman commented 4 years ago

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.

lyleunderwood commented 4 years ago

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. :+1:

raDiesle commented 4 years ago

good @shilman you pointed out Chromatic, which seems to envolved much further the last time I checked. Nevertheless do you have any hint for current Storyshots setup how to parallelize? About Chromatic, so to run 200 pages for tests would required ramp up time + time for longest page ~ 20 seconds in total BUT with Puppeteer pixel based screenshots, is it correct?

shilman commented 4 years ago

@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

felixfbecker commented 4 years ago

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?

SQReder commented 4 years ago

Maybe this would help? https://jestjs.io/docs/en/api#testconcurrentname-fn-timeout

ср, 23 сент. 2020 г., 15:41 Felix Becker notifications@github.com:

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 https://github.com/storybookjs/storybook/issues/7863#issuecomment-697338118, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALTBCL7LQL2KRDS6CBADWTSHHUITANCNFSM4IPNX5NA .

felixfbecker commented 3 years ago

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)

shilman commented 3 years ago

Working proof of concept: https://twitter.com/tmeasday/status/1316992773493436416?s=20

vidal7 commented 3 years ago

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.

raDiesle commented 3 years ago

It is also worth to mention a very promising solution: https://github.com/reg-viz/storycap I vote for replacing puppeteer-shoryshots with this one. I seems to be modular and high effective

jdelStrother commented 3 years ago

Working proof of concept: https://twitter.com/tmeasday/status/1316992773493436416?s=20

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: 1 jest file that uses initStoryshots

Test Suites: 1 passed, 1 total
Tests:       133 passed, 133 total
Snapshots:   133 passed, 133 total
Time:        9.372 s, estimated 10 s

After: Using @tmeasday's approach to transform my 61 stories into test files, with jest running those 4-in-parallel:

Test Suites: 61 passed, 61 total
Tests:       133 passed, 133 total
Snapshots:   133 passed, 133 total
Time:        15.874 s

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:

 PASS  assets/javascripts/components/stories/error_row.tsx
  ✓ blankFile (26 ms)
  ✓ fileTooSmall (3 ms)
  ✓ fileTooLarge (2 ms)
  ✓ invalidFormat (2 ms)
  ✓ invalidFile (2 ms)
  ✓ textFile (2 ms)

Test Suites: 1 passed, 1 total
Tests:       6 passed, 6 total
Snapshots:   6 passed, 6 total
Time:        5.383 s

running the actual tests is only taking a few milliseconds. The other 5 seconds is ... requiring dependencies? Compiling typescript? I'm not quite sure.

lyleunderwood commented 3 years ago

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.

petermikitsh commented 3 years ago

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 🔥

tmeasday commented 3 years ago

@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.

tmeasday commented 3 years ago

One thing to do would be to compare on this repo (storybookjs/storybook).

jdelStrother commented 3 years ago

We aren't using typescript on that codebase right now, perhaps that's a factor.

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.

Does anyone have a complete example (git repo) with parallelized tests?

I'll see if I can throw something together

jdelStrother commented 3 years ago

@petermikitsh FWIW, I hacked this up : https://github.com/jdelStrother/storybook/commits/parallel-storyshots

Clone it, run yarn in the root directory to install dependencies, then cd examples/cra-kitchen-sink.

That directory has three approaches for generating snapshots. Running yarn jest will run all three.

  1. Classic approach. yarn jest storyshots - run a single jest worker that tests all of your stories' snapshots (via https://github.com/jdelStrother/storybook/blob/parallel-storyshots/examples/cra-kitchen-sink/src/storyshots.test.js)

  2. One-worker-per-story approach. yarn jest 'stories/button.*stories' - this is @tmeasday's approach of using a jest transform (https://github.com/jdelStrother/storybook/blob/parallel-storyshots/examples/cra-kitchen-sink/.storybook/transformer.js) to convert each story to a test file.

  3. test.concurrent approach - yarn jest concurrent - just for curiosity's sake, I added a concurrentJest option to initStoryshots which uses jest's test.concurrent runner. It's possible I'm doing something wrong - couldn't find much in the way of documentation on test.concurrent - but it seems virtually no different than option 1 in terms of speed. (https://github.com/jdelStrother/storybook/blob/parallel-storyshots/examples/cra-kitchen-sink/src/concurrentshots.test.js)

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.

tmeasday commented 3 years ago

@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.

jdelStrother commented 3 years ago

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. (yarn jest 'stories/button.*simple')

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.)

dsilvasc commented 3 years ago

@jdelStrother where did you get concurrentJest from? I don't see it in the storyshots source as an option:

https://github.com/storybookjs/storybook/search?q=concurrentJest&type=code

jdelStrother commented 3 years ago

That was something I hacked together here: https://github.com/jdelStrother/storybook/commit/72f58da6e43622945b2237b240bfcb73524ecf83#diff-34c8b266627e8b81cae2511f5e31f5fd16febdaec479844a263e05ca447c31e5R31-R45

dsilvasc commented 3 years ago

Thanks for that 👍

petermikitsh commented 3 years ago

@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:

  1. transformer.js: Jest transformer that converts each story file into a test suite. TL,DR it takes the story code, transpiles to plain JS, and appends the jest describe, it functions you'd ordinarily be writing yourself as unit test code. https://github.com/petermikitsh/storyshots-parallel-demo/blob/a22d7c5f8b68852cd3ceb6a6f1291950b667291e/.storybook/transformer.js#L1
  2. testCsf.js: The test code that gets appended to each story. It defines the jest test setup, manages puppeteer, takes the screenshot, and does cleanup. https://github.com/petermikitsh/storyshots-parallel-demo/blob/a22d7c5f8b68852cd3ceb6a6f1291950b667291e/.storybook/testCsf.js#L1
  3. jest.config.js: Jest configuration. You need to apply the custom transformer to your stories and ts/tsx/js/etc files. https://github.com/petermikitsh/storyshots-parallel-demo/blob/a22d7c5f8b68852cd3ceb6a6f1291950b667291e/jest.config.js#L7-L8
  4. jest.setup.js: You need ways to hook into the test execution process, to provide things like storybookUrl, custom puppeteer instance, etc. My implementation provides this via a global variable accessible to the test code.
shilman commented 3 years ago

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.

raDiesle commented 3 years ago

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.

AlonMiz commented 2 years ago

Waiting for this as well. takes a ton of time in our CI (almost 10 minutes 😱 )

kftsehk commented 2 years ago

Not sure if it is solved in new version I am using.

Version: 

    "@storybook/addon-actions": "^6.3.12",
    "@storybook/addon-essentials": "^6.3.12",
    "@storybook/addon-links": "^6.3.12",
    "@storybook/addon-storyshots": "^6.3.12",
    "@storybook/addon-storyshots-puppeteer": "^6.3.12",
    "@storybook/addon-viewport": "^6.3.12",
    "@storybook/builder-webpack5": "^6.3.12",
    "@storybook/manager-webpack5": "^6.3.12",
    "@storybook/react": "^6.3.12",
    "puppeteer": "^10.4.0"

My Test Task: Taking snapshot of 2 views, at 14 different resolutions per view

Baseline

2 Views:
Test Suites: 1 passed, 1 total
Tests:       28 passed, 28 total
Snapshots:   28 passed, 28 total
Time:        16.246 s

Parallelize

use storyKindRegex and storyNameRegex to partition views into different test files Half of my view's storyName is I.... (Apple product resolutions)

snapshot.test.js

# starts with "I"
initStoryshots({
  storyNameRegex: /^I.*/,
  storyKindRegex: /.*Business.*/,
  test: imageSnapshot({ beforeScreenshot, getMatchOptions }),
});

snapshot2.test.js

# Not start with "I"
initStoryshots({
  storyNameRegex: /^[^I].*/,
  storyKindRegex: /.*Business.*/,
  test: imageSnapshot({ beforeScreenshot, getMatchOptions }),
});

Jest command

jest .storybook/snapshot*.test.js --verbose --maxWorkers 2

Parallel Result:

Test Suites: 2 passed, 2 total
Tests:       28 passed, 28 total
Snapshots:   28 passed, 28 total
Time:        10.575 s, estimated 17 s

Note:

  1. initStoryshots twice in the same file doesn't work, even with different suite parameter
  2. If some regex matches no story, storyshots found 0 stories is a failure even --passWithNoTests is set in jest
  3. For whole test suite, I get from 46s to 18s on a 8 core laptop, not small overhead, definitely need powerful machine to take advantage

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 jest --maxWorkers 1

Expected image to match or be a close match to snapshot but was 0.11187629937629937% different from snapshot (861 differing pixels)

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.

vidal7 commented 2 years ago

@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;
        }
    }
}
vidal7 commented 2 years ago

@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', () => { });
    }
}
kftsehk commented 2 years ago

@vidal7 ended up generate the snapshot.test.js according to *.snapshot.js using hygen, something like this

find src -name '*.snapshot.js' -exec realpath --relative-to src {} \\; | HYGEN_OVERWRITE=1 xargs -n 1 -P $(nproc) -I {} npx hygen  snapshot-test with-prompt --testfile={}
ndelangen commented 1 year ago

The future of storyshots is the test-runner: https://storybook.js.org/docs/react/writing-tests/test-runner#page-top

And to add assertions you can use the play function: https://storybook.js.org/docs/react/writing-stories/play-function#page-top

myufa commented 1 year ago

The future of storyshots is the test-runner: https://storybook.js.org/docs/react/writing-tests/test-runner#page-top

And to add assertions you can use the play function: https://storybook.js.org/docs/react/writing-stories/play-function#page-top

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?

ndelangen commented 1 year ago

Does this help you @myufa : https://storybook.js.org/addons/@storybook/test-runner

myufa commented 1 year ago

Does this help you @myufa : https://storybook.js.org/addons/@storybook/test-runner

Yes, thank you!