From 8ba709e988e3c95dcf676eb6bfbefcf2c98a1ed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 11 Aug 2026 14:11:57 +0200 Subject: [PATCH] Uninstall E2E apps via the Admin API instead of the store admin UI The E2E teardown uninstalled apps by driving the store admin UI in Playwright: Polaris selectors, an uninstall-reason survey modal, and hard-coded waits. It was the flakiest step of teardown, and because store and app deletion are gated on it, one flaky modal leaked both a store and an app. Replace it with two HTTP requests: mint an app access token with the client credentials grant (the E2E org owns both the app and the dev store, so the grant applies) and call the appUninstall mutation, which uninstalls the calling app. The client secret comes from `app env show`. The token mint can transiently 400 with application_cannot_be_found right after app creation while records propagate, so it retries briefly. There is no browser fallback: the API path is deterministic, and a failure should surface loudly instead of hiding behind the flaky click-through. The per-run cleanup jobs remain the safety net. Specs now pass appDir to teardownAll and remove the temp dir after teardown, since the API path reads the app config from disk. Co-Authored-By: Claude Fable 5 --- packages/e2e/helpers/loadtest-header.ts | 10 ++ packages/e2e/setup/admin-api.ts | 124 ++++++++++++++++++++ packages/e2e/setup/teardown.ts | 37 ++++-- packages/e2e/tests/app-dev-server.spec.ts | 6 +- packages/e2e/tests/dev-hot-reload.spec.ts | 18 ++- packages/e2e/tests/multi-config-dev.spec.ts | 12 +- packages/e2e/tests/toml-config.spec.ts | 6 +- 7 files changed, 187 insertions(+), 26 deletions(-) create mode 100644 packages/e2e/setup/admin-api.ts diff --git a/packages/e2e/helpers/loadtest-header.ts b/packages/e2e/helpers/loadtest-header.ts index ed71a7e3e10..821291755c0 100644 --- a/packages/e2e/helpers/loadtest-header.ts +++ b/packages/e2e/helpers/loadtest-header.ts @@ -3,6 +3,16 @@ import type {BrowserContext} from '@playwright/test' const LOADTEST_HEADER_PATTERN = /^X-Shopify-Loadtest-[\da-f]{8}(?:-[\da-f]{4}){3}-[\da-f]{12}$/i const LOADTEST_HEADER_DOMAINS = ['shopify.com', 'myshopify.com'] +/** + * Loadtest header as a plain record for direct API requests made by the + * harness. Empty when the env var is unset so local runs still work. + */ +export function loadtestHeaderRecord(): {[header: string]: string} { + const loadtestHeader = process.env.E2E_LOADTEST_HEADER?.trim() + if (!loadtestHeader || !LOADTEST_HEADER_PATTERN.test(loadtestHeader)) return {} + return {[loadtestHeader]: 'true'} +} + export async function addLoadtestHeader(context: BrowserContext): Promise { const loadtestHeader = process.env.E2E_LOADTEST_HEADER?.trim() diff --git a/packages/e2e/setup/admin-api.ts b/packages/e2e/setup/admin-api.ts new file mode 100644 index 00000000000..d7c0b83a70d --- /dev/null +++ b/packages/e2e/setup/admin-api.ts @@ -0,0 +1,124 @@ +/* eslint-disable no-restricted-globals -- the harness calls Shopify endpoints + directly, like the Playwright browser does; the cli-kit fetch wrapper (and + its proxy support) is for the CLI under test, which this package must not + depend on */ +import {extractClientId} from './app.js' +import {CLI_TIMEOUT} from './constants.js' +import {loadtestHeaderRecord} from '../helpers/loadtest-header.js' +import {stripAnsi} from '../helpers/strip-ansi.js' +import type {CLIProcess} from './cli.js' + +/** Admin API version for harness-side GraphQL requests. */ +const ADMIN_API_VERSION = '2026-04' + +interface AdminApiUninstallCtx { + cli: Pick + appDir: string + storeFqdn: string +} + +/** + * Uninstall the app from the store over the Admin API instead of driving the + * store admin UI: + * 1. Read the app's client secret with `app env show`. + * 2. Mint an app access token with the client credentials grant. The grant + * works here because the E2E org owns both the app and the dev store. + * 3. Run the `appUninstall` mutation, which uninstalls the calling app. + * + * Throws on any failure so teardown surfaces the error instead of leaking. + */ +export async function uninstallAppWithAdminApi(ctx: AdminApiUninstallCtx): Promise { + const clientId = extractClientId(ctx.appDir) + const clientSecret = await fetchClientSecret(ctx) + const accessToken = await mintAppAccessTokenWithRetry(ctx.storeFqdn, clientId, clientSecret) + await runAppUninstall(ctx.storeFqdn, accessToken) +} + +/** + * Freshly created apps and stores can transiently 400 with + * `application_cannot_be_found` while records propagate — retry briefly. + */ +async function mintAppAccessTokenWithRetry(storeFqdn: string, clientId: string, clientSecret: string): Promise { + const attempts = 3 + const retryDelayMs = 5000 + + for (let attempt = 1; ; attempt++) { + try { + // eslint-disable-next-line no-await-in-loop + return await mintAppAccessToken(storeFqdn, clientId, clientSecret) + } catch (err) { + if (attempt === attempts) throw err + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => setTimeout(resolve, retryDelayMs)) + } + } +} + +async function fetchClientSecret(ctx: AdminApiUninstallCtx): Promise { + const result = await ctx.cli.exec(['app', 'env', 'show', '--path', ctx.appDir], {timeout: CLI_TIMEOUT.short}) + if (result.exitCode !== 0) { + throw new Error(`app env show failed (exit ${result.exitCode}): ${result.stderr}`) + } + const secret = stripAnsi(result.stdout).match(/SHOPIFY_API_SECRET=(\S+)/)?.[1] + if (!secret) { + throw new Error('app env show output did not include SHOPIFY_API_SECRET') + } + return secret +} + +async function mintAppAccessToken(storeFqdn: string, clientId: string, clientSecret: string): Promise { + const response = await fetch(`https://${storeFqdn}/admin/oauth/access_token`, { + method: 'POST', + headers: {'Content-Type': 'application/x-www-form-urlencoded', ...loadtestHeaderRecord()}, + body: new URLSearchParams({ + grant_type: 'client_credentials', + client_id: clientId, + client_secret: clientSecret, + }).toString(), + }) + if (!response.ok) { + throw new Error( + `access token request failed for client_id ${clientId} on ${storeFqdn} ` + + `(status ${response.status}): ${summarizeErrorBody(await response.text())}`, + ) + } + const payload = (await response.json()) as {access_token?: string} + if (!payload.access_token) { + throw new Error('access token response did not include access_token') + } + return payload.access_token +} + +async function runAppUninstall(storeFqdn: string, accessToken: string): Promise { + const response = await fetch(`https://${storeFqdn}/admin/api/${ADMIN_API_VERSION}/graphql.json`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Shopify-Access-Token': accessToken, + ...loadtestHeaderRecord(), + }, + body: JSON.stringify({query: 'mutation { appUninstall { userErrors { field message } } }'}), + }) + if (!response.ok) { + throw new Error( + `appUninstall request failed (status ${response.status}): ${summarizeErrorBody(await response.text())}`, + ) + } + const payload = (await response.json()) as { + errors?: unknown + data?: {appUninstall?: {userErrors?: {message: string}[]}} + } + if (payload.errors) { + throw new Error(`appUninstall returned errors: ${JSON.stringify(payload.errors)}`) + } + const userErrors = payload.data?.appUninstall?.userErrors ?? [] + if (userErrors.length > 0) { + throw new Error(`appUninstall returned user errors: ${userErrors.map((error) => error.message).join(', ')}`) + } +} + +/** Shopify OAuth errors come back as full HTML pages — keep only the , which carries the error code. */ +function summarizeErrorBody(body: string): string { + const htmlTitle = body.match(/<title>([^<]*)<\/title>/)?.[1] + return htmlTitle ?? body.slice(0, 300) +} diff --git a/packages/e2e/setup/teardown.ts b/packages/e2e/setup/teardown.ts index 727e7936b49..fcfb0aa32f5 100644 --- a/packages/e2e/setup/teardown.ts +++ b/packages/e2e/setup/teardown.ts @@ -1,4 +1,5 @@ /* 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' @@ -14,6 +15,8 @@ interface BaseTeardownCtx { appName: string /** Direct Dev Dashboard app URL. Prefer this when available to avoid slow org-wide pagination. */ appUrl?: string + /** Local app directory. When present, uninstall goes through the Admin API instead of the store admin UI. */ + appDir?: string workerIndex?: number } @@ -40,20 +43,30 @@ export async function teardownAll(ctx: TeardownCtx): Promise<void> { const storeSlug = ctx.storeFqdn.replace('.myshopify.com', '') e2eSection(wCtx, `Teardown: store ${ctx.storeFqdn}`) - // Phase 1: Uninstall app from store + // 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 - 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 + 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}`) } - 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) { diff --git a/packages/e2e/tests/app-dev-server.spec.ts b/packages/e2e/tests/app-dev-server.spec.ts index 55dd85fe069..19bbf43ec57 100644 --- a/packages/e2e/tests/app-dev-server.spec.ts +++ b/packages/e2e/tests/app-dev-server.spec.ts @@ -15,6 +15,7 @@ test.describe('App dev server', () => { const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('dev') let appUrl: string | undefined + let appDir: string | undefined try { // Step 1: Create an extension-only app (no scopes needed) @@ -29,7 +30,7 @@ test.describe('App dev server', () => { expect(initResult.exitCode, `createApp failed:\nstdout: ${initResult.stdout}\nstderr: ${initResult.stderr}`).toBe( 0, ) - const appDir = initResult.appDir + appDir = initResult.appDir appUrl = devDashboardAppUrl(appDir, env.orgId) // Step 2: Start dev server via PTY, targeting the worker's store @@ -53,16 +54,17 @@ test.describe('App dev server', () => { } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, storeFqdn, workerIndex: env.workerIndex, }) + fs.rmSync(parentDir, {recursive: true, force: true}) } } }) diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts index d62ffbdc5cb..f976451aaf5 100644 --- a/packages/e2e/tests/dev-hot-reload.spec.ts +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -46,11 +46,12 @@ test.describe('Dev hot reload', () => { const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('hot-reload') let appUrl: string | undefined + let appDir: string | undefined try { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) - const appDir = initResult.appDir + appDir = initResult.appDir appUrl = devDashboardAppUrl(appDir, env.orgId) injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -88,16 +89,17 @@ test.describe('Dev hot reload', () => { } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, storeFqdn, workerIndex: env.workerIndex, }) + fs.rmSync(parentDir, {recursive: true, force: true}) } } }) @@ -109,11 +111,12 @@ test.describe('Dev hot reload', () => { const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('hot-create') let appUrl: string | undefined + let appDir: string | undefined try { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) - const appDir = initResult.appDir + appDir = initResult.appDir appUrl = devDashboardAppUrl(appDir, env.orgId) injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -144,16 +147,17 @@ test.describe('Dev hot reload', () => { } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, storeFqdn, workerIndex: env.workerIndex, }) + fs.rmSync(parentDir, {recursive: true, force: true}) } } }) @@ -165,11 +169,12 @@ test.describe('Dev hot reload', () => { const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('hot-delete') let appUrl: string | undefined + let appDir: string | undefined try { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) - const appDir = initResult.appDir + appDir = initResult.appDir appUrl = devDashboardAppUrl(appDir, env.orgId) injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -206,16 +211,17 @@ test.describe('Dev hot reload', () => { } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, 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 c8088f903b6..d2559fc783e 100644 --- a/packages/e2e/tests/multi-config-dev.spec.ts +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -21,11 +21,12 @@ test.describe('Multi-config dev', () => { const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('multi-cfg') let appUrl: string | undefined + let appDir: string | undefined try { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) - const appDir = initResult.appDir + appDir = initResult.appDir appUrl = devDashboardAppUrl(appDir, env.orgId) // Inject the fully populated TOML as the default config @@ -91,16 +92,17 @@ extensions_summary = "E2E staging app extensions" } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, storeFqdn, workerIndex: env.workerIndex, }) + fs.rmSync(parentDir, {recursive: true, force: true}) } } }) @@ -112,11 +114,12 @@ extensions_summary = "E2E staging app extensions" const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('mcfg-def') let appUrl: string | undefined + let appDir: string | undefined try { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) - const appDir = initResult.appDir + appDir = initResult.appDir appUrl = devDashboardAppUrl(appDir, env.orgId) injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -170,16 +173,17 @@ extensions_summary = "E2E staging app extensions" } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, 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 88b00a04494..3f93816cb95 100644 --- a/packages/e2e/tests/toml-config.spec.ts +++ b/packages/e2e/tests/toml-config.spec.ts @@ -58,11 +58,12 @@ test.describe('TOML config regression', () => { const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) const appName = e2eAppName('toml-dev') let appUrl: string | undefined + let appDir: string | undefined try { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) - const appDir = initResult.appDir + appDir = initResult.appDir injectFixtureToml(appDir, FIXTURE_TOML, appName) appUrl = devDashboardAppUrl(appDir, env.orgId) @@ -84,16 +85,17 @@ test.describe('TOML config regression', () => { } finally { // 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({ cli, browserPage, appName, appUrl, + appDir, orgId: env.orgId, storeFqdn, workerIndex: env.workerIndex, }) + fs.rmSync(parentDir, {recursive: true, force: true}) } } })