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

Fix npx chromatic timing out on build-storybook #305

Merged
merged 19 commits into from Apr 29, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 16 additions & 12 deletions bin/tasks/build.js
Expand Up @@ -3,6 +3,7 @@ import fs from 'fs';
import path from 'path';
import semver from 'semver';
import tmp from 'tmp-promise';
import yarnOrNpm, { hasYarn } from 'yarn-or-npm';

import { createTask, transitionTo } from '../lib/tasks';
import buildFailed from '../ui/messages/errors/buildFailed';
Expand All @@ -21,22 +22,21 @@ export const setSourceDir = async (ctx) => {
};

export const setSpawnParams = (ctx) => {
// Run either:
// npm/yarn run scriptName (depending on npm_execpath)
// node path/to/npm.js run scriptName (if npm run via node)
// Based on https://github.com/mysticatea/npm-run-all/blob/52eaf86242ba408dedd015f53ca7ca368f25a026/lib/run-task.js#L156-L174
const npmExecPath = process.env.npm_execpath;
const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath));
const isYarn = npmExecPath && /^yarn(\.js)?$/.test(path.basename(npmExecPath));
ctx.spawnParams = {
command: (isJsPath ? process.execPath : npmExecPath) || 'npm',
clientArgs: isJsPath ? [npmExecPath, 'run'] : ['run', '--silent'],
client: yarnOrNpm(),
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
platform: process.platform,
command: yarnOrNpm(),
clientArgs: ['run', '--silent'],
scriptArgs: [
ctx.options.buildScriptName,
isYarn ? '' : '--',
hasYarn() ? '' : '--',
'--output-dir',
ctx.sourceDir,
].filter(Boolean),
spawnOptions: {
preferLocal: true,
localDir: path.resolve('node_modules/.bin'),
},
};
};

Expand All @@ -52,9 +52,13 @@ export const buildStorybook = async (ctx) => {
});

try {
const { command, clientArgs, scriptArgs } = ctx.spawnParams;
const { command, clientArgs, scriptArgs, spawnOptions } = ctx.spawnParams;
ctx.log.debug('Using spawnParams:', JSON.stringify(ctx.spawnParams, null, 2));
await Promise.race([
execa(command, [...clientArgs, ...scriptArgs], { stdio: [null, logFile, logFile] }),
execa(command, [...clientArgs, ...scriptArgs], {
stdio: [null, logFile, logFile],
...spawnOptions,
}),
timeoutAfter(ctx.env.STORYBOOK_BUILD_TIMEOUT),
]);
} catch (e) {
Expand Down
30 changes: 29 additions & 1 deletion bin/tasks/build.test.js
Expand Up @@ -36,9 +36,32 @@ describe('setSpawnParams', () => {
const ctx = { sourceDir: './source-dir/', options: { buildScriptName: 'build:storybook' } };
await setSpawnParams(ctx);
expect(ctx.spawnParams).toEqual({
client: 'npm',
platform: expect.stringMatching(/darwin|linux|win32/),
command: 'npm',
clientArgs: ['run', '--silent'],
scriptArgs: ['build:storybook', '--', '--output-dir', './source-dir/'],
spawnOptions: {
preferLocal: true,
localDir: expect.stringMatching(/node_modules[/\\]\.bin$/),
},
});
});

it('supports yarn', async () => {
process.env.npm_execpath = '/path/to/yarn';
const ctx = { sourceDir: './source-dir/', options: { buildScriptName: 'build:storybook' } };
await setSpawnParams(ctx);
expect(ctx.spawnParams).toEqual({
client: 'yarn',
platform: expect.stringMatching(/darwin|linux|win32/),
command: '/path/to/yarn',
clientArgs: ['run', '--silent'],
scriptArgs: ['build:storybook', '--output-dir', './source-dir/'],
spawnOptions: {
preferLocal: true,
localDir: expect.stringMatching(/node_modules[/\\]\.bin$/),
},
});
});

Expand All @@ -63,6 +86,7 @@ describe('buildStorybook', () => {
scriptArgs: ['--script-args'],
},
env: { STORYBOOK_BUILD_TIMEOUT: 1000 },
log: { debug: jest.fn() },
};
await buildStorybook(ctx);
expect(ctx.buildLogFile).toMatch(/build-storybook\.log$/);
Expand All @@ -71,6 +95,10 @@ describe('buildStorybook', () => {
['--client-args', '--script-args'],
expect.objectContaining({ stdio: expect.any(Array) })
);
expect(ctx.log.debug).toHaveBeenCalledWith(
'Using spawnParams:',
JSON.stringify(ctx.spawnParams, null, 2)
);
});

it('fails when build times out', async () => {
Expand All @@ -82,7 +110,7 @@ describe('buildStorybook', () => {
},
options: { buildScriptName: '' },
env: { STORYBOOK_BUILD_TIMEOUT: 0 },
log: { error: jest.fn() },
log: { debug: jest.fn(), error: jest.fn() },
};
execa.mockReturnValue(new Promise((resolve) => setTimeout(resolve, 10)));
await expect(buildStorybook(ctx)).rejects.toThrow('Command failed');
Expand Down
3 changes: 2 additions & 1 deletion bin/ui/messages/errors/fatalError.js
Expand Up @@ -14,7 +14,7 @@ export default function fatalError(ctx, error, timestamp = new Date().toISOStrin
const website = link(pkg.docs);
const errors = [].concat(error);

const { git = {}, storybook, exitCode, isolatorUrl, cachedUrl, build } = ctx;
const { git = {}, storybook, spawnParams, exitCode, isolatorUrl, cachedUrl, build } = ctx;
const debugInfo = {
timestamp,
sessionId,
Expand All @@ -27,6 +27,7 @@ export default function fatalError(ctx, error, timestamp = new Date().toISOStrin
flags,
...(options.buildScriptName ? { buildScript: scripts[options.buildScriptName] } : {}),
...(options.scriptName ? { storybookScript: scripts[options.scriptName] } : {}),
...(spawnParams ? { spawnParams } : {}),
exitCode,
errorType: errors.map((err) => err.name).join('\n'),
errorMessage: stripAnsi(errors[0].message.split('\n')[0].trim()),
Expand Down
4 changes: 2 additions & 2 deletions bin/ui/tasks/build.js
Expand Up @@ -7,7 +7,7 @@ export const initial = {

export const pending = (ctx) => ({
status: 'pending',
title: 'Building your Storybook',
title: `Building your Storybook with ${ctx.spawnParams.client}`,
output: `Running command: ${ctx.spawnParams.scriptArgs.join(' ')}`,
});

Expand All @@ -25,6 +25,6 @@ export const skipped = (ctx) => ({

export const failed = (ctx) => ({
status: 'error',
title: 'Building your Storybook',
title: `Building your Storybook with ${ctx.spawnParams.client}`,
output: `Command failed: ${ctx.spawnParams.scriptArgs.join(' ')}`,
});
14 changes: 9 additions & 5 deletions bin/ui/tasks/build.stories.js
@@ -1,17 +1,19 @@
import task from '../components/task';
import { initial, pending, skipped, success } from './build';
import { initial, pending, skipped, success, failed } from './build';

export default {
title: 'CLI/Tasks/Build',
decorators: [(storyFn) => task(storyFn())],
};

const spawnParams = {
client: 'yarn',
scriptArgs: ['build-storybook', '-o', 'storybook-static'],
};

export const Initial = () => initial;

export const Building = () =>
pending({
spawnParams: { scriptArgs: ['build-storybook', '-o', 'storybook-static'] },
});
export const Building = () => pending({ spawnParams });

export const Built = () =>
success({
Expand All @@ -24,3 +26,5 @@ export const Skipped = () =>
skipped({
options: { storybookBuildDir: '/users/me/project/storybook-static' },
});

export const Failed = () => failed({ spawnParams });
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "chromatic",
"version": "5.7.1",
"version": "5.8.0-debug.2",
"description": "Automate visual testing across browsers. Gather UI feedback. Versioned documentation.",
"homepage": "https://www.chromatic.com",
"bugs": {
Expand Down