From 388b650fb639edbefc681a359b416a9e2991bda4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 19 Oct 2026 15:45:33 +0200 Subject: [PATCH 1/2] Parallelize legacy E2E store cleanup --- docs/cli/testing-strategy.md | 2 + packages/e2e/README.md | 13 + packages/e2e/playwright.config.ts | 2 + packages/e2e/scripts/cleanup-stores.ts | 493 +++++++++++++------ packages/e2e/setup/persistent-stores.ts | 28 ++ packages/e2e/setup/store.ts | 31 +- packages/e2e/tests/persistent-stores.spec.ts | 47 ++ 7 files changed, 466 insertions(+), 150 deletions(-) create mode 100644 packages/e2e/README.md create mode 100644 packages/e2e/setup/persistent-stores.ts create mode 100644 packages/e2e/tests/persistent-stores.spec.ts diff --git a/docs/cli/testing-strategy.md b/docs/cli/testing-strategy.md index a8c4ed6d1c6..11d079cbdf8 100644 --- a/docs/cli/testing-strategy.md +++ b/docs/cli/testing-strategy.md @@ -95,6 +95,8 @@ For Vitest problems, see [our troubleshooting page](troubleshooting.md) or the [ End-to-end tests live under `packages/e2e` and are implemented using [Playwright](https://playwright.dev/). They test full user journeys by invoking the CLI and verifying outputs. Run them with `pnpm test:e2e`. +See the [E2E setup guide](../../packages/e2e/README.md) for legacy resource cleanup. + ## GitHub Actions Use [the PR workflow](../../.github/workflows/tests-pr.yml) and [CLI pre-submit CI guide](../../.agents/skills/cli-pre-submit-ci/SKILL.md) to choose local checks and generated files. Follow the check requirements of the active automation task. diff --git a/packages/e2e/README.md b/packages/e2e/README.md new file mode 100644 index 00000000000..7d90ce63432 --- /dev/null +++ b/packages/e2e/README.md @@ -0,0 +1,13 @@ +# E2E tests + +Run local tests without Shopify credentials: + +```sh +pnpm --filter e2e exec playwright test --project local +``` + +## Old store cleanup + +`cleanup-stores.ts` remains available for old temporary stores. It excludes the `no-delete-e2e-` prefix and all stores configured in `E2E_STORE_FQDNS` or `E2E_STORE_FQDN`. Full and delete modes use five browser workers and the existing `store delete` behavior. Force mode sends API deletion requests with twenty workers and skips app checks. Confirmed deletions and requests still awaiting confirmation are reported separately. Force deletion can leave app installation records that prevent app deletion until the backend clears them. + +Use `--list` first and select a narrow `--pattern`. These cleanup commands do not run as part of the persistent-store test lifecycle. diff --git a/packages/e2e/playwright.config.ts b/packages/e2e/playwright.config.ts index c4e6e148c8f..f213aa9980a 100644 --- a/packages/e2e/playwright.config.ts +++ b/packages/e2e/playwright.config.ts @@ -31,6 +31,7 @@ export default defineConfig({ 'tests/smoke-pty.spec.ts', 'tests/fixture-toml.spec.ts', 'tests/auth-diagnostics.spec.ts', + 'tests/persistent-stores.spec.ts', ], }, { @@ -45,6 +46,7 @@ export default defineConfig({ 'tests/smoke-pty.spec.ts', 'tests/fixture-toml.spec.ts', 'tests/auth-diagnostics.spec.ts', + 'tests/persistent-stores.spec.ts', ], dependencies: ['remote-auth'], }, diff --git a/packages/e2e/scripts/cleanup-stores.ts b/packages/e2e/scripts/cleanup-stores.ts index 61108caca04..51002c491b2 100644 --- a/packages/e2e/scripts/cleanup-stores.ts +++ b/packages/e2e/scripts/cleanup-stores.ts @@ -3,13 +3,14 @@ /** * E2E Store Cleanup Utility * - * Finds and deletes leftover E2E dev stores from the Dev Dashboard. + * Finds and deletes leftover E2E dev stores with bounded concurrency. * Stores are matched by the "e2e-w" prefix in their name (default). * * Usage: * pnpm --filter e2e exec tsx scripts/cleanup-stores.ts # Full: uninstall apps + delete stores * pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --list # List stores with app counts * pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --delete # Delete only stores with 0 apps installed + * pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --force # Delete stores via API, without checking installed apps * pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --headed # Show browser window * pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --pattern X # Match stores containing "X" (default: "e2e-w") * @@ -28,14 +29,15 @@ import {chromium} from '@playwright/test' import {BROWSER_TIMEOUT, CLI_TIMEOUT} from '../setup/constants.js' import {deleteDevStoreWithCli, dismissDevConsole, isStoreAppsEmpty} from '../setup/store.js' import {executables} from '../setup/env.js' +import {isProtectedStore, readPersistentStores} from '../setup/persistent-stores.js' import {refreshIfPageError, trackMainFrameStatus} from '../setup/browser.js' import {completeLogin} from '../helpers/browser-login.js' import {addLoadtestHeader} from '../helpers/loadtest-header.js' +import {DeleteAppDevelopmentStore} from '../../store/dist/cli/api/graphql/business-platform-organizations/generated/delete_app_development_store.js' import { - ListAppDevStores, - type ListAppDevStoresQuery, -} from '../../app/dist/cli/api/graphql/business-platform-organizations/generated/list_app_dev_stores.js' -import {businessPlatformOrganizationsRequestDoc} from '../../cli-kit/dist/public/node/api/business-platform.js' + businessPlatformOrganizationsRequest, + businessPlatformOrganizationsRequestDoc, +} from '../../cli-kit/dist/public/node/api/business-platform.js' import {ensureAuthenticatedBusinessPlatform} from '../../cli-kit/dist/public/node/session.js' import {extractHost} from '../../cli-kit/dist/public/common/url.js' import {execa} from 'execa' @@ -57,12 +59,19 @@ if ( // Core cleanup logic // --------------------------------------------------------------------------- -export type CleanupStoresMode = 'full' | 'list' | 'delete' +export type CleanupStoresMode = 'full' | 'list' | 'delete' | 'force' + +type CleanupOutcome = 'deleted' | 'requested' | 'skipped' | 'failed' + +const CLEANUP_WORKER_COUNT = 5 +// Force mode is pure API calls (no browser pages), so it can run much wider. +const FORCE_DELETE_WORKER_COUNT = 20 const MODE_LABELS: Record = { - full: 'Uninstall apps + Delete stores', + full: 'Uninstall apps + Request store deletion', list: 'List only', - delete: 'Delete empty stores only', + delete: 'Request deletion of empty stores only', + force: 'Request deletion via API, without checking installed apps', } export interface CleanupStoresOptions { @@ -121,6 +130,7 @@ export async function cleanupStores(opts: CleanupStoresOptions = {}): Promise !isProtectedStore(store.fqdn, protectedStores)) + const protectedCount = discoveredStores.length - stores.length + if (protectedCount > 0) console.log(`[cleanup-stores] Excluded ${protectedCount} persistent store(s).`) console.log(`[cleanup-stores] Found ${stores.length} store(s) matching pattern "${pattern}"`) console.log('') @@ -180,105 +193,255 @@ export async function cleanupStores(opts: CleanupStoresOptions = {}): Promise = {deleted: 0, requested: 0, skipped: 0, failed: 0} + for (const outcome of outcomes) { + stats[outcome]++ + } - for (let i = 0; i < stores.length; i++) { - const store = stores[i]! - const tag = `[cleanup-stores] [${i + 1}/${stores.length}]` - const storeStart = Date.now() + // Summary + const parts = [`${stats.deleted} deletions confirmed`] + if (stats.requested > 0) parts.push(`${stats.requested} deletion requests accepted (confirmation pending)`) + if (stats.skipped > 0) parts.push(`${stats.skipped} skipped`) + if (stats.failed > 0) parts.push(`${stats.failed} failed`) + const totalElapsed = ((Date.now() - totalStart) / 1000).toFixed(1) + console.log('') + console.log(`[cleanup-stores] Complete: ${parts.join(', ')} (${totalElapsed}s total)`) + if (stats.failed > 0) process.exitCode = 1 + } finally { + await browser.close() + } +} - console.log(`${tag} ${store.name}`) +async function cleanupStoresInParallel(opts: { + dashboardPage: Page + mode: CleanupStoresMode + stores: StoreInfo[] + orgId: string +}): Promise { + const {dashboardPage, mode, stores, orgId} = opts + const outcomes = new Array(stores.length) + const workerCount = Math.min(CLEANUP_WORKER_COUNT, stores.length) + let nextStoreIndex = 0 + + const workers = await Promise.allSettled( + Array.from({length: workerCount}, async (_, workerIndex) => { + const workerPage = await dashboardPage.context().newPage() + trackMainFrameStatus(workerPage) try { - const storeSlug = store.fqdn.replace('.myshopify.com', '') + // eslint-disable-next-line no-constant-condition + while (true) { + const storeIndex = nextStoreIndex++ + if (storeIndex >= stores.length) break + + outcomes[storeIndex] = await cleanupStore({ + page: workerPage, + mode, + store: stores[storeIndex]!, + orgId, + workerNumber: workerIndex + 1, + storeNumber: storeIndex + 1, + foundCount: stores.length, + }) + } + } finally { + await workerPage.close() + } + }), + ) - // Navigate to apps settings page once - await page.goto(`https://admin.shopify.com/store/${storeSlug}/settings/apps`, { - waitUntil: 'domcontentloaded', - }) - await page.waitForTimeout(BROWSER_TIMEOUT.long) - await dismissDevConsole(page) - - // Wait for page to settle: either the empty state or at least one app menu button - const emptyState = page.locator('text=Add apps to your store') - const firstMenuBtn = page.locator('.Polaris-Layout__Section button[aria-label="More actions"]').first() - await Promise.race([ - emptyState.waitFor({state: 'visible', timeout: BROWSER_TIMEOUT.max}).catch(() => {}), - firstMenuBtn.waitFor({state: 'visible', timeout: BROWSER_TIMEOUT.max}).catch(() => {}), - ]) - - // Gate: confirm zero apps before attempting delete. - let safeToDelete = false + const failures = workers.filter((worker): worker is PromiseRejectedResult => worker.status === 'rejected') + if (failures.length > 0) + throw new AggregateError( + failures.map((worker) => worker.reason), + 'Store cleanup workers failed', + ) + return outcomes +} + +async function cleanupStore(opts: { + page: Page + mode: CleanupStoresMode + store: StoreInfo + orgId: string + workerNumber: number + storeNumber: number + foundCount: number +}): Promise { + const {page, mode, store, orgId, workerNumber, storeNumber, foundCount} = opts + const tag = `[cleanup-stores] [worker ${workerNumber}] [${storeNumber}/${foundCount}] ${store.name}` + const storeStart = Date.now() + let outcome: CleanupOutcome = 'failed' + + console.log(`${tag}: Starting`) + + try { + const storeSlug = store.fqdn.replace('.myshopify.com', '') + + // Navigate to apps settings page once + await page.goto(`https://admin.shopify.com/store/${storeSlug}/settings/apps`, { + waitUntil: 'domcontentloaded', + }) + await page.waitForTimeout(BROWSER_TIMEOUT.long) + await dismissDevConsole(page) + + // Wait for page to settle: either the empty state or at least one app menu button + const emptyState = page.locator('text=Add apps to your store') + const firstMenuBtn = page.locator('.Polaris-Layout__Section button[aria-label="More actions"]').first() + await Promise.race([ + emptyState.waitFor({state: 'visible', timeout: BROWSER_TIMEOUT.max}).catch(() => {}), + firstMenuBtn.waitFor({state: 'visible', timeout: BROWSER_TIMEOUT.max}).catch(() => {}), + ]) + + // Gate: confirm zero apps before attempting delete. + let safeToDelete = false + if (await isStoreAppsEmpty(page)) { + console.log(`${tag}: No apps installed (empty state confirmed)`) + safeToDelete = true + } else { + const appMenuButtons = await page.locator('.Polaris-Layout__Section button[aria-label="More actions"]').all() + console.log(`${tag}: ${appMenuButtons.length || '?'} app(s) installed`) + + if (mode === 'delete') { + console.log(`${tag}: Skipped (still has apps)`) + outcome = 'skipped' + } else { + // Full mode: uninstall all apps, then re-gate. + console.log(`${tag}: Uninstalling apps...`) + await uninstallAllAppsFromStore(page, tag) if (await isStoreAppsEmpty(page)) { - console.log(' No apps installed (empty state confirmed)') + console.log(`${tag}: Apps uninstalled (empty state confirmed)`) safeToDelete = true } else { - const appMenuButtons = await page.locator('.Polaris-Layout__Section button[aria-label="More actions"]').all() - console.log(` ${appMenuButtons.length || '?'} app(s) installed`) - - if (mode === 'delete') { - console.log(' Skipped (still has apps)') - skipped++ - } else { - // Full mode: uninstall all apps, then re-gate. - console.log(' Uninstalling apps...') - await uninstallAllAppsFromStore(page) - if (await isStoreAppsEmpty(page)) { - console.log(' Apps uninstalled (empty state confirmed)') - safeToDelete = true - } else { - console.warn(' Apps may still be installed (empty state not confirmed) — skipping delete') - skipped++ - } - } + console.warn(`${tag}: Apps may still be installed (empty state not confirmed) — skipping delete`) + outcome = 'skipped' } + } + } - if (safeToDelete) { - console.log(' Deleting store...') - let deletionRequested = false - for (let attempt = 1; attempt <= 3; attempt++) { - try { - const deletionConfirmed = await deleteDevStoreWithCli({cli: cleanupCli, storeFqdn: store.fqdn, orgId}) - console.log(deletionConfirmed ? ' Deletion confirmed by CLI' : ' Deletion requested with CLI') - deletionRequested = true - break - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (err) { - console.log(` (${attempt}/3) deletion failed: ${err instanceof Error ? err.message : err}`) - } - } - if (deletionRequested) { - succeeded++ - } else { - console.warn(' Failed after 3 attempts') - failed++ - } + if (safeToDelete) { + console.log(`${tag}: Deleting store...`) + for (let attempt = 1; attempt <= 3; attempt++) { + try { + const confirmed = await deleteDevStoreWithCli({cli: cleanupCli, storeFqdn: store.fqdn, orgId}) + console.log(`${tag}: ${confirmed ? 'Deletion confirmed' : 'Deletion requested; confirmation is pending'}`) + outcome = confirmed ? 'deleted' : 'requested' + break + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (err) { + console.log(`${tag}: (${attempt}/3) deletion failed: ${err instanceof Error ? err.message : err}`) } - } catch (err) { - const msg = err instanceof Error ? err.message : String(err) - console.warn(` Failed: ${msg}`) - failed++ } - - const storeElapsed = ((Date.now() - storeStart) / 1000).toFixed(1) - console.log(` (${storeElapsed}s)`) - console.log('') + if (outcome === 'failed') { + console.warn(`${tag}: Failed after 3 attempts`) + } } + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + console.warn(`${tag}: Failed: ${msg}`) + outcome = 'failed' + } - // Summary - const parts = [`${succeeded} succeeded`] - if (skipped > 0) parts.push(`${skipped} skipped`) - if (failed > 0) parts.push(`${failed} failed`) - const totalElapsed = ((Date.now() - totalStart) / 1000).toFixed(1) - console.log('') - console.log(`[cleanup-stores] Complete: ${parts.join(', ')} (${totalElapsed}s total)`) - if (failed > 0) process.exitCode = 1 - } finally { - await browser.close() + const storeElapsed = ((Date.now() - storeStart) / 1000).toFixed(1) + console.log(`${tag}: ${outcome} (${storeElapsed}s)`) + return outcome +} + +/** + * Force mode: request deletion for every store through the Business Platform API. + * + * This skips the per-store admin visit, the app check, the CLI subprocess, and the CLI's + * deletion-confirmation polling — deletions are requested and left to complete asynchronously. + * Stores that still have apps installed will leave those apps undeletable in the Dev Dashboard + * until their install records clear. + */ +async function forceDeleteStoresInParallel(opts: {stores: StoreInfo[]; orgId: string}): Promise { + const {stores, orgId} = opts + const token = await ensureAuthenticatedBusinessPlatform([], {noPrompt: true}) + const outcomes = new Array(stores.length) + const workerCount = Math.min(FORCE_DELETE_WORKER_COUNT, stores.length) + let nextStoreIndex = 0 + + const workers = await Promise.allSettled( + Array.from({length: workerCount}, async (_, workerIndex) => { + // eslint-disable-next-line no-constant-condition + while (true) { + const storeIndex = nextStoreIndex++ + if (storeIndex >= stores.length) break + + outcomes[storeIndex] = await forceDeleteStore({ + store: stores[storeIndex]!, + orgId, + token, + workerNumber: workerIndex + 1, + storeNumber: storeIndex + 1, + foundCount: stores.length, + }) + } + }), + ) + + const failures = workers.filter((worker): worker is PromiseRejectedResult => worker.status === 'rejected') + if (failures.length > 0) + throw new AggregateError( + failures.map((worker) => worker.reason), + 'Store cleanup workers failed', + ) + return outcomes +} + +async function forceDeleteStore(opts: { + store: StoreInfo + orgId: string + token: string + workerNumber: number + storeNumber: number + foundCount: number +}): Promise { + const {store, orgId, token, workerNumber, storeNumber, foundCount} = opts + const tag = `[cleanup-stores] [worker ${workerNumber}] [${storeNumber}/${foundCount}] ${store.name}` + const storeStart = Date.now() + let outcome: CleanupOutcome = 'failed' + + for (let attempt = 1; attempt <= 3; attempt++) { + try { + const result = await businessPlatformOrganizationsRequestDoc({ + query: DeleteAppDevelopmentStore, + token, + organizationId: orgId, + variables: {storeFqdn: store.fqdn}, + unauthorizedHandler: { + type: 'token_refresh', + handler: async () => ({token: await ensureAuthenticatedBusinessPlatform([], {noPrompt: true})}), + }, + }) + + const deletion = result.deleteAppDevelopmentStore + if (!deletion) throw new Error('Unexpected empty response from deleteAppDevelopmentStore') + const userErrors = deletion.userErrors ?? [] + if (userErrors.length > 0) throw new Error(userErrors.map((error) => error.message).join(', ')) + if (deletion.success !== true) throw new Error('Deletion was not accepted') + + outcome = 'requested' + break + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (err) { + console.log(`${tag}: (${attempt}/3) deletion failed: ${err instanceof Error ? err.message : err}`) + if (attempt < 3) await new Promise((resolve) => setTimeout(resolve, BROWSER_TIMEOUT.medium)) + } } + + const storeElapsed = ((Date.now() - storeStart) / 1000).toFixed(1) + const summary = outcome === 'requested' ? 'Deletion requested' : 'Failed after 3 attempts' + console.log(`${tag}: ${summary} (${storeElapsed}s)`) + return outcome } // --------------------------------------------------------------------------- @@ -311,46 +474,97 @@ async function findStores(page: Page, opts: FindStoresOptions): Promise { console.log('[cleanup-stores] Discovering stores via Business Platform API...') const token = await ensureAuthenticatedBusinessPlatform([], {noPrompt: true}) - const result = await businessPlatformOrganizationsRequestDoc({ - query: ListAppDevStores, - token, - organizationId: orgId, - variables: {searchTerm: namePattern}, - unauthorizedHandler: { - type: 'token_refresh', - handler: async () => ({token: await ensureAuthenticatedBusinessPlatform([], {noPrompt: true})}), - }, - }) - - const accessibleShops = result.organization?.accessibleShops - if (!accessibleShops) return [] - if (accessibleShops.pageInfo.hasNextPage) { - console.warn( - `[cleanup-stores] API discovery has more pages for pattern "${namePattern}"; use a narrower pattern if matches are missing.`, - ) - } - const seen = new Set() const stores: StoreInfo[] = [] - for (const edge of accessibleShops.edges) { - const store = toStoreInfo(edge.node, namePattern) - if (!store || seen.has(store.fqdn)) continue - seen.add(store.fqdn) - stores.push(store) + let after: string | undefined + + for (let pageNumber = 1; pageNumber <= STORE_DISCOVERY_MAX_PAGES; pageNumber++) { + const result = await businessPlatformOrganizationsRequest({ + query: ListAppDevStoresPaginated, + token, + organizationId: orgId, + variables: {searchTerm: namePattern, first: STORE_DISCOVERY_PAGE_SIZE, ...(after ? {after} : {})}, + unauthorizedHandler: { + type: 'token_refresh', + handler: async () => ({token: await ensureAuthenticatedBusinessPlatform([], {noPrompt: true})}), + }, + }) + + const accessibleShops = result.organization?.accessibleShops + if (!accessibleShops) throw new Error('Store discovery returned no organization or shop list') + + for (const edge of accessibleShops.edges) { + const store = toStoreInfo(edge.node, namePattern) + if (!store || seen.has(store.fqdn)) continue + seen.add(store.fqdn) + stores.push(store) + } + + const {hasNextPage, endCursor} = accessibleShops.pageInfo + if (!hasNextPage) return stores + if (!endCursor || endCursor === after) throw new Error('Store discovery returned an invalid pagination cursor') + after = endCursor + console.log(`[cleanup-stores] ...${stores.length} stores after ${pageNumber} page(s)`) } - return stores + throw new Error(`Store discovery exceeded ${STORE_DISCOVERY_MAX_PAGES} pages for pattern "${namePattern}"`) } -type AppDevStoreNode = NonNullable< - NonNullable['accessibleShops']>['edges'][number]['node'] -> - function toStoreInfo(node: AppDevStoreNode, namePattern: string): StoreInfo | undefined { const fqdn = normalizeStoreFqdn(node.primaryDomain) ?? @@ -483,9 +697,9 @@ async function countInstalledApps(page: Page, storeFqdn: string): Promise false))) break - const isNextDisabled = await nextBtn.evaluate( - (el) => el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('disabled'), - ).catch(() => true) + const isNextDisabled = await nextBtn + .evaluate((el) => el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('disabled')) + .catch(() => true) if (isNextDisabled) break await nextBtn.click() @@ -500,7 +714,7 @@ async function countInstalledApps(page: Page, storeFqdn: string): Promise { +async function uninstallAllAppsFromStore(page: Page, tag: string): Promise { // Uninstall apps one at a time using the ⋯ "More actions" menu buttons. // The admin paginates installed apps, so after clearing the current page // we check for a "Next" button and continue on subsequent pages. @@ -519,13 +733,15 @@ async function uninstallAllAppsFromStore(page: Page): Promise { if (!(await menuBtn.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) break // Get the app name from the list item container - const appName = await menuBtn.evaluate((el) => { - const row = el.closest('div[role="listitem"]') - if (!row) return 'unknown' - // The app name is in a inside the clickable link - const link = row.querySelector('a span') - return link?.textContent?.trim() || 'unknown' - }).catch(() => 'unknown') + const appName = await menuBtn + .evaluate((el) => { + const row = el.closest('div[role="listitem"]') + if (!row) return 'unknown' + // The app name is in a inside the clickable link + const link = row.querySelector('a span') + return link?.textContent?.trim() || 'unknown' + }) + .catch(() => 'unknown') await menuBtn.click() await page.waitForTimeout(BROWSER_TIMEOUT.short) @@ -546,10 +762,10 @@ async function uninstallAllAppsFromStore(page: Page): Promise { await confirmBtn.click() await page.waitForTimeout(BROWSER_TIMEOUT.medium) consecutiveSkips = 0 - console.log(` Uninstalled ${appName}`) + console.log(`${tag}: Uninstalled ${appName}`) } else { // Confirm never appeared — skip this app to avoid infinite loop - console.log(` Uninstall confirm not found for ${appName}, skipping`) + console.log(`${tag}: Uninstall confirm not found for ${appName}, skipping`) consecutiveSkips++ } @@ -562,9 +778,9 @@ async function uninstallAllAppsFromStore(page: Page): Promise { // Check for pagination — if there's a next page, navigate to it const nextBtn = page.locator('button#nextURL') if (!(await nextBtn.isVisible({timeout: BROWSER_TIMEOUT.short}).catch(() => false))) break - const isNextDisabled = await nextBtn.evaluate( - (el) => el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('disabled'), - ).catch(() => true) + const isNextDisabled = await nextBtn + .evaluate((el) => el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('disabled')) + .catch(() => true) if (isNextDisabled) break await nextBtn.click() @@ -595,6 +811,7 @@ async function main() { let mode: CleanupStoresMode = 'full' if (args.includes('--list')) mode = 'list' else if (args.includes('--delete')) mode = 'delete' + else if (args.includes('--force')) mode = 'force' await cleanupStores({mode, pattern, headed}) } diff --git a/packages/e2e/setup/persistent-stores.ts b/packages/e2e/setup/persistent-stores.ts new file mode 100644 index 00000000000..b21ca9224cb --- /dev/null +++ b/packages/e2e/setup/persistent-stores.ts @@ -0,0 +1,28 @@ +/** Read the persistent store pool. A single store is also supported for local runs. */ +export function readPersistentStores(env: NodeJS.ProcessEnv = process.env, required = true): string[] { + const value = env.E2E_STORE_FQDNS ?? env.E2E_STORE_FQDN + if (!value?.trim()) { + if (!required) return [] + throw new Error('E2E_STORE_FQDNS must contain existing development-store domains.') + } + + const stores = value + .trim() + .split(/[\s,]+/) + .map((store) => store.toLowerCase()) + for (const store of stores) { + if (!/^[a-z0-9][a-z0-9-]*\.myshopify\.com$/.test(store)) { + throw new Error(`Invalid E2E store domain: ${store}. Use a full myshopify.com domain.`) + } + } + if (new Set(stores).size !== stores.length) { + throw new Error('E2E_STORE_FQDNS must not contain duplicate stores.') + } + return stores +} + +/** Persistent test stores must never be removed by the cleanup scripts. */ +export function isProtectedStore(storeFqdn: string, configuredStores: string[] = []): boolean { + const domain = storeFqdn.toLowerCase() + return domain.startsWith('no-delete-e2e-') || configuredStores.includes(domain) +} diff --git a/packages/e2e/setup/store.ts b/packages/e2e/setup/store.ts index 135aa7e228b..5dff5c3d50f 100644 --- a/packages/e2e/setup/store.ts +++ b/packages/e2e/setup/store.ts @@ -3,6 +3,7 @@ import {appTestFixture} from './app.js' import {isVisibleWithin} from './browser.js' import {BROWSER_TIMEOUT, CLI_TIMEOUT} from './constants.js' import {createLogger, e2eRunSegment, e2eSection, requireEnv} from './env.js' +import {isProtectedStore, readPersistentStores} from './persistent-stores.js' import type {CLIProcess, ExecResult} from './cli.js' import type {Locator, Page} from '@playwright/test' @@ -54,10 +55,20 @@ export async function deleteDevStoreWithCli(options: { storeFqdn: string orgId: string }): Promise { - const result = await options.cli.exec( - ['store', 'delete', '--store', options.storeFqdn, '--organization-id', options.orgId, '--force', '--json'], - {timeout: CLI_TIMEOUT.store}, - ) + if (isProtectedStore(options.storeFqdn, readPersistentStores(process.env, false))) { + throw new Error(`Refusing to delete persistent E2E store ${options.storeFqdn}.`) + } + const args = [ + 'store', + 'delete', + '--store', + options.storeFqdn, + '--organization-id', + options.orgId, + '--force', + '--json', + ] + const result = await options.cli.exec(args, {timeout: CLI_TIMEOUT.store}) assertCommandSucceeded('delete development store', result) const output = parseJsonOutput(result, 'delete development store') @@ -162,7 +173,9 @@ export async function uninstallAppFromStore(page: Page, storeSlug: string, appNa // Force a DOM click to bypass Playwright actionability (the button can read as // disabled mid-transition). - await confirmBtn.evaluate((button) => button.click()) + await confirmBtn.evaluate((button) => { + if ('click' in button && typeof button.click === 'function') button.click() + }) await page.waitForTimeout(BROWSER_TIMEOUT.medium) } @@ -250,13 +263,7 @@ async function dismissOpenPopover(page: Page, modal: Locator): Promise { /** 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 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() - return menuButtons.length === 0 + return isVisibleWithin(page.locator('text=Add apps to your store'), BROWSER_TIMEOUT.medium) } // --------------------------------------------------------------------------- diff --git a/packages/e2e/tests/persistent-stores.spec.ts b/packages/e2e/tests/persistent-stores.spec.ts new file mode 100644 index 00000000000..0d37f315c39 --- /dev/null +++ b/packages/e2e/tests/persistent-stores.spec.ts @@ -0,0 +1,47 @@ +import {readPersistentStores, isProtectedStore} from '../setup/persistent-stores.js' +import {deleteDevStoreWithCli, isStoreAppsEmpty} from '../setup/store.js' +import {expect, test, type Page} from '@playwright/test' + +const stores = Array.from({length: 6}, (_, index) => `no-delete-e2e-reusable-store-${index + 1}.myshopify.com`) + +test('reads a pool and supports one local store', () => { + expect(readPersistentStores({E2E_STORE_FQDNS: ' ONE.myshopify.com,\ntwo.myshopify.com '})).toEqual([ + 'one.myshopify.com', + 'two.myshopify.com', + ]) + expect(readPersistentStores({E2E_STORE_FQDN: 'one.myshopify.com'})).toEqual(['one.myshopify.com']) + expect(readPersistentStores({}, false)).toEqual([]) + expect(() => readPersistentStores({})).toThrow('existing development-store domains') +}) + +test('protects named persistent stores and configured stores from cleanup', async () => { + expect(isProtectedStore(stores[0]!)).toBe(true) + expect(isProtectedStore('other.myshopify.com', ['other.myshopify.com'])).toBe(true) + expect(isProtectedStore('e2e-w0-old.myshopify.com', stores)).toBe(false) + let invoked = false + const cli = { + async exec() { + invoked = true + return {exitCode: 0, stdout: '{}', stderr: ''} + }, + } + await expect(deleteDevStoreWithCli({cli, storeFqdn: stores[0]!, orgId: '123'})).rejects.toThrow( + 'Refusing to delete persistent E2E store', + ) + expect(invoked).toBe(false) +}) + +test('legacy store cleanup requires an explicit empty state', async () => { + const page = (empty: boolean) => + ({ + locator(selector: string) { + return { + async waitFor() { + if (selector !== 'text=Add apps to your store' || !empty) throw new Error('Element is not visible') + }, + } + }, + }) as unknown as Page + expect(await isStoreAppsEmpty(page(true))).toBe(true) + expect(await isStoreAppsEmpty(page(false))).toBe(false) +}) From 7ba48f57654acc772561d68bd5a1d1c44ff498cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 21 Oct 2026 13:23:03 +0200 Subject: [PATCH 2/2] Protect every configured E2E store from cleanup --- packages/e2e/setup/persistent-stores.ts | 11 +-- packages/e2e/tests/persistent-stores.spec.ts | 80 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/e2e/setup/persistent-stores.ts b/packages/e2e/setup/persistent-stores.ts index b21ca9224cb..5020b159dff 100644 --- a/packages/e2e/setup/persistent-stores.ts +++ b/packages/e2e/setup/persistent-stores.ts @@ -1,7 +1,7 @@ -/** Read the persistent store pool. A single store is also supported for local runs. */ +/** Read every configured persistent store that cleanup must protect. */ export function readPersistentStores(env: NodeJS.ProcessEnv = process.env, required = true): string[] { - const value = env.E2E_STORE_FQDNS ?? env.E2E_STORE_FQDN - if (!value?.trim()) { + const value = [env.E2E_STORE_FQDNS, env.E2E_STORE_FQDN].join(' ') + if (!value.trim()) { if (!required) return [] throw new Error('E2E_STORE_FQDNS must contain existing development-store domains.') } @@ -15,10 +15,7 @@ export function readPersistentStores(env: NodeJS.ProcessEnv = process.env, requi throw new Error(`Invalid E2E store domain: ${store}. Use a full myshopify.com domain.`) } } - if (new Set(stores).size !== stores.length) { - throw new Error('E2E_STORE_FQDNS must not contain duplicate stores.') - } - return stores + return [...new Set(stores)] } /** Persistent test stores must never be removed by the cleanup scripts. */ diff --git a/packages/e2e/tests/persistent-stores.spec.ts b/packages/e2e/tests/persistent-stores.spec.ts index 0d37f315c39..7a243193685 100644 --- a/packages/e2e/tests/persistent-stores.spec.ts +++ b/packages/e2e/tests/persistent-stores.spec.ts @@ -14,6 +14,78 @@ test('reads a pool and supports one local store', () => { expect(() => readPersistentStores({})).toThrow('existing development-store domains') }) +test('combines and deduplicates all configured stores for cleanup protection', () => { + expect( + readPersistentStores({ + E2E_STORE_FQDNS: 'e2e-w-pool.myshopify.com, E2E-W-SHARED.myshopify.com', + E2E_STORE_FQDN: ' E2E-W-LOCAL.myshopify.com ', + }), + ).toEqual(['e2e-w-pool.myshopify.com', 'e2e-w-shared.myshopify.com', 'e2e-w-local.myshopify.com']) + expect( + readPersistentStores({ + E2E_STORE_FQDNS: 'e2e-w-shared.myshopify.com, E2E-W-SHARED.myshopify.com', + E2E_STORE_FQDN: 'e2e-w-shared.myshopify.com', + }), + ).toEqual(['e2e-w-shared.myshopify.com']) + expect(readPersistentStores({E2E_STORE_FQDNS: ' ', E2E_STORE_FQDN: 'e2e-w-local.myshopify.com'})).toEqual([ + 'e2e-w-local.myshopify.com', + ]) + expect(() => + readPersistentStores({E2E_STORE_FQDNS: 'e2e-w-pool.myshopify.com', E2E_STORE_FQDN: 'not-a-store'}), + ).toThrow('Invalid E2E store domain') +}) + +for (const scenario of [ + {name: 'both variables contain different stores', list: 'e2e-w-pool.myshopify.com'}, + {name: 'both variables contain the same store', list: 'e2e-w-local.myshopify.com'}, + {name: 'the plural variable is empty', list: ''}, + {name: 'the plural variable is unset', list: undefined}, +]) { + test(`refuses deletion of the local override when ${scenario.name}`, async () => { + const restoreEnv = setStoreEnvironment({ + E2E_STORE_FQDNS: scenario.list, + E2E_STORE_FQDN: 'e2e-w-local.myshopify.com', + }) + let invoked = false + const cli = { + async exec() { + invoked = true + return {exitCode: 0, stdout: '{"store":{"deletionRequested":true}}', stderr: ''} + }, + } + + try { + await expect(deleteDevStoreWithCli({cli, storeFqdn: 'e2e-w-local.myshopify.com', orgId: '123'})).rejects.toThrow( + 'Refusing to delete persistent E2E store', + ) + expect(invoked).toBe(false) + } finally { + restoreEnv() + } + }) +} + +test('still allows deletion of an unprotected legacy store', async () => { + const restoreEnv = setStoreEnvironment({ + E2E_STORE_FQDNS: 'e2e-w-pool.myshopify.com', + E2E_STORE_FQDN: 'e2e-w-local.myshopify.com', + }) + let invocations = 0 + const cli = { + async exec() { + invocations++ + return {exitCode: 0, stdout: '{"store":{"deletionRequested":true,"deletionConfirmed":true}}', stderr: ''} + }, + } + + try { + expect(await deleteDevStoreWithCli({cli, storeFqdn: 'e2e-w-legacy.myshopify.com', orgId: '123'})).toBe(true) + expect(invocations).toBe(1) + } finally { + restoreEnv() + } +}) + test('protects named persistent stores and configured stores from cleanup', async () => { expect(isProtectedStore(stores[0]!)).toBe(true) expect(isProtectedStore('other.myshopify.com', ['other.myshopify.com'])).toBe(true) @@ -45,3 +117,11 @@ test('legacy store cleanup requires an explicit empty state', async () => { expect(await isStoreAppsEmpty(page(true))).toBe(true) expect(await isStoreAppsEmpty(page(false))).toBe(false) }) + +function setStoreEnvironment(overrides: NodeJS.ProcessEnv): () => void { + const previousEnv = process.env + process.env = {...previousEnv, ...overrides} + return () => { + process.env = previousEnv + } +}