Storybook: Storyshots Puppeteer is broken with latest update

[mklanjsek]

Describe the bug
Bumped the storybook versions from 5.2.8 to 5.3.x and can no longer run our storyshots-puppeteer tests.

To Reproduce
Steps to reproduce the behavior:

1. Install 5.3.x
2. Create an __tests__/imageStoryshots.test.js using imageSnapshot for the test (code snippet below)
3. We narrowed the problem down to the call to imageSnapshot function in the snippet below. If we replace it with a function that returns undefined, everything works fine.

Expected behavior
Tests should pass or fail as they used to before the update.

Code snippets
Here is the content of imageStoryshots.test.js:

import 'document-register-element';
import initStoryshots from '@storybook/addon-storyshots';
import { imageSnapshot } from '@storybook/addon-storyshots-puppeteer';

const getFailureThreshold = story => {
  if (['xxx', 'yyy'].includes(story)) return 0.06;
  if (['zzz'].includes(story)) return 0.5;
  return 0.01;
};

const getMatchOptions = ({ context: { story } }) => ({
  failureThreshold: getFailureThreshold(story),
  failureThresholdType: 'percent'
});

initStoryshots({
  suite: 'Image storyshots',
  test: imageSnapshot({
    storybookUrl: 'http://localhost:9001?rng=abc&now=1558699200000',
    getMatchOptions
  })
});

Here is the error we are seeing:

Error when running puppeteer test for Overview|Getting Started - Introduction : It seems the headless browser is not running.
 Error when running puppeteer test for Overview|Getting Started - Installation : It seems the headless browser is not running.

```
Cannot find module 'puppeteer' from 'puppeteerTest.js'

  at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:259:17)
  at node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:145:33
  at step (node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:44:23)

Image storyshots › TEST_XXX

no-headless-browser-running

  at node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:78:35
  at step (node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:44:23)
  at Object.next (node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:25:53)
  at node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:19:71
  at Object.<anonymous>.__awaiter (node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:15:12)
  at testFn (node_modules/@storybook/addon-storyshots-puppeteer/dist/puppeteerTest.js:60:16)
  at Object.<anonymous> (node_modules/@storybook/addon-storyshots/dist/api/snapshotsTestsTemplate.js:42:20)

**System:**

→ npx -p @storybook/cli@next sb info

Environment Info:

System:
OS: macOS Mojave 10.14.6
CPU: (12) x64 Intel(R) Core(TM) i7-8700B CPU @ 3.20GHz
Binaries:
Node: 12.5.0 - /usr/local/bin/node
Yarn: 1.16.0 - /usr/local/bin/yarn
npm: 6.9.0 - /usr/local/bin/npm
Browsers:
Chrome: 78.0.3904.97
Firefox: 68.0.1
Safari: 13.0.4
npmPackages:
@storybook/addon-a11y: v5.3.6 => 5.3.6
@storybook/addon-actions: v5.3.6 => 5.3.6
@storybook/addon-console: 1.2.1 => 1.2.1
@storybook/addon-docs: v5.3.6 => 5.3.6
@storybook/addon-info: v5.3.6 => 5.3.6
@storybook/addon-knobs: v5.3.6 => 5.3.6
@storybook/addon-options: v5.3.6 => 5.3.6
@storybook/addon-storyshots: v5.3.6 => 5.3.6
@storybook/addon-storyshots-puppeteer: ^v5.3.6 => 5.3.6
@storybook/addons: v5.3.6 => 5.3.6
@storybook/components: v5.3.6 => 5.3.6
@storybook/core-events: v5.3.6 => 5.3.6
@storybook/preset-create-react-app: ^1.5.2 => 1.5.2
@storybook/react: v5.3.6 => 5.3.6
@storybook/theming: v5.3.6 => 5.3.6```

[alex-f1v]

@mklanjsek We were able to solve this error by adding puppeteer directly to our project.

[mklanjsek]

Thanks, adding puppeteer to project fixed it for us as well.

Would be good if don't need to do that, though.

[shilman]

It looks like this was the guilty commit: https://github.com/storybookjs/storybook/commit/aad29ed53d63daf145ed226dc2848913c6c3b9e5#diff-7618e1ea33b2f7a83797c63c2c55acf0

@gaetanmaisse any chance you can:

• [ ] Update MIGRATION.md
• [ ] Update the package README?

[gaetanmaisse]

@mklanjsek it was an intentional move in order to:

• give SB users more control on puppeteer version used by the addon: update puppeteer (i.e. Chromium version) independently from @storybook/addon-storyshots-puppeteer
• not download puppeteer dep for projects that don't need it

Sorry for having missed the doc update...

@shilman here it is: https://github.com/storybookjs/storybook/pull/9526 I targeted next, is it right?

[shilman]

@gaetanmaisse I think it's a huge improvement for users to install it themselves. It's also a breaking change in a minor release, which we've gotten a lot of criticism for in the past (rightfully?). Anyway, I'm cool with it, but let's watch out for this next time! (Next-next time, since we're starting 6.0 now and can break as much stuff as we want this milestone 😂) Thanks for the updates!!!

[gaetanmaisse]

@shilman you're 💯right and I'm a bit frustrated because I'm one of them, claiming to have breaking changes only in major updates 🙃! Looking back, I was overconfident when I was working on this addon a few months ago, I will definitely be more meticulous next time.

[shilman]

It's all good--it happens to everybody. This is great work: I'd rather err on the side of upgrading the product than be scared to change anything. 💯