Merge pull request #8932 from storybookjs/fix-image-snapshot

Fix image snapshots setup in official-storybook

addons/storyshots/storyshots-core/package.json

@@ -35,9 +35,11 @@
     "@jest/transform": "^24.9.0",
     "@storybook/addons": "5.3.0-beta.6",
     "@storybook/client-api": "5.3.0-beta.6",
+    "@storybook/core": "5.3.0-beta.6",
     "@types/glob": "^7.1.1",
     "@types/jest": "^24.0.16",
     "@types/jest-specific-snapshot": "^0.5.3",
+    "babel-plugin-require-context-hook": "^1.0.0",
     "core-js": "^3.0.1",
     "glob": "^7.1.3",
     "global": "^4.3.2",

addons/storyshots/storyshots-core/src/api/index.ts

@@ -12,11 +12,11 @@ type TestMethod = 'beforeAll' | 'beforeEach' | 'afterEach' | 'afterAll';
 const methods: TestMethod[] = ['beforeAll', 'beforeEach', 'afterEach', 'afterAll'];
 
 function callTestMethodGlobals(
-  testMethod: { [key in TestMethod]?: Function } & { [key in string]: any }
+  testMethod: { [key in TestMethod]?: Function & { timeout?: number } } & { [key in string]: any }
 ) {
   methods.forEach(method => {
     if (typeof testMethod[method] === 'function') {
-      global[method](testMethod[method]);
+      global[method](testMethod[method], testMethod[method].timeout);
     }
   });
 }

addons/storyshots/storyshots-core/src/api/snapshotsTestsTemplate.ts

@@ -18,15 +18,19 @@ function snapshotTest({ item, asyncJest, framework, testMethod, testMethodParams
             context,
             ...testMethodParams,
           })
-        )
+        ),
+      testMethod.timeout
     );
   } else {
-    it(name, () =>
-      testMethod({
-        story: item,
-        context,
-        ...testMethodParams,
-      })
+    it(
+      name,
+      () =>
+        testMethod({
+          story: item,
+          context,
+          ...testMethodParams,
+        }),
+      testMethod.timeout
     );
   }
 }

addons/storyshots/storyshots-core/src/frameworks/configure.ts

@@ -1,9 +1,13 @@
 import fs from 'fs';
 import path from 'path';
-import glob from 'glob';
+import { toRequireContext } from '@storybook/core/server';
+import registerRequireContextHook from 'babel-plugin-require-context-hook/register';
+import global from 'global';
 import { ClientApi } from './Loader';
 import { StoryshotsOptions } from '../api/StoryshotsOptions';
 
+registerRequireContextHook();
+
 const isFile = (file: string): boolean => {
   try {
     return fs.lstatSync(file).isFile();
@@ -64,9 +68,18 @@ function getConfigPathParts(input: string): Output {
     if (main) {
       const { stories = [] } = require.requireActual(main);
 
-      const result = stories.reduce((acc: string[], i: string) => [...acc, ...glob.sync(i)], []);
-
-      output.stories = result;
+      output.stories = stories.map(
+        (pattern: string | { path: string; recursive: boolean; match: string }) => {
+          const { path: basePath, recursive, match } = toRequireContext(pattern);
+          // eslint-disable-next-line no-underscore-dangle
+          return global.__requireContext(
+            configDir,
+            basePath,
+            recursive,
+            new RegExp(match.slice(1, -1))
+          );
+        }
+      );
     }
 
     return output;
@@ -94,8 +107,7 @@ function configure(
   });
 
   if (stories && stories.length) {
-    // eslint-disable-next-line global-require, import/no-dynamic-require
-    storybook.configure(() => stories.map(f => require(f)), false);
+    storybook.configure(stories, false);
   }
 }
 

addons/storyshots/storyshots-core/src/typings.d.ts

@@ -2,3 +2,5 @@ declare module 'global';
 declare module 'jest-preset-angular/*';
 declare module 'preact-render-to-json';
 declare module 'react-test-renderer*';
+declare module '@storybook/core/server';
+declare module 'babel-plugin-require-context-hook/register';

addons/storyshots/storyshots-puppeteer/README.md

@@ -196,6 +196,11 @@ initStoryshots({
 });
 ```
 
+### Specifying setup and tests timeout
+
+By default, `@storybook/addon-storyshots-puppeteer` uses 15 second timeouts for browser setup and test functions.
+Those can be customized with `setupTimeout` and `testTimeout` parameters.
+
 ### Integrate image storyshots with regular app
 
 You may want to use another Jest project to run your image snapshots as they require more resources: Chrome and Storybook built/served.

addons/storyshots/storyshots-puppeteer/src/ImageSnapshotConfig.ts

@@ -15,4 +15,6 @@ export interface ImageSnapshotConfig {
   getGotoOptions: (options: { context: Context; url: string }) => DirectNavigationOptions;
   customizePage: (page: Page) => Promise<void>;
   getCustomBrowser: () => Promise<Browser>;
+  setupTimeout: number;
+  testTimeout: number;
 }

addons/storyshots/storyshots-puppeteer/src/imageSnapshot.ts

@@ -1,4 +1,4 @@
-import puppeteer, { Browser, Page } from 'puppeteer';
+import { Browser, Page } from 'puppeteer';
 import { toMatchImageSnapshot } from 'jest-image-snapshot';
 import { logger } from '@storybook/node-logger';
 import { constructUrl } from './url';
@@ -21,6 +21,8 @@ const defaultConfig: ImageSnapshotConfig = {
   getGotoOptions: noop,
   customizePage: asyncNoop,
   getCustomBrowser: undefined,
+  setupTimeout: 15000,
+  testTimeout: 15000,
 };
 
 export const imageSnapshot = (customConfig: Partial<ImageSnapshotConfig> = {}) => {
@@ -33,6 +35,8 @@ export const imageSnapshot = (customConfig: Partial<ImageSnapshotConfig> = {}) =
     getGotoOptions,
     customizePage,
     getCustomBrowser,
+    setupTimeout,
+    testTimeout,
   } = { ...defaultConfig, ...customConfig };
 
   let browser: Browser; // holds ref to browser. (ie. Chrome)
@@ -75,19 +79,22 @@ export const imageSnapshot = (customConfig: Partial<ImageSnapshotConfig> = {}) =
 
     expect(image).toMatchImageSnapshot(getMatchOptions({ context, url }));
   };
+  testFn.timeout = testTimeout;
 
-  testFn.afterAll = () => {
+  testFn.afterAll = async () => {
     if (getCustomBrowser && page) {
-      return page.close();
+      await page.close();
+    } else if (browser) {
+      await browser.close();
     }
-
-    return browser.close();
   };
 
-  testFn.beforeAll = async () => {
+  const beforeAll = async () => {
     if (getCustomBrowser) {
       browser = await getCustomBrowser();
     } else {
+      // eslint-disable-next-line global-require
+      const puppeteer = require('puppeteer');
       // add some options "no-sandbox" to make it work properly on some Linux systems as proposed here: https://github.com/Googlechrome/puppeteer/issues/290#issuecomment-322851507
       browser = await puppeteer.launch({
         args: ['--no-sandbox ', '--disable-setuid-sandbox', '--disable-dev-shm-usage'],
@@ -97,6 +104,8 @@ export const imageSnapshot = (customConfig: Partial<ImageSnapshotConfig> = {}) =
 
     page = await browser.newPage();
   };
+  beforeAll.timeout = setupTimeout;
+  testFn.beforeAll = beforeAll;
 
   return testFn;
 };

examples/official-storybook/image-snapshots/storyshots-image.runner.js

@@ -20,7 +20,7 @@ if (!fs.existsSync(pathToStorybookStatic)) {
 } else {
   initStoryshots({
     suite: 'Image snapshots',
-    storyKindRegex: /^Addons\|Storyshots/,
+    storyKindRegex: /^Addons\/Storyshots/,
     framework: 'react',
     configPath: path.join(__dirname, '..'),
     test: imageSnapshot({

examples/official-storybook/package.json

@@ -61,5 +61,8 @@
     "ts-loader": "^6.0.0",
     "uuid": "^3.3.2",
     "webpack": "^4.33.0"
+  },
+  "optionalDependencies": {
+    "puppeteer": "^2.0.0"
   }
 }

lib/api/src/version.ts

@@ -1 +1 @@
-export const version = '5.3.0-beta.3';
+export const version = '5.3.0-beta.5';

lib/core/package.json

@@ -66,14 +66,14 @@
     "find-up": "^4.1.0",
     "fs-extra": "^8.0.1",
     "glob-base": "^0.3.0",
-    "glob-regex": "^0.3.2",
     "global": "^4.3.2",
     "html-webpack-plugin": "^4.0.0-beta.2",
     "inquirer": "^7.0.0",
     "interpret": "^2.0.0",
     "ip": "^1.1.5",
     "json5": "^2.1.1",
     "lazy-universal-dotenv": "^3.0.1",
+    "micromatch": "^4.0.2",
     "node-fetch": "^2.6.0",
     "open": "^7.0.0",
     "pnp-webpack-plugin": "1.5.0",

lib/core/server.js

@@ -2,6 +2,7 @@ const defaultWebpackConfig = require('./dist/server/preview/base-webpack.config'
 const serverUtils = require('./dist/server/utils/template');
 const buildStatic = require('./dist/server/build-static');
 const buildDev = require('./dist/server/build-dev');
+const toRequireContext = require('./dist/server/preview/to-require-context');
 
 const managerPreset = require.resolve('./dist/server/manager/manager-preset');
 
@@ -11,4 +12,5 @@ module.exports = {
   ...buildStatic,
   ...buildDev,
   ...serverUtils,
+  ...toRequireContext,
 };

lib/core/src/client/preview/start.js

@@ -361,7 +361,7 @@ export default function start(render, { decorateStory } = {}) {
       });
     } else {
       const exported = loadable();
-      if (Array.isArray(exported) && !exported.find(obj => !obj.default)) {
+      if (Array.isArray(exported) && exported.every(obj => obj.default != null)) {
         currentExports = new Map(exported.map(fileExports => [fileExports, null]));
       } else if (exported) {
         logger.warn(

lib/core/src/server/preview/iframe-webpack.config.js

@@ -9,14 +9,11 @@ import CoreJSUpgradeWebpackPlugin from 'corejs-upgrade-webpack-plugin';
 import VirtualModulePlugin from 'webpack-virtual-modules';
 
 import resolveFrom from 'resolve-from';
-import toRegex from 'glob-regex';
-import globBase from 'glob-base';
 
 import babelLoader from '../common/babel-loader';
-import { nodeModulesPaths, loadEnv } from '../config/utils';
-import { getPreviewHeadHtml, getPreviewBodyHtml } from '../utils/template';
-
-const isObject = val => val != null && typeof val === 'object' && Array.isArray(val) === false;
+import { loadEnv, nodeModulesPaths } from '../config/utils';
+import { getPreviewBodyHtml, getPreviewHeadHtml } from '../utils/template';
+import { toRequireContextString } from './to-require-context';
 
 const reactPaths = {};
 try {
@@ -26,27 +23,6 @@ try {
   //
 }
 
-const toRequireContext = input => {
-  switch (true) {
-    case typeof input === 'string': {
-      const { base, glob } = globBase(input);
-      const regex = toRegex(glob)
-        .toString()
-        .replace('^([^\\/]+)', '');
-
-      return `require.context('${base}', true, ${regex})`;
-    }
-    case isObject(input): {
-      const { path: p, recursive: r, match: m } = input;
-      return `require.context('${p}', ${r}, ${m})`;
-    }
-
-    default: {
-      throw new Error('the provided input cannot be transformed into a require.context');
-    }
-  }
-};
-
 export default ({
   configDir,
   babelOptions,
@@ -77,7 +53,7 @@ export default ({
             [path.resolve(path.join(configDir, `generated-entry.js`))]: `
               import { configure, addDecorator, addParameters } from '@storybook/${framework}';
 
-              configure([${stories.map(toRequireContext).join(',')}
+              configure([${stories.map(toRequireContextString).join(',')}
               ], module);
             `,
           })

lib/core/src/server/preview/to-require-context.js

@@ -0,0 +1,30 @@
+import globBase from 'glob-base';
+import { makeRe } from 'micromatch';
+
+const isObject = val => val != null && typeof val === 'object' && Array.isArray(val) === false;
+export const toRequireContext = input => {
+  switch (true) {
+    case typeof input === 'string': {
+      const { base, glob } = globBase(input);
+      const regex = makeRe(glob)
+        .toString()
+        // webpack prepends the relative path with './'
+        .replace(/^\/\^/, '/^\\.\\/')
+        .replace(/\?:\^/g, '?:');
+
+      return { path: base, recursive: glob.startsWith('**'), match: regex };
+    }
+    case isObject(input): {
+      return input;
+    }
+
+    default: {
+      throw new Error('the provided input cannot be transformed into a require.context');
+    }
+  }
+};
+
+export const toRequireContextString = input => {
+  const { path: p, recursive: r, match: m } = toRequireContext(input);
+  return `require.context('${p}', ${r}, ${m})`;
+};

scripts/babel-jest.js

@@ -2,5 +2,5 @@ const path = require('path');
 const babelJest = require('babel-jest');
 
 module.exports = babelJest.createTransformer({
-  configFile: path.resolve('.babelrc'),
+  configFile: path.resolve(__dirname, '../.babelrc'),
 });