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.

Sign up for GitHub

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

Addon Test: Fix duplicate test.include patterns #30029

Merged
merged 4 commits into from
Dec 12, 2024
+22 −14
Merged

Addon Test: Fix duplicate test.include patterns #30029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aa75a98
remove existing test.include config
JReinhold Dec 11, 2024
9a680a3
remove unnecessary test.include in test kitchen sink
JReinhold Dec 11, 2024
d5b2286
Merge branch 'next' into jeppe/fix-duplicate-include
JReinhold Dec 11, 2024
0ece491
Merge branch 'next' of github.com:storybookjs/storybook into jeppe/fi…
JReinhold Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions code/addons/test/src/vitest-plugin/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
.replace('</head>', `${headHtmlSnippet ?? ''}</head>`)
.replace('<body>', `<body>${bodyHtmlSnippet ?? ''}`);
},
async config(inputConfig_DoNotMutate) {
async config(inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED) {
// ! We're not mutating the input config, instead we're returning a new partial config
// ! see https://vite.dev/guide/api-plugin.html#config
try {
Expand Down Expand Up @@ -155,8 +155,9 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
setupFiles: [
join(packageDir, 'dist/vitest-plugin/setup-file.mjs'),
// if the existing setupFiles is a string, we have to include it otherwise we're overwriting it
typeof inputConfig_DoNotMutate.test?.setupFiles === 'string' &&
inputConfig_DoNotMutate.test?.setupFiles,
typeof inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test
?.setupFiles === 'string' &&
inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test?.setupFiles,
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
].filter(Boolean) as string[],

...(finalOptions.storybookScript
Expand All @@ -181,7 +182,8 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
.map((path) => convertPathToPattern(path)),

// if the existing deps.inline is true, we keep it as-is, because it will inline everything
...(inputConfig_DoNotMutate.test?.server?.deps?.inline !== true
...(inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test?.server?.deps
?.inline !== true
? {
server: {
deps: {
Expand All @@ -192,7 +194,7 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
: {}),

browser: {
...inputConfig_DoNotMutate.test?.browser,
...inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test?.browser,
commands: {
getInitialGlobals: () => {
const envConfig = JSON.parse(process.env.VITEST_STORYBOOK_CONFIG ?? '{}');
Expand All @@ -209,8 +211,9 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
},
},
// if there is a test.browser config AND test.browser.screenshotFailures is not explicitly set, we set it to false
...(inputConfig_DoNotMutate.test?.browser &&
inputConfig_DoNotMutate.test.browser.screenshotFailures === undefined
...(inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test?.browser &&
inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test.browser
.screenshotFailures === undefined
? {
screenshotFailures: false,
}
Expand All @@ -219,7 +222,11 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
},

envPrefix: Array.from(
new Set([...(inputConfig_DoNotMutate.envPrefix || []), 'STORYBOOK_', 'VITE_'])
new Set([
...(inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.envPrefix || []),
'STORYBOOK_',
'VITE_',
])
),

resolve: {
Expand Down Expand Up @@ -258,8 +265,12 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin> => {
);

// alert the user of problems
if (inputConfig_DoNotMutate.test.include?.length > 0) {
console.warn(
if (
inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test.include?.length > 0
) {
// remove the user's existing include, because we're replacing it with our own heuristic based on main.ts#stories
inputConfig_ONLY_MUTATE_WHEN_STRICTLY_NEEDED_OR_YOU_WILL_BE_FIRED.test.include = [];
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
console.log(
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
picocolors.yellow(dedent`
Warning: Starting in Storybook 8.5.0-alpha.18, the "test.include" option in Vitest is discouraged in favor of just using the "stories" field in your Storybook configuration.

Expand Down
5 changes: 1 addition & 4 deletions test-storybooks/portable-stories-kitchen-sink/react/vitest.workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ export default defineWorkspace([
test: {
name: "storybook",
pool: "threads",
include: [
"stories/AddonTest.stories.?(c|m)[jt]s?(x)",
],
deps: {
optimizer: {
web: {
Expand All @@ -30,4 +27,4 @@ export default defineWorkspace([
environment: "happy-dom",
},
},
]);
]);
Loading