storybookjs / storybook

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

Feature request: add testing utilities #10145

Closed lifeiscontent closed 3 years ago

lifeiscontent commented 4 years ago

Is your feature request related to a problem? Please describe. I'd love to have some testing utilities exposed from storybook to be able to get a fully composed story (including decorators)

Describe the solution you'd like

expose a function to gather both the global and local decorators of a story so we can use a story as a fixture in tests.

Describe alternatives you've considered

import { render, waitFor, screen } from '@testing-library/react';
import story, { renders, asUser } from './index.stories';
import { defaultDecorateStory } from '@storybook/client-api';
import { action } from '@storybook/addon-actions';

jest.mock('@storybook/addon-actions');

describe('EditorPage', () => {
  describe('when not logged in', () => {
    afterEach(() => {
      action('router.replace').mockClear();
    });
    it('redirects', async () => {
      render(defaultDecorateStory(renders, story.decorators)(renders.story));

      await waitFor(() => {
        expect(action('router.replace')).toHaveBeenCalledWith('/editor', '/', {
          shallow: true
        });
      });
    });
  });

  describe('when logged in', () => {
    it('does not redirect', async () => {
      render(defaultDecorateStory(asUser, story.decorators)(asUser.story));

      await waitFor(() => {
        expect(action('router.replace')).not.toHaveBeenCalled();
      });
    });
  });
});

Are you able to assist bring the feature to reality? Potentially

Additional context Add any other context or screenshots about the feature request here.

shilman commented 4 years ago

I think we might be able to do this cleanly for jest specifically:

  1. Starting in 6.0 we have .storybook/preview.js exporting decorators
  2. Starting in 5.2 (CSF) we have stories exporting default.decorators and StoryName.story.decorators

So we could provide a jest transform that decorates stories through some library function.

import { decorate } from `@storybook/jest`
import { MyStory } from './Foo.stories';

describe( ...,
  it(...
    const foo = decorate(MyStory)();
  )
)

Where calls to decorate are transformed to something like:

  decorate(MyStory, { 
    story: MyStory?.story?.decorators,
    component: require('./Foo.stories')?.default?.decorators,
    global: require('/path/to/preview')?.decorators
  })();

Also, we should probably move defaultDecorateStory out of @storybook/client-api and into @storybook/csf to make it more easily accessible.

tmeasday commented 4 years ago

Some issues:

I'm not sure the magic you described above is the right approach. I wonder if it might be better to just make it as easy as possible to do it manually. For instance, it would not be a big burden for a user to one time:

// decorateStory.js
import { defaultDecorateStory } from '@storybook/csf';
import { decorators as globalDecorators } from './preview';

// We might drop one or two global decorators at this point.
// We could also import decorators from addons that we need for a story to render in Jest, if required.

export function decorateStory(example, componentDecorators) {
  return defaultDecorateStory(example, [...globalDecorators, ...componentDecorators, ... example.story.decorators]);
}

And then in each test file they would do:


// Component.tests.js
import { decorateStory } from '../decorateStory';
import { example, decorators } from './Component.stories';

const decoratedExample = decorateStory(example, decorators);
lifeiscontent commented 4 years ago

@tmeasday how would you get the global context and merge it with the story context?

tmeasday commented 4 years ago

@lifeiscontent -- what do you mean by "context" in this setting?

If you mean decorators, you'll notice in the code snippet above the decorateStory that's exported (by the user) automatically decorates the story w/ the globalDecorators. My thinking here is that you might drop one or two of those decorators if they are problematic in Jest (in fact I'll update my comment to demonstrate that).

lifeiscontent commented 4 years ago

@tmeasday I'm specifically talking about the parameters, options that might be added in preview.js

tmeasday commented 4 years ago

Interesting. Do you have a use case where you need access to those parameters or are you just wondering?

lifeiscontent commented 4 years ago

@tmeasday yeah, I do, I have a custom decorator for Apollo, which does a lot of boilerplate setup for the cache, and then in the individual story I'll reconfigure it via the default export or the individual story.

lifeiscontent commented 4 years ago

@tmeasday just pinging to pick back up where we left off from yesterday.

tmeasday commented 4 years ago

Oh, hey, sorry, lots of distractions around at the moment.

So does your custom decorator rely on global parameters that are added in preview.js?

You could probably wire those parameters into your decorateStory function exported globally? Something like:

// decorateStory.js
import { combineParameters } from '@storybook/client-api';
import { decorators as globalDecorators, parameters as globalParameters } from './preview';

export function decorateStory(example, componentDecorators) {
  const decoratedExample = defaultDecorateStory(example, [...globalDecorators, ...componentDecorators, ... example.story.decorators]);

  return (context) => decoratedExample({
    ...context, parameters: combineParameters(globalParameters, context.parameters)
  })
}

Then I guess you should also pass the story-level and component-level parameters when you render the story?

// Component.tests.js
import { decorateStory } from '../decorateStory';
import { combineParameters } from '@storybook/client-api';
import { example, decorators, parameters } from './Component.stories';

const decoratedExample = () => decorateStory(example, decorators)({ parameters: combineParameters(parameters, example.story.parameters });

I guess perhaps the @storybook/csf package could export a bunch of utilities to do this heavy lifting for you. WDYT?

lifeiscontent commented 4 years ago

@tmeasday yeah, that'd be pretty cool! does combineParameters already exist? Or was that just something you were envisioning.

tmeasday commented 4 years ago

does combineParameters already exist?

Yup! https://github.com/storybookjs/storybook/blob/a723b46f4f9baae3bc5667ea2a42c7797e36cb33/lib/client-api/src/index.ts#L19

tmeasday commented 4 years ago

PS I really like your apollo package, can't wait to try it.

lifeiscontent commented 4 years ago

@tmeasday thanks! I plan on releasing another addon to do behavior testing with the next.js router

lifeiscontent commented 4 years ago

@tmeasday I tried implementing this today, but in our codebase in preview.js this is what we're doing:

addDecorator(withMuiTheme);
addDecorator(withKnobs);
addDecorator(withRouter);
addDecorator(
  withApolloClient({
    cache,
    addTypename: true,
    defaultOptions,
    resolvers,
    typeDefs,
  })
);

so AFAIK your example of

import { decorators as globalDecorators, parameters as globalParameters } from './preview';

would not work, is there another way to add decorators globally?

shilman commented 4 years ago

@lifeiscontent @tmeasday is using the 6.0 syntax:

export const decorators = [withMuiTheme, withKnobs, etc.]

You could make the same thing work in 5.3:

export const decorators = [withMuiTheme, withKnobs, etc.]

decorators.forEach(deco => { addDecorator(deco) });
lifeiscontent commented 4 years ago

@shilman ah, so I guess that's why I wasn't seeing combineParameters as a function that was exported.

lifeiscontent commented 4 years ago

@tmeasday @shilman I think I'll try to take a stab at this once I can get 6.0 to compile for me.

stale[bot] commented 4 years ago

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[bot] commented 4 years ago

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!

yannbf commented 4 years ago

Hey @lifeiscontent did you manage to succeed in your experiments? Any interesting feedback? ๐Ÿ˜„

lifeiscontent commented 4 years ago

@yannbf hey, sorry I haven't gotten around to putting anything together but from what I can tell @tmeasday highlighted all the things necessary to move forward with this.

yannbf commented 4 years ago

Hey @shilman, after talking to @tmeasday and getting really nice insights I came up with an experiment that gave me promising results.

Here's an example of trying to reuse a Button story that needs Styled components in my tests. First I'm just importing the story directly, which will fail of course because it's not wrapped in a ThemeProvider. Later, I replace the code with the solution that I'll be talking about: Kapture 2020-10-22 at 23 22 56

The solution

decorateStory(meta, story, globalConfig?)

A method that has access to the set of (args|parameters|globals|decorators) of all possible levels of a story and returns a component that has all of these things combined under the hood.

// lib/client-api/src/decorators

export const decorateStory = (meta, story, globalConfig = {}) => {
  // combine decorators from preview, meta and story
  const combinedDecorators = [
    ...(globalConfig?.decorators || []),
    ...(meta?.decorators || []),
    ...(story.decorators || story.story?.decorators || []),
  ]

  const decorated = defaultDecorateStory(story, combinedDecorators) // existing function from the file

  return (extraArgs) =>
    decorated({
      globals: globalConfig.globalTypes,
      parameters: {
        ...globalConfig.parameters,
        ...meta.parameters,
        ...story.parameters,
      },
      ...story.args,
      ...extraArgs, // whatever props are passed in the final component
    })
}

Possible api number 1:

import React from 'react'
import { render } from '@testing-library/react'
import Meta, { Basic } from './Button.stories'
import { decorateStory } from '@storybook/client-api'; // maybe @storybook/react? likely there will be small tweaks 
 per framework
import * as globalConfig from '../../.storybook/preview'

test('renders button', () => {
  const BasicButton = decorateStory(Meta, Basic, globalConfig) // optional globalConfig to access decorators, parameters etc from preview.js
  const { getByText } = render(<BasicButton>Hello world</BasicButton>)
  const buttonElement = getByText(/Hello world/i)
  expect(buttonElement).toBeInTheDocument()
})

Possible api number 2:

Users would install a babel transformer that would identify exports from .stories and essentially transform the code into the snippet in api number 1. Could be tricky to find the preview import in projects where they have custom paths for storybook config, and would be quite magical for users, although very convenient. I think Storyshots does something similar already.

import React from 'react'
import { render } from '@testing-library/react'
import { Basic } from './Button.stories'

test('renders button', () => {
  const { getByText } = render(<Basic>Hello world</Basic>) // babel transformation will do its magic
  const buttonElement = getByText(/Hello world/i)
  expect(buttonElement).toBeInTheDocument()
})

This is all work in progress and surely has to mature, but I'm super excited that we can achieve this without much effort!

What do you guys think? :D

tmeasday commented 4 years ago

Nice job @yannbf! Yeah this highlights the challenges involved here and some of the tradeoffs. A couple of things I wanted to note about your code snippets above just so we don't lose track of them:

  1. The way the store passes args "first" with passArgsFirst is like so: https://github.com/storybookjs/storybook/blob/f6681e23173b5a11692e01cce2e6a64a41212456/lib/client-api/src/story_store.ts#L366-L369

You should do the same here rather than spreading them into the story context.

  1. We should definitely wrap all this logic around pulling decorators / params / et al and combining it into the context / story into an exportable function from @storybook/core so we don't reimplement it (here and likely in storyshots also).

  2. Definitely it is tricky to get the path for .storybook/preview, however I guess you could pass it as an option to a babel plugin?

  3. An extra challenge is any decorators/parameters/etc added by addons automatically via presets. A work around is for the user to re-export them from their .storybook/preview but we need to figure out a principled approach (can we evaluate main.js in the babel plugin? Is that insane?) here too.

shilman commented 4 years ago

Great stuff @yannbf @tmeasday! I love API 1 and think we should provide it as an export from core. I'm still on the fence about API 2.

Additionally:

Not addressed:

tmeasday commented 4 years ago

Re: other frameworks: There's two things here:

  1. Decorators -- defaultDecorateStory isn't used by every framework, so you'd have to ensure you used the right decorator "composer". For this reason I suppose prepareStory should be exported by the framework (with the decorator composer built in).

  2. Rendering the story -- as @shilman mentions in some frameworks it is non-trivial to render the result of a story function. I'm not sure to what degree each framework has a standard construct like a react element (or function that returns one) that is common to every testing library, this is probably something that needs to be figured out per framework. But the original idea was to have a renderStory function that did that too.

yannbf commented 4 years ago

Hey @tmeasday @shilman I added a WIP for the utilities at feature/testing-utilities (please check the draft PR). I am going on vacation for a couple weeks so might not be touching that, but I'd love if you could check, test and maybe propose some improvements for it. Feel free to change anything there as you like. Currently the utility function is in the react package, but if it's possible to somehow move to core then it's better indeed! See you later! ๐Ÿ–๏ธ

shilman commented 4 years ago

@yannbf boa viagem! ๐Ÿ–

tmeasday commented 4 years ago

@yannbf looks great. My main comment is that I was thinking the babel plugin would work the other way: operate on .stories.* files, something like:

// Add this line
import globalConfig from './path/to/.storybook/preview.js';

// Update all imports from other story files to use raw exports
// import { Foo } from './Other.stories'
import { _Foo_raw } from './Other.stories';

// Ensure default export is referable
// export default {...} =>
const Meta = { ... };
export default Meta;

// Decorate all named exports
// export const Named = Template.bind({});
// Named.args = { ... };
export const _Named_raw = Template.bind({});
_Named_raw.args = { ... };

// Add composed export
export const Named = composeStory(_Named_raw, Meta, globalConfig);

The complexity here is the need to remap the raw exports in case stories import from each other. I'm not sure what's best?

shilman commented 3 years ago

Related but different request that's probably worth discussing here: #13403

@pimlie wants to use storyshots as an API for getting this information. I don't think it's quite right, but Storyshots should already contain the logic for dealing with decorators and such, so it might be a better approach to this general problem. WDYT?

tmeasday commented 3 years ago

@shilman the problem with the storyshots (storystore-based) approach is you need to load all the stories (and their dependencies) in order to get access to say just a single story in a test.

So I think ultimately it is too slow, especially for use cases where you only want one or two tests/stories.

OTOH we have done an approach similar to the above for smoke testing the Chromatic code base (cf this gist) and I will say I have been extremely happy with it. Someone should turn it into Storyshots 2.0 ;)

lifeiscontent commented 3 years ago

@shilman @yannbf should we close this issue now that @storybook/testing-react is a thing?

shilman commented 3 years ago

Good call @lifeiscontent

I believe @jonniebigodes is adding this to the official documentation. Closing this one! ๐Ÿš€