Merge pull request #37 from storybookjs/fix/cleanup-has-play-fn

Cleanup play function checks
storybookjs · Feb 3, 2022 · b1997ee · b1997ee
2 parents b1db5ef + 86a700a
commit b1997ee
Show file tree

Hide file tree

Showing 10 changed files with 37 additions and 53 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -30,3 +30,7 @@ jobs:
           yarn build
           STORY_STORE_V7=true yarn test-storybook:ci
 
+      - name: Run test runner (stories json mode)
+        run: |
+          yarn build
+          yarn test-storybook:ci-json
diff --git a/bin/test-storybook.js b/bin/test-storybook.js
@@ -100,8 +100,6 @@ async function fetchStoriesJson(url) {
   return tmpDir;
 }
 
-const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
-
 const main = async () => {
   const targetURL = sanitizeURL(process.env.TARGET_URL || `http://localhost:6006`);
   await checkStorybook(targetURL);

diff --git a/package.json b/package.json
@@ -45,6 +45,7 @@
     "release": "yarn build && auto shipit",
     "test-storybook": "node bin/test-storybook.js --no-cache",
     "test-storybook:ci": "concurrently -k -s first -n \"SB,TEST\" -c \"magenta,blue\" \"yarn build-storybook --quiet && npx http-server storybook-static --port 6006 --silent\" \"wait-on tcp:6006 && yarn test-storybook\"",
+    "test-storybook:ci-json": "concurrently -k -s first -n \"SB,TEST\" -c \"magenta,blue\" \"yarn build-storybook --quiet && npx http-server storybook-static --port 6006 --silent\" \"wait-on tcp:6006 && yarn test-storybook --stories-json\"",
     "test-storybook:jsdom": "jest --no-cache --config ./jsdom-jest.config.js -i",
     "test-storybook:playwright-no-cache": "jest --no-cache --config ./test-runner-jest.config.js",
     "test-storybook:playwright": "./bin/test-storybook.js",

diff --git a/playwright/custom-environment.js b/playwright/custom-environment.js
@@ -49,11 +49,11 @@ class CustomEnvironment extends PlaywrightEnvironment {
     await page.addScriptTag({
       content: `
         class StorybookTestRunnerError extends Error {
-          constructor(storyId, hasPlayFn, errorMessage) {
+          constructor(storyId, errorMessage) {
             super(errorMessage);
             this.name = 'StorybookTestRunnerError';
             const storyUrl = \`${referenceURL || targetURL}?path=/story/\${storyId}\`;
-            const finalStoryUrl = storyUrl + (hasPlayFn ? '&addonPanel=storybook/interactions/panel' : '');
+            const finalStoryUrl = \`\${storyUrl}&addonPanel=storybook/interactions/panel\`;
 
             this.message = \`\nAn error occurred in the following story:\n\${finalStoryUrl}\n\nMessage:\n \${errorMessage}\`;
           }
@@ -90,19 +90,18 @@ class CustomEnvironment extends PlaywrightEnvironment {
           });
         }
 
-        async function __test(storyId, hasPlayFn) {
+        async function __test(storyId) {
           try {
             await __waitForElement('#root');
           } catch(err) {
             const message = \`Timed out waiting for Storybook to load after 10 seconds. Are you sure the Storybook is running correctly in that URL? Is the Storybook private (e.g. under authentication layers)?\n\n\nHTML: \${document.body.innerHTML}\`;
-            throw new StorybookTestRunnerError(storyId, hasPlayFn, message);
+            throw new StorybookTestRunnerError(storyId, message);
           }
 
           const channel = window.__STORYBOOK_ADDONS_CHANNEL__;
           if(!channel) {
             throw new StorybookTestRunnerError(
               storyId,
-              hasPlayFn,
               'The test runner could not access the Storybook channel. Are you sure the Storybook is running correctly in that URL?'
             );
           }
@@ -111,13 +110,13 @@ class CustomEnvironment extends PlaywrightEnvironment {
             channel.on('storyRendered', () => resolve(document.getElementById('root')));
             channel.on('storyUnchanged', () => resolve(document.getElementById('root')));
             channel.on('storyErrored', ({ description }) => reject(
-              new StorybookTestRunnerError(storyId, hasPlayFn, description))
+              new StorybookTestRunnerError(storyId, description))
             );
             channel.on('storyThrewException', (error) => reject(
-              new StorybookTestRunnerError(storyId, hasPlayFn, error.message))
+              new StorybookTestRunnerError(storyId, error.message))
             );
             channel.on('storyMissing', (id) => id === storyId && reject(
-              new StorybookTestRunnerError(storyId, hasPlayFn, 'The story was missing when trying to access it.'))
+              new StorybookTestRunnerError(storyId, 'The story was missing when trying to access it.'))
             );
 
             channel.emit('setCurrentStory', { storyId });

diff --git a/src/csf/transformCsf.ts b/src/csf/transformCsf.ts
@@ -12,7 +12,6 @@ export interface TestContext {
   name: t.Literal;
   title: t.Literal;
   id: t.Literal;
-  hasPlayFn?: t.BooleanLiteral;
 }
 type FilePrefixer = () => t.Statement[];
 type TestPrefixer = (context: TestContext) => t.Statement[];
@@ -35,9 +34,8 @@ const prefixFunction = (
   const context: TestContext = {
     storyExport: t.identifier(key),
     name: t.stringLiteral(name), // FIXME .name annotation
-    title: t.stringLiteral(title), // FIXME: auto-title
+    title: t.stringLiteral(title),
     id: t.stringLiteral(toId(title, name)),
-    hasPlayFn: t.booleanLiteral(!!input),
   };
 
   // instead, let's just make the prefixer return the function

diff --git a/src/jsdom/transformJsdom.ts b/src/jsdom/transformJsdom.ts
@@ -8,7 +8,7 @@ const filePrefixer = template(`
 
 const testPrefixer = template(
   `
-    console.log({ id: %%id%%, title: %%title%%, name: %%name%%, storyExport: %%storyExport%%, hasPlayFn: %%hasPlayFn%%});
+    console.log({ id: %%id%%, title: %%title%%, name: %%name%%, storyExport: %%storyExport%% });
     async () => {
       const Composed = await composeStory(%%storyExport%%, exports.default);
       const { container } = render(<Composed />);

diff --git a/src/playwright/transformPlaywright.test.ts b/src/playwright/transformPlaywright.test.ts
@@ -60,11 +60,9 @@ describe('Playwright', () => {
               });
             });
             return page.evaluate(({
-              id,
-              hasPlayFn
-            }) => __test(id, hasPlayFn), {
-              id: "foo-bar--a",
-              hasPlayFn: true
+              id
+            }) => __test(id), {
+              id: "foo-bar--a"
             });
           });
         });
@@ -96,11 +94,9 @@ describe('Playwright', () => {
               });
             });
             return page.evaluate(({
-              id,
-              hasPlayFn
-            }) => __test(id, hasPlayFn), {
-              id: "foo-bar--a",
-              hasPlayFn: false
+              id
+            }) => __test(id), {
+              id: "foo-bar--a"
             });
           });
         });
@@ -133,11 +129,9 @@ describe('Playwright', () => {
               });
             });
             return page.evaluate(({
-              id,
-              hasPlayFn
-            }) => __test(id, hasPlayFn), {
-              id: "example-header--a",
-              hasPlayFn: false
+              id
+            }) => __test(id), {
+              id: "example-header--a"
             });
           });
         });

diff --git a/src/playwright/transformPlaywright.ts b/src/playwright/transformPlaywright.ts
@@ -13,9 +13,8 @@ export const testPrefixer = template(
         page.evaluate(({ id, err }) => __throwError(id, err), { id: %%id%%, err: err.message });
       });
 
-      return page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
-        id: %%id%%,
-        hasPlayFn: %%hasPlayFn%%,
+      return page.evaluate(({ id }) => __test(id), {
+        id: %%id%%
       });
     }
   `,

diff --git a/src/playwright/transformPlaywrightJson.test.ts b/src/playwright/transformPlaywrightJson.test.ts
@@ -62,11 +62,9 @@ Object {
         });
       });
       return page.evaluate(({
-        id,
-        hasPlayFn
-      }) => __test(id, hasPlayFn), {
-        id: \\"example-header--logged-in\\",
-        hasPlayFn: false
+        id
+      }) => __test(id), {
+        id: \\"example-header--logged-in\\"
       });
     });
   });
@@ -82,11 +80,9 @@ Object {
         });
       });
       return page.evaluate(({
-        id,
-        hasPlayFn
-      }) => __test(id, hasPlayFn), {
-        id: \\"example-header--logged-out\\",
-        hasPlayFn: false
+        id
+      }) => __test(id), {
+        id: \\"example-header--logged-out\\"
       });
     });
   });
@@ -104,11 +100,9 @@ Object {
         });
       });
       return page.evaluate(({
-        id,
-        hasPlayFn
-      }) => __test(id, hasPlayFn), {
-        id: \\"example-page--logged-in\\",
-        hasPlayFn: false
+        id
+      }) => __test(id), {
+        id: \\"example-page--logged-in\\"
       });
     });
   });
@@ -164,11 +158,9 @@ Object {
         });
       });
       return page.evaluate(({
-        id,
-        hasPlayFn
-      }) => __test(id, hasPlayFn), {
-        id: \\"example-page--logged-in\\",
-        hasPlayFn: false
+        id
+      }) => __test(id), {
+        id: \\"example-page--logged-in\\"
       });
     });
   });

diff --git a/src/playwright/transformPlaywrightJson.ts b/src/playwright/transformPlaywrightJson.ts
@@ -13,7 +13,6 @@ const makeTest = (story: Story): t.Statement => {
     id: t.stringLiteral(story.id),
     // FIXME
     storyExport: t.identifier(story.id),
-    hasPlayFn: t.booleanLiteral(false),
   });
   const stmt = result[1] as t.ExpressionStatement;
   return t.expressionStatement(