Merge pull request #115 from happo/prevent-loop

Prevent infinite loop when error is thrown during `play` invocation

.github/workflows/test.yml

@@ -12,56 +12,71 @@ jobs:
     env:
       HAPPO_API_KEY: ${{ secrets.HAPPO_API_KEY }}
       HAPPO_API_SECRET: ${{ secrets.HAPPO_API_SECRET }}
+      HAPPO_PROJECT: storybook8
+      YARN_IGNORE_PATH: 1
 
     steps:
       - uses: actions/checkout@v2
+        with:
+          fetch-depth: 100
       - name: Use Node.js 18
         uses: actions/setup-node@v2
         with:
           node-version: 18
       - run: corepack enable
-      - run: yarn install --frozen-lockfile
+      - run: yarn set version 1.22.22
+      - run: yarn -v
+      - run: yarn install --immutable
       - run: yarn lint
       - run: yarn test
-      - run: yarn build && yarn happo run
-      - run: yarn build-local-storybook && HAPPO_USE_PREBUILT_PACKAGE=yes yarn happo run
+      - run: yarn build && yarn happo-ci-github-actions
+      - run: yarn build-local-storybook && HAPPO_USE_PREBUILT_PACKAGE=yes HAPPO_PROJECT=storybook8-prebuilt yarn happo-ci-github-actions
 
   storybook7:
     runs-on: ubuntu-latest
     env:
       HAPPO_API_KEY: ${{ secrets.HAPPO_API_KEY }}
       HAPPO_API_SECRET: ${{ secrets.HAPPO_API_SECRET }}
+      HAPPO_PROJECT: storybook7
+      YARN_IGNORE_PATH: 1
 
     steps:
       - uses: actions/checkout@v2
+        with:
+          fetch-depth: 100
       - name: Use Node.js 18
         uses: actions/setup-node@v2
         with:
           node-version: 18
       - run: corepack enable
-      - run: yarn install --frozen-lockfile
+      - run: yarn set version 1.22.22
+      - run: yarn install --immutable
       - run: yarn add --dev @storybook/addon-actions@7 @storybook/addon-interactions@7 @storybook/api@7 @storybook/manager-api@7 @storybook/preview-api@7 @storybook/react@7 @storybook/react-webpack5@7 storybook@7
-      - run: yarn build && yarn happo run
+      - run: yarn build && yarn happo-ci-github-actions
 
   storybook6:
     runs-on: ubuntu-latest
     env:
       HAPPO_API_KEY: ${{ secrets.HAPPO_API_KEY }}
-      HAPPO_API_SECRET: ${{ secrets.HAPPO_API_SECRET }}
       DISABLE_REACT_WEBPACK5_FRAMEWORK: true
       DISABLE_ADDONS: true
       HAPPO_DEBUG: true
+      HAPPO_PROJECT: storybook6
+      YARN_IGNORE_PATH: 1
 
     steps:
       - uses: actions/checkout@v2
+        with:
+          fetch-depth: 100
       - name: Use Node.js 18
         uses: actions/setup-node@v2
         with:
           node-version: 18
       - run: corepack enable
-      - run: yarn install --frozen-lockfile
+      - run: yarn set version 1.22.22
+      - run: yarn install --immutable
       - run: rm .storybook/Interactive.stories.js
       - run: yarn add --dev storybook@6 @storybook/addon-actions@6 @storybook/addon-interactions@6 @storybook/api@6 @storybook/react@6 global @storybook/channels@6 @storybook/channel-websocket@6
       - run: yarn build
       - run: yarn storybook build --version
-      - run: yarn happo run
+      - run: yarn happo-ci-github-actions

.gitignore

@@ -5,6 +5,5 @@ node_modules
 /preset.js
 /decorator.js
 /constants.js
-.yarnrc.yml
 .yarn
 dist

.happo.js

@@ -5,19 +5,27 @@ const { RemoteBrowserTarget } = require('happo.io');
 const happoPluginStorybook = require('.');
 
 module.exports = {
+  project: process.env.HAPPO_PROJECT,
   targets: {
     chrome: new RemoteBrowserTarget('chrome', {
       viewport: '800x600',
       chunks: 4,
+      allowPointerEvents: true,
     }),
     firefox: new RemoteBrowserTarget('firefox', {
       viewport: '800x600',
       chunks: 2,
+      allowPointerEvents: true,
+    }),
+    edge: new RemoteBrowserTarget('edge', {
+      viewport: '800x600',
+      chunks: 1,
+      allowPointerEvents: true,
     }),
-    edge: new RemoteBrowserTarget('edge', { viewport: '800x600', chunks: 1 }),
     safari: new RemoteBrowserTarget('safari', {
       viewport: '800x600',
       chunks: 2,
+      allowPointerEvents: true,
     }),
     iosSafari: new RemoteBrowserTarget('ios-safari', {
       viewport: '375x600',

.storybook/Interactive.stories.js

@@ -41,3 +41,22 @@ export const Demo = {
     });
   },
 };
+
+export const InteractiveThrowsError = {
+  // This story exists to test what happens when the play function throws an
+  // error that isn't caused by `forceHappoScreenshot`.
+  play: async ({ args, canvasElement, step }) => {
+    const canvas = within(canvasElement);
+    await new Promise((r) => setTimeout(r, 200));
+
+    await step('clicked', async () => {
+      await userEvent.click(canvas.getByRole('button'));
+      await expect(canvas.getByText('I was clicked')).toBeInTheDocument();
+      await forceHappoScreenshot('clicked');
+      throw new Error('Whoops');
+
+      // We will never reach this line
+      await forceHappoScreenshot('clicked2');
+    });
+  },
+};

.yarnrc.yml

@@ -0,0 +1,3 @@
+nodeLinker: node-modules
+
+yarnPath: .yarn/releases/yarn-1.22.22.cjs

package.json

@@ -81,5 +81,5 @@
     "archiver": "^3.0.0",
     "rimraf": "^2.6.3"
   },
-  "packageManager": "yarn@4.0.2"
+  "packageManager": "yarn@1.22.22"
 }

src/register.js

@@ -165,9 +165,14 @@ function renderStory(story, { force = false } = {}) {
       ) {
         channel.off('storyRenderPhaseChanged', handleRenderPhaseChanged);
         clearTimeout(timeout);
+        const pausedAtStep =
+          forcedHappoScreenshotSteps[forcedHappoScreenshotSteps.length - 1];
+        if (pausedAtStep.done) {
+          // Already processed this step
+          return resolve();
+        }
         return resolve({
-          pausedAtStep:
-            forcedHappoScreenshotSteps[forcedHappoScreenshotSteps.length - 1],
+          pausedAtStep,
         });
       }
       if (ev.newPhase === 'playing') {
@@ -240,7 +245,7 @@ window.happo.nextExample = async () => {
       )) || {};
     pausedAtStep = renderResult.pausedAtStep;
     if (pausedAtStep) {
-      variant = `${variant}-${pausedAtStep}`;
+      variant = `${variant}-${pausedAtStep.stepLabel}`;
     } else {
       forcedHappoScreenshotSteps = undefined;
     }
@@ -273,6 +278,8 @@ window.happo.nextExample = async () => {
   } finally {
     if (!pausedAtStep) {
       currentIndex++;
+    } else {
+      pausedAtStep.done = true;
     }
   }
 };
@@ -292,14 +299,14 @@ export function forceHappoScreenshot(stepLabel) {
 
   if (
     forcedHappoScreenshotSteps &&
-    forcedHappoScreenshotSteps.includes(stepLabel)
+    forcedHappoScreenshotSteps.some((s) => s.stepLabel === stepLabel)
   ) {
     // ignore, this step has already been handled
     return;
   }
 
   forcedHappoScreenshotSteps = forcedHappoScreenshotSteps || [];
-  forcedHappoScreenshotSteps.push(stepLabel);
+  forcedHappoScreenshotSteps.push({ stepLabel, done: false });
 
   const e = new Error(`Forced screenshot with label "${stepLabel}"`);
   e.type = 'ForcedHappoScreenshot';