From 9830af52643d58c2c9d7e226f6df3ef2f96fe73c Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 24 Nov 2023 16:01:35 +0100 Subject: [PATCH 1/2] fix sync issues between tests --- playwright/jest-setup.js | 9 +- .../__snapshots__/transformCsf.test.ts.snap | 15 ---- src/playwright/hooks.ts | 15 ---- src/playwright/transformPlaywright.test.ts | 50 ----------- src/playwright/transformPlaywright.ts | 7 -- .../transformPlaywrightJson.test.ts | 50 ----------- src/setup-page-script.ts | 88 +++++++++++++------ src/setup-page.ts | 15 ++-- 8 files changed, 68 insertions(+), 181 deletions(-) diff --git a/playwright/jest-setup.js b/playwright/jest-setup.js index 76c234e..7e31956 100644 --- a/playwright/jest-setup.js +++ b/playwright/jest-setup.js @@ -1,10 +1,4 @@ -const { - getTestRunnerConfig, - setPreVisit, - setPostVisit, - setupPage, - throwUncaughtPageError, -} = require('../dist'); +const { getTestRunnerConfig, setPreVisit, setPostVisit, setupPage } = require('../dist'); const testRunnerConfig = getTestRunnerConfig(process.env.STORYBOOK_CONFIG_DIR); if (testRunnerConfig) { @@ -27,5 +21,4 @@ if (testRunnerConfig) { // If the transformed tests need a dependency, it has to be globally available // in order to work both in default (file transformation) and stories/index.json mode. globalThis.__sbSetupPage = setupPage; -globalThis.__sbThrowUncaughtPageError = throwUncaughtPageError; globalThis.__sbCollectCoverage = process.env.STORYBOOK_COLLECT_COVERAGE === 'true'; diff --git a/src/csf/__snapshots__/transformCsf.test.ts.snap b/src/csf/__snapshots__/transformCsf.test.ts.snap index e83f453..e04d4dd 100644 --- a/src/csf/__snapshots__/transformCsf.test.ts.snap +++ b/src/csf/__snapshots__/transformCsf.test.ts.snap @@ -26,10 +26,6 @@ if (!require.main) { title: "Button", name: "Primary" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -51,7 +47,6 @@ if (!require.main) { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -98,10 +93,6 @@ if (!require.main) { title: "Button", name: "Primary" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -123,7 +114,6 @@ if (!require.main) { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -156,10 +146,6 @@ if (!require.main) { title: "Button", name: "Primary" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -181,7 +167,6 @@ if (!require.main) { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { diff --git a/src/playwright/hooks.ts b/src/playwright/hooks.ts index 72512c7..abc9845 100644 --- a/src/playwright/hooks.ts +++ b/src/playwright/hooks.ts @@ -81,18 +81,3 @@ export const waitForPageReady = async (page: Page) => { await page.waitForLoadState('networkidle'); await page.evaluate(() => document.fonts.ready); }; - -export const throwUncaughtPageError = (err: Error, context: TestContext) => { - const storybookUrl = process.env.REFERENCE_URL || process.env.TARGET_URL; - const storyUrl = `${storybookUrl}?path=/story/${context.id}`; - - const errorMessage = dedent` - An uncaught error occurred when visiting the following story. - Please access the link and check the logs in the browser: - ${storyUrl} - - Message: - ${err.stack || err.message}\n`; - - throw new Error(errorMessage); -}; diff --git a/src/playwright/transformPlaywright.test.ts b/src/playwright/transformPlaywright.test.ts index 0299971..2e43f5f 100644 --- a/src/playwright/transformPlaywright.test.ts +++ b/src/playwright/transformPlaywright.test.ts @@ -66,10 +66,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "A" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -91,7 +87,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -116,10 +111,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "B" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -141,7 +132,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -184,10 +174,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "B" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -209,7 +195,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -252,10 +237,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "A" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -277,7 +258,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -302,10 +282,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "B" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -327,7 +303,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -379,10 +354,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "B" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -404,7 +375,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -429,10 +399,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "C" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -454,7 +420,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -511,10 +476,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "A" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -536,7 +497,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -577,10 +537,6 @@ describe('Playwright', () => { title: "Example/foo/bar", name: "A" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -602,7 +558,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -643,10 +598,6 @@ describe('Playwright', () => { title: "Example/Header", name: "A" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -668,7 +619,6 @@ describe('Playwright', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { diff --git a/src/playwright/transformPlaywright.ts b/src/playwright/transformPlaywright.ts index 5aed7df..f74699a 100644 --- a/src/playwright/transformPlaywright.ts +++ b/src/playwright/transformPlaywright.ts @@ -20,12 +20,6 @@ export const testPrefixer: TestPrefixer = (context) => { async () => { const testFn = async() => { const context = { id: %%id%%, title: %%title%%, name: %%name%% }; - - const onPageError = (err) => { - globalThis.__sbThrowUncaughtPageError(err, context); - } - - page.on('pageerror', onPageError); if(globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); @@ -48,7 +42,6 @@ export const testPrefixer: TestPrefixer = (context) => { await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; diff --git a/src/playwright/transformPlaywrightJson.test.ts b/src/playwright/transformPlaywrightJson.test.ts index 72659c4..2d16d73 100644 --- a/src/playwright/transformPlaywrightJson.test.ts +++ b/src/playwright/transformPlaywrightJson.test.ts @@ -50,10 +50,6 @@ describe('Playwright Json', () => { title: "Example/Header", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -75,7 +71,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -100,10 +95,6 @@ describe('Playwright Json', () => { title: "Example/Header", name: "Logged Out" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -125,7 +116,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -152,10 +142,6 @@ describe('Playwright Json', () => { title: "Example/Page", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -177,7 +163,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -251,10 +236,6 @@ describe('Playwright Json', () => { title: "Example/Header", name: "Logged Out" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -276,7 +257,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -303,10 +283,6 @@ describe('Playwright Json', () => { title: "Example/Page", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -328,7 +304,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -378,10 +353,6 @@ describe('Playwright Json', () => { title: "Example/Page", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -403,7 +374,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -474,10 +444,6 @@ describe('Playwright Json', () => { title: "Example/Header", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -499,7 +465,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -524,10 +489,6 @@ describe('Playwright Json', () => { title: "Example/Header", name: "Logged Out" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -549,7 +510,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -576,10 +536,6 @@ describe('Playwright Json', () => { title: "Example/Page", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -601,7 +557,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { @@ -660,10 +615,6 @@ describe('Playwright Json', () => { title: "Example/Page", name: "Logged In" }; - const onPageError = err => { - globalThis.__sbThrowUncaughtPageError(err, context); - }; - page.on('pageerror', onPageError); if (globalThis.__sbPreVisit) { await globalThis.__sbPreVisit(page, context); } @@ -685,7 +636,6 @@ describe('Playwright Json', () => { } await jestPlaywright.saveCoverage(page); } - page.off('pageerror', onPageError); return result; }; try { diff --git a/src/setup-page-script.ts b/src/setup-page-script.ts index bba39fe..3c8fbaa 100644 --- a/src/setup-page-script.ts +++ b/src/setup-page-script.ts @@ -6,14 +6,19 @@ */ // All of these variables will be replaced once this file is processed. -const referenceURL: string | undefined = '{{referenceURL}}'; -const targetURL: string = '{{targetURL}}'; +const storybookUrl: string = '{{storybookUrl}}'; const testRunnerVersion: string = '{{testRunnerVersion}}'; const failOnConsole: string = '{{failOnConsole}}'; const renderedEvent: string = '{{renderedEvent}}'; const viewMode: string = '{{viewMode}}'; const debugPrintLimit = parseInt('{{debugPrintLimit}}', 10); +// Type definitions for globals +declare global { + // this is defined in setup-page.ts and can be used for logging from the browser to node, helpful for debugging + var logToPage: (message: string) => void; +} + // Type definitions for function parameters and return types type Colorizer = (message: string) => string; @@ -189,7 +194,7 @@ class StorybookTestRunnerError extends Error { constructor(storyId: string, errorMessage: string, logs: string[] = []) { super(errorMessage); this.name = 'StorybookTestRunnerError'; - const storyUrl = `${referenceURL ?? targetURL}?path=/story/${storyId}`; + const storyUrl = `${storybookUrl}?path=/story/${storyId}`; const finalStoryUrl = `${storyUrl}&addonPanel=storybook/interactions/panel`; const separator = '\n\n--------------------------------------------------'; const extraLogs = @@ -294,33 +299,58 @@ async function __test(storyId: string): Promise { spyOnConsole(method as ConsoleMethod, color(method)); }); - return new Promise((resolve, reject) => { - channel.on(renderedEvent, () => { - if (hasErrors) { - return reject(new StorybookTestRunnerError(storyId, 'Browser console errors', logs)); - } - return resolve(document.getElementById('root')); + const cleanup = (_listeners: Record) => { + Object.entries(_listeners).forEach(([eventName, listener]) => { + channel.off(eventName, listener); }); - channel.on('storyUnchanged', () => resolve(document.getElementById('root'))); - channel.on('storyErrored', ({ description }: { description: string }) => - reject(new StorybookTestRunnerError(storyId, description, logs)) - ); - channel.on('storyThrewException', (error: Error) => - reject(new StorybookTestRunnerError(storyId, error.message, logs)) - ); - channel.on('playFunctionThrewException', (error: Error) => - reject(new StorybookTestRunnerError(storyId, error.message, logs)) - ); - channel.on('storyMissing', (id: string) => { - if (id === storyId) { - reject( - new StorybookTestRunnerError( - storyId, - 'The story was missing when trying to access it.', - logs - ) - ); - } + }; + + return new Promise((resolve, reject) => { + const listeners = { + [renderedEvent]: () => { + cleanup(listeners); + if (hasErrors) { + return reject(new StorybookTestRunnerError(storyId, 'Browser console errors', logs)); + } + return resolve(document.getElementById('root')); + }, + + storyUnchanged: () => { + cleanup(listeners); + resolve(document.getElementById('root')); + }, + + storyErrored: ({ description }: { description: string }) => { + cleanup(listeners); + reject(new StorybookTestRunnerError(storyId, description, logs)); + }, + + storyThrewException: (error: Error) => { + cleanup(listeners); + reject(new StorybookTestRunnerError(storyId, error.message, logs)); + }, + + playFunctionThrewException: (error: Error) => { + cleanup(listeners); + reject(new StorybookTestRunnerError(storyId, error.message, logs)); + }, + + storyMissing: (id: string) => { + cleanup(listeners); + if (id === storyId) { + reject( + new StorybookTestRunnerError( + storyId, + 'The story was missing when trying to access it.', + logs + ) + ); + } + }, + }; + + Object.entries(listeners).forEach(([eventName, listener]) => { + channel.on(eventName, listener); }); channel.emit('setCurrentStory', { storyId, viewMode }); diff --git a/src/setup-page.ts b/src/setup-page.ts index c73a5c7..ea2bf64 100644 --- a/src/setup-page.ts +++ b/src/setup-page.ts @@ -60,15 +60,16 @@ export const setupPage = async (page: Page, browserContext: BrowserContext) => { // if we ever want to log something from the browser to node await page.exposeBinding('logToPage', (_, message) => console.log(message)); + const finalStorybookUrl = referenceURL ?? targetURL ?? ''; const scriptLocation = require.resolve(path.join(__dirname, 'setup-page-script.mjs')); - // read the contents of setup-page-scripts.ts and replace the referenceUrl + + // read the content of setup-page-script.mjs and replace the placeholders with the actual values const content = (await readFile(scriptLocation, 'utf-8')) - .replace('{{targetURL}}', targetURL ?? '') - .replace('{{referenceUrl}}', referenceURL ?? '') - .replace('{{failOnConsole}}', failOnConsole ?? 'false') - .replace('{{renderedEvent}}', renderedEvent) - .replace('{{testRunnerVersion}}', testRunnerVersion) - .replace('{{debugPrintLimit}}', debugPrintLimit.toString()); + .replaceAll('{{storybookUrl}}', finalStorybookUrl) + .replaceAll('{{failOnConsole}}', failOnConsole ?? 'false') + .replaceAll('{{renderedEvent}}', renderedEvent) + .replaceAll('{{testRunnerVersion}}', testRunnerVersion) + .replaceAll('{{debugPrintLimit}}', debugPrintLimit.toString()); await page.addScriptTag({ content }); }; From 6dbd144a6ed6ab369af03fecf889363ba6583c41 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 24 Nov 2023 16:04:21 +0100 Subject: [PATCH 2/2] fix lint --- src/playwright/hooks.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/playwright/hooks.ts b/src/playwright/hooks.ts index abc9845..f3c2bbd 100644 --- a/src/playwright/hooks.ts +++ b/src/playwright/hooks.ts @@ -1,6 +1,5 @@ import type { BrowserContext, Page } from 'playwright'; import type { StoryContext } from '@storybook/csf'; -import dedent from 'ts-dedent'; export type TestContext = { id: string;