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

Do not prompt to install chromatic script during E2E builds #941

Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 12 additions & 2 deletions node-src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { DNSResolveAgent } from './io/getDNSResolveAgent';
import getEnv from './lib/getEnv';
import parseArgs from './lib/parseArgs';
import TestLogger from './lib/testLogger';
import * as checkPackageJson from './lib/checkPackageJson';
import { uploadFiles } from './lib/uploadFiles';
import { writeChromaticDiagnostics } from './lib/writeChromaticDiagnostics';
import { Context } from './types';
Expand Down Expand Up @@ -41,8 +42,9 @@ vi.mock('execa');
// NOTE: we'd prefer to mock the require.resolve() of `@chromatic-com/playwright/..` but
// vitest doesn't allow you to do that.
const mockedBuildCommand = 'mocked build command';
vi.mock('./lib/getE2eBuildCommand', () => ({
getE2eBuildCommand: () => mockedBuildCommand,
vi.mock('./lib/e2e', async (importOriginal) => ({
...await importOriginal(),
getE2EBuildCommand: () => mockedBuildCommand,
}));

const execaCommand = vi.mocked(execaDefault);
Expand Down Expand Up @@ -711,6 +713,14 @@ it('prompts you to add a script to your package.json', async () => {
expect(confirm).toHaveBeenCalled();
});

it('does not propmpt you to add a script to your package.json for E2E builds', async () => {
const ctx = getContext(['--project-token=asdf1234', '--playwright']);
await runAll(ctx);
const spy = vi.spyOn(checkPackageJson, 'default');
expect(spy).not.toHaveBeenCalled();
expect(confirm).not.toHaveBeenCalled();
});

Comment on lines +716 to +723
Copy link
Member

Choose a reason for hiding this comment

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

Nice test.

it('ctx should be JSON serializable', async () => {
const ctx = getContext(['--project-token=asdf1234']);
expect(() => writeChromaticDiagnostics(ctx)).not.toThrow();
Expand Down
3 changes: 2 additions & 1 deletion node-src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import noPackageJson from './ui/messages/errors/noPackageJson';
import runtimeError from './ui/messages/errors/runtimeError';
import taskError from './ui/messages/errors/taskError';
import intro from './ui/messages/info/intro';
import { isE2EBuild } from './lib/e2e';

/**
Make keys of `T` outside of `R` optional.
Expand Down Expand Up @@ -176,7 +177,7 @@ export async function runAll(ctx: InitialContext) {
// Run these in parallel; neither should ever reject
await Promise.all([runBuild(ctx), checkForUpdates(ctx)]).catch(onError);

if ([0, 1].includes(ctx.exitCode)) {
if (!isE2EBuild(ctx.options) && [0, 1].includes(ctx.exitCode)) {
await checkPackageJson(ctx);
}

Expand Down
8 changes: 6 additions & 2 deletions node-src/lib/getE2eBuildCommand.ts → node-src/lib/e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { getCliCommand, Runner, AGENTS } from '@antfu/ni';

import { Context } from '../types';
import { Context, Options } from '../types';
import missingDependency from '../ui/messages/errors/missingDependency';
import { exitCodes, setExitCode } from './setExitCode';
import { failed } from '../ui/tasks/build';
Expand All @@ -27,7 +27,7 @@ const parseNexec = <Runner>((agent, args) => {
return command.replace('{0}', args.map(quote).join(' ')).trim();
});

export async function getE2eBuildCommand(
export async function getE2EBuildCommand(
ctx: Context,
flag: 'playwright' | 'cypress',
buildCommandOptions: string[]
Expand Down Expand Up @@ -59,3 +59,7 @@ export async function getE2eBuildCommand(
throw err;
}
}

export function isE2EBuild(options: Options) {
return options.playwright || options.cypress;
}
Comment on lines +63 to +65
Copy link
Member

Choose a reason for hiding this comment

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

Nice. We could use this for the "flag the project as e2e" thing in the future (if we ever feel the need to handle projects switching from storybook-e2e or vice versa).

3 changes: 2 additions & 1 deletion node-src/lib/getOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import missingBuildScriptName from '../ui/messages/errors/missingBuildScriptName
import missingProjectToken from '../ui/messages/errors/missingProjectToken';
import deprecatedOption from '../ui/messages/warnings/deprecatedOption';
import invalidPackageJson from '../ui/messages/errors/invalidPackageJson';
import { isE2EBuild } from './e2e';

const takeLast = (input: string | string[]) =>
Array.isArray(input) ? input[input.length - 1] : input;
Expand Down Expand Up @@ -263,7 +264,7 @@ export default function getOptions({
return options;
}

if (options.playwright || options.cypress) {
if (isE2EBuild(options)) {
return options;
}

Expand Down
8 changes: 4 additions & 4 deletions node-src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { endActivity, startActivity } from '../ui/components/activity';
import buildFailed from '../ui/messages/errors/buildFailed';
import { failed, initial, pending, skipped, success } from '../ui/tasks/build';
import { getPackageManagerRunCommand } from '../lib/getPackageManager';
import { buildBinName as e2EbuildBinName, getE2eBuildCommand } from '../lib/getE2eBuildCommand';
import { buildBinName as e2EbuildBinName, getE2EBuildCommand, isE2EBuild } from '../lib/e2e';
import missingDependency from '../ui/messages/errors/missingDependency';

export const setSourceDir = async (ctx: Context) => {
Expand Down Expand Up @@ -43,8 +43,8 @@ export const setBuildCommand = async (ctx: Context) => {
ctx.git.changedFiles && webpackStatsSupported && ctx.sourceDir,
].filter(Boolean);

if (ctx.options.playwright || ctx.options.cypress) {
ctx.buildCommand = await getE2eBuildCommand(
if (isE2EBuild(ctx.options)) {
ctx.buildCommand = await getE2EBuildCommand(
ctx,
ctx.options.playwright ? 'playwright' : 'cypress',
buildCommandOptions
Expand Down Expand Up @@ -88,7 +88,7 @@ export const buildStorybook = async (ctx: Context) => {
// installed the right dependency or run from the right directory
if (
ctx.options.inAction &&
(ctx.options.playwright || ctx.options.cypress) &&
(isE2EBuild(ctx.options)) &&
e.message.match(e2EbuildBinName)
) {
const flag = ctx.options.playwright ? 'playwright' : 'cypress';
Expand Down
Loading