diff --git a/packages/e2e/helpers/browser-login.ts b/packages/e2e/helpers/browser-login.ts index 591008936f5..7c00bc94689 100644 --- a/packages/e2e/helpers/browser-login.ts +++ b/packages/e2e/helpers/browser-login.ts @@ -1,3 +1,4 @@ +import {isVisibleWithin} from '../setup/browser.js' import {BROWSER_TIMEOUT} from '../setup/constants.js' import type {Page} from '@playwright/test' @@ -47,12 +48,12 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, ]).catch(() => {}) // If passkey page shown, navigate to password login - if (await differentMethodBtn.isVisible({timeout: BROWSER_TIMEOUT.short}).catch(() => false)) { + if (await isVisibleWithin(differentMethodBtn, BROWSER_TIMEOUT.short)) { await differentMethodBtn.click() await page.waitForTimeout(BROWSER_TIMEOUT.short) const continueWithPassword = page.locator('text=Continue with password') - if (await continueWithPassword.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) { + if (await isVisibleWithin(continueWithPassword, BROWSER_TIMEOUT.medium)) { await continueWithPassword.click() await page.waitForTimeout(BROWSER_TIMEOUT.short) } @@ -67,7 +68,7 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, await page.waitForTimeout(BROWSER_TIMEOUT.medium) try { const btn = page.locator('button[type="submit"]').first() - if (await btn.isVisible({timeout: BROWSER_TIMEOUT.long})) await btn.click() + if (await isVisibleWithin(btn, BROWSER_TIMEOUT.long)) await btn.click() // eslint-disable-next-line no-catch-all/no-catch-all } catch (_error) { // No confirmation page — expected diff --git a/packages/e2e/setup/app.ts b/packages/e2e/setup/app.ts index b9584911856..30499a2c3aa 100644 --- a/packages/e2e/setup/app.ts +++ b/packages/e2e/setup/app.ts @@ -1,6 +1,6 @@ /* eslint-disable no-restricted-imports, no-await-in-loop */ import {authFixture} from './auth.js' -import {navigateToDashboard, refreshIfPageError} from './browser.js' +import {getLastPageStatus, isVisibleWithin, navigateToDashboard, refreshIfPageError} from './browser.js' import {CLI_TIMEOUT, BROWSER_TIMEOUT} from './constants.js' import {updateTomlValues} from '@shopify/toml-patch' import * as toml from '@iarna/toml' @@ -9,6 +9,26 @@ import * as fs from 'fs' import type {CLIContext, CLIProcess, ExecResult} from './cli.js' import type {Page} from '@playwright/test' +/** + * Race the given promise builders. When the winner resolves, losers are + * cancelled via `AbortController.abort()` so their timers and `outputWaiters` + * entries inside `waitForOutput` are freed immediately rather than lingering + * until they hit their own timeout. Loser rejections are swallowed so they + * don't surface as unhandled promise rejections. + */ +async function raceWaiters(build: (signal: AbortSignal) => Promise[]): Promise { + const ctrl = new AbortController() + const promises = build(ctrl.signal) + promises.forEach((promise) => { + promise.catch(() => {}) + }) + try { + return await Promise.race(promises) + } finally { + ctrl.abort() + } +} + // --------------------------------------------------------------------------- // CLI helpers — thin wrappers around cli.exec() // --------------------------------------------------------------------------- @@ -27,20 +47,16 @@ export async function createApp(ctx: { const template = ctx.template ?? 'reactRouter' const packageManager = ctx.packageManager ?? (process.env.E2E_PACKAGE_MANAGER as 'npm' | 'yarn' | 'pnpm' | 'bun') ?? 'pnpm' - - const args = [ - '--name', - name, - '--path', - parentDir, - '--package-manager', - packageManager, - '--local', - '--template', - template, - ] + // reactRouter/remix both require a --flavor or they'll hang on the language + // prompt in non-interactive runs. Default to javascript when template needs + // it. For `--template none` (extension-only) flavor is ignored. + const flavor = ctx.flavor ?? (template === 'none' ? undefined : 'javascript') + + const args = ['--template', template] + if (flavor) args.push('--flavor', flavor) + args.push('--name', name, '--package-manager', packageManager, '--local') if (ctx.orgId) args.push('--organization-id', ctx.orgId) - if (ctx.flavor) args.push('--flavor', ctx.flavor) + args.push('--path', parentDir) const result = await cli.execCreateApp(args, { env: {FORCE_COLOR: '0'}, @@ -116,8 +132,9 @@ export async function generateExtension( flavor?: string }, ): Promise { - const args = ['app', 'generate', 'extension', '--name', ctx.name, '--path', ctx.appDir, '--template', ctx.template] + const args = ['app', 'generate', 'extension', '--template', ctx.template] if (ctx.flavor) args.push('--flavor', ctx.flavor) + args.push('--name', ctx.name, '--path', ctx.appDir) return ctx.cli.exec(args, {timeout: CLI_TIMEOUT.long}) } @@ -134,12 +151,13 @@ export async function deployApp( noBuild?: boolean }, ): Promise { - const args = ['app', 'deploy', '--path', ctx.appDir] - if (ctx.force ?? true) args.push('--force') - if (ctx.noBuild) args.push('--no-build') + const args = ['app', 'deploy'] if (ctx.version) args.push('--version', ctx.version) if (ctx.message) args.push('--message', ctx.message) if (ctx.config) args.push('--config', ctx.config) + if (ctx.force ?? true) args.push('--force') + if (ctx.noBuild) args.push('--no-build') + args.push('--path', ctx.appDir) return ctx.cli.exec(args, {timeout: CLI_TIMEOUT.long}) } @@ -152,7 +170,7 @@ export async function appInfo(ctx: CLIContext): Promise<{ entrySourceFilePath: string }[] }> { - const result = await ctx.cli.exec(['app', 'info', '--path', ctx.appDir, '--json']) + const result = await ctx.cli.exec(['app', 'info', '--json', '--path', ctx.appDir]) if (result.exitCode !== 0) { throw new Error(`app info failed (exit ${result.exitCode}):\nstdout: ${result.stdout}\nstderr: ${result.stderr}`) } @@ -168,25 +186,130 @@ export async function functionRun( inputPath: string }, ): Promise { - return ctx.cli.exec(['app', 'function', 'run', '--path', ctx.appDir, '--input', ctx.inputPath], { + return ctx.cli.exec(['app', 'function', 'run', '--input', ctx.inputPath, '--path', ctx.appDir], { timeout: CLI_TIMEOUT.short, }) } -export async function versionsList(ctx: CLIContext): Promise { - return ctx.cli.exec(['app', 'versions', 'list', '--path', ctx.appDir, '--json'], { - timeout: CLI_TIMEOUT.short, - }) +export async function versionsList( + ctx: CLIContext & { + config?: string + }, +): Promise { + const args = ['app', 'versions', 'list', '--json'] + if (ctx.config) args.push('--config', ctx.config) + args.push('--path', ctx.appDir) + return ctx.cli.exec(args, {timeout: CLI_TIMEOUT.short}) } +/** + * Run `app config link` to create a brand-new app on Shopify interactively. + * Answers the prompts: + * "Which organization is this work for?" → filter by orgId → Enter + * "Create this project as a new app on Shopify?" → Yes (default) + * "App name" → appName + * "Configuration file name" → skipped via `--config` flag + * + * Env overrides (via PTY spawn): + * CI=undefined — drop the key so prompts render. + * Fixture default is CI=1; Ink's `is-in-ci` + * treats `'CI' in env` as CI even when ''. + * In CI mode Ink suppresses prompt frames + * (only emitted on unmount), so waitForOutput + * hangs until the process is killed. + * SHOPIFY_CLI_NEVER_USE_PARTNERS_API=1 — skip Partners client in fetchOrganizations. + * Without this, fetchOrganizations iterates + * AppManagement AND Partners sequentially. + * Partners requires SHOPIFY_CLI_PARTNERS_TOKEN + * (not set in OAuth-auth'd tests) and hangs + * for minutes trying to authenticate. The e2e + * test org (161686155) lives in AppManagement. + */ export async function configLink( ctx: CLIContext & { - clientId: string + appName: string + orgId: string + configName?: string }, ): Promise { - return ctx.cli.exec(['app', 'config', 'link', '--path', ctx.appDir, '--client-id', ctx.clientId], { - timeout: CLI_TIMEOUT.medium, + const args = ['app', 'config', 'link'] + // Pass configName as --config flag. link.ts → loadConfigurationFileName skips + // the "Configuration file name" prompt when options.configName is set, which + // also side-steps a painful interactive quirk: that prompt uses + // `initialAnswer = remoteApp.title`, so any text we write would be appended + // to the app name rather than replacing it. + if (ctx.configName) args.push('--config', ctx.configName) + args.push('--path', ctx.appDir) + + const proc = await ctx.cli.spawn(args, { + env: { + CI: undefined, + SHOPIFY_CLI_NEVER_USE_PARTNERS_API: '1', + }, }) + + // Short sleep so Ink's useInput hooks attach before we start writing. + // Without this, an Enter press arrives mid-mount and a subsequent render can + // flip the prompt state unexpectedly (e.g. turning a select into search mode). + const settle = (ms = 50) => new Promise((resolve) => setTimeout(resolve, ms)) + + try { + // The first prompt is either the multi-org selector or — when the account + // has only one org, or none of the orgs have existing apps — we jump + // straight to `createAsNewAppPrompt`. Race all three; the loser + // waitForOutput calls are cancelled via AbortSignal so their timers and + // outputWaiter entries are freed immediately when the winner resolves. + const firstPrompt = await raceWaiters((signal) => [ + proc.waitForOutput('Which organization', {timeoutMs: CLI_TIMEOUT.medium, signal}).then(() => 'org' as const), + proc + .waitForOutput('Create this project as a new app', {timeoutMs: CLI_TIMEOUT.medium, signal}) + .then(() => 'create' as const), + proc.waitForOutput('App name', {timeoutMs: CLI_TIMEOUT.medium, signal}).then(() => 'appName' as const), + ]) + + if (firstPrompt === 'org') { + // Type the orgId to filter the autocomplete prompt to exactly one match. + // selectOrganizationPrompt's label includes `(${org.id})` when duplicate + // org names exist (which is true for the e2e test account), so substring + // matching on the numeric ID is unique. Avoids relying on MRU ordering. + await settle() + proc.ptyProcess.write(ctx.orgId) + await settle() + proc.sendKey('\r') + // After org selection the CLI fetches apps for the chosen org. If + // the org has existing apps → "Create this project" prompt. If it has + // zero apps → selectOrCreateApp skips straight to appNamePrompt. + const next = await raceWaiters((signal) => [ + proc + .waitForOutput('Create this project as a new app', {timeoutMs: CLI_TIMEOUT.medium, signal}) + .then(() => 'create' as const), + proc.waitForOutput('App name', {timeoutMs: CLI_TIMEOUT.medium, signal}).then(() => 'appName' as const), + ]) + if (next === 'create') { + await settle() + proc.sendKey('\r') + } + } else if (firstPrompt === 'create') { + await settle() + proc.sendKey('\r') + } + + // Wait for "App name" text prompt and submit the desired name. + // Important: Ink parses each PTY data event as ONE keypress. If we write + // "name\r" in one call, parseKeypress sees the whole string and treats + // it as text (not Enter), so the prompt never submits. We must write the + // text, wait for it to be consumed, then write \r separately. + await proc.waitForOutput('App name', CLI_TIMEOUT.medium) + await settle() + proc.ptyProcess.write(ctx.appName) + await settle() + proc.sendKey('\r') + + const exitCode = await proc.waitForExit(CLI_TIMEOUT.long) + return {exitCode, stdout: proc.getOutput(), stderr: ''} + } finally { + proc.kill() + } } // --------------------------------------------------------------------------- @@ -213,7 +336,7 @@ export async function findAppOnDevDashboard(page: Page, appName: string, orgId?: // Check for next page const nextLink = page.locator('a[href*="next_cursor"]').first() - if (!(await nextLink.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) break + if (!(await isVisibleWithin(nextLink, BROWSER_TIMEOUT.medium))) break const nextHref = await nextLink.getAttribute('href') if (!nextHref) break const nextUrl = nextHref.startsWith('http') ? nextHref : `https://dev.shopify.com${nextHref}` @@ -225,70 +348,61 @@ export async function findAppOnDevDashboard(page: Page, appName: string, orgId?: return null } -/** Delete an app from its dev dashboard settings page. Returns true if deleted, false if not. */ +/** + * Delete an app from its dev dashboard settings page. Returns true if deleted. + * + * Single attempt — caller owns the retry loop. + * + * Fail-fast on STILL_HAS_INSTALLS: the Delete button stays disabled while + * installs exist, so we throw to let the caller skip instead of spinning. + */ export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Promise { - // Step 1: Navigate to settings page + // Step 1: Navigate to the app's settings page. 404 → already deleted. 5xx → throw for retry. await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'}) await page.waitForTimeout(BROWSER_TIMEOUT.medium) - await refreshIfPageError(page) - - // Step 2: Wait for "Delete app" button to be enabled, then click (retry with error check) - const deleteAppBtn = page.locator('button:has-text("Delete app")').first() - for (let attempt = 1; attempt <= 3; attempt++) { - if (await refreshIfPageError(page)) continue - const isDisabled = await deleteAppBtn.getAttribute('disabled').catch(() => 'true') - if (!isDisabled) break + const gotoStatus = getLastPageStatus(page) + if (gotoStatus === 404) return true + if (gotoStatus !== undefined && gotoStatus >= 500) { + throw new Error(`Server error loading app settings page (status ${gotoStatus})`) + } + + // Step 2: Click "Delete app" to open the confirmation modal. + // Button can be below the fold, and takes ~1-2s to enable after uninstall (one reload covers propagation lag). + // If it stays disabled after reload, installs remain — fail fast for caller. + const deleteBtn = page.locator('button:has-text("Delete app")').first() + await deleteBtn.scrollIntoViewIfNeeded() + if (!(await deleteBtn.isEnabled())) { await page.reload({waitUntil: 'domcontentloaded'}) await page.waitForTimeout(BROWSER_TIMEOUT.medium) + await deleteBtn.scrollIntoViewIfNeeded() + if (!(await deleteBtn.isEnabled())) throw new Error('STILL_HAS_INSTALLS') } - - // Click the delete button — if it's not found, the page didn't load properly - const deleteClicked = await deleteAppBtn - .click({timeout: BROWSER_TIMEOUT.long}) - .then(() => true) - .catch(() => false) - if (!deleteClicked) return false + await deleteBtn.click({timeout: 2 * BROWSER_TIMEOUT.long}) await page.waitForTimeout(BROWSER_TIMEOUT.medium) - // Step 3: Click confirm "Delete" in the modal (retry step 2+3 if not visible) - // The dev dashboard modal has a submit button with class "critical" inside a form - const confirmAppBtn = page.locator('button.critical[type="submit"]') - for (let attempt = 1; attempt <= 3; attempt++) { - if (await confirmAppBtn.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) break - if (attempt === 3) return false - // Retry: re-click the delete button to reopen modal - await page.keyboard.press('Escape') + // Step 3: Some confirmation modals require typing "DELETE". Fill if the input is present. + const confirmInput = page.locator('input[type="text"]').last() + if (await isVisibleWithin(confirmInput, BROWSER_TIMEOUT.medium)) { + await confirmInput.fill('DELETE') await page.waitForTimeout(BROWSER_TIMEOUT.short) - const retryClicked = await deleteAppBtn - .click({timeout: BROWSER_TIMEOUT.long}) - .then(() => true) - .catch(() => false) - if (!retryClicked) return false - await page.waitForTimeout(BROWSER_TIMEOUT.medium) } - const urlBefore = page.url() - const confirmClicked = await confirmAppBtn - .click({timeout: BROWSER_TIMEOUT.long}) - .then(() => true) - .catch(() => false) - if (!confirmClicked) return false + // Step 4: Click the confirm button (second "Delete app" button on the page, inside the modal). + const confirmBtn = page.locator('button:has-text("Delete app")').last() + await confirmBtn.click({timeout: BROWSER_TIMEOUT.long}) + await page.waitForTimeout(BROWSER_TIMEOUT.medium) - // Wait for page to navigate away after deletion - try { - await page.waitForURL((url) => url.toString() !== urlBefore, {timeout: BROWSER_TIMEOUT.max}) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (_err) { - // URL didn't change — check if page error occurred during redirect - if (await refreshIfPageError(page)) { - // After refresh, 404 means the app was deleted (settings page no longer exists) - const bodyText = (await page.textContent('body')) ?? '' - if (bodyText.includes('404: Not Found')) return true - return false - } - await page.waitForTimeout(BROWSER_TIMEOUT.medium) + // Step 5: Reload the settings page to confirm deletion. + // Success → 404. + // Failure → same settings page. + await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'}) + await page.waitForTimeout(BROWSER_TIMEOUT.short) + const afterStatus = getLastPageStatus(page) + if (afterStatus === 404) return true + if (afterStatus !== undefined && afterStatus >= 500) { + throw new Error(`Server error verifying app deletion (status ${afterStatus})`) } - return page.url() !== urlBefore + return false } // --------------------------------------------------------------------------- diff --git a/packages/e2e/setup/browser.ts b/packages/e2e/setup/browser.ts index c6eb151f7ab..0683e5bf25d 100644 --- a/packages/e2e/setup/browser.ts +++ b/packages/e2e/setup/browser.ts @@ -1,6 +1,6 @@ import {cliFixture} from './cli.js' import {BROWSER_TIMEOUT} from './constants.js' -import {chromium, type Page} from '@playwright/test' +import {chromium, type Locator, type Page} from '@playwright/test' import * as fs from 'fs' // --------------------------------------------------------------------------- @@ -11,6 +11,33 @@ export interface BrowserContext { browserPage: Page } +// --------------------------------------------------------------------------- +// Main-frame response status tracking +// --------------------------------------------------------------------------- + +/** + * Records the HTTP status of the most recent main-frame document response per + * page. Populated by a `response` listener attached when the page is created + * (see fixture below). Read via `getLastPageStatus(page)`. + * + * A WeakMap lets the entry be garbage-collected with the page — no manual + * cleanup required. + */ +const lastMainFrameStatus = new WeakMap() + +export function trackMainFrameStatus(page: Page): void { + page.on('response', (response) => { + if (response.frame() !== page.mainFrame()) return + if (response.request().resourceType() !== 'document') return + lastMainFrameStatus.set(page, response.status()) + }) +} + +/** Get the HTTP status of the last main-frame document response on `page`. */ +export function getLastPageStatus(page: Page): number | undefined { + return lastMainFrameStatus.get(page) +} + // --------------------------------------------------------------------------- // Fixture // --------------------------------------------------------------------------- @@ -40,6 +67,7 @@ export const browserFixture = cliFixture.extend<{}, {browserPage: Page}>({ context.setDefaultTimeout(BROWSER_TIMEOUT.max) context.setDefaultNavigationTimeout(BROWSER_TIMEOUT.max) const page = await context.newPage() + trackMainFrameStatus(page) await use(page) await browser.close() }, @@ -52,13 +80,32 @@ export const browserFixture = cliFixture.extend<{}, {browserPage: Page}>({ // --------------------------------------------------------------------------- /** - * Check if the current page shows a server error (500, 502). If so, refresh and return true. - * Call this in retry loops when a selector isn't found — the page might be an error page. + * Wait up to `timeoutMs` for `locator` to become visible. Returns `true` if it + * appears in time, `false` on timeout or any other locator error (detached + * element, closed context, etc.). + * + * Use this instead of `locator.isVisible({timeout})` — that API does not + * actually wait in modern Playwright; it returns the current visibility state. + * This helper uses `waitFor({state: 'visible', timeout})` for true polling. + */ +export async function isVisibleWithin(locator: Locator, timeoutMs: number): Promise { + return locator + .waitFor({state: 'visible', timeout: timeoutMs}) + .then(() => true) + .catch(() => false) +} + +/** + * If the most recent main-frame response was a 5xx server error, reload the + * page and return true. Otherwise return false. Call this in retry loops when + * a selector isn't found — the page might be an error page. + * + * Uses the HTTP status captured by `trackMainFrameStatus` rather than scraping + * body text, so it works regardless of how an error page is rendered. */ export async function refreshIfPageError(page: Page): Promise { - const pageText = (await page.textContent('body')) ?? '' - if (!pageText.includes('Internal Server Error') && !pageText.includes('502 Bad Gateway')) return false - // if (process.env.DEBUG === '1') process.stdout.write(' page refreshing...\n') + const status = getLastPageStatus(page) + if (status === undefined || status < 500) return false await page.reload({waitUntil: 'domcontentloaded'}) await page.waitForTimeout(BROWSER_TIMEOUT.medium) return true @@ -83,7 +130,7 @@ export async function navigateToDashboard( // Handle account picker (skip if email not provided) if (ctx.email) { const accountButton = browserPage.locator(`text=${ctx.email}`).first() - if (await accountButton.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) { + if (await isVisibleWithin(accountButton, BROWSER_TIMEOUT.medium)) { await accountButton.click() await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium) } diff --git a/packages/e2e/setup/cli.ts b/packages/e2e/setup/cli.ts index 8ce425ebd18..ad1793d171f 100644 --- a/packages/e2e/setup/cli.ts +++ b/packages/e2e/setup/cli.ts @@ -11,9 +11,19 @@ export interface ExecResult { exitCode: number } +export interface WaitForOutputOptions { + timeoutMs?: number + /** Cancel the wait early — frees the timer and removes the waiter entry. */ + signal?: AbortSignal +} + export interface SpawnedProcess { - /** Wait for a string to appear in the PTY output */ - waitForOutput(text: string, timeoutMs?: number): Promise + /** + * Wait for a string to appear in the PTY output. + * Pass a number for the legacy positional `timeoutMs` form, or an options + * object to also supply an `AbortSignal` for cancellation. + */ + waitForOutput(text: string, opts?: number | WaitForOutputOptions): Promise /** Send a single key to the PTY */ sendKey(key: string): void /** Send a line of text followed by Enter */ @@ -132,13 +142,13 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ process.stdout.write(data) } - // Check if any waiters are satisfied (check both raw and stripped output) + // Check if any waiters are satisfied (check both raw and stripped + // output). resolve() removes the waiter from outputWaiters internally, + // so we iterate over a snapshot to avoid index shifting during the loop. const stripped = stripAnsi(output) - for (let idx = outputWaiters.length - 1; idx >= 0; idx--) { - const waiter = outputWaiters[idx] - if (waiter && (stripped.includes(waiter.text) || output.includes(waiter.text))) { + for (const waiter of [...outputWaiters]) { + if (stripped.includes(waiter.text) || output.includes(waiter.text)) { waiter.resolve() - outputWaiters.splice(idx, 1) } } }) @@ -151,26 +161,39 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ if (exitResolve) { exitResolve(code) } - // Reject any remaining output waiters - for (const waiter of outputWaiters) { + // Reject any remaining output waiters. reject() removes each waiter + // from outputWaiters, so iterate over a snapshot to avoid skipping. + for (const waiter of [...outputWaiters]) { waiter.reject(new Error(`Process exited (code ${code}) while waiting for output: "${waiter.text}"`)) } - outputWaiters.length = 0 }) const spawned: SpawnedProcess = { ptyProcess, - waitForOutput(text: string, timeoutMs = CLI_TIMEOUT.medium) { + waitForOutput(text: string, opts: number | WaitForOutputOptions = {}) { + const {timeoutMs = CLI_TIMEOUT.medium, signal} = + typeof opts === 'number' ? {timeoutMs: opts, signal: undefined} : opts + // Check if already in output (raw or stripped) if (stripAnsi(output).includes(text) || output.includes(text)) { return Promise.resolve() } + if (signal?.aborted) { + return Promise.reject(new Error(`Cancelled waiting for output: "${text}"`)) + } return new Promise((resolve, reject) => { + // eslint-disable-next-line prefer-const + let waiter: {text: string; resolve: () => void; reject: (err: Error) => void} + + const removeWaiter = () => { + const idx = outputWaiters.indexOf(waiter) + if (idx >= 0) outputWaiters.splice(idx, 1) + } + const timer = setTimeout(() => { - const waiterIdx = outputWaiters.findIndex((waiter) => waiter.text === text) - if (waiterIdx >= 0) outputWaiters.splice(waiterIdx, 1) + removeWaiter() reject( new Error( `Timed out after ${timeoutMs}ms waiting for output: "${text}"\n\nCaptured output:\n${stripAnsi( @@ -180,17 +203,32 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ ) }, timeoutMs) - outputWaiters.push({ + waiter = { text, resolve: () => { clearTimeout(timer) + removeWaiter() resolve() }, reject: (err) => { clearTimeout(timer) + removeWaiter() reject(err) }, - }) + } + outputWaiters.push(waiter) + + if (signal) { + signal.addEventListener( + 'abort', + () => { + clearTimeout(timer) + removeWaiter() + reject(new Error(`Cancelled waiting for output: "${text}"`)) + }, + {once: true}, + ) + } }) }, diff --git a/packages/e2e/setup/global-auth.ts b/packages/e2e/setup/global-auth.ts index a6669aa7f29..739cebccbd5 100644 --- a/packages/e2e/setup/global-auth.ts +++ b/packages/e2e/setup/global-auth.ts @@ -6,6 +6,7 @@ */ /* eslint-disable no-restricted-imports */ +import {isVisibleWithin} from './browser.js' import {directories, executables, globalLog} from './env.js' import {CLI_TIMEOUT, BROWSER_TIMEOUT} from './constants.js' import {stripAnsi} from '../helpers/strip-ansi.js' @@ -154,7 +155,7 @@ async function visitAndHandleAccountPicker(page: Page, url: string, email: strin await page.waitForTimeout(BROWSER_TIMEOUT.medium) if (isAccountsShopifyUrl(page.url())) { const accountButton = page.locator(`text=${email}`).first() - if (await accountButton.isVisible({timeout: BROWSER_TIMEOUT.long}).catch(() => false)) { + if (await isVisibleWithin(accountButton, BROWSER_TIMEOUT.long)) { await accountButton.click() await page.waitForTimeout(BROWSER_TIMEOUT.medium) } diff --git a/packages/e2e/setup/store.ts b/packages/e2e/setup/store.ts index 51507c98e0c..6b5f033de5a 100644 --- a/packages/e2e/setup/store.ts +++ b/packages/e2e/setup/store.ts @@ -1,5 +1,6 @@ /* eslint-disable no-await-in-loop */ import {appTestFixture} from './app.js' +import {isVisibleWithin} from './browser.js' import {BROWSER_TIMEOUT} from './constants.js' import {createLogger, e2eSection} from './env.js' import * as fs from 'fs' @@ -9,7 +10,7 @@ import type {Page} from '@playwright/test' const log = createLogger('browser') // --------------------------------------------------------------------------- -// Dev store management — create and delete stores via browser automation +// Dev store provisioning — create new stores via browser automation // --------------------------------------------------------------------------- /** Generate a unique store name for a worker. */ @@ -63,7 +64,7 @@ export async function createDevStore( if (browserPage.url().includes('accounts.shopify.com') && email) { const accountButton = browserPage.locator(`text=${email}`).first() - if (await accountButton.isVisible({timeout: BROWSER_TIMEOUT.long}).catch(() => false)) { + if (await isVisibleWithin(accountButton, BROWSER_TIMEOUT.long)) { await accountButton.click() await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium) } @@ -73,7 +74,7 @@ export async function createDevStore( // Wait for the store creation form to load — retry if page didn't render const nameInput = browserPage.locator('s-internal-text-field[label="Store name"]').locator('input') for (let attempt = 1; attempt <= 3; attempt++) { - if (await nameInput.isVisible({timeout: BROWSER_TIMEOUT.max}).catch(() => false)) break + if (await isVisibleWithin(nameInput, BROWSER_TIMEOUT.max)) break log.log(ctx, `store form not loaded (attempt ${attempt}/3), url=${browserPage.url()}`) await browserPage.goto(`https://admin.shopify.com/store-create/organization/${orgId}`, { @@ -122,72 +123,71 @@ export async function createDevStore( } // --------------------------------------------------------------------------- -// Store admin browser actions — uninstall apps and delete stores +// Store admin browser actions — uninstall apps, delete stores, and helpers // --------------------------------------------------------------------------- /** Dismiss the Dev Console panel if visible on a store admin page. */ export async function dismissDevConsole(page: Page): Promise { const devConsole = page.locator('h2:has-text("Dev Console")') - if (!(await devConsole.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) return + if (!(await isVisibleWithin(devConsole, BROWSER_TIMEOUT.medium))) return const hideBtn = page.locator('button[aria-label="hide"]').first() - if (await hideBtn.isVisible({timeout: BROWSER_TIMEOUT.short}).catch(() => false)) { + if (await isVisibleWithin(hideBtn, BROWSER_TIMEOUT.short)) { await hideBtn.click() await page.waitForTimeout(BROWSER_TIMEOUT.short) } } /** - * Uninstall an app from a store's admin settings page. - * Navigates to /settings/apps, finds the app by name, uninstalls it, and verifies removal. - * Returns true if app is confirmed gone, false if still present. + * Uninstall an app from a store's admin settings/apps page. Returns true if confirmed uninstalled. + * + * Single attempt — caller owns the retry loop. */ export async function uninstallAppFromStore(page: Page, storeSlug: string, appName: string): Promise { + // Step 1: Navigate to the store's settings/apps page. await page.goto(`https://admin.shopify.com/store/${storeSlug}/settings/apps`, { waitUntil: 'domcontentloaded', }) await page.waitForTimeout(BROWSER_TIMEOUT.long) await dismissDevConsole(page) + // Step 2: Find the app by name. Not visible → already uninstalled. const appSpan = page.locator(`span:has-text("${appName}"):not([class*="Polaris"])`).first() - if (!(await appSpan.isVisible({timeout: BROWSER_TIMEOUT.long}).catch(() => false))) return true + if (!(await isVisibleWithin(appSpan, BROWSER_TIMEOUT.long))) return true - // Click ⋯ menu → Uninstall → Confirm + // Step 3: Open the ⋯ menu and click Uninstall. await appSpan.locator('xpath=./following::button[1]').click() await page.waitForTimeout(BROWSER_TIMEOUT.short) - const uninstallOpt = page.locator('text=Uninstall').last() - if (!(await uninstallOpt.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) return false + if (!(await isVisibleWithin(uninstallOpt, BROWSER_TIMEOUT.medium))) return false await uninstallOpt.click() await page.waitForTimeout(BROWSER_TIMEOUT.medium) + // Step 4: Confirm the uninstall in the modal (if one appears). const confirmBtn = page.locator('button:has-text("Uninstall"), button:has-text("Confirm")').last() - if (await confirmBtn.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) { + if (await isVisibleWithin(confirmBtn, BROWSER_TIMEOUT.medium)) { await confirmBtn.click() await page.waitForTimeout(BROWSER_TIMEOUT.medium) } - // Verify: check the specific app is gone - const check = async () => - page - .locator(`span:has-text("${appName}"):not([class*="Polaris"])`) - .first() - .isVisible({timeout: BROWSER_TIMEOUT.medium}) - .catch(() => false) - - if (!(await check())) return true - - // If still visible — reload and check again + // Step 5: Reload the page to confirm the app is no longer listed. + // Success → app is not on listed on the page. + // Failure → app is still listed. await page.reload({waitUntil: 'domcontentloaded'}) await page.waitForTimeout(BROWSER_TIMEOUT.long) - return !(await check()) + await dismissDevConsole(page) + const stillVisible = await isVisibleWithin( + page.locator(`span:has-text("${appName}"):not([class*="Polaris"])`).first(), + BROWSER_TIMEOUT.medium, + ) + return !stillVisible } /** Check if the current page shows the empty state (zero apps installed). Caller must navigate first. */ export async function isStoreAppsEmpty(page: Page): Promise { // "Add apps to your store" empty state is the definitive zero-apps signal const emptyState = page.locator('text=Add apps to your store') - if (await emptyState.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) return true + if (await isVisibleWithin(emptyState, BROWSER_TIMEOUT.medium)) return true // Fallback: no "More actions" menu buttons in the app list const menuButtons = await page.locator('.Polaris-Layout__Section button[aria-label="More actions"]').all() @@ -195,76 +195,61 @@ export async function isStoreAppsEmpty(page: Page): Promise { } /** - * Delete a store via the admin settings plan page. - * Caller must verify no apps are installed before calling. - * Returns true if deleted, false if not. + * Delete a store via the admin /settings/plan/cancel page. Returns true if deleted. + * + * Gate: store must have zero apps installed. + * Caller should verify via `isStoreAppsEmpty` and skip this call if apps remain, + * otherwise step 4 will exhaust its micro-retry (Delete button never enables) and throw. + * + * Single attempt — caller owns the retry loop. */ export async function deleteStore(page: Page, storeSlug: string): Promise { - // Step 1: Navigate to plan page and click delete button to open modal (retry navigation on failure) - const planUrl = `https://admin.shopify.com/store/${storeSlug}/settings/plan` - const deleteButton = page.locator('s-internal-button[tone="critical"]').locator('button') - - for (let attempt = 1; attempt <= 3; attempt++) { - try { - await page.goto(planUrl, {waitUntil: 'domcontentloaded'}) - await page.waitForTimeout(BROWSER_TIMEOUT.long) - // If redirected to access_account, store is already deleted - if (page.url().includes('access_account')) return true - await deleteButton.click({timeout: BROWSER_TIMEOUT.long}) - break - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (_err) { - if (attempt === 3) return false - } + // Step 1: Navigate to /settings/plan/cancel (auto-opens the "Review before deleting store" modal). + const cancelUrl = `https://admin.shopify.com/store/${storeSlug}/settings/plan/cancel` + await page.goto(cancelUrl, {waitUntil: 'domcontentloaded'}) + + // Step 2: Race — modal renders (normal) vs. redirect to /access_account (already deleted). + // The redirect can fire post-DOMContentLoaded, so a URL check right after goto is too early. + const modal = page.locator('.Polaris-Modal-Dialog__Modal:has-text("Review before deleting store")') + const checkbox = modal.locator('input[type="checkbox"]') + try { + await Promise.race([ + checkbox.waitFor({state: 'visible', timeout: BROWSER_TIMEOUT.max}), + page.waitForURL(/access_account/, {timeout: BROWSER_TIMEOUT.max}), + ]) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // Both branches timed out — fall through so outer retry can decide. } - await page.waitForTimeout(BROWSER_TIMEOUT.medium) + if (page.url().includes('access_account')) return true - const modal = page.locator('.Polaris-Modal-Dialog__Modal') + // Step 3: Check the confirmation checkbox (enables the Delete button). + await checkbox.check() - // Step 2: Check the confirmation checkbox (retry step 1+2 if fails) - for (let attempt = 1; attempt <= 3; attempt++) { - const checkbox = modal.locator('input[type="checkbox"]') - if (await checkbox.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) { - await checkbox.check({force: true}) - await page.waitForTimeout(BROWSER_TIMEOUT.short) - break - } - if (attempt === 3) return false - // Retry: close modal and re-click delete - await page.keyboard.press('Escape') - await page.waitForTimeout(BROWSER_TIMEOUT.short) - await deleteButton.click({timeout: BROWSER_TIMEOUT.max}) - await page.waitForTimeout(BROWSER_TIMEOUT.medium) - } - - // Step 3: Click confirm (retry step 2+3 if button is still disabled) + // Step 4: Wait for the Delete button to enable, then click. + // Micro-retry for flaky checkbox state — different concern from caller's retry loop. const confirmButton = modal.locator('button:has-text("Delete store")') - for (let attempt = 1; attempt <= 3; attempt++) { - const isDisabled = await confirmButton - .evaluate((el) => el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('disabled')) - .catch(() => true) - if (!isDisabled) break - if (attempt === 3) return false - // Retry: re-check the checkbox - const checkbox = modal.locator('input[type="checkbox"]') - await checkbox.check({force: true}) + for (let i = 1; i <= 3; i++) { + if (await confirmButton.isEnabled().catch(() => false)) break + if (i === 3) throw new Error('Confirm button still disabled') + await checkbox.check() await page.waitForTimeout(BROWSER_TIMEOUT.short) } + await confirmButton.click() - const confirmClicked = await confirmButton - .click({force: true}) - .then(() => true) - .catch(() => false) - if (!confirmClicked) return false - - // Verify: URL reaching access_account confirms store is deleted + // Step 5: Wait for the delete POST to finish before reloading. try { - await page.waitForURL(/access_account/, {timeout: BROWSER_TIMEOUT.max}) - return true + await page.waitForLoadState('networkidle', {timeout: BROWSER_TIMEOUT.max}) // eslint-disable-next-line no-catch-all/no-catch-all - } catch (_err) { - return false + } catch { + // networkidle can miss on busy pages — fall through to reload anyway. } + + // Step 6: Reload /settings/plan/cancel to confirm deletion. + // Success → redirect to /access_account. + // Failure → still on /settings/plan/cancel + await page.reload({waitUntil: 'domcontentloaded'}) + return page.url().includes('access_account') } // --------------------------------------------------------------------------- diff --git a/packages/e2e/setup/teardown.ts b/packages/e2e/setup/teardown.ts index ea28db8737f..2166564ff7d 100644 --- a/packages/e2e/setup/teardown.ts +++ b/packages/e2e/setup/teardown.ts @@ -18,15 +18,12 @@ interface TeardownCtx { } /** - * Best-effort per-test teardown with escalating retry. - * - * Each phase is independent — failure never prevents later phases. - * Escalation: retry same step → go back one step and retry both. + * Best-effort per-test teardown. Each phase retries up to 3 times. * * App + store flow: * Phase 1: uninstall app from store admin - * Phase 2: delete store (escalates to phase 1 on failure) - * Phase 3: delete app from dev dashboard (always runs) + * Phase 2: delete store (skipped if phase 1 failed) + * Phase 3: delete app from dev dashboard (skipped if phase 1 failed) * * App-only flow: * Phase 3 only @@ -63,66 +60,79 @@ export async function teardownAll(ctx: TeardownCtx): Promise { // Phase 2: Delete store log.log(wCtx, 'deleting store') let storeDeleted = false - for (let attempt = 1; attempt <= 3; attempt++) { - try { - // Navigate to apps page to check state - await page.goto(`https://admin.shopify.com/store/${storeSlug}/settings/apps`, { - waitUntil: 'domcontentloaded', - }) - await page.waitForTimeout(BROWSER_TIMEOUT.long) + let safeToDelete = false - // Store already deleted? - if (page.url().includes('access_account')) { - log.log(wCtx, 'store already deleted') - storeDeleted = true - break - } + // Gate: confirm the store has zero apps before attempting delete store. + try { + await page.goto(`https://admin.shopify.com/store/${storeSlug}/settings/apps`, { + waitUntil: 'domcontentloaded', + }) + await page.waitForTimeout(BROWSER_TIMEOUT.long) + if (page.url().includes('access_account')) { + log.log(wCtx, 'store already deleted') + storeDeleted = true + } else { await dismissDevConsole(page) - - // Apps still installed? Reload once in case page is stale + // Reload once in case the page is stale (Phase 1 just uninstalled) if (!(await isStoreAppsEmpty(page))) { await page.reload({waitUntil: 'domcontentloaded'}) await page.waitForTimeout(BROWSER_TIMEOUT.long) await dismissDevConsole(page) - if (!(await isStoreAppsEmpty(page))) { - log.error(wCtx, 'store still has apps installed, skipping delete') - break - } } + if (await isStoreAppsEmpty(page)) { + safeToDelete = true + } else { + log.error(wCtx, 'store has apps installed, skipping delete') + } + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (err) { + log.error(wCtx, `store empty state unclear, skipping delete: ${err instanceof Error ? err.message : err}`) + } - // Safe to delete - const deleted = await deleteStore(page, storeSlug) - if (deleted) { - log.log(wCtx, 'store deleted') - storeDeleted = true - break + if (safeToDelete) { + for (let attempt = 1; attempt <= 3; attempt++) { + try { + if (await deleteStore(page, storeSlug)) { + log.log(wCtx, 'store deleted') + storeDeleted = true + break + } + log.log(wCtx, `(${attempt}/3) store deletion failed`) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (err) { + log.log(wCtx, `(${attempt}/3) store deletion failed: ${err instanceof Error ? err.message : err}`) } - log.log(wCtx, `(${attempt}/3) store deletion failed`) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (err) { - log.log(wCtx, `(${attempt}/3) store deletion failed: ${err instanceof Error ? err.message : err}`) + } + if (!storeDeleted) { + log.error(wCtx, 'store deletion failed after 3 attempts') } } - if (!storeDeleted) { - log.error(wCtx, 'store deletion failed after 3 attempts') + + // Gate: confirm the store has zero apps before attempting delete app. + if (!uninstalled) { + log.log(wCtx, 'skipping app delete — uninstall failed, run `pnpm test:e2e-cleanup-apps` after') + return } } - // Phase 3: Delete app from dev dashboard — ALWAYS runs + // Phase 3: Delete app from dev dashboard e2eSection(wCtx, `Teardown: app ${ctx.appName}`) log.log(wCtx, 'deleting app') let appDeleted = false + let stillHasInstalls = false for (let attempt = 1; attempt <= 3; attempt++) { try { const appUrl = await findAppOnDevDashboard(page, ctx.appName, ctx.orgId) if (!appUrl) { - // Check if the page actually loaded — a 500/502 page also returns null + // null could mean "app not in the list" OR "pagination ended on a stuck error page" + // — findAppOnDevDashboard's refresh-on-error doesn't cover every iteration. + // Detect and retry so we don't misclassify an error page as "already deleted". if (await refreshIfPageError(page)) { log.log(wCtx, `page error, refreshing...`) continue } - // Page loaded correctly and app wasn't found = already deleted log.log(wCtx, 'app already deleted') appDeleted = true break @@ -137,10 +147,17 @@ export async function teardownAll(ctx: TeardownCtx): Promise { log.log(wCtx, `(${attempt}/3) app deletion failed`) // eslint-disable-next-line no-catch-all/no-catch-all } catch (err) { + // Fail fast: Delete button stays disabled while installs exist — retries won't help. + // cleanup-apps.ts reaps the orphan. + if (err instanceof Error && err.message === 'STILL_HAS_INSTALLS') { + log.log(wCtx, 'app delete skipped — still has installs, run `pnpm test:e2e-cleanup-apps` after') + stillHasInstalls = true + break + } log.log(wCtx, `(${attempt}/3) app deletion failed: ${err instanceof Error ? err.message : err}`) } } - if (!appDeleted) { + if (!appDeleted && !stillHasInstalls) { log.error(wCtx, 'app deletion failed after 3 attempts') } } diff --git a/packages/e2e/tests/app-deploy.spec.ts b/packages/e2e/tests/app-deploy.spec.ts index a02bb93a923..e3505b805c9 100644 --- a/packages/e2e/tests/app-deploy.spec.ts +++ b/packages/e2e/tests/app-deploy.spec.ts @@ -41,8 +41,8 @@ test.describe('App deploy', () => { expect(listResult.exitCode, `versions list failed:\n${listOutput}`).toBe(0) expect(listOutput).toContain(versionTag) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, diff --git a/packages/e2e/tests/app-dev-server.spec.ts b/packages/e2e/tests/app-dev-server.spec.ts index 846b987d259..ae8305debb9 100644 --- a/packages/e2e/tests/app-dev-server.spec.ts +++ b/packages/e2e/tests/app-dev-server.spec.ts @@ -49,8 +49,8 @@ test.describe('App dev server', () => { const exitCode = await dev.waitForExit(CLI_TIMEOUT.short) expect(exitCode, `dev exited with non-zero code. Output:\n${dev.getOutput()}`).toBe(0) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, diff --git a/packages/e2e/tests/app-scaffold.spec.ts b/packages/e2e/tests/app-scaffold.spec.ts index 790d370c169..a53d9d66229 100644 --- a/packages/e2e/tests/app-scaffold.spec.ts +++ b/packages/e2e/tests/app-scaffold.spec.ts @@ -45,8 +45,8 @@ test.describe('App scaffold', () => { `buildApp failed:\nstdout: ${buildResult.stdout}\nstderr: ${buildResult.stderr}`, ).toBe(0) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, @@ -80,8 +80,8 @@ test.describe('App scaffold', () => { expect(fs.existsSync(initResult.appDir)).toBe(true) expect(fs.existsSync(path.join(initResult.appDir, 'shopify.app.toml'))).toBe(true) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, @@ -138,8 +138,8 @@ test.describe('App scaffold', () => { `buildApp failed:\nstdout: ${buildResult.stdout}\nstderr: ${buildResult.stderr}`, ).toBe(0) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts index c3f53de191e..b16c9c6d5b3 100644 --- a/packages/e2e/tests/dev-hot-reload.spec.ts +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -83,8 +83,8 @@ test.describe('Dev hot reload', () => { proc.kill() } } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, @@ -135,8 +135,8 @@ test.describe('Dev hot reload', () => { proc.kill() } } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, @@ -193,8 +193,8 @@ test.describe('Dev hot reload', () => { proc.kill() } } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, diff --git a/packages/e2e/tests/multi-config-dev.spec.ts b/packages/e2e/tests/multi-config-dev.spec.ts index cbb8efd6c22..7084cdb471d 100644 --- a/packages/e2e/tests/multi-config-dev.spec.ts +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -84,8 +84,8 @@ include_config_on_deploy = true proc.kill() } } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, @@ -156,8 +156,8 @@ api_version = "2025-01" proc.kill() } } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, diff --git a/packages/e2e/tests/toml-config-invalid.spec.ts b/packages/e2e/tests/toml-config-invalid.spec.ts index 7d8cbbf05e3..f0bee68d644 100644 --- a/packages/e2e/tests/toml-config-invalid.spec.ts +++ b/packages/e2e/tests/toml-config-invalid.spec.ts @@ -36,8 +36,8 @@ test.describe('TOML config invalid', () => { expect(result.exitCode, `expected deploy to fail for ${label}, but it succeeded:\n${output}`).not.toBe(0) expect(output.toLowerCase(), `expected error output for ${label}:\n${output}`).toMatch(/error|invalid|failed/) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(appDir, {recursive: true, force: true}) } } diff --git a/packages/e2e/tests/toml-config.spec.ts b/packages/e2e/tests/toml-config.spec.ts index f3ff4f854c9..8d1fa0c65cd 100644 --- a/packages/e2e/tests/toml-config.spec.ts +++ b/packages/e2e/tests/toml-config.spec.ts @@ -35,8 +35,8 @@ test.describe('TOML config regression', () => { const output = result.stdout + result.stderr expect(result.exitCode, `deploy failed:\n${output}`).toBe(0) } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage, @@ -77,8 +77,8 @@ test.describe('TOML config regression', () => { proc.kill() } } finally { - // E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward. - if (!process.env.E2E_SKIP_CLEANUP) { + // E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward. + if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ browserPage,