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

Add storyshots migration guides #380

Merged
merged 9 commits into from
Nov 14, 2023
Merged

Add storyshots migration guides #380

merged 9 commits into from
Nov 14, 2023

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Nov 8, 2023

No description provided.

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (33f2385) 76.66% compared to head (fc2e351) 76.66%.
Report is 2 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next     #380   +/-   ##
=======================================
  Coverage   76.66%   76.66%           
=======================================
  Files          11       11           
  Lines         180      180           
  Branches       40       40           
=======================================
  Hits          138      138           
  Misses         42       42           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yannbf yannbf added the skip-release Preserve the current version when merged label Nov 9, 2023
```ts
// your-setup-file.js
import * as projectAnnotations from './.storybook/preview';
import { setProjectAnnotations } from '@storybook/react';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 95 to 97
import { render } from '@testing-library/react';
import { composeStories } from '@storybook/react';
import type { Meta, StoryFn } from '@storybook/react';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { render } from '@testing-library/vue';
import { composeStories } from '@storybook/testing-vue3';
import type { Meta, StoryFn } from '@storybook/vue3';

Comment on lines 234 to 236
import { render } from '@testing-library/react';
import { composeStories } from '@storybook/react';
import type { Meta, StoryFn } from '@storybook/react';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonniebigodes
Copy link
Contributor

@yannbf I've pushed a couple of commits updating the documentation as discussed. I polished it up in terms of grammar and readability as much as possible and updated some links for outdated content or accuracy. Feel free to let me know if you need anything else added, and I'll gladly adjust it.


## Getting started

We recommend you turn off your current storyshots tests to start the migration process. To do this, rename the configuration file (i.e., `storybook.test.ts` or similar) to a different name. This will prevent the tests from being detected, as you'll be creating a new testing configuration file with the same name. By doing this, you'll be able to preserve your existing tests while transitioning to portable stories.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonniebigodes the reason I asked people to rename storybook.test.ts to storybook.test.ts.old was so that the test was not detected anymore. If we just tell them to rename to something else, the tests will still run and that will collide with the migration

MIGRATION.md Outdated Show resolved Hide resolved

### 4 - (Optional) Extend your testing coverage

The examples above will give you the closest possible experience with the Storyshots addon. However, if you are using Storyshots for other use cases, such as accessibility testing, image snapshot testing, or different testing scenarios, you can extend them to suit your needs or extend your testing solution to use the [Storybook test-runner](https://github.com/storybookjs/test-runner), that provides out-of-the-box solutions for such use cases.
Copy link
Member Author

@yannbf yannbf Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that provides out-of-the-box solutions for such use cases.

That's not true for some cases, such as accessibility and image snapshot testing. People still need to install dependencies and follow recipes

// Replace your-testing-library with one of the supported testing libraries (e.g., react, vue)
import { render } from '@testing-library/your-testing-library';

// Adjust the import based on the supported framework or Storybook's testing libraries (e.g., react, testing-vue3)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if people use @storybook/react they're good, but if they use @storybook/testing-vue3, they have to install the package. In the future it will be integrated into @storybook/vue3, but right now that's the case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yannbf I've pushed a commit that added a callout to this situation. As Vue 3 is the wildcard here, users must install and use it whether they are enabling setProjectAnnotations or using the composeStories API. I removed the mention of the integration to avoid confusion and have users ask us about the integration into the Vue 3 renderer as we don't have a precise date on it. I don't want to set expectations until that work is planned correctly and added.

@jonniebigodes
Copy link
Contributor

@yannbf thanks for taking the time to check this and provide some feedback. Appreciate it 🙏 ! I've pushed a commit that addressed your feedback. Let me know if you need anything else changed and I'll gladly do it.

@jonniebigodes jonniebigodes marked this pull request as ready for review November 14, 2023 17:32
@jonniebigodes
Copy link
Contributor

@yannbf going to merge this to get the information available as soon as possible.

@jonniebigodes jonniebigodes merged commit abdeace into next Nov 14, 2023
13 checks passed
Copy link

🚀 PR was released in v0.15.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released skip-release Preserve the current version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants