diff --git a/packages/e2e/playwright.config.ts b/packages/e2e/playwright.config.ts index c4e6e148c8f..2e8b62e894b 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/app-management-api.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/app-management-api.spec.ts', ], dependencies: ['remote-auth'], }, diff --git a/packages/e2e/setup/app-management-api.ts b/packages/e2e/setup/app-management-api.ts new file mode 100644 index 00000000000..139117fb460 --- /dev/null +++ b/packages/e2e/setup/app-management-api.ts @@ -0,0 +1,79 @@ +/* eslint-disable no-restricted-imports -- this wrapper runs the cli-kit API client in a tsx subprocess */ +import {execa} from 'execa' +import * as path from 'path' +import {fileURLToPath} from 'url' + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) +const RESULT_PREFIX = 'E2E_APP_MANAGEMENT_RESULT=' + +export type AppDeletionReadiness = + | {status: 'ready'; app: {id: string; key: string}} + | {status: 'already-deleted'} + | {status: 'still-installed'; installCount: number} + +export interface AppManagementAppState { + id: string + key: string + installCount?: number | null + activeRelease: {version: {name: string}} +} + +interface AppDeletionReadinessOptions { + appName: string + clientId?: string + orgId: string +} + +/** + * Inspect the app and wait for its install count to reach zero. + * + * The subprocess is required because Playwright's transformed module graph + * cannot load cli-kit's dist ESM on every supported CI Node version. Running + * the API work under tsx also lets teardown use cli-kit's normal GraphQL + * throttling, network retry, and token-refresh behavior. + */ +export async function waitForAppDeletionReadiness( + sessionEnv: NodeJS.ProcessEnv, + options: AppDeletionReadinessOptions, +): Promise { + const script = path.join(__dirname, 'inspect-app-management-state.ts') + const result = await execa('tsx', [script], { + env: {...sessionEnv, SHOPIFY_FLAG_VERBOSE: undefined}, + extendEnv: false, + preferLocal: true, + localDir: path.resolve(__dirname, '..'), + input: JSON.stringify(options), + timeout: 120_000, + }) + + const resultLine = result.stdout.split('\n').findLast((line) => line.startsWith(RESULT_PREFIX)) + if (!resultLine) { + throw new Error('App Management inspection did not return a result') + } + + return JSON.parse(resultLine.slice(RESULT_PREFIX.length)) as AppDeletionReadiness +} + +export function appDeletionReadinessFromApps( + apps: AppManagementAppState[], + appName: string, + clientId?: string, +): AppDeletionReadiness { + const exactNameMatches = apps.filter((app) => app.activeRelease.version.name === appName) + const clientIdMatches = clientId ? exactNameMatches.filter((app) => app.key === clientId) : [] + const matchingApps = clientIdMatches.length > 0 ? clientIdMatches : exactNameMatches + + if (matchingApps.length === 0) return {status: 'already-deleted'} + if (matchingApps.length > 1) { + throw new Error(`App Management API returned multiple apps named ${appName}`) + } + + const app = matchingApps[0]! + if (typeof app.installCount !== 'number') { + throw new Error(`App Management API did not return installCount for ${appName}`) + } + + return app.installCount === 0 + ? {status: 'ready', app: {id: app.id, key: app.key}} + : {status: 'still-installed', installCount: app.installCount} +} diff --git a/packages/e2e/setup/app.ts b/packages/e2e/setup/app.ts index 1f8a3b94fb4..92cb3a90d9f 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 */ +/* eslint-disable no-restricted-imports */ import {authFixture} from './auth.js' -import {getLastPageStatus, isVisibleWithin, navigateToDashboard, refreshIfPageError} from './browser.js' +import {getLastPageStatus, isVisibleWithin} from './browser.js' import {CLI_TIMEOUT, BROWSER_TIMEOUT} from './constants.js' import * as toml from '@iarna/toml' import * as path from 'path' @@ -320,39 +320,39 @@ export async function configLink( } // --------------------------------------------------------------------------- -// Dev dashboard browser actions — find and delete apps +// Dev dashboard browser actions — delete apps // --------------------------------------------------------------------------- -/** Search dev dashboard for an app by name. Returns the app URL or null. */ -export async function findAppOnDevDashboard(page: Page, appName: string, orgId?: string): Promise { - const org = orgId ?? (process.env.E2E_ORG_ID ?? '').trim() - const email = process.env.E2E_ACCOUNT_EMAIL +/** + * Load the app's settings page, clicking through the accounts.shopify.com + * account picker if the session bounces there — which is common when the + * browser context has not visited the Dev Dashboard yet. + */ +async function gotoAppSettings(page: Page, appUrl: string): Promise { + await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'}) + await page.waitForTimeout(BROWSER_TIMEOUT.medium) - await navigateToDashboard({browserPage: page, email, orgId: org}) + if (!isAccountsShopifyUrl(page.url())) return - // Scan current page + pagination for the app - while (true) { - const allLinks = await page.locator('a[href*="/apps/"]').all() - for (const link of allLinks) { - const text = (await link.textContent()) ?? '' - if (text.includes(appName)) { - const href = await link.getAttribute('href') - if (href) return href.startsWith('http') ? href : `https://dev.shopify.com${href}` - } + const email = process.env.E2E_ACCOUNT_EMAIL + if (email) { + const accountButton = page.locator(`text=${email}`).first() + if (await isVisibleWithin(accountButton, BROWSER_TIMEOUT.long)) { + await accountButton.click() + await page.waitForTimeout(BROWSER_TIMEOUT.medium) } - - // Check for next page - const nextLink = page.locator('a[href*="next_cursor"]').first() - 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}` - await page.goto(nextUrl, {waitUntil: 'domcontentloaded'}) - await page.waitForTimeout(BROWSER_TIMEOUT.medium) - await refreshIfPageError(page) } + await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'}) + await page.waitForTimeout(BROWSER_TIMEOUT.medium) +} - return null +function isAccountsShopifyUrl(rawUrl: string): boolean { + try { + return new URL(rawUrl).hostname === 'accounts.shopify.com' + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return false + } } /** @@ -360,13 +360,12 @@ export async function findAppOnDevDashboard(page: Page, appName: string, orgId?: * * 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. + * Throws STILL_HAS_INSTALLS when the Delete button remains disabled after a + * reload so the caller can apply its bounded retry policy. */ export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Promise { // 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 gotoAppSettings(page, appUrl) const gotoStatus = getLastPageStatus(page) if (gotoStatus === 404) return true if (gotoStatus !== undefined && gotoStatus >= 500) { @@ -377,6 +376,11 @@ export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Pro // 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() + if (!(await isVisibleWithin(deleteBtn, BROWSER_TIMEOUT.long))) { + // Include the landed URL: the usual cause is a session bounce that the + // account-picker handling above did not cover. + throw new Error(`Delete app button not found (page: ${page.url()})`) + } await deleteBtn.scrollIntoViewIfNeeded({timeout: BROWSER_TIMEOUT.long}) if (!(await deleteBtn.isEnabled())) { await page.reload({waitUntil: 'domcontentloaded'}) diff --git a/packages/e2e/setup/inspect-app-management-state.ts b/packages/e2e/setup/inspect-app-management-state.ts new file mode 100644 index 00000000000..bfb8d71ec17 --- /dev/null +++ b/packages/e2e/setup/inspect-app-management-state.ts @@ -0,0 +1,106 @@ +/* eslint-disable @nx/enforce-module-boundaries, no-await-in-loop -- this + subprocess uses cli-kit's built API client with the isolated E2E session */ +import {appDeletionReadinessFromApps} from './app-management-api.js' +import {appManagementFqdn} from '../../cli-kit/dist/public/node/context/fqdn.js' +import {graphqlRequest} from '../../cli-kit/dist/public/node/api/graphql.js' +import {ensureAuthenticatedAppManagementAndBusinessPlatform} from '../../cli-kit/dist/public/node/session.js' +import {loadtestHeaderRecord} from '../helpers/loadtest-header.js' +import type {AppDeletionReadiness, AppManagementAppState} from './app-management-api.js' + +const RESULT_PREFIX = 'E2E_APP_MANAGEMENT_RESULT=' + +interface InspectionOptions { + appName: string + clientId?: string + orgId: string +} + +interface AppsQueryResult { + appsConnection?: {edges: {node: AppManagementAppState}[]} | null +} + +const query = ` + query E2ETeardownApps($query: String) { + appsConnection(query: $query, first: 50) { + edges { + node { + id + key + installCount + activeRelease { + version { + name + } + } + } + } + } + } +` + +const options = JSON.parse(await readStandardInput()) as InspectionOptions +const deadline = Date.now() + 30_000 +const missingAppConfirmationsRequired = 2 +const apiUrl = `https://${await appManagementFqdn()}/app_management/unstable/graphql.json` + +let {appManagementToken} = await ensureAuthenticatedAppManagementAndBusinessPlatform({noPrompt: true}) + +async function inspectApp(): Promise { + const result = await graphqlRequest({ + api: 'App Management', + url: apiUrl, + token: appManagementToken, + addedHeaders: loadtestHeaderRecord(), + query, + variables: { + query: `title:${options.appName}`, + // App Management reads this undeclared variable to route the request to the organization. + organizationId: options.orgId, + }, + unauthorizedHandler: { + type: 'token_refresh', + handler: async () => { + const refreshed = await ensureAuthenticatedAppManagementAndBusinessPlatform({ + noPrompt: true, + forceRefresh: true, + }) + appManagementToken = refreshed.appManagementToken + return {token: appManagementToken} + }, + }, + }) + + if (!result.appsConnection) { + throw new Error('App Management API did not return appsConnection') + } + + return appDeletionReadinessFromApps( + result.appsConnection.edges.map((edge) => edge.node), + options.appName, + options.clientId, + ) +} + +let readiness: AppDeletionReadiness +let missingAppConfirmations = 0 + +while (true) { + readiness = await inspectApp() + missingAppConfirmations = readiness.status === 'already-deleted' ? missingAppConfirmations + 1 : 0 + + if (readiness.status === 'ready') break + if (readiness.status === 'already-deleted' && missingAppConfirmations >= missingAppConfirmationsRequired) break + if (Date.now() >= deadline) break + + await new Promise((resolve) => setTimeout(resolve, 2_000)) +} + +process.stdout.write(`\n${RESULT_PREFIX}${JSON.stringify(readiness)}\n`) + +async function readStandardInput(): Promise { + let input = '' + for await (const chunk of process.stdin) { + input += chunk.toString() + } + return input +} diff --git a/packages/e2e/setup/store.ts b/packages/e2e/setup/store.ts index 135aa7e228b..73378a69a8a 100644 --- a/packages/e2e/setup/store.ts +++ b/packages/e2e/setup/store.ts @@ -162,7 +162,7 @@ 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) => (button as HTMLButtonElement).click()) await page.waitForTimeout(BROWSER_TIMEOUT.medium) } diff --git a/packages/e2e/setup/teardown.ts b/packages/e2e/setup/teardown.ts index fcfb0aa32f5..b11c2d14ee1 100644 --- a/packages/e2e/setup/teardown.ts +++ b/packages/e2e/setup/teardown.ts @@ -1,182 +1,219 @@ /* eslint-disable no-await-in-loop */ import {uninstallAppWithAdminApi} from './admin-api.js' -import {findAppOnDevDashboard, deleteAppFromDevDashboard} from './app.js' -import {refreshIfPageError} from './browser.js' -import {createLogger, e2eSection} from './env.js' +import {waitForAppDeletionReadiness} from './app-management-api.js' +import {deleteAppFromDevDashboard, extractClientId} from './app.js' +import {e2eSection} from './env.js' import {BROWSER_TIMEOUT} from './constants.js' -import {uninstallAppFromStore, deleteDevStoreWithCli, isStoreAppsEmpty, dismissDevConsole} from './store.js' +import {deleteDevStoreWithCli} from './store.js' import type {CLIProcess} from './cli.js' +import type {E2EEnv} from './env.js' import type {Page} from '@playwright/test' -const log = createLogger('browser') - -interface BaseTeardownCtx { +interface BaseTeardownContext { browserPage: Page appName: string - /** Direct Dev Dashboard app URL. Prefer this when available to avoid slow org-wide pagination. */ + env: E2EEnv appUrl?: string - /** Local app directory. When present, uninstall goes through the Admin API instead of the store admin UI. */ - appDir?: string - workerIndex?: number } -type TeardownCtx = BaseTeardownCtx & - ({storeFqdn: string; orgId: string; cli: CLIProcess} | {storeFqdn?: undefined; orgId?: string; cli?: CLIProcess}) +type StoreTeardownContext = BaseTeardownContext & { + storeFqdn: string + cli: CLIProcess + appDir: string | undefined +} + +type TeardownContext = StoreTeardownContext | (BaseTeardownContext & {storeFqdn?: undefined; appDir?: string}) + +interface CleanupPhaseRecord { + phase: CleanupPhase + status: 'completed' | 'failed' | 'skipped' + detail: string +} + +type CleanupPhase = 'uninstall-app' | 'wait-for-zero-installs' | 'delete-app' | 'delete-store' /** - * 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 (skipped if phase 1 failed) - * Phase 3: delete app from dev dashboard (skipped if phase 1 failed) + * Best-effort per-test teardown. * - * App-only flow: - * Phase 3 only + * Store-backed tests use this order: + * uninstall app, wait for zero installs, delete app, then delete store. + * Every phase records its own result and teardown never replaces the test result. */ -export async function teardownAll(ctx: TeardownCtx): Promise { - const wCtx = {workerIndex: ctx.workerIndex ?? 0} - const page = ctx.browserPage - - // Phase 1 + 2: Store cleanup (app+store tests only) - if (ctx.storeFqdn) { - const storeSlug = ctx.storeFqdn.replace('.myshopify.com', '') - e2eSection(wCtx, `Teardown: store ${ctx.storeFqdn}`) - - // Phase 1: Uninstall app from store — Admin API when the app dir is known. - // No browser fallback: an API failure must surface loudly so it gets fixed - // instead of hiding behind the flaky store-admin click-through. - let uninstalled = false - if (ctx.appDir) { - log.log(wCtx, 'uninstalling app via admin API') - await uninstallAppWithAdminApi({cli: ctx.cli, appDir: ctx.appDir, storeFqdn: ctx.storeFqdn}) - uninstalled = true - log.log(wCtx, 'app uninstalled via admin API') - } - if (!uninstalled) { - log.log(wCtx, 'uninstalling app from store') - for (let attempt = 1; attempt <= 3; attempt++) { - try { - uninstalled = await uninstallAppFromStore(page, storeSlug, ctx.appName) - if (uninstalled) { - log.log(wCtx, 'app uninstalled') - break - } - log.log(wCtx, `(${attempt}/3) app uninstall attempt failed, app still visible`) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (err) { - log.log(wCtx, `(${attempt}/3) app uninstall attempt failed: ${err instanceof Error ? err.message : err}`) - } - } - } - if (!uninstalled) { - log.error(wCtx, 'app uninstall failed after 3 attempts') +export async function teardownAll(context: TeardownContext): Promise { + const {workerIndex} = context.env + const clientId = resolveClientId(context) + const storeContext = hasStore(context) ? context : undefined + e2eSection({workerIndex}, `Teardown: app ${context.appName}`) + + if (storeContext) { + const {appDir} = storeContext + if (appDir) { + await runCleanupPhase(workerIndex, 'uninstall-app', 'app uninstalled', () => + uninstallAppWithAdminApi({ + cli: storeContext.cli, + appDir, + storeFqdn: storeContext.storeFqdn, + }), + ) + } else { + recordCleanupPhase(workerIndex, { + phase: 'uninstall-app', + status: 'skipped', + detail: 'app directory unavailable', + }) } + } + + let readiness + try { + readiness = await waitForAppDeletionReadiness(context.env.processEnv, { + appName: context.appName, + clientId, + orgId: context.env.orgId, + }) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + recordCleanupPhase(workerIndex, { + phase: 'wait-for-zero-installs', + status: 'failed', + detail: errorMessage(error), + }) + recordSkippedDeletion(workerIndex, Boolean(storeContext), 'installation state is unknown') + return + } - // Phase 2: Delete store - log.log(wCtx, 'deleting store') - let storeDeletionRequested = false - let safeToDelete = false + if (readiness.status === 'still-installed') { + recordCleanupPhase(workerIndex, { + phase: 'wait-for-zero-installs', + status: 'failed', + detail: `app still has ${readiness.installCount} install(s)`, + }) + recordSkippedDeletion(workerIndex, Boolean(storeContext), 'app still has installs') + return + } - // 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') - storeDeletionRequested = true - } else { - await dismissDevConsole(page) - // 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)) { - 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}`) + recordCleanupPhase(workerIndex, { + phase: 'wait-for-zero-installs', + status: 'completed', + detail: readiness.status === 'already-deleted' ? 'app already deleted' : 'app has zero installs', + }) + + let appDeleted = true + if (readiness.status === 'already-deleted') { + recordCleanupPhase(workerIndex, {phase: 'delete-app', status: 'completed', detail: 'app already deleted'}) + } else { + appDeleted = await runCleanupPhase(workerIndex, 'delete-app', 'app deleted', () => + deleteAppWithRetry(context, readiness.app), + ) + } + + if (!appDeleted) { + if (storeContext) { + recordCleanupPhase(workerIndex, {phase: 'delete-store', status: 'skipped', detail: 'app deletion failed'}) } + return + } - if (safeToDelete) { - for (let attempt = 1; attempt <= 3; attempt++) { - try { - const deletionConfirmed = await deleteDevStoreWithCli({ - cli: ctx.cli, - storeFqdn: ctx.storeFqdn, - orgId: ctx.orgId, - }) - log.log(wCtx, deletionConfirmed ? 'store deletion confirmed by CLI' : 'store deletion requested with CLI') - storeDeletionRequested = true - break - // 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 (!storeDeletionRequested) { - log.error(wCtx, 'store deletion request failed after 3 attempts') + if (storeContext) { + await runCleanupPhase(workerIndex, 'delete-store', 'store deletion requested', () => + deleteStoreWithRetry(storeContext), + ) + } +} + +function hasStore(context: TeardownContext): context is StoreTeardownContext { + return context.storeFqdn !== undefined +} + +async function runCleanupPhase( + workerIndex: number, + phase: CleanupPhase, + detail: string, + operation: () => Promise, +): Promise { + try { + await operation() + recordCleanupPhase(workerIndex, {phase, status: 'completed', detail}) + return true + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + recordCleanupPhase(workerIndex, {phase, status: 'failed', detail: errorMessage(error)}) + return false + } +} + +function recordSkippedDeletion(workerIndex: number, hasStore: boolean, detail: string): void { + recordCleanupPhase(workerIndex, {phase: 'delete-app', status: 'skipped', detail}) + if (hasStore) recordCleanupPhase(workerIndex, {phase: 'delete-store', status: 'skipped', detail}) +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +async function deleteAppWithRetry(context: TeardownContext, app: {id: string; key: string}): Promise { + const numericAppId = app.id.match(/(\d+)$/)?.[1] + // The dashboard exposes the Delete button on its numeric app route. Keep the + // client-key URL only as a fallback when the API does not return a numeric GID. + const appUrl = + numericAppId && context.env.orgId + ? `https://dev.shopify.com/dashboard/${context.env.orgId}/apps/${numericAppId}` + : (context.appUrl ?? `https://dev.shopify.com/dashboard/${context.env.orgId}/apps/${app.key}`) + + await retryCleanup( + async () => { + if (!(await deleteAppFromDevDashboard(context.browserPage, appUrl))) { + throw new Error('app deletion was not confirmed') } - } + }, + () => context.browserPage.waitForTimeout(BROWSER_TIMEOUT.medium), + ) +} + +async function deleteStoreWithRetry(context: StoreTeardownContext): Promise { + await retryCleanup(async () => { + await deleteDevStoreWithCli({ + cli: context.cli, + storeFqdn: context.storeFqdn, + orgId: context.env.orgId, + }) + }) +} - // 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') +async function retryCleanup(operation: () => Promise, waitBeforeRetry?: () => Promise): Promise { + let lastError: unknown + for (let attempt = 1; attempt <= 3; attempt++) { + try { + await operation() return + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + lastError = error } + + if (attempt < 3) await waitBeforeRetry?.() } - // 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++) { + throw lastError instanceof Error ? lastError : new Error(String(lastError)) +} + +function recordCleanupPhase(workerIndex: number, record: CleanupPhaseRecord): void { + const output = record.status === 'failed' ? process.stderr : process.stdout + output.write( + `[e2e][w${workerIndex}][teardown] phase=${record.phase} status=${record.status} detail=${record.detail}\n`, + ) +} + +function resolveClientId(context: TeardownContext): string | undefined { + if (context.appDir) { try { - const appUrl = ctx.appUrl ?? (await findAppOnDevDashboard(page, ctx.appName, ctx.orgId)) - log.log(wCtx, ctx.appUrl ? 'using direct app URL for delete' : 'using dashboard search for delete') - if (!appUrl) { - // 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 - } - log.log(wCtx, 'app already deleted') - appDeleted = true - break - } - log.log(wCtx, 'app found, deleting') - const deleted = await deleteAppFromDevDashboard(page, appUrl) - if (deleted) { - log.log(wCtx, 'app deleted') - appDeleted = true - break - } - log.log(wCtx, `(${attempt}/3) app deletion failed`) + return extractClientId(context.appDir) // 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}`) + } catch { + // The app TOML can be unavailable when setup failed partway through. } } - if (!appDeleted && !stillHasInstalls) { - log.error(wCtx, 'app deletion failed after 3 attempts') - } + + const urlSegment = context.appUrl?.match(/\/apps\/([^/?#]+)/)?.[1] + return urlSegment && !/^\d+$/.test(urlSegment) ? urlSegment : undefined } diff --git a/packages/e2e/tests/app-deploy.spec.ts b/packages/e2e/tests/app-deploy.spec.ts index a8c1c0fac37..2174eedc7ed 100644 --- a/packages/e2e/tests/app-deploy.spec.ts +++ b/packages/e2e/tests/app-deploy.spec.ts @@ -190,15 +190,13 @@ test.describe('App deploy', () => { browserPage, appName, appUrl: primaryAppUrl, - orgId: env.orgId, - workerIndex: env.workerIndex, + env, }) await teardownAll({ browserPage, appName: secondaryAppName, appUrl: secondaryAppUrl, - orgId: env.orgId, - workerIndex: env.workerIndex, + env, }) } } diff --git a/packages/e2e/tests/app-dev-server.spec.ts b/packages/e2e/tests/app-dev-server.spec.ts index 19bbf43ec57..a286ff42cc0 100644 --- a/packages/e2e/tests/app-dev-server.spec.ts +++ b/packages/e2e/tests/app-dev-server.spec.ts @@ -60,9 +60,8 @@ test.describe('App dev server', () => { appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) } diff --git a/packages/e2e/tests/app-management-api.spec.ts b/packages/e2e/tests/app-management-api.spec.ts new file mode 100644 index 00000000000..e104ada4d99 --- /dev/null +++ b/packages/e2e/tests/app-management-api.spec.ts @@ -0,0 +1,41 @@ +import {appDeletionReadinessFromApps} from '../setup/app-management-api.js' +import {expect, test} from '@playwright/test' +import type {AppManagementAppState} from '../setup/app-management-api.js' + +test.describe('App Management teardown state', () => { + test('uses the client ID to disambiguate apps with the same name', () => { + const matchingApp = appState({id: 'gid://organization/App/2', key: 'expected-client-id'}) + + expect( + appDeletionReadinessFromApps( + [appState({id: 'gid://organization/App/1', key: 'other-client-id'}), matchingApp], + 'E2E app', + 'expected-client-id', + ), + ).toEqual({status: 'ready', app: {id: matchingApp.id, key: matchingApp.key}}) + }) + + test('does not treat a missing install count as zero', () => { + expect(() => appDeletionReadinessFromApps([appState({installCount: null})], 'E2E app')).toThrow( + 'App Management API did not return installCount for E2E app', + ) + }) + + test('reports installed and deleted apps without ambiguity', () => { + expect(appDeletionReadinessFromApps([appState({installCount: 2})], 'E2E app')).toEqual({ + status: 'still-installed', + installCount: 2, + }) + expect(appDeletionReadinessFromApps([], 'E2E app')).toEqual({status: 'already-deleted'}) + }) +}) + +function appState(overrides: Partial = {}): AppManagementAppState { + return { + id: 'gid://organization/App/1', + key: 'client-id', + installCount: 0, + activeRelease: {version: {name: 'E2E app'}}, + ...overrides, + } +} diff --git a/packages/e2e/tests/app-scaffold.spec.ts b/packages/e2e/tests/app-scaffold.spec.ts index 5a62c8659f2..072566f7092 100644 --- a/packages/e2e/tests/app-scaffold.spec.ts +++ b/packages/e2e/tests/app-scaffold.spec.ts @@ -54,8 +54,7 @@ test.describe('App scaffold', () => { browserPage, appName, appUrl, - orgId: env.orgId, - workerIndex: env.workerIndex, + env, }) } } @@ -92,8 +91,7 @@ test.describe('App scaffold', () => { browserPage, appName, appUrl, - orgId: env.orgId, - workerIndex: env.workerIndex, + env, }) } } @@ -150,8 +148,7 @@ test.describe('App scaffold', () => { browserPage, appName, appUrl, - orgId: env.orgId, - workerIndex: env.workerIndex, + env, }) } } diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts index f976451aaf5..da08a6b195c 100644 --- a/packages/e2e/tests/dev-hot-reload.spec.ts +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -95,9 +95,8 @@ test.describe('Dev hot reload', () => { appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) } @@ -153,9 +152,8 @@ test.describe('Dev hot reload', () => { appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) } @@ -217,9 +215,8 @@ test.describe('Dev hot reload', () => { appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) } diff --git a/packages/e2e/tests/multi-config-dev.spec.ts b/packages/e2e/tests/multi-config-dev.spec.ts index d2559fc783e..2fbf9ae26ea 100644 --- a/packages/e2e/tests/multi-config-dev.spec.ts +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -98,9 +98,8 @@ extensions_summary = "E2E staging app extensions" appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) } @@ -179,9 +178,8 @@ extensions_summary = "E2E staging app extensions" appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) } diff --git a/packages/e2e/tests/toml-config.spec.ts b/packages/e2e/tests/toml-config.spec.ts index 3f93816cb95..6b58ebae62b 100644 --- a/packages/e2e/tests/toml-config.spec.ts +++ b/packages/e2e/tests/toml-config.spec.ts @@ -44,8 +44,7 @@ test.describe('TOML config regression', () => { browserPage, appName, appUrl, - orgId: env.orgId, - workerIndex: env.workerIndex, + env, }) } } @@ -91,9 +90,8 @@ test.describe('TOML config regression', () => { appName, appUrl, appDir, - orgId: env.orgId, + env, storeFqdn, - workerIndex: env.workerIndex, }) fs.rmSync(parentDir, {recursive: true, force: true}) }