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

Storyshots overhaul #4996

Closed
ndelangen opened this issue Dec 14, 2018 · 15 comments
Closed

Storyshots overhaul #4996

ndelangen opened this issue Dec 14, 2018 · 15 comments

Comments

@ndelangen
Copy link
Member

Storyshots currently has 2 major problems:

  1. There is zero feedback on the progress
  2. Running storyshots is slow

Both problems are caused by the same:
We ask users to write a extra testfile which then encapsulates all of storyshots.

If instead we were able to have jest run the story-files directly jest would be able to:

  • run all storyshots in parallel
  • provide feedback on progress

To have jest be able to load *.stories.js we must tell jest what to do / what to mock in for those files.

This is what we'd be creating: jest-storybook.

Essentially it would allow jest to open any story-file and it would then snapshot all rendered stories.

@ndelangen
Copy link
Member Author

jest-storybook would be a jest-plugin makes sure all stories are actually rendered and then snapshotted.

screenshot 2018-12-14 at 09 57 04

@ndelangen
Copy link
Member Author

jest-storybook should use the names of storiesOf and add as the text for describe and it.

screenshot 2018-12-14 at 10 00 12

@abdallahalsamman
Copy link

Hey @ndelangen can you assign this issue to me please?

@tmeasday
Copy link
Member

Sorry I am in a rush and I will write this up properly later, but I did a POC of such an approach:

// jest.config.js
module.exports = {
  testMatch: ['**/*.stories.js'],
  setupTestFrameworkScriptFile: '<rootDir>/storyshots-setup.js',
};

// storyshots-setup.js
import mockRenderer from 'react-test-renderer';

const mockDescribe = describe;
const mockIt = it;

jest.mock('@storybook/react', () => ({
  storiesOf: kind => {
    // too hacky?
    let resolver;
    mockDescribe(kind, async () => {
      await new Promise(r => {
        resolver = r;
      });
    });

    setTimeout(() => resolver, 0);

    const api = {
      add: (name, story) => {
        mockIt(name, () => {
          const tree = mockRenderer.create(story());
          expect(tree.toJSON()).toMatchSnapshot();
        });
        return api;
      },
      addDecorator: () => api,
      addParameters: () => api,
    };
    return api;
  },
}));

It seemed to work with some small problems

@tmeasday
Copy link
Member

unknown

Also another advantage of this approach is the snapshots end up in a sensible location

@ndelangen
Copy link
Member Author

I'm not sure we actually need to mock storybook at all @tmeasday .

If we mock it, we need a mock implementation, which we'd need to create, test, keep up to date, etc.

@ndelangen
Copy link
Member Author

Though super awesome you had an initial success already! 🎈

@tmeasday
Copy link
Member

tmeasday commented Dec 14, 2018 via email

@alcferreira
Copy link

We are experiencing slower storyshots as well after updating to storybook v4: CraveFood/farmblocks#663 (comment)

@stale stale bot added the inactive label Jan 19, 2019
@storybookjs storybookjs deleted a comment from stale bot Jan 19, 2019
@stale
Copy link

stale bot commented Feb 18, 2019

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

@ericsoco
Copy link

Bump to reopen -- we have ~40 storybook files now with > 200 stories and all of @ndelangen 's original points are painfully valid for us as well.

I found https://github.com/storybookjs/jest-storybook but it looks like it's just the POC that @evexoio wrote up in 2018. Has anything moved on this in the past 18mo? thanks!

@shilman shilman reopened this Jul 1, 2020
@stale stale bot removed the inactive label Jul 1, 2020
@ndelangen
Copy link
Member Author

Unfortunately no work has been done, would you be interested in contributing?

@ericsoco
Copy link

ericsoco commented Jul 7, 2020

Possibly? Though not likely I'll have time until Aug/Sept. Is @evexoio 's approach the right one? If something needs to change, can you please clarify (to someone unfamiliar w/ the codebase except as an end user) what/how?

@ndelangen
Copy link
Member Author

I can assist getting up and running in a online meeting, if you'd like:

You can schedule a meeting with me here:
https://calendly.com/ndelangen/storybook

@stale
Copy link

stale bot commented Aug 8, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants