Merge pull request #237 from chromaui/fix-spawn-params

Avoid passing --silent when invoking npm through Node.js script

bin/lib/getEnv.js

@@ -9,6 +9,7 @@ const {
   CHROMATIC_TIMEOUT = 5 * 60 * 1000,
   CHROMATIC_STORYBOOK_VERSION,
   LOGGLY_CUSTOMER_TOKEN = 'b5e26204-cdc5-4c78-a9cc-c69eb7fabad3',
+  STORYBOOK_BUILD_TIMEOUT = 10 * 60 * 1000,
 } = process.env;
 
 const ENVIRONMENT_WHITELIST = [/^GERRIT/, /^TRAVIS/];
@@ -24,16 +25,17 @@ const CHROMATIC_PROJECT_TOKEN =
   process.env.CHROMA_APP_CODE; // backwards compatibility
 
 export default () => ({
-  CHROMATIC_SERVER_PORT,
+  CHROMATIC_SERVER_PORT: parseInt(CHROMATIC_SERVER_PORT, 10),
   CHROMATIC_INDEX_URL,
   CHROMATIC_TUNNEL_URL,
   CHROMATIC_CREATE_TUNNEL,
   CHROMATIC_PROJECT_TOKEN,
-  CHROMATIC_RETRIES,
-  CHROMATIC_POLL_INTERVAL,
-  CHROMATIC_TIMEOUT,
+  CHROMATIC_RETRIES: parseInt(CHROMATIC_RETRIES, 10),
+  CHROMATIC_POLL_INTERVAL: parseInt(CHROMATIC_POLL_INTERVAL, 10),
+  CHROMATIC_TIMEOUT: parseInt(CHROMATIC_TIMEOUT, 10),
   CHROMATIC_STORYBOOK_VERSION,
+  ENVIRONMENT_WHITELIST,
   LOGGLY_CUSTOMER_TOKEN,
+  STORYBOOK_BUILD_TIMEOUT: parseInt(STORYBOOK_BUILD_TIMEOUT, 10),
   STORYBOOK_CLI_FLAGS_BY_VERSION,
-  ENVIRONMENT_WHITELIST,
 });

bin/tasks/build.js

@@ -29,8 +29,8 @@ export const setSpawnParams = (ctx) => {
   const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath));
   const isYarn = npmExecPath && path.basename(npmExecPath) === 'yarn.js';
   ctx.spawnParams = {
-    command: isJsPath ? process.execPath : npmExecPath || 'npm',
-    clientArgs: [isJsPath ? npmExecPath : '', isYarn ? '' : 'run', '--silent'].filter(Boolean),
+    command: (isJsPath ? process.execPath : npmExecPath) || 'npm',
+    clientArgs: isJsPath ? [npmExecPath, 'run'] : ['run', '--silent'],
     scriptArgs: [
       ctx.options.buildScriptName,
       isYarn ? '' : '--',
@@ -40,6 +40,9 @@ export const setSpawnParams = (ctx) => {
   };
 };
 
+const timeoutAfter = (ms) =>
+  new Promise((resolve, reject) => setTimeout(reject, ms, new Error(`Operation timed out`)));
+
 export const buildStorybook = async (ctx) => {
   ctx.buildLogFile = path.resolve('./build-storybook.log');
   const logFile = fs.createWriteStream(ctx.buildLogFile);
@@ -50,7 +53,10 @@ export const buildStorybook = async (ctx) => {
 
   try {
     const { command, clientArgs, scriptArgs } = ctx.spawnParams;
-    await execa(command, [...clientArgs, ...scriptArgs], { stdio: [null, logFile, logFile] });
+    await Promise.race([
+      execa(command, [...clientArgs, ...scriptArgs], { stdio: [null, logFile, logFile] }),
+      timeoutAfter(ctx.env.STORYBOOK_BUILD_TIMEOUT),
+    ]);
   } catch (e) {
     const buildLog = fs.readFileSync(ctx.buildLogFile, 'utf8');
     ctx.log.error(buildFailed(ctx, e, buildLog));

bin/tasks/build.test.js

@@ -51,6 +51,7 @@ describe('buildStorybook', () => {
         clientArgs: ['--client-args'],
         scriptArgs: ['--script-args'],
       },
+      env: { STORYBOOK_BUILD_TIMEOUT: 1000 },
     };
     await buildStorybook(ctx);
     expect(ctx.buildLogFile).toMatch(/build-storybook\.log$/);
@@ -60,4 +61,20 @@ describe('buildStorybook', () => {
       expect.objectContaining({ stdio: expect.any(Array) })
     );
   });
+
+  it('fails when build times out', async () => {
+    const ctx = {
+      spawnParams: {
+        command: 'build:storybook',
+        clientArgs: ['--client-args'],
+        scriptArgs: ['--script-args'],
+      },
+      options: { buildScriptName: '' },
+      env: { STORYBOOK_BUILD_TIMEOUT: 0 },
+      log: { error: jest.fn() },
+    };
+    execa.mockReturnValue(new Promise((resolve) => setTimeout(resolve, 10)));
+    await expect(buildStorybook(ctx)).rejects.toThrow('Command failed');
+    expect(ctx.log.error).toHaveBeenCalledWith(expect.stringContaining('Operation timed out'));
+  });
 });

package.json

@@ -1,6 +1,6 @@
 {
   "name": "chromatic",
-  "version": "5.5.0",
+  "version": "5.5.1-dev.1",
   "description": "Visual Testing for Storybook",
   "homepage": "https://www.chromatic.com",
   "bugs": {