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

Test: Create @storybook/test package based on vitest #24392

Merged
merged 45 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
29f8596
Create @storybook/test package
kasperpeulen Aug 17, 2023
f0d68a1
Replace expect (from jest) with @vitest/expect
kasperpeulen Aug 25, 2023
97350aa
Fix build
kasperpeulen Aug 25, 2023
daeffb5
Move testing library to test package + some improvements
kasperpeulen Sep 19, 2023
1f02904
Add testing library assertion on expect
kasperpeulen Sep 19, 2023
b406b6d
Make sure synchronous methods are always promises, even in the first run
kasperpeulen Sep 20, 2023
76c109a
Revert mistake
kasperpeulen Sep 20, 2023
d034a44
Clear spies and instrument spies by event handler name
kasperpeulen Sep 20, 2023
e6b0f0a
Show deprecation message
kasperpeulen Sep 20, 2023
9d88027
Revert making get* method's promises and make sure we instrument `not`
kasperpeulen Sep 21, 2023
ea2c3c3
fix yarn lock
yannbf Sep 21, 2023
33cbcae
Fix eslint
kasperpeulen Sep 21, 2023
6d19861
Comment out deprecation for now
kasperpeulen Sep 22, 2023
0b76716
Fix types
kasperpeulen Sep 22, 2023
da91e3d
Make getters lazy and fix workspace
kasperpeulen Sep 22, 2023
17b3a33
Fix yarn.lock
kasperpeulen Sep 22, 2023
fae8299
Fix type fest range
kasperpeulen Sep 22, 2023
2085dd4
Patch vitest to make it compatible with the jest runner
kasperpeulen Sep 22, 2023
39ed28d
Add jest types to lock
kasperpeulen Sep 22, 2023
ddd8bf1
Better patch
kasperpeulen Sep 22, 2023
27ded25
Fix lock
kasperpeulen Sep 22, 2023
d44e0bd
Fix lazy getters
kasperpeulen Sep 23, 2023
8657c59
Fix gitignore
kasperpeulen Sep 23, 2023
db9bb66
Merge branch 'next' into kasper/test-package
kasperpeulen Oct 3, 2023
a5cdd31
Fix getter
kasperpeulen Oct 3, 2023
d62d5b2
Merge remote-tracking branch 'origin/next' into kasper/test-package
kasperpeulen Oct 3, 2023
de86f5d
Always externalize @vitest deps in tsup, as they are ESM-only. But st…
kasperpeulen Oct 4, 2023
6ee27f2
Always install extra deps
kasperpeulen Oct 3, 2023
47586d9
Fix getters that use `this`
kasperpeulen Oct 4, 2023
4ae6a77
Fix deps issue
kasperpeulen Oct 4, 2023
5497868
Merge remote-tracking branch 'origin/next' into kasper/bladiebla
kasperpeulen Oct 5, 2023
6de0d49
Fix lock files
kasperpeulen Oct 5, 2023
2b9981a
Fix check script
kasperpeulen Oct 5, 2023
fb84613
Calculate the diff for chai errors
kasperpeulen Oct 10, 2023
6801e04
Add moduleNameMapper for ESM only modules with package json exports
kasperpeulen Oct 10, 2023
ef847b9
Downcast function types that are mocks in meta args
kasperpeulen Oct 10, 2023
d843514
Merge branch 'next' into kapser/test-package2
ndelangen Oct 11, 2023
f42e9ef
Pre bundle most deps of the test package
kasperpeulen Oct 25, 2023
43ee7d5
Merge remote-tracking branch 'origin/next' into kapser/test-package2
kasperpeulen Oct 25, 2023
a8edebd
Merge branch 'next' into kapser/test-package2
kasperpeulen Oct 26, 2023
bcf87ea
Fix eslint
kasperpeulen Oct 27, 2023
19f2409
Merge branch 'next' into kapser/test-package2
kasperpeulen Oct 27, 2023
d888f60
Fix next version
kasperpeulen Oct 27, 2023
4f08e77
Merge remote-tracking branch 'origin/kapser/test-package2' into kapse…
kasperpeulen Oct 27, 2023
4e993be
Merge remote-tracking branch 'origin/next' into kapser/test-package2
kasperpeulen Oct 27, 2023
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ test-results
!/**/.yarn/plugins
!/**/.yarn/sdks
!/**/.yarn/versions
!/**/.yarn/patches
/**/.pnp.*
!/node_modules

Expand Down
37 changes: 37 additions & 0 deletions code/.yarn/patches/@vitest-expect-npm-0.34.5-8031508efe.patch

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion code/addons/actions/src/addArgs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import type { ArgsEnhancer } from '@storybook/types';
import { addActionsFromArgTypes, inferActionsFromArgTypesRegex } from './addArgsHelpers';
import {
addActionsFromArgTypes,
attachActionsToFunctionMocks,
inferActionsFromArgTypesRegex,
} from './addArgsHelpers';

export const argsEnhancers: ArgsEnhancer[] = [
addActionsFromArgTypes,
inferActionsFromArgTypesRegex,
attachActionsToFunctionMocks,
];
33 changes: 32 additions & 1 deletion code/addons/actions/src/addArgsHelpers.ts
Copy link

@jalooc jalooc Jun 4, 2024

Choose a reason for hiding this comment

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

Hey, nice job with the PR, useful stuff. One problem I found (not sure if this code is the cause, but it's the closest that I was able to find) is that now not only the fn() arg types are logged in the Actions tab, but all the fn()s defined anywhere. Not sure if this is expected or a bug, but looking at the code here, that's not how it was meant to work 🧐

A sample case I've been tackling with after upgrading SB to v8 is that I've got quite a lot of stubs defined using fn() for mocked api modules in my application (using the __mock__ folder method). All their invocations get logged in the Actions panel when the component story is invoked, but it does not make sense to have them logged in there - they're no actions, nor anything relevant to the person using the story. In some cases, when a more complex component, like a page component, makes lots of those mocked api requests, this behaviour renders the Actions tab completely unusable, littering it with irrelevant entries.

Does that behaviour sound familiar? Anything that can be bypassed / fixed?

Copy link
Contributor Author

@kasperpeulen kasperpeulen Jun 4, 2024

Choose a reason for hiding this comment

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

Could you make an issue for this? I have some plans to bypass this behavior, but nothing concrete yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the easiest thing we can do as a starter, is only log it if the mockName is set:
https://vitest.dev/api/mock.html#mockname

As otherwise it will only log spy and nothing something useful anyway.

I'm also thinking about enriching the fn with a type.

fn().type('action')
fn().type('query')

And only show the type action by default. And hide all other types (or those without type), but still be able to show them by changing the action filter or something.

Copy link

Choose a reason for hiding this comment

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

Here it is: #27534

Wasn't sure what type of issue that should be; went with bug since seems to me that both the documentation and the code indicate that only the fn()s used in args should be logged in the Actions tab.

Copy link

Choose a reason for hiding this comment

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

Answering your suggestions @kasperpeulen - could you point out how/where it happens that all fn()s get intercepted and logged in the tab? Looking at the code here, I'd say it's clear that only the ones defined in args should be enhanced 🧐

That knowledge would help me provide better feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I added a link to the code in the response of your issue!

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-underscore-dangle,no-param-reassign */
import type { Args, Renderer, ArgsEnhancer } from '@storybook/types';
import { action } from './runtime/action';

Expand Down Expand Up @@ -31,7 +32,7 @@ export const inferActionsFromArgTypesRegex: ArgsEnhancer<Renderer> = (context) =

return argTypesMatchingRegex.reduce((acc, [name, argType]) => {
if (isInInitialArgs(name, initialArgs)) {
acc[name] = action(name);
acc[name] = action(name, { implicit: true });
}
return acc;
}, {} as Args);
Expand Down Expand Up @@ -61,3 +62,33 @@ export const addActionsFromArgTypes: ArgsEnhancer<Renderer> = (context) => {
return acc;
}, {} as Args);
};

export const attachActionsToFunctionMocks: ArgsEnhancer<Renderer> = (context) => {
const {
initialArgs,
argTypes,
parameters: { actions },
} = context;
if (actions?.disable || !argTypes) {
return {};
}

const argTypesWithAction = Object.entries(initialArgs).filter(
([, value]) =>
typeof value === 'function' &&
'_isMockFunction' in value &&
value._isMockFunction &&
!value._actionAttached
);

return argTypesWithAction.reduce((acc, [key, value]) => {
const previous = value.getMockImplementation();
value.mockImplementation((...args: unknown[]) => {
action(key)(...args);
return previous?.(...args);
});
// this enhancer is being called multiple times
value._actionAttached = true;
return acc;
}, {} as Args);
};
1 change: 1 addition & 0 deletions code/addons/actions/src/models/ActionOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ interface Options {
depth: number; // backards compatibility, remove in 7.0
clearOnStoryChange: boolean;
limit: number;
implicit: boolean;
}

export type ActionOptions = Partial<Options> & Partial<TelejsonOptions>;
17 changes: 17 additions & 0 deletions code/addons/actions/src/runtime/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ export function action(name: string, options: ActionOptions = {}): HandlerFuncti
};

const handler = function actionHandler(...args: any[]) {
// TODO: Enable once codemods are finished
// if (options.implicit) {
// const preview =
// '__STORYBOOK_PREVIEW__' in global
// ? (global.__STORYBOOK_PREVIEW__ as PreviewWeb<Renderer>)
// : undefined;
// if (
// preview?.storyRenders.some(
// (render) => render.phase === 'playing' || render.phase === 'rendering'
// )
// ) {
// console.warn(
// 'Can not use implicit actions during rendering or playing of a story.'
// );
// }
// }

const channel = addons.getChannel();
// this makes sure that in js enviroments like react native you can still get an id
const id = generateId();
Expand Down
25 changes: 24 additions & 1 deletion code/addons/interactions/src/components/Interaction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { type Call, CallStates, type ControlStates } from '@storybook/instrument
import { styled, typography } from '@storybook/theming';
import { transparentize } from 'polished';

import { MatcherResult } from './MatcherResult';
import { Expected, MatcherResult, Received } from './MatcherResult';
import { MethodCall } from './MethodCall';
import { StatusIcon } from './StatusIcon';

Expand Down Expand Up @@ -120,6 +120,29 @@ const Exception = ({ exception }: { exception: Call['exception'] }) => {
return (
<RowMessage>
<pre>{paragraphs[0]}</pre>

{exception.showDiff && exception.diff ? (
<>
<br />
<MatcherResult message={exception.diff} style={{ padding: 0 }} />
</>
) : (
<pre>
<br />
{exception.expected && (
<>
Expected: <Expected value={exception.expected} />
<br />
</>
)}
{exception.actual && (
<>
Received: <Received value={exception.actual} />
<br />
</>
)}
</pre>
)}
{more && <p>See the full stack trace in the browser console.</p>}
</RowMessage>
);
Expand Down
9 changes: 8 additions & 1 deletion code/addons/interactions/src/components/MatcherResult.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,21 @@ export const Expected = ({ value, parsed }: { value: any; parsed?: boolean }) =>
return <StyledExpected>{value}</StyledExpected>;
};

export const MatcherResult = ({ message }: { message: string }) => {
export const MatcherResult = ({
message,
style = {},
}: {
message: string;
style?: React.CSSProperties;
}) => {
const lines = message.split('\n');
return (
<pre
style={{
margin: 0,
padding: '8px 10px 8px 36px',
fontSize: typography.size.s1,
...style,
}}
>
{lines.flatMap((line: string, index: number) => {
Expand Down
25 changes: 22 additions & 3 deletions code/addons/interactions/src/preview.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-param-reassign,no-underscore-dangle */
/// <reference types="node" />

import { addons } from '@storybook/preview-api';
Expand All @@ -9,6 +10,7 @@ import type {
PlayFunction,
PlayFunctionContext,
StepLabel,
Args,
} from '@storybook/types';
import { instrument } from '@storybook/instrumenter';
import { ModuleMocker } from 'jest-mock';
Expand All @@ -30,14 +32,13 @@ const addSpies = (id: string, val: any, key?: string): any => {
try {
if (Object.prototype.toString.call(val) === '[object Object]') {
// We have to mutate the original object for this to survive HMR.
// eslint-disable-next-line no-restricted-syntax, no-param-reassign
// eslint-disable-next-line no-restricted-syntax
for (const [k, v] of Object.entries(val)) val[k] = addSpies(id, v, k);
return val;
}
if (Array.isArray(val)) {
return val.map((item, index) => addSpies(id, item, `${key}[${index}]`));
}
// eslint-disable-next-line no-underscore-dangle
if (typeof val === 'function' && val.isAction && !val._isMockFunction) {
Object.defineProperty(val, 'name', { value: key, writable: false });
Object.defineProperty(val, '__storyId__', { value: id, writable: false });
Expand All @@ -54,7 +55,25 @@ const addSpies = (id: string, val: any, key?: string): any => {
const addActionsFromArgTypes: ArgsEnhancer<Renderer> = ({ id, initialArgs }) =>
addSpies(id, initialArgs);

export const argsEnhancers = [addActionsFromArgTypes];
const instrumentSpies: ArgsEnhancer = ({ initialArgs }) => {
const argTypesWithAction = Object.entries(initialArgs).filter(
([, value]) =>
typeof value === 'function' &&
'_isMockFunction' in value &&
value._isMockFunction &&
!value._instrumented
);

return argTypesWithAction.reduce((acc, [key, value]) => {
const instrumented = instrument({ [key]: () => value }, { retain: true })[key];
acc[key] = instrumented();
// this enhancer is being called multiple times
value._instrumented = true;
return acc;
}, {} as Args);
};

export const argsEnhancers = [addActionsFromArgTypes, instrumentSpies];

export const { step: runStep } = instrument(
{
Expand Down
3 changes: 3 additions & 0 deletions code/jest.config.base.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const modulesToTransform = [
'@angular',
'@lit',
'@mdx-js',
'@vitest',
'ccount',
'character-entities',
'decode-named-character-reference',
Expand Down Expand Up @@ -60,6 +61,8 @@ module.exports = {
path.resolve('./__mocks__/fileMock.js'),
'\\.(css|scss|stylesheet)$': path.resolve('./__mocks__/styleMock.js'),
'\\.(md)$': path.resolve('./__mocks__/htmlMock.js'),
'@vitest/utils/(.*)': '@vitest/utils/dist/$1.js',
'@vitest/utils': '@vitest/utils/dist/index.js',
},
transform: {
'^.+\\.(t|j)sx?$': ['@swc/jest', swcrc],
Expand Down
4 changes: 3 additions & 1 deletion code/lib/instrumenter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@
"@storybook/client-logger": "workspace:*",
"@storybook/core-events": "workspace:*",
"@storybook/global": "^5.0.0",
"@storybook/preview-api": "workspace:*"
"@storybook/preview-api": "workspace:*",
"@vitest/utils": "^0.34.6",
"util": "^0.12.4"
},
"devDependencies": {
"typescript": "~4.9.3"
Expand Down
42 changes: 40 additions & 2 deletions code/lib/instrumenter/src/instrumenter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,44 @@ describe('Instrumenter', () => {
expect(result.fn1.fn2.__originalFn__).toBe(fn1.fn2);
});

it('patches functions correctly that reference this', () => {
const object = {
name: 'name',
method() {
return this.name;
},
};

const instrumented = instrument(object);
expect(object.method()).toEqual(instrumented.method());

expect(instrumented.method).toEqual(expect.any(Function));
expect(instrumented.method.__originalFn__).toBe(object.method);
});

it('patches functions correctly that use proxies', () => {
const object = new Proxy(
{
name: 'name',
method() {
return this.name;
},
},
{
get(target, prop, receiver) {
if (prop === 'name') return `${target[prop]}!`;
return Reflect.get(target, prop, receiver);
},
}
);

const instrumented = instrument(object);
expect(object.method()).toEqual(instrumented.method());

expect(instrumented.method).toEqual(expect.any(Function));
expect(instrumented.method.__originalFn__).toBe(object.method);
});

it('patched functions call the original function when invoked', () => {
const { fn } = instrument({ fn: jest.fn() });
const obj = {};
Expand Down Expand Up @@ -510,12 +548,12 @@ describe('Instrumenter', () => {
expect(callSpy).toHaveBeenCalledWith(
expect.objectContaining({
id: 'kind--story [0] fn',
exception: {
exception: expect.objectContaining({
name: 'Error',
message: 'Boom!',
stack: expect.stringContaining('Error: Boom!'),
callId: 'kind--story [0] fn',
},
}),
})
);
});
Expand Down
Loading