From 355e6acf0da6fb5c4c3115b2d958060d444efab7 Mon Sep 17 00:00:00 2001 From: Jace Browning Date: Thu, 27 Mar 2025 11:41:25 -0400 Subject: [PATCH] Make tests fail when there are console errors (#6015) * Add a test to confirm console errors fail tests * Check for console errors on all browsers * Ignore error impacting lots of tests * Add more detected errors to the allowlist for now --- .env.development | 3 + e2e/playwright/lib/console-error-whitelist.ts | 40 +++++++++++ e2e/playwright/projects.spec.ts | 3 + e2e/playwright/regression-tests.spec.ts | 13 ++++ e2e/playwright/test-utils.ts | 72 +++++++++---------- 5 files changed, 91 insertions(+), 40 deletions(-) diff --git a/.env.development b/.env.development index 449af161a..eea7f7a5e 100644 --- a/.env.development +++ b/.env.development @@ -1,5 +1,6 @@ NODE_ENV=development DEV=true + VITE_KC_API_WS_MODELING_URL=wss://api.dev.zoo.dev/ws/modeling/commands VITE_KC_API_BASE_URL=https://api.dev.zoo.dev VITE_KC_SITE_BASE_URL=https://dev.zoo.dev @@ -8,3 +9,5 @@ VITE_KC_SKIP_AUTH=false VITE_KC_CONNECTION_TIMEOUT_MS=5000 # ONLY add your token in .env.development.local if you want to skip auth, otherwise this token takes precedence! #VITE_KC_DEV_TOKEN="your token from dev.zoo.dev should go in .env.development.local" + +FAIL_ON_CONSOLE_ERRORS=true diff --git a/e2e/playwright/lib/console-error-whitelist.ts b/e2e/playwright/lib/console-error-whitelist.ts index 176c335ef..b3bd369f6 100644 --- a/e2e/playwright/lib/console-error-whitelist.ts +++ b/e2e/playwright/lib/console-error-whitelist.ts @@ -257,6 +257,46 @@ export const isErrorWhitelisted = (exception: Error) => { project: 'Google Chrome', foundInSpec: 'e2e/playwright/testing-settings.spec.ts', }, + // TODO: fix this error in the code + { + name: 'TypeError', + message: "Cannot read properties of undefined (reading 'length')", + stack: '', + project: 'Google Chrome', + foundInSpec: '', // many tests are impacted by this error + }, + // TODO: fix this error in the code + { + name: 'ReferenceError', + message: '_testUtils is not defined', + stack: '', + project: 'Google Chrome', + foundInSpec: 'e2e/playwright/snapshot-tests.spec.ts', + }, + // TODO: fix this error in the code + { + name: 'TypeError', + message: 'Failed to fetch', + stack: '', + project: 'Google Chrome', + foundInSpec: 'e2e/playwright/snapshot-tests.spec.ts', + }, + // TODO: fix this error in the code + { + name: 'ReferenceError', + message: 'originalCode is not defined', + stack: '', + project: 'Google Chrome', + foundInSpec: 'e2e/playwright/onboarding-tests.spec.ts', + }, + // TODO: fix this error in the code + { + name: 'ReferenceError', + message: 'createNewVariableCheckbox is not defined', + stack: '', + project: 'Google Chrome', + foundInSpec: 'e2e/playwright/testing-constraints.spec.ts', + }, ] const cleanString = (str: string) => str.replace(/[`"]/g, '') diff --git a/e2e/playwright/projects.spec.ts b/e2e/playwright/projects.spec.ts index 8f8a6b66e..c8d57b112 100644 --- a/e2e/playwright/projects.spec.ts +++ b/e2e/playwright/projects.spec.ts @@ -473,6 +473,9 @@ test.describe('Can export from electron app', () => { if (!tronApp) { fail() } + if (runningOnWindows()) { + test.fixme(orRunWhenFullSuiteEnabled()) + } await context.folderSetupFn(async (dir) => { const bracketDir = path.join(dir, 'bracket') diff --git a/e2e/playwright/regression-tests.spec.ts b/e2e/playwright/regression-tests.spec.ts index a938b9fb5..deb17303f 100644 --- a/e2e/playwright/regression-tests.spec.ts +++ b/e2e/playwright/regression-tests.spec.ts @@ -778,6 +778,19 @@ plane002 = offsetPlane(XZ, offset = -2 * x)` await editor.expectEditor.not.toContain(`plane002`) }) }) + + test.fail( + 'Console errors cause tests to fail', + async ({ page, homePage }) => { + const u = await getUtils(page) + await homePage.goToModelingScene() + await u.openAndClearDebugPanel() + + await page.getByTestId('custom-cmd-input').fill('foobar') + await page.getByTestId('custom-cmd-send-button').scrollIntoViewIfNeeded() + await page.getByTestId('custom-cmd-send-button').click() + } + ) }) async function clickExportButton(page: Page) { diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts index 6f266ad4d..aae8a922c 100644 --- a/e2e/playwright/test-utils.ts +++ b/e2e/playwright/test-utils.ts @@ -935,47 +935,39 @@ export async function setup( } function failOnConsoleErrors(page: Page, testInfo?: TestInfo) { - // enabled for chrome for now - if (page.context().browser()?.browserType().name() === 'chromium') { - // No idea wtf exception is - page.on('pageerror', (exception: any) => { - if (isErrorWhitelisted(exception)) { - return - } + page.on('pageerror', (exception: any) => { + if (isErrorWhitelisted(exception)) { + return + } + // Only disable this environment variable if you want to collect console errors + if (process.env.FAIL_ON_CONSOLE_ERRORS !== 'false') { + // Use expect to prevent page from closing and not cleaning up + expect(`An error was detected in the console: \r\n message:${exception.message} \r\n name:${exception.name} \r\n stack:${exception.stack} - // only set this env var to false if you want to collect console errors - // This can be configured in the GH workflow. This should be set to true by default (we want tests to fail when - // unwhitelisted console errors are detected). - if (process.env.FAIL_ON_CONSOLE_ERRORS === 'true') { - // Fail when running on CI and FAIL_ON_CONSOLE_ERRORS is set - // use expect to prevent page from closing and not cleaning up - expect(`An error was detected in the console: \r\n message:${exception.message} \r\n name:${exception.name} \r\n stack:${exception.stack} - - *Either fix the console error or add it to the whitelist defined in ./lib/console-error-whitelist.ts (if the error can be safely ignored) - `).toEqual('Console error detected') - } else { - // the (test-results/exceptions.txt) file will be uploaded as part of an upload artifact in GH - fsp - .appendFile( - './test-results/exceptions.txt', - [ - '~~~', - `triggered_by_test:${ - testInfo?.file + ' ' + (testInfo?.title || ' ') - }`, - `name:${exception.name}`, - `message:${exception.message}`, - `stack:${exception.stack}`, - `project:${testInfo?.project.name}`, - '~~~', - ].join('\n') - ) - .catch((err) => { - console.error(err) - }) - } - }) - } + *Either fix the console error or add it to the whitelist defined in ./lib/console-error-whitelist.ts (if the error can be safely ignored) + `).toEqual('Console error detected') + } else { + // Add errors to `test-results/exceptions.txt` as a test artifact + fsp + .appendFile( + './test-results/exceptions.txt', + [ + '~~~', + `triggered_by_test:${ + testInfo?.file + ' ' + (testInfo?.title || ' ') + }`, + `name:${exception.name}`, + `message:${exception.message}`, + `stack:${exception.stack}`, + `project:${testInfo?.project.name}`, + '~~~', + ].join('\n') + ) + .catch((err) => { + console.error(err) + }) + } + }) } export async function isOutOfViewInScrollContainer( element: Locator,