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

Improve type safety and code quality #383

Merged
merged 11 commits into from
Nov 21, 2023
2 changes: 1 addition & 1 deletion .storybook/test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const config: TestRunnerConfig = {
});

const elementHandler = (await page.$('#root')) || (await page.$('#storybook-root'));
const innerHTML = await elementHandler.innerHTML();
const innerHTML = await elementHandler?.innerHTML();
// HTML snapshot tests
expect(innerHTML).toMatchSnapshot();
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"@babel/preset-env": "^7.19.4",
"@babel/preset-react": "^7.18.6",
"@babel/preset-typescript": "^7.18.6",
"@jest/types": "^29.6.3",
"@storybook/addon-coverage": "^0.0.9",
"@storybook/addon-essentials": "^7.5.3",
"@storybook/addon-interactions": "^7.5.3",
Expand Down Expand Up @@ -96,6 +95,7 @@
"@babel/generator": "^7.22.5",
"@babel/template": "^7.22.5",
"@babel/types": "^7.22.5",
"@jest/types": "^29.6.3",
"@storybook/core-common": "^7.0.0-beta.0 || ^7.0.0-rc.0 || ^7.0.0",
"@storybook/csf": "^0.1.1",
"@storybook/csf-tools": "^7.0.0-beta.0 || ^7.0.0-rc.0 || ^7.0.0",
Expand Down
21 changes: 11 additions & 10 deletions src/config/jest-playwright.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import path from 'path';
import { getProjectRoot } from '@storybook/core-common';
import type { Config } from '@jest/types';

const TEST_RUNNER_PATH = process.env.STORYBOOK_TEST_RUNNER_PATH || '@storybook/test-runner';
const TEST_RUNNER_PATH = process.env.STORYBOOK_TEST_RUNNER_PATH ?? '@storybook/test-runner';

/**
* IMPORTANT NOTE:
Expand All @@ -15,7 +16,7 @@ const TEST_RUNNER_PATH = process.env.STORYBOOK_TEST_RUNNER_PATH || '@storybook/t
* This function does the same thing as `preset: 'jest-playwright-preset` but makes sure that the
* necessary moving parts are all required within the correct path.
* */
const getJestPlaywrightConfig = () => {
const getJestPlaywrightConfig = (): Config.InitialOptions => {
const presetBasePath = path.dirname(
require.resolve('jest-playwright-preset', {
paths: [path.join(__dirname, '../node_modules')],
Expand All @@ -28,18 +29,18 @@ const getJestPlaywrightConfig = () => {
);
return {
runner: path.join(presetBasePath, 'runner.js'),
globalSetup: require.resolve(TEST_RUNNER_PATH + '/playwright/global-setup.js'),
globalTeardown: require.resolve(TEST_RUNNER_PATH + '/playwright/global-teardown.js'),
testEnvironment: require.resolve(TEST_RUNNER_PATH + '/playwright/custom-environment.js'),
globalSetup: require.resolve(`${TEST_RUNNER_PATH}/playwright/global-setup.js`),
globalTeardown: require.resolve(`${TEST_RUNNER_PATH}/playwright/global-teardown.js`),
testEnvironment: require.resolve(`${TEST_RUNNER_PATH}/playwright/custom-environment.js`),
setupFilesAfterEnv: [
require.resolve(TEST_RUNNER_PATH + '/playwright/jest-setup.js'),
require.resolve(`${TEST_RUNNER_PATH}/playwright/jest-setup.js`),
expectPlaywrightPath,
path.join(presetBasePath, 'lib', 'extends.js'),
],
};
};

export const getJestConfig = () => {
export const getJestConfig = (): Config.InitialOptions => {
const {
TEST_ROOT,
TEST_MATCH,
Expand Down Expand Up @@ -69,16 +70,16 @@ export const getJestConfig = () => {

const reporters = STORYBOOK_JUNIT ? ['default', jestJunitPath] : ['default'];

const testMatch = (STORYBOOK_STORIES_PATTERN && STORYBOOK_STORIES_PATTERN.split(';')) || [];
const testMatch = STORYBOOK_STORIES_PATTERN?.split(';') ?? [];

let config = {
const config: Config.InitialOptions = {
rootDir: getProjectRoot(),
roots: TEST_ROOT ? [TEST_ROOT] : undefined,
reporters,
testMatch,
transform: {
'^.+\\.(story|stories)\\.[jt]sx?$': require.resolve(
TEST_RUNNER_PATH + '/playwright/transform'
`${TEST_RUNNER_PATH}/playwright/transform`
),
'^.+\\.[jt]sx?$': swcJestPath,
},
Expand Down
9 changes: 9 additions & 0 deletions src/csf/__snapshots__/transformCsf.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -229,3 +229,12 @@ if (!require.main) {
});
}"
`;

exports[`transformCsf returns empty result if there are no stories 1`] = `
"
export default {
title: 'Button',
};

"
`;
12 changes: 12 additions & 0 deletions src/csf/transformCsf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ describe('transformCsf', () => {
expect(result).toEqual(expectedCode);
});

it('returns empty result if there are no stories', () => {
const csfCode = `
export default {
title: 'Button',
};
`;

const result = transformCsf(csfCode, { testPrefixer });

expect(result).toMatchSnapshot();
});

it('calls the testPrefixer function for each test', () => {
const csfCode = `
export default {
Expand Down
30 changes: 16 additions & 14 deletions src/csf/transformCsf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ const makePlayTest = ({
metaOrStoryPlay?: boolean;
testPrefix: TestPrefixer;
shouldSkip?: boolean;
}): t.Statement[] => {
}): t.ExpressionStatement[] => {
return [
t.expressionStatement(
t.callExpression(shouldSkip ? t.identifier('it.skip') : t.identifier('it'), [
t.stringLiteral(!!metaOrStoryPlay ? 'play-test' : 'smoke-test'),
t.stringLiteral(metaOrStoryPlay ? 'play-test' : 'smoke-test'),
prefixFunction(key, title, testPrefix),
])
),
Expand All @@ -69,7 +69,7 @@ const makeDescribe = (
key: string,
tests: t.Statement[],
beforeEachBlock?: t.ExpressionStatement
): t.Statement | null => {
): t.ExpressionStatement => {
const blockStatements = beforeEachBlock ? [beforeEachBlock, ...tests] : tests;
return t.expressionStatement(
t.callExpression(t.identifier('describe'), [
Expand Down Expand Up @@ -100,22 +100,25 @@ export const transformCsf = (
) => {
const { includeTags, excludeTags, skipTags } = getTagOptions();

const csf = loadCsf(code, { makeTitle: makeTitle || ((userTitle: string) => userTitle) });
const csf = loadCsf(code, { makeTitle: makeTitle ?? ((userTitle: string) => userTitle) });
csf.parse();

const storyExports = Object.keys(csf._stories);
const title = csf.meta?.title;

const storyAnnotations = storyExports.reduce((acc, key) => {
const annotations = csf._storyAnnotations[key];
acc[key] = {};
if (annotations?.play) {
acc[key].play = annotations.play;
}
const storyAnnotations = storyExports.reduce<Record<string, { play?: t.Node; tags?: string[] }>>(
(acc, key) => {
const annotations = csf._storyAnnotations[key];
acc[key] = {};
if (annotations?.play) {
acc[key].play = annotations.play;
}

acc[key].tags = csf._stories[key].tags || csf.meta?.tags || [];
return acc;
}, {} as Record<string, { play?: t.Node; tags?: string[] }>);
acc[key].tags = csf._stories[key].tags || csf.meta?.tags || [];
return acc;
},
{}
);

const allTests = storyExports
.filter((key) => {
Expand Down Expand Up @@ -148,7 +151,6 @@ export const transformCsf = (
if (tests.length) {
return makeDescribe(key, tests);
}
return null;
})
.filter(Boolean) as babel.types.Statement[];

Expand Down
4 changes: 2 additions & 2 deletions src/playwright/transformPlaywright.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ jest.mock('@storybook/core-common', () => ({
jest.mock('../util/getTestRunnerConfig');

expect.addSnapshotSerializer({
print: (val: any) => val.trim(),
test: (val: any) => true,
print: (val: unknown) => (typeof val === 'string' ? val.trim() : String(val)),
test: () => true,
});

describe('Playwright', () => {
Expand Down
16 changes: 9 additions & 7 deletions src/playwright/transformPlaywright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ const coverageErrorMessage = dedent`
More info: https://github.com/storybookjs/test-runner#setting-up-code-coverage
`;

export const testPrefixer = template(
`
export const testPrefixer: TestPrefixer = (context) => {
return template(
`
console.log({ id: %%id%%, title: %%title%%, name: %%name%%, storyExport: %%storyExport%% });
async () => {
const testFn = async() => {
Expand Down Expand Up @@ -66,14 +67,15 @@ export const testPrefixer = template(
}
}
`,
{
plugins: ['jsx'],
}
) as unknown as TestPrefixer;
{
plugins: ['jsx'],
}
)({ ...context });
};

const makeTitleFactory = (filename: string) => {
const { workingDir, normalizedStoriesEntries } = getStorybookMetadata();
const filePath = './' + relative(workingDir, filename);
const filePath = `./${relative(workingDir, filename)}`;

return (userTitle: string) =>
userOrAutoTitle(filePath, normalizedStoriesEntries, userTitle) as string;
Expand Down
73 changes: 48 additions & 25 deletions src/playwright/transformPlaywrightJson.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { transformPlaywrightJson } from './transformPlaywrightJson';
import {
UnsupportedVersion,
V3StoriesIndex,
V4Index,
makeDescribe,
transformPlaywrightJson,
} from './transformPlaywrightJson';
import * as t from '@babel/types';

jest.mock('../util/getTestRunnerConfig');

Expand All @@ -18,23 +25,20 @@ describe('Playwright Json', () => {
id: 'example-header--logged-in',
title: 'Example/Header',
name: 'Logged In',
importPath: './stories/basic/Header.stories.js',
tags: ['play-fn'],
},
'example-header--logged-out': {
id: 'example-header--logged-out',
title: 'Example/Header',
name: 'Logged Out',
importPath: './stories/basic/Header.stories.js',
},
'example-page--logged-in': {
id: 'example-page--logged-in',
title: 'Example/Page',
name: 'Logged In',
importPath: './stories/basic/Page.stories.js',
},
},
};
} satisfies V4Index;
expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
{
"example-header": "describe("Example/Header", () => {
Expand Down Expand Up @@ -355,16 +359,14 @@ describe('Playwright Json', () => {
id: 'example-introduction--page',
title: 'Example/Introduction',
name: 'Page',
importPath: './stories/basic/Introduction.stories.mdx',
},
'example-page--logged-in': {
id: 'example-page--logged-in',
title: 'Example/Page',
name: 'Logged In',
importPath: './stories/basic/Page.stories.js',
},
},
};
} satisfies V4Index;
expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
{
"example-page": "describe("Example/Page", () => {
Expand Down Expand Up @@ -433,9 +435,6 @@ describe('Playwright Json', () => {
id: 'example-header--logged-in',
title: 'Example/Header',
name: 'Logged In',
importPath: './stories/basic/Header.stories.js',
kind: 'Example/Header',
story: 'Logged In',
parameters: {
__id: 'example-header--logged-in',
docsOnly: false,
Expand All @@ -446,9 +445,6 @@ describe('Playwright Json', () => {
id: 'example-header--logged-out',
title: 'Example/Header',
name: 'Logged Out',
importPath: './stories/basic/Header.stories.js',
kind: 'Example/Header',
story: 'Logged Out',
parameters: {
__id: 'example-header--logged-out',
docsOnly: false,
Expand All @@ -459,17 +455,14 @@ describe('Playwright Json', () => {
id: 'example-page--logged-in',
title: 'Example/Page',
name: 'Logged In',
importPath: './stories/basic/Page.stories.js',
kind: 'Example/Page',
story: 'Logged In',
parameters: {
__id: 'example-page--logged-in',
docsOnly: false,
fileName: './stories/basic/Page.stories.js',
},
},
},
};
} satisfies V3StoriesIndex;
expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
{
"example-header": "describe("Example/Header", () => {
Expand Down Expand Up @@ -638,9 +631,6 @@ describe('Playwright Json', () => {
id: 'example-introduction--page',
title: 'Example/Introduction',
name: 'Page',
importPath: './stories/basic/Introduction.stories.mdx',
kind: 'Example/Introduction',
story: 'Page',
parameters: {
__id: 'example-introduction--page',
docsOnly: true,
Expand All @@ -651,17 +641,14 @@ describe('Playwright Json', () => {
id: 'example-page--logged-in',
title: 'Example/Page',
name: 'Logged In',
importPath: './stories/basic/Page.stories.js',
kind: 'Example/Page',
story: 'Logged In',
parameters: {
__id: 'example-page--logged-in',
docsOnly: false,
fileName: './stories/basic/Page.stories.js',
},
},
},
};
} satisfies V3StoriesIndex;
expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
{
"example-page": "describe("Example/Page", () => {
Expand Down Expand Up @@ -721,3 +708,39 @@ describe('Playwright Json', () => {
});
});
});

describe('unsupported index', () => {
it('throws an error for unsupported versions', () => {
const unsupportedVersion = { v: 1 } satisfies UnsupportedVersion;
expect(() => transformPlaywrightJson(unsupportedVersion)).toThrowError(
`Unsupported version ${unsupportedVersion.v}`
);
});
});

describe('makeDescribe', () => {
it('should generate a skipped describe block with a no-op test when stmts is empty', () => {
const title = 'Test Title';
const stmts: t.Statement[] = []; // Empty array

const result = makeDescribe(title, stmts);

// Create the expected AST manually for a skipped describe block with a no-op test
const noOpIt = t.expressionStatement(
t.callExpression(t.identifier('it'), [
t.stringLiteral('no-op'),
t.arrowFunctionExpression([], t.blockStatement([])),
])
);

const expectedAST = t.expressionStatement(
t.callExpression(t.memberExpression(t.identifier('describe'), t.identifier('skip')), [
t.stringLiteral(title),
t.arrowFunctionExpression([], t.blockStatement([noOpIt])),
])
);

// Compare the generated AST with the expected AST
expect(result).toEqual(expectedAST);
});
});
Loading
Loading