Skip to content

Commit

Permalink
ci(vrt): Stop the snapshot madness (#17163)
Browse files Browse the repository at this point in the history
* Remove redundant Playwright specs

* Increase tolerance in Playwright

* Wait for profile pictures to resolve

* Ensure scene is loaded before proceeding

* Always wait for loaders to disappear

* Disable Gravatars in snapshots

* Don't wait for loaders in more stories with loaders

* Improve resiliency of batch export story

* Fix one more loading story

* Unskip Playwright tests

* Improve inconsistencies

* Upgrade Playwright from 1.29.2 to 1.37.1

* Wait for images to load

* Unify code editors

* Properly wait for images to load

* Go back to Playwright 1.29.2

* Wait for loader selector in LemonTable stories

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Twixes and github-actions[bot] authored Aug 29, 2023
1 parent 2660367 commit 59d4398
Show file tree
Hide file tree
Showing 76 changed files with 278 additions and 233 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ module.exports = {
element: 'Collapse',
message: 'use <LemonCollapse> instead',
},
{
element:'MonacoEditor',
message: 'use <CodeEditor> instead',
}
],
},
],
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/storybook-chromatic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
runs-on: buildjet-4vcpu-ubuntu-2204
timeout-minutes: 30
container:
image: mcr.microsoft.com/playwright:v1.29.2
image: mcr.microsoft.com/playwright:v1.29.2 # Same as @storybook/[email protected]'s dependency
strategy:
fail-fast: false
matrix:
Expand Down
39 changes: 22 additions & 17 deletions .storybook/test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ type StoryContext = ReturnType<typeof getStoryContext> extends Promise<infer T>
type SupportedBrowserName = 'chromium' | 'webkit'

// Extend Storybook interface `Parameters` with Chromatic parameters
declare module '@storybook/react' {
declare module '@storybook/csf' {
interface Parameters {
options?: any
layout?: 'padded' | 'fullscreen' | 'centered'
Expand All @@ -20,22 +20,21 @@ declare module '@storybook/react' {
skip?: boolean
/**
* Whether we should wait for all loading indicators to disappear before taking a snapshot.
*
* This is on by default for stories that have a layout of 'fullscreen', and off otherwise.
* Override that behavior by setting this to `true` or `false` manually.
*
* You can also provide a selector string instead of a boolean - in that case we'll wait
* for a matching element to be be visible once all loaders are gone.
* @default true
*/
waitForLoadersToDisappear?: boolean | string
waitForLoadersToDisappear?: boolean
/** If set, we'll wait for the given selector to be satisfied. */
waitForSelector?: string
/**
* Whether navigation (sidebar + topbar) should be excluded from the snapshot.
* Warning: Fails if enabled for stories in which navigation is not present.
* @default false
*/
excludeNavigationFromSnapshot?: boolean
/**
* The test will always run for all the browers, but snapshots are only taken in Chromium by default.
* Override this to take snapshots in other browsers too.
*
* @default ['chromium']
*/
snapshotBrowsers?: SupportedBrowserName[]
Expand All @@ -51,7 +50,7 @@ declare module '@storybook/react' {
}

const RETRY_TIMES = 5
const LOADER_SELECTORS = ['.ant-skeleton', '.Spinner', '.LemonSkeleton', '.LemonTableLoader']
const LOADER_SELECTORS = ['.ant-skeleton', '.Spinner', '.LemonSkeleton', '.LemonTableLoader', '[aria-busy="true"]']

const customSnapshotsDir = `${process.cwd()}/frontend/__snapshots__`

Expand All @@ -65,7 +64,7 @@ module.exports = {
const storyContext = await getStoryContext(page, context)
const { skip = false, snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}

browserContext.setDefaultTimeout(1000) // Reduce the default timeout from 30 s to 1 s to pre-empt Jest timeouts
browserContext.setDefaultTimeout(3000) // Reduce the default timeout from 30 s to 3 s to pre-empt Jest timeouts
if (!skip) {
const currentBrowser = browserContext.browser()!.browserType().name() as SupportedBrowserName
if (snapshotBrowsers.includes(currentBrowser)) {
Expand All @@ -81,9 +80,9 @@ async function expectStoryToMatchSnapshot(
storyContext: StoryContext,
browser: SupportedBrowserName
): Promise<void> {
// await page.setViewportSize(DEFAULT_PAGE_DIMENSIONS)
const {
waitForLoadersToDisappear = storyContext.parameters?.layout === 'fullscreen',
waitForLoadersToDisappear = true,
waitForSelector,
excludeNavigationFromSnapshot = false,
} = storyContext.parameters?.testOptions ?? {}

Expand All @@ -110,13 +109,19 @@ async function expectStoryToMatchSnapshot(
document.body.classList.add('storybook-test-runner')
})
if (waitForLoadersToDisappear) {
await page.waitForTimeout(300) // Wait for initial UI to load
await Promise.all(LOADER_SELECTORS.map((selector) => page.waitForSelector(selector, { state: 'detached' })))
if (typeof waitForLoadersToDisappear === 'string') {
await page.waitForSelector(waitForLoadersToDisappear)
}
}
await page.waitForTimeout(100) // Just a bit of extra delay for things to settle
if (waitForSelector) {
await page.waitForSelector(waitForSelector)
}

await page.waitForTimeout(400) // Wait for animations to finish

// Wait for all images to load
await page.waitForFunction(() =>
Array.from(document.querySelectorAll('img')).every((i: HTMLImageElement) => i.complete)
)

await check(page, context, browser, storyContext.parameters?.testOptions?.snapshotTargetSelector)
}

Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.playwright
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ COPY package.json pnpm-lock.yaml ./

ENV CYPRESS_INSTALL_BINARY=0

RUN pnpm install
RUN pnpm install --frozen-lockfile

COPY playwright.config.ts webpack.config.js babel.config.js tsconfig.json ./

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-divider--vertical.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-snack--complex-content.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-profile-bubbles--one-bubble.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-annotations--annotations.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-dashboards--create-template.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-notebooks--notebooks-list.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-other-billing-v2--billing-v-2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-other-invitesignup--logged-in.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 9 additions & 7 deletions frontend/src/layout/navigation-3000/Navigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,25 @@ import './Navigation.scss'
import { themeLogic } from './themeLogic'
import { navigation3000Logic } from './navigationLogic'
import clsx from 'clsx'
import { sceneLogic } from 'scenes/sceneLogic'
import { NotebookPopover } from 'scenes/notebooks/Notebook/NotebookPopover'
import { Scene, SceneConfig } from 'scenes/sceneTypes'

export function Navigation({ children }: { children: ReactNode }): JSX.Element {
export function Navigation({
children,
sceneConfig,
}: {
children: ReactNode
scene: Scene | null
sceneConfig: SceneConfig | null
}): JSX.Element {
useMountedLogic(themeLogic)
const { sceneConfig } = useValues(sceneLogic)
const { activeNavbarItem } = useValues(navigation3000Logic)

useEffect(() => {
// FIXME: Include debug notice in a non-obstructing way
document.getElementById('bottom-notice')?.remove()
}, [])

if (sceneConfig?.layout === 'plain') {
throw new Error('Navigation should never be rendered for a plain scene')
}

return (
<div className="Navigation3000">
<Navbar />
Expand Down
19 changes: 12 additions & 7 deletions frontend/src/layout/navigation/Navigation.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
import clsx from 'clsx'
import { useValues } from 'kea'
import { BillingAlertsV2 } from 'lib/components/BillingAlertsV2'
import { sceneLogic } from 'scenes/sceneLogic'
import { Scene } from 'scenes/sceneTypes'
import { Scene, SceneConfig } from 'scenes/sceneTypes'
import { Breadcrumbs } from './Breadcrumbs/Breadcrumbs'
import { ProjectNotice } from './ProjectNotice'
import { SideBar } from './SideBar/SideBar'
import { TopBar } from './TopBar/TopBar'
import { ReactNode } from 'react'

export function Navigation({ children }: { children: any }): JSX.Element {
const { sceneConfig, activeScene } = useValues(sceneLogic)

export function Navigation({
children,
scene,
sceneConfig,
}: {
children: ReactNode
scene: Scene | null
sceneConfig: SceneConfig | null
}): JSX.Element {
return (
<div className="h-screen flex flex-col">
{activeScene !== Scene.Ingestion && <TopBar />}
{scene !== Scene.Ingestion && <TopBar />}
<SideBar>
<div className={clsx('main-app-content', sceneConfig?.layout === 'plain' && 'main-app-content--plain')}>
{sceneConfig?.layout !== 'plain' && (
Expand Down
29 changes: 29 additions & 0 deletions frontend/src/lib/components/CodeEditors.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { Spinner } from 'lib/lemon-ui/Spinner'
import { inStorybookTestRunner } from 'lib/utils'
import MonacoEditor, { type EditorProps } from '@monaco-editor/react'
import { themeLogic } from '~/layout/navigation-3000/themeLogic'
import { useValues } from 'kea'

export type CodeEditorProps = Omit<EditorProps, 'loading' | 'theme'>

export function CodeEditor({ options, ...editorProps }: CodeEditorProps): JSX.Element {
const { isDarkModeOn } = useValues(themeLogic)

const scrollbarRendering = !inStorybookTestRunner() ? 'auto' : 'hidden'

return (
<MonacoEditor // eslint-disable-line react/forbid-elements
theme={isDarkModeOn ? 'vs-dark' : 'vs-light'}
loading={<Spinner />}
options={{
...options,
scrollbar: {
vertical: scrollbarRendering,
horizontal: scrollbarRendering,
...options?.scrollbar,
},
}}
{...editorProps}
/>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const meta: Meta<typeof TaxonomicFilter> = {
component: TaxonomicFilter,
decorators: [taxonomicFilterMocksDecorator],
parameters: {
testOptions: { waitForLoadersToDisappear: '.definition-popover' },
testOptions: { waitForSelector: '.definition-popover' },
},
}
export default meta
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,14 @@ export const DisabledWithReason = (): JSX.Element => {
}
// TODO: Add DisabledWithReason.play for a proper snapshot showcasing the tooltip

export const Loading = (): JSX.Element => {
export const Loading: Story = (): JSX.Element => {
return <TypesAndStatusesTemplate loading />
}
Loading.parameters = {
testOptions: {
waitForLoadersToDisappear: false,
},
}

export const LoadingViaOnClick = (): JSX.Element => {
const { loading, onEvent } = useAsyncHandler(async () => await delay(1000))
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/lib/lemon-ui/LemonRow/LemonRow.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ Loading.args = {
...Default.args,
loading: true,
}
Loading.parameters = {
testOptions: {
waitForLoadersToDisappear: false,
},
}

export const Small: Story = Template.bind({})
Small.args = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ Loading.args = {
options: [],
loading: true,
}
Loading.parameters = {
testOptions: {
waitForLoadersToDisappear: false,
},
}

export const NoOptions: Story = Template.bind({})
NoOptions.args = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const meta: Meta<typeof LemonSkeleton> = {
Skeleton screens are used to indicate that a screen is loading, are perceived as being shorter in duration when compared against a blank screen (our control) and a spinner — but not by much`,
},
},
testOptions: {
waitForLoadersToDisappear: false,
},
},
tags: ['autodocs'],
}
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/lib/lemon-ui/LemonTable/LemonTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,30 @@ BorderlessRows.args = { borderedRows: false }

export const Loading: Story = BasicTemplate.bind({})
Loading.args = { loading: true }
Loading.parameters = {
testOptions: {
waitForLoadersToDisappear: false,
waitForSelector: '.LemonTableLoader',
},
}

export const EmptyLoading: Story = EmptyTemplate.bind({})
EmptyLoading.args = { loading: true }
EmptyLoading.parameters = {
testOptions: {
waitForLoadersToDisappear: false,
waitForSelector: '.LemonTableLoader',
},
}

export const EmptyLoadingWithManySkeletonRows: Story = EmptyTemplate.bind({})
EmptyLoadingWithManySkeletonRows.args = { loading: true, loadingSkeletonRows: 10 }
EmptyLoadingWithManySkeletonRows.parameters = {
testOptions: {
waitForLoadersToDisappear: false,
waitForSelector: '.LemonTableLoader',
},
}

export const WithoutHeader: Story = BasicTemplate.bind({})
WithoutHeader.args = { showHeader: false }
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lib/lemon-ui/LemonTable/LemonTableLoader.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
z-index: 10;
position: absolute;
left: 0;
padding: 0;
bottom: -1px;
width: 100%;
height: 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ const DUMMIES: ProfileBubblesProps['people'] = [
const meta: Meta<typeof ProfileBubblesComponent> = {
title: 'Lemon UI/Profile Bubbles',
component: ProfileBubblesComponent,
parameters: {
testOptions: {
waitForLoadersToDisappear: true,
},
},
args: {
people: DUMMIES,
},
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/lib/lemon-ui/ProfilePicture/ProfilePicture.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { userLogic } from 'scenes/userLogic'
import { IconRobot } from '../icons'
import { Lettermark, LettermarkColor } from '../Lettermark/Lettermark'
import './ProfilePicture.scss'
import { inStorybookTestRunner } from 'lib/utils'

export interface ProfilePictureProps {
name?: string
Expand Down Expand Up @@ -39,12 +40,15 @@ export function ProfilePicture({
const combinedNameAndEmail = name && email ? `${name} <${email}>` : name || email

useEffect(() => {
if (inStorybookTestRunner()) {
return // There are no guarantees on how long it takes to fetch a Gravatar, so we skip this in snapshots
}
// Check if Gravatar exists
const emailOrNameWithEmail = email || (name?.includes('@') ? name : undefined)
if (emailOrNameWithEmail) {
const emailHash = md5(emailOrNameWithEmail.trim().toLowerCase())
const tentativeUrl = `https://www.gravatar.com/avatar/${emailHash}?s=96&d=404`
// The image will be cached, so it's better to do a full GET request in this check
// The image will be cached, so it's best to do GET request check before trying to render it
fetch(tentativeUrl).then((response) => {
if (response.status === 200) {
setGravatarUrl(tentativeUrl)
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/lib/lemon-ui/Spinner/Spinner.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
}

.SpinnerOverlay {
transition: opacity 0.2s ease;
position: absolute;
top: 0;
bottom: 0;
Expand All @@ -55,6 +56,10 @@
display: flex;
align-items: center;
justify-content: center;
&[aria-hidden='true'] {
opacity: 0;
pointer-events: none;
}
&::before {
content: '';
position: absolute;
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/lib/lemon-ui/Spinner/Spinner.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import { LemonButton } from '@posthog/lemon-ui'
const meta: Meta<typeof Spinner> = {
title: 'Lemon UI/Spinner',
component: Spinner,
parameters: {
testOptions: {
waitForLoadersToDisappear: false,
},
},
tags: ['autodocs'],
}
export default meta
Expand Down
Loading

0 comments on commit 59d4398

Please sign in to comment.