diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index 0a92c6d1..ab408788 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -8,10 +8,11 @@ import { styled, useTheme, } from "@storybook/theming"; -import type { Decorator, Preview } from "@storybook/react"; +import type { Decorator, Loader, Preview } from "@storybook/react"; import { initialize, mswLoader } from "msw-storybook-addon"; import React from "react"; import { baseModes } from "../src/modes"; +import { graphql } from "msw"; // Initialize MSW initialize({ @@ -122,9 +123,53 @@ const withManagerApi: Decorator = (Story, { argsByTarget }) => ( ); +/** + * An experiment with targeted args for GraphQL. This loader will serve a graphql + * response for any arg nested under $graphql. + * We serve the arg value for the query by the name of arg name, e.g. + * + * { + * args: { + * $graphql: { + * AddonVisualTestsBuild: { project: { name: 'acme', ... } }, + * }, + * }, + * } + * + * Additionally, if you want to map the arg (optionally based on variables), + * you can set `argTypes.$graphql.X.map`, + * + * eg. + * + * { + * argTypes: { + * $graphql: { + * AddonVisualTestsBuild: { + * map: ({ lastBuildOnBranch }, { selectedBuildId }) => + * ({ project: { name: 'acme', ... } }), + * }, + * }, + * }, + * } + */ +export const graphQLArgLoader: Loader = async ({ argTypes, argsByTarget, parameters }) => { + const handlers = Object.entries(argsByTarget.graphql?.$graphql || []).map( + ([argName, inputResult]: [string, any]) => + graphql.query(argName, (req, res, ctx) => { + const result = argTypes.$graphql[argName]?.map?.(inputResult, req.variables) ?? inputResult; + + return res(ctx.data(result)); + }) + ); + + return mswLoader({ + parameters: { msw: { handlers: [...handlers, ...(parameters.msw?.handlers || [])] } }, + }); +}; + const preview: Preview = { decorators: [withTheme, withManagerApi], - loaders: [mswLoader], + loaders: [graphQLArgLoader], parameters: { actions: { argTypesRegex: "^on[A-Z].*", @@ -148,6 +193,9 @@ const preview: Preview = { }, layout: "fullscreen", }, + argTypes: { + $graphql: { target: "graphql" }, + }, globalTypes: { theme: { name: "Theme", diff --git a/src/screens/VisualTests/VisualTests.stories.tsx b/src/screens/VisualTests/VisualTests.stories.tsx index 6721ea39..11469d68 100644 --- a/src/screens/VisualTests/VisualTests.stories.tsx +++ b/src/screens/VisualTests/VisualTests.stories.tsx @@ -1,9 +1,9 @@ -import type { ResultOf } from "@graphql-typed-document-node/core"; +/* eslint-disable @typescript-eslint/no-non-null-assertion */ +// eslint-disable-next-line import/no-unresolved +import { VariablesOf } from "@graphql-typed-document-node/core"; import { action } from "@storybook/addon-actions"; import { expect } from "@storybook/jest"; -import type { API, State } from "@storybook/manager-api"; -import { ManagerContext } from "@storybook/manager-api"; -import type { Decorator, Meta, StoryObj } from "@storybook/react"; +import type { Meta, StoryObj } from "@storybook/react"; import { findByRole, findByTestId, @@ -13,20 +13,20 @@ import { waitFor, within, } from "@storybook/testing-library"; -import { getOperationAST } from "graphql"; -import { graphql } from "msw"; import React from "react"; -import { TypedDocumentNode } from "urql"; import { INITIAL_BUILD_PAYLOAD } from "../../buildSteps"; import type { LastBuildOnBranchBuildFieldsFragment, + MakeOptional, SelectedBuildFieldsFragment, - StoryTestFieldsFragment, } from "../../gql/graphql"; import { Browser, ComparisonResult, TestResult, TestStatus } from "../../gql/graphql"; import { panelModes } from "../../modes"; -import { SelectedBuildWithTests } from "../../types"; +import { + withGraphQLMutationParameters, + withGraphQLQueryParameters, +} from "../../utils/gqlStoryHelpers"; import { storyWrapper } from "../../utils/graphQLClient"; import { playAll } from "../../utils/playAll"; import { makeComparison, makeTest, makeTests } from "../../utils/storyData"; @@ -48,56 +48,42 @@ import { pendingTestsNewMode, pendingTestsNewStory, startedBuild, + withTests, } from "./mocks"; -import { VisualTests } from "./VisualTests"; +import { VisualTests, VisualTestsWithoutSelectedBuildId } from "./VisualTests"; const browsers = [Browser.Chrome, Browser.Safari]; -const withManagerApi: Decorator = (Story, { argsByTarget }) => ( - - - -); - -const withGraphQLQuery = (...args: Parameters) => ({ - msw: { - handlers: [graphql.query(...args)], - }, -}); - -function withGraphQLQueryResult>( - query: TQuery, - result: ResultOf -) { - const queryName = getOperationAST(query)?.name?.value; - if (queryName) return withGraphQLQuery(queryName, (req, res, ctx) => res(ctx.data(result))); - throw new Error(`Couldn't determine query name from query`); -} +// A build that satisfies both the result of each part of the query. We just use +// builds that satsify both to make our lives easier +type LastOrSelectedBuildFragment = SelectedBuildFieldsFragment & + LastBuildOnBranchBuildFieldsFragment; -const withGraphQLMutation = (...args: Parameters) => ({ - msw: { - handlers: [graphql.mutation(...args)], - }, -}); +type QueryInput = { + /** If `lastBuildOnBranch` is unset, there will be no last build on the branch */ + lastBuildOnBranch?: LastOrSelectedBuildFragment; + + /** If `selectedBuild` is unset, `lastBuildOnBranch` will be used *if* it matches `selectedBuildId` */ + selectedBuild?: LastOrSelectedBuildFragment; -const withBuilds = ({ - lastBuildOnBranch, - selectedBuild, - userCanReview = true, -}: { - selectedBuild?: SelectedBuildFieldsFragment; - lastBuildOnBranch?: LastBuildOnBranchBuildFieldsFragment; userCanReview?: boolean; -}) => { - return withGraphQLQueryResult(QueryBuild, { +}; +function mapQuery( + { lastBuildOnBranch, selectedBuild: selectedBuildInput, userCanReview = true }: QueryInput, + { selectedBuildId }: VariablesOf +) { + if (selectedBuildInput && selectedBuildInput?.id !== selectedBuildId) { + throw new Error("Invalid story, selectedBuild does not match selectedBuildId"); + } + + const selectedBuild = + selectedBuildInput ?? + (lastBuildOnBranch?.id === selectedBuildId ? lastBuildOnBranch : undefined); + + return { project: { name: "acme", - lastBuildOnBranch: lastBuildOnBranch || selectedBuild, + lastBuildOnBranch, }, selectedBuild, viewer: { @@ -105,32 +91,34 @@ const withBuilds = ({ userCanReview, }, }, - }); -}; + }; +} -const withTests = ( - build: T, - fullTests: StoryTestFieldsFragment[] -) => ({ - ...build, - testsForStatus: { nodes: fullTests }, - testsForStory: { nodes: fullTests }, -}); +// We don't have jest.mock() so we have to make something similar for typing +function mock any>(f: T) { + return f as unknown as T & { + mock: { calls: Parameters[] }; + }; +} +type StoryArgs = Parameters[0] & { + addNotification: () => void; + $graphql?: { AddonVisualTestsBuild?: QueryInput }; +}; const meta = { title: "screens/VisualTests/VisualTests", - component: VisualTests, - decorators: [storyWrapper, withManagerApi], - parameters: { - ...withBuilds({ selectedBuild: passedBuild }), - chromatic: { - modes: panelModes, - }, - }, + component: VisualTestsWithoutSelectedBuildId, + decorators: [storyWrapper], + parameters: { chromatic: { modes: panelModes } }, argTypes: { addNotification: { type: "function", target: "manager-api" }, + $graphql: { + AddonVisualTestsBuild: { map: mapQuery }, + }, }, args: { + setSelectedBuildInfo: action("setSelectedBuildInfo"), + dismissBuildError: action("dismissBuildError"), gitInfo: { userEmailHash: "xyz987", branch: "feature-branch", @@ -146,16 +134,18 @@ const meta = { setOutdated: action("setOutdated"), updateBuildStatus: action("updateBuildStatus") as any, addNotification: action("addNotification"), + $graphql: { AddonVisualTestsBuild: {} }, }, -} satisfies Meta[0] & { addNotification: () => void }>; +} satisfies Meta; export default meta; -type Story = StoryObj; +type Story = StoryObj>; export const Loading = { + args: { $graphql: {} }, parameters: { - ...withGraphQLQuery("AddonVisualTestsBuild", (req, res, ctx) => - res(ctx.status(200), ctx.data({}), ctx.delay("infinite")) + ...withGraphQLQueryParameters("AddonVisualTestsBuild", (req, res, ctx) => + res(ctx.status(200), ctx.data({} as any), ctx.delay("infinite")) ), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-304933&t=0rxMQnkxsVpVj1qy-4" @@ -163,34 +153,19 @@ export const Loading = { }, } satisfies Story; -const pendingBuildNewStory = withTests( - { ...pendingBuild }, - pendingTests.map((test) => ({ - ...test, - result: TestResult.Added, - comparisons: test.comparisons.map((comparison) => ({ - ...comparison, - result: ComparisonResult.Added, - baseCapture: null, - })), - })) -); - -const newStoryNoTests = withTests({ ...pendingBuild }, []); - export const GraphQLError = { + args: { $graphql: {} }, parameters: { - ...withGraphQLQuery("AddonVisualTestsBuild", (req, res, ctx) => + ...withGraphQLQueryParameters("AddonVisualTestsBuild", (req, res, ctx) => res(ctx.status(200), ctx.errors([{ message: "Something went wrong on the server" }])) ), }, } satisfies Story; export const EmptyBranch = { - parameters: { - ...withBuilds({ selectedBuild: undefined }), - }, - render: ({ ...args }) => { + // @ts-expect-error Type conflict due to us explicitly defining `StoryArgs` above, + // as it cannot be auto-inferred from meta + render: (args: typeof meta.args) => { // custom render for mapping `updateBuildStatus` to a function which is mocked, but returns data instead of a function return ( { + const graphqlArgs = argsByTarget.graphql?.$graphql as typeof args.$graphql; // We need to type argsByTarget + await waitFor(() => { + const lastUpdater = mock(args.setSelectedBuildInfo!).mock.calls.at(-1)?.[0]; + + const result = + typeof lastUpdater === "function" ? lastUpdater(args.selectedBuildInfo) : lastUpdater; + + expect(result).toEqual({ + buildId: graphqlArgs?.AddonVisualTestsBuild?.lastBuildOnBranch?.id, + storyId: meta.args.storyId, + }); + }); + }, +} satisfies Story; + +/** Complete builds should always be switched to */ +export const EmptyBranchCIBuildPending = { + args: { + $graphql: EmptyBranchLocalBuildCapturedCurrentStory.args.$graphql, + }, + play: EmptyBranchLocalBuildCapturedCurrentStory.play, +} satisfies Story; -export const StoryAddedNotInBuildStarting: Story = { +// There is a selected build, but this story is new (not on the build at all) +export const StoryAddedNotInBuild = { + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests({ ...pendingBuild }, []), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: newStoryNoTests, - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=1898-562751&mode=design&t=ciag0nGKx2OGmoSR-4" ), }, +} satisfies Story; + +export const StoryAddedNotInBuildStarting = { args: { + ...StoryAddedNotInBuild.args, localBuildProgress: { buildProgressPercentage: 1, currentStep: "initialize", @@ -313,16 +332,22 @@ export const StoryAddedNotInBuildStarting: Story = { }, }, }, -}; - -export const StoryAddedNotInBuildCompletedLocalProgressIsOnSelectedBuild: Story = { parameters: { - ...withBuilds({ - selectedBuild: withTests({ ...pendingBuild, id: "Build:shared-id" }, []), - lastBuildOnBranch: withTests(pendingBuild, pendingTestsNewStory), - }), + ...withFigmaDesign( + "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=1898-562751&mode=design&t=ciag0nGKx2OGmoSR-4" + ), }, +} satisfies Story; + +export const StoryAddedNotInBuildCompletedLocalProgressIsOnSelectedBuild = { args: { + selectedBuildInfo: { buildId: "Build:shared-id", storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, pendingTestsNewStory), + selectedBuild: withTests({ ...pendingBuild, id: "Build:shared-id" }, []), + }, + }, localBuildProgress: { ...INITIAL_BUILD_PAYLOAD, buildId: "shared-id", @@ -330,48 +355,66 @@ export const StoryAddedNotInBuildCompletedLocalProgressIsOnSelectedBuild: Story currentStep: "complete", }, }, -}; +} satisfies Story; -export const StoryAddedInSelectedBuild: Story = { +export const StoryAddedInSelectedBuild = { + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, pendingTestsNewStory), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTestsNewStory), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=1898-562751&mode=design&t=ciag0nGKx2OGmoSR-4" ), }, -}; +} satisfies Story; -// This case isn't handled -export const StoryAddedInLastBuildOnBranchNotInSelected: Story = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, []), - lastBuildOnBranch: withTests(pendingBuild, pendingTestsNewStory), - }), +/** + * Although this state doesn't immediately render the captured story (it probably should), + * it should switch to the lastBuildOnBranch immediately. + */ +export const StoryAddedInLastBuildOnBranchNotInSelected = { + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests({ ...pendingBuild, id: "2" }, pendingTestsNewStory), + selectedBuild: withTests(pendingBuild, []), + }, + }, }, -}; + play: EmptyBranchLocalBuildCapturedCurrentStory.play, +} satisfies Story; export const StoryAddedAndAccepted = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, [ - makeTest({ - result: TestResult.Added, - status: TestStatus.Accepted, - comparisonResults: [ComparisonResult.Added], - }), - ]), - }), + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, [ + makeTest({ + result: TestResult.Added, + status: TestStatus.Accepted, + comparisonResults: [ComparisonResult.Added], + }), + ]), + }, + }, }, -}; +} satisfies Story; -export const ModeAddedInSelectedBuild: Story = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTestsNewMode), - }), +export const ModeAddedInSelectedBuild = { + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, pendingTestsNewMode), + }, + }, }, play: playAll(async ({ canvasElement, canvasIndex }) => { const canvas = within(canvasElement); @@ -380,30 +423,36 @@ export const ModeAddedInSelectedBuild: Story = { const items = await screen.findAllByText("1200px"); await userEvent.click(items[canvasIndex]); }), -}; +} satisfies Story; export const ModeAddedAndAccepted = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, [ - makeTest({ - result: TestResult.Added, - status: TestStatus.Accepted, - comparisonResults: [ComparisonResult.Added], - }), - makeTest({ - result: TestResult.Equal, - }), - ]), - }), + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, [ + makeTest({ + result: TestResult.Added, + status: TestStatus.Accepted, + comparisonResults: [ComparisonResult.Added], + }), + makeTest({ + result: TestResult.Equal, + }), + ]), + }, + }, }, -}; +} satisfies Story; -export const BrowserAddedInSelectedBuild: Story = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTestsNewBrowser), - }), +export const BrowserAddedInSelectedBuild = { + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, pendingTestsNewBrowser), + }, + }, }, play: playAll(async ({ canvasElement, canvasIndex }) => { const canvas = within(canvasElement); @@ -412,71 +461,45 @@ export const BrowserAddedInSelectedBuild: Story = { const items = await screen.findAllByText("Safari"); await userEvent.click(items[canvasIndex]); }), -}; +} satisfies Story; export const BrowserAddedAndAccepted: Story = { - parameters: { - ...withBuilds({ - selectedBuild: withTests( - pendingBuild, - makeTests({ - browsers: [Browser.Chrome, Browser.Safari], - viewports: [ - { - viewport: 480, - result: TestResult.Changed, - status: TestStatus.Accepted, - comparisons: [ - makeComparison({ result: ComparisonResult.Added, browser: Browser.Chrome }), - makeComparison({ result: ComparisonResult.Equal, browser: Browser.Safari }), - ], - }, - ], - }) - ), - }), + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests( + pendingBuild, + makeTests({ + browsers: [Browser.Chrome, Browser.Safari], + viewports: [ + { + viewport: 480, + result: TestResult.Changed, + status: TestStatus.Accepted, + comparisons: [ + makeComparison({ result: ComparisonResult.Added, browser: Browser.Chrome }), + makeComparison({ result: ComparisonResult.Equal, browser: Browser.Safari }), + ], + }, + ], + }) + ), + }, + }, }, }; -/** At this point, we should switch to the next build */ -export const EmptyBranchLocalBuildCapturedCurrentStory = { - parameters: { - ...withBuilds({ - selectedBuild: undefined, - lastBuildOnBranch: withTests(pendingBuild, pendingTests), - }), - }, +export const NoChanges = { args: { - localBuildProgress: { - ...EmptyBranchLocalBuildCapturing.args.localBuildProgress, - buildProgressPercentage: 90, - stepProgress: { - ...EmptyBranchLocalBuildCapturing.args.localBuildProgress.stepProgress, - snapshot: { - ...EmptyBranchLocalBuildCapturing.args.localBuildProgress.stepProgress.snapshot, - numerator: 310, - }, + selectedBuildInfo: { buildId: passedBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(passedBuild, passedTests), }, }, }, -} satisfies Story; - -/** Complete builds should always be switched to */ -export const EmptyBranchCIBuildPending = { - parameters: { - ...withBuilds({ - selectedBuild: undefined, - lastBuildOnBranch: withTests(pendingBuild, pendingTests), - }), - }, - // In theory we might have a complete running build here, it should behave the same either way -} satisfies Story; - -export const NoChanges = { parameters: { - ...withBuilds({ - selectedBuild: withTests(passedBuild, passedTests), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-304933&t=0rxMQnkxsVpVj1qy-4" ), @@ -484,17 +507,22 @@ export const NoChanges = { } satisfies Story; /** We just switched branches so the selected build is out of date */ -export const NoChangesOnWrongBranch: Story = { +export const NoChangesOnWrongBranch = { args: { + selectedBuildInfo: { buildId: passedBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + selectedBuild: withTests(passedBuild, passedTests), + }, + }, gitInfo: { ...meta.args.gitInfo, branch: "new-branch" }, }, parameters: { - ...withBuilds({ selectedBuild: passedBuild, lastBuildOnBranch: undefined }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-304933&t=0rxMQnkxsVpVj1qy-4" ), }, -}; +} satisfies Story; /** * We've started a new build but it's not done yet @@ -502,11 +530,12 @@ export const NoChangesOnWrongBranch: Story = { export const PendingLocalBuildStarting = { args: { ...EmptyBranchStartedLocalBuild.args, - }, - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - }), + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, pendingTests), + }, + }, }, } satisfies Story; @@ -514,14 +543,15 @@ export const PendingLocalBuildStarting = { * As above but we started the next build */ export const PendingLocalBuildCapturing = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - lastBuildOnBranch: withTests({ ...startedBuild, id: "2" }, inProgressTests), - }), - }, args: { ...EmptyBranchLocalBuildCapturing.args, + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests({ ...startedBuild, id: "2" }, inProgressTests), + selectedBuild: withTests(pendingBuild, pendingTests), + }, + }, }, } satisfies Story; @@ -529,47 +559,92 @@ export const PendingLocalBuildCapturing = { * The next build is snapshotting and has captured this story */ export const PendingLocalBuildCapturedStory = { - ...PendingLocalBuildCapturing, + args: { + ...EmptyBranchLocalBuildCapturing.args, + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests({ ...startedBuild, id: "2" }, pendingTests), + selectedBuild: withTests(pendingBuild, pendingTests), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - lastBuildOnBranch: withTests({ ...startedBuild, id: "2" }, pendingTests), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=2303-374529&t=qjmuGHxoALrVuhvX-0" ), }, + // NOTE: this does not current work, the story is auto-selected + // play: async ({ canvasElement, args, argsByTarget }) => { + // // We have to wait a moment for the story to render + // const canvas = within(canvasElement); + // await canvas.findAllByText("Latest"); + + // // Ensure we don't switch to the new build, the user has to opt-in + // mock(args.setSelectedBuildInfo!).mock.calls.forEach(([updater]) => { + // const result = typeof updater === "function" ? updater(args.selectedBuildInfo) : updater; + // expect(result).toEqual(args.selectedBuildInfo); // Unchanged + // }); + + // // Now opt in + // const viewLatestSnapshot = (await canvas.findAllByText("View latest snapshot"))[0]; + // await userEvent.click(viewLatestSnapshot); + + // const graphqlArgs = argsByTarget.graphql?.$graphql as typeof args.$graphql; // We need to type argsByTarget + // await waitFor(() => { + // expect(args.setSelectedBuildInfo).toHaveBeenCalledWith({ + // buildId: graphqlArgs?.AddonVisualTestsBuild?.lastBuildOnBranch?.id, + // storyId: meta.args.storyId, + // }); + // }); + // }, } satisfies Story; /** * The next build is snapshotting but hasn't yet reached this story (we didn't start it) */ export const PendingCIBuildInProgress = { - parameters: PendingLocalBuildCapturing.parameters, + args: { + selectedBuildInfo: PendingLocalBuildCapturing.args.selectedBuildInfo, + $graphql: PendingLocalBuildCapturing.args.$graphql, + }, } satisfies Story; /** * The next build is snapshotting and has captured this story */ export const PendingCIBuildCapturedStory = { + args: { + selectedBuildInfo: PendingLocalBuildCapturedStory.args.selectedBuildInfo, + $graphql: PendingLocalBuildCapturedStory.args.$graphql, + }, parameters: { - ...PendingLocalBuildCapturedStory.parameters, ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=2303-374529&t=qjmuGHxoALrVuhvX-0" ), }, + // NOTE: this does not current work, the story is auto-selected + // Should behave the same as when the local build captures the current story + // play: PendingLocalBuildCapturedStory.play, } satisfies Story; export const Pending = { + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, pendingTests), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-304718&t=0rxMQnkxsVpVj1qy-4" ), }, - render: ({ ...args }) => { + // @ts-expect-error Type conflict due to us explicitly defining `StoryArgs` above, + // as it cannot be auto-inferred from meta + render: ({ ...args }: typeof meta.args) => { // custom render for mapping `updateBuildStatus` to a function which is mocked, but returns data instead of a function return ( - res(ctx.status(200), ctx.data({}), ctx.delay("infinite")) - ).msw.handlers, - ], - }, + ...withGraphQLMutationParameters("ReviewTest", (req, res, ctx) => + res(ctx.status(200), ctx.data({}), ctx.delay("infinite")) + ), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-304718&t=0rxMQnkxsVpVj1qy-4" ), @@ -665,17 +747,11 @@ export const Accepting = { } satisfies Story; export const AcceptingFailed = { + args: { ...Accepting.args }, parameters: { - msw: { - handlers: [ - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - }).msw.handlers, - ...withGraphQLMutation("ReviewTest", (req, res, ctx) => - res(ctx.status(200), ctx.errors([{ message: "Accepting failed" }])) - ).msw.handlers, - ], - }, + ...withGraphQLMutationParameters("ReviewTest", (req, res, ctx) => + res(ctx.status(200), ctx.errors([{ message: "Accepting failed" }])) + ), }, play: playAll(async ({ canvasElement, argsByTarget }) => { const button = await findByRole(canvasElement, "button", { name: "Accept" }); @@ -687,10 +763,15 @@ export const AcceptingFailed = { } satisfies Story; export const Accepted = { + args: { + selectedBuildInfo: { buildId: acceptedBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(acceptedBuild, acceptedTests), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: withTests(acceptedBuild, acceptedTests), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-305053&t=0rxMQnkxsVpVj1qy-4" ), @@ -700,20 +781,23 @@ export const Accepted = { export const Skipped = { args: { storyId: "button--tertiary", + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(pendingBuild, [ + makeTest({ + id: "31", + status: TestStatus.Passed, + result: TestResult.Skipped, + browsers, + viewport: 1200, + storyId: "button--tertiary", + }), + ]), + }, + }, }, parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, [ - makeTest({ - id: "31", - status: TestStatus.Passed, - result: TestResult.Skipped, - browsers, - viewport: 1200, - storyId: "button--tertiary", - }), - ]), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=2255-42087&t=a8NRPgQk3kXMyxqZ-0" ), @@ -721,10 +805,15 @@ export const Skipped = { } satisfies Story; export const CaptureError = { + args: { + selectedBuildInfo: { buildId: brokenBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(brokenBuild, brokenTests), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: withTests(brokenBuild, brokenTests), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-305053&t=0rxMQnkxsVpVj1qy-4" ), @@ -732,10 +821,15 @@ export const CaptureError = { } satisfies Story; export const InteractionFailure = { + args: { + selectedBuildInfo: { buildId: brokenBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: withTests(brokenBuild, interactionFailureTests), + }, + }, + }, parameters: { - ...withBuilds({ - selectedBuild: withTests(brokenBuild, interactionFailureTests), - }), ...withFigmaDesign( "https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=508-305053&t=0rxMQnkxsVpVj1qy-4" ), @@ -743,38 +837,57 @@ export const InteractionFailure = { } satisfies Story; export const InfrastructureError = { - parameters: { - ...withBuilds({ - selectedBuild: failedBuild, - }), + args: { + selectedBuildInfo: { buildId: failedBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + lastBuildOnBranch: failedBuild, + }, + }, }, } satisfies Story; /** The new build is newer than the story build (but we didn't run it) */ export const CIBuildNewer = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - lastBuildOnBranch: { - ...withTests(pendingBuild, pendingTests), - id: "2", - committedAt: meta.args.gitInfo.committedAt, + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + selectedBuild: withTests(pendingBuild, pendingTests), + lastBuildOnBranch: { + ...withTests(pendingBuild, pendingTests), + id: "2", + committedAt: meta.args.gitInfo.committedAt, + }, }, - }), + }, }, + // Similarly to a captured story, we should only switch when the user is ready + // NOTE: this does not current work, the story is auto-selected + // play: PendingLocalBuildCapturedStory.play, } satisfies Story; /** The new build is newer than the story build and the git info */ export const CIBuildNewerThanCommit = { - parameters: { - ...withBuilds({ - selectedBuild: withTests(pendingBuild, pendingTests), - lastBuildOnBranch: { - ...withTests(pendingBuild, pendingTests), - id: "2", - committedAt: meta.args.gitInfo.committedAt + 1, - }, - }), + args: { + selectedBuildInfo: { buildId: pendingBuild.id, storyId: meta.args.storyId }, + $graphql: { + AddonVisualTestsBuild: { + selectedBuild: withTests(pendingBuild, pendingTests), + lastBuildOnBranch: { + ...withTests(pendingBuild, pendingTests), + id: "2", + committedAt: meta.args.gitInfo.committedAt + 1, + }, + }, + }, + }, + play: async ({ args }) => { + // We should not switch + mock(args.setSelectedBuildInfo!).mock.calls.forEach(([updater]) => { + const result = typeof updater === "function" ? updater(args.selectedBuildInfo) : updater; + expect(result).toEqual(args.selectedBuildInfo); // Unchanged + }); }, } satisfies Story; diff --git a/src/screens/VisualTests/VisualTests.tsx b/src/screens/VisualTests/VisualTests.tsx index 5164015b..bf13c13a 100644 --- a/src/screens/VisualTests/VisualTests.tsx +++ b/src/screens/VisualTests/VisualTests.tsx @@ -29,6 +29,8 @@ const createEmptyStoryStatusUpdate = (state: API_StatusState) => { }; interface VisualTestsProps { + selectedBuildInfo?: SelectedBuildInfo; + setSelectedBuildInfo: ReturnType>[1]; dismissBuildError: () => void; localBuildProgress?: LocalBuildProgress; startDevBuild: () => void; @@ -43,7 +45,9 @@ interface VisualTestsProps { storyId: string; } -export const VisualTests = ({ +export const VisualTestsWithoutSelectedBuildId = ({ + selectedBuildInfo, + setSelectedBuildInfo, dismissBuildError, localBuildProgress, startDevBuild, @@ -56,10 +60,6 @@ export const VisualTests = ({ }: VisualTestsProps) => { const { addNotification } = useStorybookApi(); - // The storyId and buildId that drive the test(s) we are currently looking at - // The user can choose when to change story (via sidebar) and build (via opting into new builds) - const [selectedBuildInfo, setSelectedBuildInfo] = useState({ storyId }); - const [{ data, error: queryError, operation }, rerunQuery] = useQuery({ query: QueryBuild, variables: { @@ -70,16 +70,13 @@ export const VisualTests = ({ ...(gitInfo.slug ? { slug: gitInfo.slug } : {}), gitUserEmailHash: gitInfo.userEmailHash, selectedBuildId: selectedBuildInfo?.buildId || "", - hasSelectedBuildId: !!selectedBuildInfo?.buildId, + hasSelectedBuildId: !!selectedBuildInfo, }, }); // When you change story, for a period the query will return the previous set of data, and indicate // that with the operation being for the previous query. - const storyDataIsStale = - operation && - selectedBuildInfo?.storyId && - operation.variables.storyId !== selectedBuildInfo.storyId; + const storyDataIsStale = operation && storyId && operation.variables.storyId !== storyId; // Poll for updates useEffect(() => { @@ -200,6 +197,7 @@ export const VisualTests = ({ const shouldSwitchToLastBuildOnBranch = canSwitchToLastBuildOnBranch && lastBuildOnBranchCompletedStory; + // Ensure we are holding the right story build useEffect(() => { setSelectedBuildInfo((oldSelectedBuildInfo) => @@ -209,13 +207,13 @@ export const VisualTests = ({ storyId, }) ); - }, [shouldSwitchToLastBuildOnBranch, lastBuildOnBranch?.id, storyId]); + }, [shouldSwitchToLastBuildOnBranch, lastBuildOnBranch?.id, storyId, setSelectedBuildInfo]); const switchToLastBuildOnBranch = useCallback( () => canSwitchToLastBuildOnBranch && - setSelectedBuildInfo({ storyId, buildId: lastBuildOnBranch.id }), - [canSwitchToLastBuildOnBranch, lastBuildOnBranch?.id, storyId] + setSelectedBuildInfo({ buildId: lastBuildOnBranch.id, storyId }), + [setSelectedBuildInfo, canSwitchToLastBuildOnBranch, lastBuildOnBranch?.id, storyId] ); return !selectedBuildHasCorrectBranch || !selectedBuild || storyDataIsStale || queryError ? ( @@ -251,3 +249,19 @@ export const VisualTests = ({ /> ); }; + +// We split this part of the component out for testing purposes, so we can control the +// selected build id in the stories and be super explicit about what happens when the +// selected build & last build on branch are out of sync. +// +// If the selectedBuildInfo is internal state of the component it is harder to do this, +// as we need to change the query results over time. +export const VisualTests = ( + props: Omit +) => { + const [selectedBuildInfo, setSelectedBuildInfo] = useState(); + + return ( + + ); +}; diff --git a/src/screens/VisualTests/mocks.ts b/src/screens/VisualTests/mocks.ts index 4cce9ebf..b8f10b7a 100644 --- a/src/screens/VisualTests/mocks.ts +++ b/src/screens/VisualTests/mocks.ts @@ -9,11 +9,22 @@ import { CompletedBuild, PublishedBuild, StartedBuild, + StoryTestFieldsFragment, TestResult, TestStatus, } from "../../gql/graphql"; +import { SelectedBuildWithTests } from "../../types"; import { makeBrowserInfo, makeComparison, makeTest, makeTests } from "../../utils/storyData"; +export const withTests = ( + build: T, + fullTests: StoryTestFieldsFragment[] +) => ({ + ...build, + testsForStatus: { nodes: fullTests }, + testsForStory: { nodes: fullTests }, +}); + export const passedTests = makeTests({ browsers: [Browser.Chrome, Browser.Safari], viewports: [ diff --git a/src/utils/gqlStoryHelpers.ts b/src/utils/gqlStoryHelpers.ts new file mode 100644 index 00000000..e44673dc --- /dev/null +++ b/src/utils/gqlStoryHelpers.ts @@ -0,0 +1,32 @@ +/* eslint-disable import/no-extraneous-dependencies */ + +import type { ResultOf, VariablesOf } from "@graphql-typed-document-node/core"; +import { getOperationAST } from "graphql"; +import { graphql } from "msw"; +import { TypedDocumentNode } from "urql"; + +export const withGraphQLQueryParameters = >( + ...args: Parameters>> +) => ({ + msw: { + handlers: [graphql.query>(...args)], + }, +}); + +export function withGraphQLQueryResultParameters>( + query: TQuery, + result: (variables: VariablesOf) => ResultOf +) { + const queryName = getOperationAST(query)?.name?.value; + if (queryName) + return withGraphQLQueryParameters(queryName, (req, res, ctx) => + res(ctx.data(result(req.variables))) + ); + throw new Error(`Couldn't determine query name from query`); +} + +export const withGraphQLMutationParameters = (...args: Parameters) => ({ + msw: { + handlers: [graphql.mutation(...args)], + }, +}); diff --git a/src/utils/updateSelectedBuildInfo.test.ts b/src/utils/updateSelectedBuildInfo.test.ts index 39d819c4..55fa86c6 100644 --- a/src/utils/updateSelectedBuildInfo.test.ts +++ b/src/utils/updateSelectedBuildInfo.test.ts @@ -2,27 +2,21 @@ import { updateSelectedBuildInfo } from "./updateSelectedBuildInfo"; it("does nothing if there is no next build", () => { expect( - updateSelectedBuildInfo( - { storyId: "storyId" }, - { - shouldSwitchToLastBuildOnBranch: false, - lastBuildOnBranchId: undefined, - storyId: "storyId", - } - ) - ).toEqual({ storyId: "storyId" }); + updateSelectedBuildInfo(undefined, { + shouldSwitchToLastBuildOnBranch: false, + lastBuildOnBranchId: undefined, + storyId: "storyId", + }) + ).toEqual(undefined); }); it("sets the story build from the next build, simple", () => { expect( - updateSelectedBuildInfo( - { storyId: "storyId" }, - { - shouldSwitchToLastBuildOnBranch: true, - lastBuildOnBranchId: "lastBuildOnBranchId", - storyId: "storyId", - } - ) + updateSelectedBuildInfo(undefined, { + shouldSwitchToLastBuildOnBranch: true, + lastBuildOnBranchId: "lastBuildOnBranchId", + storyId: "storyId", + }) ).toEqual({ buildId: "lastBuildOnBranchId", storyId: "storyId", @@ -32,15 +26,12 @@ it("sets the story build from the next build, simple", () => { // We should remain on the "new build" screen until we see a completed story it("does not set the story build from the next build, if the next build should not be switched to", () => { expect( - updateSelectedBuildInfo( - { storyId: "storyId" }, - { - shouldSwitchToLastBuildOnBranch: false, - lastBuildOnBranchId: "lastBuildOnBranchId", - storyId: "storyId", - } - ) - ).toEqual({ storyId: "storyId" }); + updateSelectedBuildInfo(undefined, { + shouldSwitchToLastBuildOnBranch: false, + lastBuildOnBranchId: "lastBuildOnBranchId", + storyId: "storyId", + }) + ).toEqual(undefined); }); it("updates the story build from the next build, simple", () => { diff --git a/src/utils/updateSelectedBuildInfo.ts b/src/utils/updateSelectedBuildInfo.ts index a67b2bfa..2d00b82e 100644 --- a/src/utils/updateSelectedBuildInfo.ts +++ b/src/utils/updateSelectedBuildInfo.ts @@ -1,10 +1,10 @@ export interface SelectedBuildInfo { storyId: string; - buildId?: string; + buildId: string; } export function updateSelectedBuildInfo( - oldSelectedBuildInfo: SelectedBuildInfo, + oldSelectedBuildInfo: SelectedBuildInfo | undefined, { shouldSwitchToLastBuildOnBranch, lastBuildOnBranchId, @@ -16,9 +16,12 @@ export function updateSelectedBuildInfo( } ) { if (!shouldSwitchToLastBuildOnBranch) { + if (!oldSelectedBuildInfo) return undefined; + return { ...oldSelectedBuildInfo, storyId }; } + if (!lastBuildOnBranchId) throw new Error("Impossible state"); return { buildId: lastBuildOnBranchId, storyId,