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

chore: Upgrade test runner #18794

Merged
merged 12 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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 .github/workflows/storybook-chromatic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ jobs:
HOME: /root
# Update snapshots for PRs on the main repo, verify on forks, which don't have access to PostHog Bot
VARIANT: ${{ github.event.pull_request.head.repo.full_name == github.repository && 'update' || 'verify' }}
STORYBOOK_SKIP_TAGS: 'test-skip,test-skip-${{ matrix.browser }}'
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
run: |
pnpm test:visual-regression:stories:ci:$VARIANT --browsers ${{ matrix.browser }} --shard ${{ matrix.shard }}/$SHARD_COUNT
Expand Down
11 changes: 0 additions & 11 deletions .storybook/decorators/withSnapshotsDisabled.tsx

This file was deleted.

2 changes: 0 additions & 2 deletions .storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { getStorybookAppContext } from './app-context'
import { withKea } from './decorators/withKea'
import { withMockDate } from './decorators/withMockDate'
import { defaultMocks } from '~/mocks/handlers'
import { withSnapshotsDisabled } from './decorators/withSnapshotsDisabled'
import { withFeatureFlags } from './decorators/withFeatureFlags'
import { withTheme } from './decorators/withTheme'

Expand Down Expand Up @@ -79,7 +78,6 @@ export const parameters: Parameters = {

// Setup storybook global decorators. See https://storybook.js.org/docs/react/writing-stories/decorators#global-decorators
export const decorators: Meta['decorators'] = [
withSnapshotsDisabled,
// Make sure the msw service worker is started, and reset the handlers to defaults.
withKea,
// Allow us to time travel to ensure our stories don't change over time.
Expand Down
20 changes: 8 additions & 12 deletions .storybook/test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ declare module '@storybook/types' {
options?: any
layout?: 'padded' | 'fullscreen' | 'centered'
testOptions?: {
/**
* Whether the test should be a no-op (doesn't jest.skip as @storybook/test-runner doesn't allow that).
* @default false
*/
skip?: boolean
/**
* Whether we should wait for all loading indicators to disappear before taking a snapshot.
* @default true
Expand Down Expand Up @@ -71,19 +66,20 @@ module.exports = {
jest.retryTimes(RETRY_TIMES, { logErrorsBeforeRetry: true })
jest.setTimeout(JEST_TIMEOUT_MS)
},
async postRender(page, context) {
async postVisit(page, context) {
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
const browserContext = page.context()
const storyContext = (await getStoryContext(page, context)) as StoryContext
const { skip = false, snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}
const { snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}

browserContext.setDefaultTimeout(PLAYWRIGHT_TIMEOUT_MS)
if (!skip) {
const currentBrowser = browserContext.browser()!.browserType().name() as SupportedBrowserName
if (snapshotBrowsers.includes(currentBrowser)) {
await expectStoryToMatchSnapshot(page, context, storyContext, currentBrowser)
}
const currentBrowser = browserContext.browser()!.browserType().name() as SupportedBrowserName
if (snapshotBrowsers.includes(currentBrowser)) {
await expectStoryToMatchSnapshot(page, context, storyContext, currentBrowser)
}
},
tags: {
skip: ['test-skip'], // NOTE: This is overridden by the CI action storybook-chromatic.yml to include browser specific skipping
},
} as TestRunnerConfig

async function expectStoryToMatchSnapshot(
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ActivityScope } from 'lib/components/ActivityLog/humanizeActivity'
const meta: Meta<typeof ActivityLog> = {
title: 'Components/ActivityLog',
component: ActivityLog,
parameters: { testOptions: { skip: true } }, // FIXME: Currently disabled as the Timeout story is flaky
tags: ['test-skip'], // FIXME: Currently disabled as the Timeout story is flaky
decorators: [
mswDecorator({
get: {
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/lib/components/Animation/Animation.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const meta: Meta<typeof Animation> = {
'Animations are [LottieFiles.com](https://lottiefiles.com/) animations that we load asynchronously.',
},
},
testOptions: { skip: true }, // Animations aren't particularly snapshotable
},
argTypes: {
size: {
Expand All @@ -25,7 +24,7 @@ const meta: Meta<typeof Animation> = {
control: { type: 'radio' },
},
},
tags: ['autodocs'],
tags: ['autodocs', 'test-skip'], // Animations aren't particularly snapshotable
}
export default meta

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import { HedgehogBuddy } from './HedgehogBuddy'
const meta: Meta<typeof HedgehogBuddy> = {
title: 'Components/Hedgehog Buddy',
component: HedgehogBuddy,
parameters: {
testOptions: { skip: true }, // Hedgehogs aren't particularly snapshotable
},
tags: ['test-skip'], // Hedgehogs aren't particularly snapshotable
}
export default meta

Expand Down
7 changes: 1 addition & 6 deletions frontend/src/lib/components/Map/Map.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,13 @@ const coordinates: [number, number] = [0.119167, 52.205276]
const meta: Meta<typeof Map> = {
title: 'Components/Map',
component: Map,
tags: ['autodocs'],
tags: ['autodocs', 'test-skip'],
// :TRICKY: We can't use markers in Storybook stories, as the Marker class is
// not JSON-serializable (circular structure).
args: {
center: coordinates,
className: 'h-60',
},
parameters: {
testOptions: {
skip: true,
},
},
}
type Story = StoryObj<typeof Map>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Meta } from '@storybook/react'
import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters'
import { AnyPropertyFilter, PropertyOperator } from '~/types'
import PropertyFiltersDisplay from 'lib/components/PropertyFilters/components/PropertyFiltersDisplay'
import { useStorybookMocks } from '~/mocks/browser'

const meta: Meta<typeof PropertyFilters> = {
title: 'Filters/PropertyFilters',
Expand Down Expand Up @@ -31,6 +32,11 @@ const propertyFilters = [
] as AnyPropertyFilter[]

export function ComparingPropertyFilters(): JSX.Element {
useStorybookMocks({
get: {
'/api/event/values/': [],
},
})
return (
<>
<h1>Pop-over enabled</h1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useMountedLogic } from 'kea'
import { PropertyGroupFilters } from './PropertyGroupFilters'
import { TaxonomicFilterGroupType } from '../TaxonomicFilter/types'
import { cohortsModel } from '~/models/cohortsModel'
import { useStorybookMocks } from '~/mocks/browser'

const meta: Meta<typeof PropertyGroupFilters> = {
title: 'Filters/PropertyGroupFilters',
Expand Down Expand Up @@ -36,6 +37,11 @@ const taxonomicGroupTypes = [
]

export function GroupPropertyFilters(): JSX.Element {
useStorybookMocks({
get: {
'/api/event/values/': [],
},
})
useMountedLogic(cohortsModel)

const [propertyGroupFilter, setPropertyGroupFilter] = useState<PropertyGroupFilter>({
Expand Down
5 changes: 1 addition & 4 deletions frontend/src/lib/components/PropertyIcon.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ type Story = StoryObj<typeof PropertyIcon>
const meta: Meta<typeof PropertyIcon> = {
title: 'Lemon UI/Icons/Property Icon',
component: PropertyIcon,
parameters: {
testOptions: { skip: true }, // There are too many icons, the snapshots are huge in table form
},
tags: ['autodocs'],
tags: ['autodocs', 'test-skip'], // There are too many icons, the snapshots are huge in table form
}
export default meta

Expand Down
3 changes: 1 addition & 2 deletions frontend/src/lib/components/hedgehogs.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ const allHedgehogs: HedgehogDefinition[] = Object.entries(hedgehogs).map(([key,

const meta: Meta = {
title: 'Lemon UI/Hog illustrations',
tags: ['test-skip', 'autodocs'], // Not valuable to take snapshots of these hedgehogs
parameters: {
testOptions: { skip: true }, // Not valuable to take snapshots of these hedgehogs
docs: {
description: {
component: `
Expand All @@ -37,7 +37,6 @@ she will get to it dependant on work load.
},
},
},
tags: ['autodocs'],
}
export default meta
export function Library(): JSX.Element {
Expand Down
7 changes: 1 addition & 6 deletions frontend/src/lib/lemon-ui/Popover/Popover.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ type Story = StoryObj<typeof Popover>
const meta: Meta<typeof Popover> = {
title: 'Lemon UI/Popover',
component: Popover,
parameters: {
testOptions: {
skip: true, // FIXME: This story needs a play test for the popup to show up in snapshots
},
},
tags: ['autodocs'],
tags: ['autodocs', 'test-skip'], // FIXME: This story needs a play test for the popup to show up in snapshots
}
export default meta

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/icons/icons.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ const LibraryTemplate: StoryFn<{ letter?: string | null }> = ({ letter }) => {

// This is for actual Storybook users
export const Library: LibraryType = LibraryTemplate.bind({})
Library.parameters = { testOptions: { skip: true } }
Library.tags = ['autodocs', 'test-skip']

// These are just for snapshots. As opposed to the full library, the stories below are segmented by the first letter
// of the icon name, which greatly optimizes both the UX and storage aspects of diffing snapshots.
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/lib/utils/eventUsageLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,10 @@ function sanitizeFilterParams(filters: AnyPartialFilterType): Record<string, any
const used_cohort_filter_ids = usedCohortFilterIds(filters.properties)

for (const entity of entities) {
properties_local = properties_local.concat(flattenProperties(entity.properties || []))
const entityProperties = Array.isArray(entity.properties) ? entity.properties : []
properties_local = properties_local.concat(flattenProperties(entityProperties))

using_groups = using_groups || hasGroupProperties(entity.properties || [])
using_groups = using_groups || hasGroupProperties(entityProperties)
if (entity.math_group_type_index != undefined) {
aggregating_by_groups = true
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataNode/DataNode.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ type Story = StoryObj<typeof Query>
const meta: Meta<typeof Query> = {
title: 'Queries/DataNode',
component: Query,
tags: ['test-skip'],
parameters: {
layout: 'fullscreen',
viewMode: 'story',
testOptions: { skip: true },
},
decorators: [
mswDecorator({
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataTable/DataTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ type Story = StoryObj<typeof Query>
const meta: Meta<typeof Query> = {
title: 'Queries/DataTable',
component: Query,
tags: ['test-skip'],
parameters: {
layout: 'fullscreen',
viewMode: 'story',
testOptions: { skip: true },
},
decorators: [
mswDecorator({
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/feedback/Feedback.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import { userInterviewSchedulerLogic } from './userInterviewSchedulerLogic'

const meta: Meta = {
title: 'Scenes-App/Feedback',
tags: ['test-skip'], // FIXME: Use mockdate in this story
parameters: {
layout: 'fullscreen',
testOptions: {
excludeNavigationFromSnapshot: true,
skip: true, // FIXME: Use mockdate in this story
},
viewMode: 'story',
// Might need to add a mockdate here, however when I do it breaks the page
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import { insightVizDataLogic } from '../insightVizDataLogic'
type Story = StoryObj<typeof App>
const meta: Meta = {
title: 'Scenes-App/Insights/Error states',
tags: ['test-skip'],
parameters: {
layout: 'fullscreen',
viewMode: 'story',
testOptions: { skip: true }, // FIXME
},
}
export default meta
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,7 @@ const meta: Meta<typeof InsightTooltip> = {
renderSeries: (value) => value,
groupTypeLabel: 'people',
},
parameters: {
testOptions: { skip: true }, // FIXME: The InWrapper story fails at locator.screenshot() for some reason
},
tags: ['test-skip'], // FIXME: The InWrapper story fails at locator.screenshot() for some reason
}
export default meta

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import recording_playlists from './__mocks__/recording_playlists.json'

const meta: Meta = {
title: 'Scenes-App/Recordings',
tags: ['test-skip'], // TODO: Fix the flakey rendering due to player playback
parameters: {
layout: 'fullscreen',
viewMode: 'story',
mockDate: '2023-02-01',
waitForSelector: '.PlayerFrame__content .replayer-wrapper iframe',
testOptions: { skip: true }, // TODO: Fix the flakey rendering due to player playback
},
decorators: [
mswDecorator({
Expand Down
6 changes: 1 addition & 5 deletions frontend/src/scenes/surveys/Surveys.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,7 @@ export const SurveyView: StoryFn = () => {
}, [])
return <App />
}
SurveyView.parameters = {
testOptions: {
skip: true, // FIXME: Fix the mocked data so that survey results can actually load
},
}
SurveyView.tags = ['test-skip'] // FIXME: Fix the mocked data so that survey results can actually load

export const SurveyTemplates: StoryFn = () => {
useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/toolbar/Toolbar.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ const toolbarParams: ToolbarParams = {

const meta: Meta = {
title: 'Scenes-Other/Toolbar',
tags: ['test-skip'], // This story is not valuable to snapshot as is
parameters: {
layout: 'fullscreen',
viewMode: 'story',
testOptions: { skip: true }, // This story is not valuable to snapshot as is
},
}
export default meta
Expand Down
4 changes: 2 additions & 2 deletions package.json
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
"@medv/finder": "^2.1.0",
"@microlink/react-json-view": "^1.21.3",
"@monaco-editor/react": "4.4.6",
"@posthog/icons": "0.4.10",
"@posthog/icons": "0.4.11",
"@posthog/plugin-scaffold": "^1.4.4",
"@react-hook/size": "^2.1.2",
"@rrweb/types": "^2.0.0-alpha.11",
Expand Down Expand Up @@ -197,7 +197,7 @@
"@storybook/csf": "^0.1.1",
"@storybook/react": "^7.5.1",
"@storybook/react-webpack5": "^7.5.1",
"@storybook/test-runner": "^0.13.0",
"@storybook/test-runner": "^0.15.2",
"@storybook/theming": "^7.5.1",
"@storybook/types": "^7.5.1",
"@sucrase/jest-plugin": "^3.0.0",
Expand Down
2 changes: 1 addition & 1 deletion playwright/pages/storybook.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, Locator, Page } from '@playwright/test'

const STORYBOOK_URL: string = process.env.STORYBOOK_URL || 'http://localhost:6006'
const STORYBOOK_URL: string = process.env.TARGET_URL || process.env.STORYBOOK_URL || 'http://localhost:6006'
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved

const PSEUDO_STATES = {
hover: 'hover',
Expand Down
Loading
Loading