Skip to content

Commit

Permalink
fix: fix storybook build cache not being used by tests in CI (twentyh…
Browse files Browse the repository at this point in the history
…q#5451)

TL;DR:
- removed `--configuration={args.scope}` from `storybook:static:test`
for the `storybook:static` part, as it was making `front-sb-test` jobs
in CI not reuse the cache from the `front-sb-build` job and re-build
storybook every time.
- replaced it with a new `test` configuration which optimizes storybook
build for tests and builds storybook 2x faster.

## Fix storybook:build cache usage in CI

`storybook:static:test` executes two scripts in parallel:
1. `storybook:static`, which depends on `storybook:build`
1.a. it builds storybook first with `storybook:build`, the output
directory is `storybook-static`.
1.b. then it launches an `http-server`, using what has been built in
`storybook-static`
2. `storybook:test` to execute tests (needs the storybook http-server to
be running)

When passing `--configuration=pages` or `--configuration=modules` to
`storybook:static` from step 1, those configurations are passed to the
`storybook:build` script from step 1.a as well.

But for Nx `storybook:build` and `storybook:build --configuration=pages`
(or `modules`) are not the same command, therefore one does not reuse
the cache of the other because they could output completely different
things.

As `front-sb-test` jobs are passing `--configuration={args.scope}` to
`storybook:static`, the cache of the previously executed
`storybook:build` (from `front-sb-build`) is not reused and therefore
each job re-builds Storybook with its own scope, which increases CI
time.

### Solution

- Removed scope configurations from `storybook:static` and
`storybook:build` scripts to avoid confusion.
- `storybook:test` and `storybook:dev` can keep scope configurations as
they can be useful and this doesn't impact storybook build cache in CI.

### Improve Storybook build time for testing

Added the `test` configuration to `storybook:build` and
`storybook:static` which makes Storybook build time 2x faster. It
disables addons that slow down build time and are not used in tests.
  • Loading branch information
thaisguigon committed May 17, 2024
1 parent 36e5411 commit 992602b
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 54 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci-front.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ concurrency:

jobs:
front-sb-build:
runs-on: ci-8-cores
runs-on: ubuntu-latest
env:
REACT_APP_SERVER_BASE_URL: http://localhost:3000
NX_REJECT_UNKNOWN_LOCAL_CACHE: 0
Expand All @@ -39,7 +39,7 @@ jobs:
- name: Front / Write .env
run: npx nx reset:env twenty-front
- name: Front / Build storybook
run: npx nx storybook:build twenty-front
run: npx nx storybook:build twenty-front --configuration=test
front-sb-test:
runs-on: ci-8-cores
needs: front-sb-build
Expand Down
10 changes: 9 additions & 1 deletion nx.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@
"command": "storybook build",
"output-dir": "storybook-static",
"config-dir": ".storybook"
},
"configurations": {
"test": {
"command": "storybook build --test"
}
}
},
"storybook:dev": {
Expand All @@ -138,6 +143,9 @@
"host": "localhost",
"port": 6006,
"silent": true
},
"configurations": {
"test": {}
}
},
"storybook:coverage": {
Expand Down Expand Up @@ -192,7 +200,7 @@
"executor": "nx:run-commands",
"options": {
"commands": [
"npx concurrently --kill-others --success=first -n SB,TEST 'nx storybook:static {projectName} --port={args.port}' 'npx wait-on tcp:{args.port} && nx storybook:test {projectName} --port={args.port}'"
"npx concurrently --kill-others --success=first -n SB,TEST 'nx storybook:static {projectName} --port={args.port} --configuration=test' 'npx wait-on tcp:{args.port} && nx storybook:test {projectName} --port={args.port}'"
],
"port": 6006
}
Expand Down
9 changes: 9 additions & 0 deletions packages/twenty-front/.storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ const config: StorybookConfig = {
name: '@storybook/react-vite',
options: {},
},
build: {
test: {
disableMDXEntries: true,
disabledAddons: [
'@storybook/addon-docs',
'@storybook/addon-essentials/docs',
],
},
},
docs: {
autodocs: false,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/twenty-front/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"scripts": {
"build": "npx vite build && sh ./scripts/inject-runtime-env.sh",
"build:sourcemaps": "VITE_BUILD_SOURCEMAP=true NODE_OPTIONS=--max-old-space-size=4096 npx nx build",
"storybook:build:chromatic": "nx storybook:build",
"storybook:build:chromatic": "nx storybook:build --configuration=test",
"start:prod": "NODE_ENV=production npx vite --host",
"tsup": "npx tsup"
},
Expand Down
32 changes: 3 additions & 29 deletions packages/twenty-front/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,7 @@
"env": { "NODE_OPTIONS": "--max_old_space_size=5000" }
},
"configurations": {
"docs": {
"env": {
"NODE_OPTIONS": "--max_old_space_size=5000",
"STORYBOOK_SCOPE": "ui-docs"
}
},
"modules": {
"env": {
"NODE_OPTIONS": "--max_old_space_size=5000",
"STORYBOOK_SCOPE": "modules"
}
},
"pages": {
"env": {
"NODE_OPTIONS": "--max_old_space_size=5000",
"STORYBOOK_SCOPE": "pages"
}
},
"performance": {
"env": {
"NODE_OPTIONS": "--max_old_space_size=5000",
"STORYBOOK_SCOPE": "performance"
}
}
"test": {}
}
},
"storybook:dev": {
Expand All @@ -102,10 +79,7 @@
"storybook:static": {
"options": { "port": 6006 },
"configurations": {
"docs": { "env": { "STORYBOOK_SCOPE": "ui-docs" } },
"modules": { "env": { "STORYBOOK_SCOPE": "modules" } },
"pages": { "env": { "STORYBOOK_SCOPE": "pages" } },
"performance": { "env": { "STORYBOOK_SCOPE": "performance" } }
"test": {}
}
},
"storybook:coverage": {
Expand Down Expand Up @@ -137,7 +111,7 @@
"storybook:static:test": {
"options": {
"commands": [
"npx concurrently --kill-others --success=first -n SB,TEST 'nx storybook:static {projectName} --configuration={args.scope} --port={args.port}' 'npx wait-on tcp:{args.port} && nx storybook:test {projectName} --port={args.port} --configuration={args.scope}'"
"npx concurrently --kill-others --success=first -n SB,TEST 'nx storybook:static {projectName} --configuration=test --port={args.port}' 'npx wait-on tcp:{args.port} && nx storybook:test {projectName} --port={args.port} --configuration={args.scope}'"
],
"port": 6006
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { IconButton, IconButtonVariant } from '../button/components/IconButton';
import { LightIconButton } from '../button/components/LightIconButton';
import { IconPickerHotkeyScope } from '../types/IconPickerHotkeyScope';

type IconPickerProps = {
export type IconPickerProps = {
disabled?: boolean;
dropdownId?: string;
onChange: (params: { iconKey: string; Icon: IconComponent }) => void;
Expand Down Expand Up @@ -154,6 +154,11 @@ export const IconPicker = ({
dropdownHotkeyScope={{ scope: IconPickerHotkeyScope.IconPicker }}
clickableComponent={
<IconButton
ariaLabel={`Click to select icon ${
selectedIconKey
? `(selected: ${selectedIconKey})`
: `(no icon selected)`
}`}
disabled={disabled}
Icon={selectedIconKey ? getIcon(selectedIconKey) : IconApps}
variant={variant}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,56 +1,80 @@
import { useArgs } from '@storybook/preview-api';
import { Meta, StoryObj } from '@storybook/react';
import { expect, userEvent, within } from '@storybook/test';
import { ComponentDecorator } from 'twenty-ui';

import { IconsProviderDecorator } from '~/testing/decorators/IconsProviderDecorator';
import { sleep } from '~/testing/sleep';

import { IconPicker } from '../IconPicker';
import { IconPicker, IconPickerProps } from '../IconPicker';

type RenderProps = IconPickerProps;
const Render = (args: RenderProps) => {
const [{ selectedIconKey }, updateArgs] = useArgs();

return (
<IconPicker
// eslint-disable-next-line react/jsx-props-no-spreading
{...args}
onChange={({ iconKey }) => {
updateArgs({ selectedIconKey: iconKey });
}}
selectedIconKey={selectedIconKey}
/>
);
};

const meta: Meta<typeof IconPicker> = {
title: 'UI/Input/IconPicker/IconPicker',
component: IconPicker,
decorators: [IconsProviderDecorator, ComponentDecorator],
render: Render,
};

export default meta;
type Story = StoryObj<typeof IconPicker>;

export const Default: Story = {};

export const WithSelectedIcon: Story = {
args: { selectedIconKey: 'IconCalendarEvent' },
};

export const WithOpen: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const iconPickerButton = await canvas.findByRole('button');
const iconPickerButton = await canvas.findByRole('button', {
name: 'Click to select icon (no icon selected)',
});

userEvent.click(iconPickerButton);
await userEvent.click(iconPickerButton);
},
};

export const WithOpenAndSelectedIcon: Story = {
export const WithSelectedIcon: Story = {
args: { selectedIconKey: 'IconCalendarEvent' },
};

export const WithOpenAndSelectedIcon: Story = {
...WithSelectedIcon,
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const iconPickerButton = await canvas.findByRole('button');
const iconPickerButton = await canvas.findByRole('button', {
name: 'Click to select icon (selected: IconCalendarEvent)',
});

userEvent.click(iconPickerButton);
await userEvent.click(iconPickerButton);
},
};

export const WithSearch: Story = {
args: { selectedIconKey: 'IconBuildingSkyscraper' },
...WithSelectedIcon,
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const iconPickerButton = await canvas.findByRole('button');
const iconPickerButton = await canvas.findByRole('button', {
name: 'Click to select icon (selected: IconCalendarEvent)',
});

userEvent.click(iconPickerButton);
await userEvent.click(iconPickerButton);

const searchInput = await canvas.findByRole('textbox');

Expand All @@ -67,13 +91,15 @@ export const WithSearch: Story = {
};

export const WithSearchAndClose: Story = {
args: { selectedIconKey: 'IconBuildingSkyscraper' },
...WithSelectedIcon,
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const iconPickerButton = await canvas.findByRole('button');
let iconPickerButton = await canvas.findByRole('button', {
name: 'Click to select icon (selected: IconCalendarEvent)',
});

userEvent.click(iconPickerButton);
await userEvent.click(iconPickerButton);

let searchInput = await canvas.findByRole('textbox');

Expand All @@ -87,13 +113,19 @@ export const WithSearchAndClose: Story = {

expect(searchedIcon).toBeInTheDocument();

userEvent.click(searchedIcon);
await userEvent.click(searchedIcon);

await sleep(100);
await sleep(500);

expect(searchedIcon).not.toBeInTheDocument();

iconPickerButton = await canvas.findByRole('button', {
name: 'Click to select icon (selected: IconBuildingSkyscraper)',
});

userEvent.click(iconPickerButton);

await sleep(100);
await sleep(500);

searchInput = await canvas.findByRole('textbox');

Expand Down
9 changes: 9 additions & 0 deletions packages/twenty-ui/.storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ const config: StorybookConfig = {
name: '@storybook/react-vite',
options: {},
},
build: {
test: {
disableMDXEntries: true,
disabledAddons: [
'@storybook/addon-docs',
'@storybook/addon-essentials/docs',
],
},
},
};

export default config;
Expand Down
9 changes: 8 additions & 1 deletion packages/twenty-ui/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,21 @@
},
"test": {},
"typecheck": {},
"storybook:build": {},
"storybook:build": {
"configurations": {
"test": {}
}
},
"storybook:dev": {
"options": { "port": 6007 }
},
"storybook:static": {
"options": {
"buildTarget": "twenty-ui:storybook:build",
"port": 6007
},
"configurations": {
"test": {}
}
},
"storybook:coverage": {},
Expand Down

0 comments on commit 992602b

Please sign in to comment.