diff --git a/packages/e2e/README.md b/packages/e2e/README.md index 49176abcd82..9163462881a 100644 --- a/packages/e2e/README.md +++ b/packages/e2e/README.md @@ -20,7 +20,11 @@ Concurrent runs can use the same stores because each test creates a separate app ## Browser use -Shared authentication and app deletion use the browser. Dev tests do not check store emptiness. App uninstall uses the Admin API. The legacy bulk cleanup scripts still use browser discovery, uninstall, and store-emptiness checks before store deletion. +Shared authentication uses Chromium once per shard. The E2E test cases do not launch a browser after authentication. App lookup and deletion use authenticated HTTP requests to the Dev Dashboard with CSRF protection, exact name and organization checks, and deletion confirmation. These HTTP routes still depend on the dashboard's HTML contract. + +Dev tests do not check store emptiness. They clean only their own app and preview; app uninstall uses the Admin API. The legacy bulk cleanup scripts still use browser discovery, uninstall, and store-emptiness checks before store deletion. + +Malformed TOML is checked by the local project without authentication. Schema validation still needs remote specifications. ## CI setup @@ -52,6 +56,8 @@ pnpm --filter e2e exec playwright test tests/store-lifecycle.spec.ts --project r Remote app cleanup failures produce warnings with the app name, store, failed step, and recovery command. Each later cleanup step is still attempted. These warnings do not fail an otherwise successful test or block the store from subsequent tests. Failure to stop a CLI process remains an error because the process can hold ports and keep making requests. +After a successful uninstall, app deletion retries the dashboard's remaining-installs redirect for up to 15 seconds. The dashboard installation count can lag behind the uninstall. Other deletion errors fail immediately, and deletion still requires a confirmed missing app. + Recover leftover apps with `cleanup-apps.ts --pattern ` or uninstall them in the store admin. Use the exact run marker from the failed run's app names and confirm that the run has stopped. Store reuse does not wait for this recovery. `E2E_SKIP_TEARDOWN=1` deliberately leaves test apps installed. Use the cleanup script to remove them after debugging; their presence does not block store reuse. diff --git a/packages/e2e/package.json b/packages/e2e/package.json index 119246f80ee..325389590dd 100644 --- a/packages/e2e/package.json +++ b/packages/e2e/package.json @@ -36,6 +36,7 @@ "dotenv": "16.6.1", "execa": "^7.2.0", "node-pty": "^1.0.0", + "parse5": "7.3.0", "strip-ansi": "^7.2.0", "tsx": "^4.23.1" }, diff --git a/packages/e2e/playwright.config.ts b/packages/e2e/playwright.config.ts index aed340bbdcc..3f8aaee6b55 100644 --- a/packages/e2e/playwright.config.ts +++ b/packages/e2e/playwright.config.ts @@ -32,6 +32,8 @@ export default defineConfig({ 'tests/fixture-toml.spec.ts', 'tests/auth-diagnostics.spec.ts', 'tests/persistent-stores.spec.ts', + 'tests/dashboard.spec.ts', + 'tests/toml-syntax.spec.ts', ], }, { @@ -47,6 +49,8 @@ export default defineConfig({ 'tests/fixture-toml.spec.ts', 'tests/auth-diagnostics.spec.ts', 'tests/persistent-stores.spec.ts', + 'tests/dashboard.spec.ts', + 'tests/toml-syntax.spec.ts', ], dependencies: ['remote-auth'], }, diff --git a/packages/e2e/scripts/cleanup-apps.ts b/packages/e2e/scripts/cleanup-apps.ts index 586f684119d..a82387074e2 100644 --- a/packages/e2e/scripts/cleanup-apps.ts +++ b/packages/e2e/scripts/cleanup-apps.ts @@ -28,7 +28,7 @@ import {fileURLToPath} from 'url' import {chromium} from '@playwright/test' import {BROWSER_TIMEOUT} from '../setup/constants.js' import {getLastPageStatus, navigateToDashboard, refreshIfPageError, trackMainFrameStatus} from '../setup/browser.js' -import {deleteAppFromDevDashboard} from '../setup/app.js' +import {createDashboardRequest, DevDashboardClient} from '../setup/dashboard.js' import {uninstallAppFromStore} from '../setup/store.js' import {completeLogin} from '../helpers/browser-login.js' import {addLoadtestHeader} from '../helpers/loadtest-header.js' @@ -205,7 +205,7 @@ async function cleanupAppsPageByPage(opts: { page: Page mode: CleanupMode pattern: string - email: string + email?: string orgId: string stats: CleanupStats }): Promise { @@ -274,7 +274,7 @@ async function cleanupAppsInParallel(opts: { dashboardPage: Page mode: CleanupMode apps: DashboardApp[] - email: string + email?: string orgId: string matchOffset: number foundCount: number @@ -319,7 +319,7 @@ async function cleanupApp(opts: { page: Page mode: CleanupMode app: DashboardApp - email: string + email?: string orgId: string workerNumber: number appNumber: number @@ -365,8 +365,12 @@ async function cleanupApp(opts: { break } console.log(`${tag}: Deleting...`) - const deleted = await deleteAppFromDevDashboard(page, app.url) - if (!deleted) throw new Error('App deletion could not be verified') + const http = await createDashboardRequest(await page.context().storageState()) + try { + await new DevDashboardClient(http, orgId).deleteApp({name: app.name, url: app.url}) + } finally { + await http.dispose() + } console.log(`${tag}: Deleted`) } diff --git a/packages/e2e/setup/app.ts b/packages/e2e/setup/app.ts index 97315519632..563764c4fa1 100644 --- a/packages/e2e/setup/app.ts +++ b/packages/e2e/setup/app.ts @@ -1,12 +1,10 @@ -/* eslint-disable no-restricted-imports, no-await-in-loop */ -import {authFixture} from './auth.js' -import {getLastPageStatus, isVisibleWithin, navigateToDashboard, refreshIfPageError} from './browser.js' -import {CLI_TIMEOUT, BROWSER_TIMEOUT} from './constants.js' +/* eslint-disable no-restricted-imports */ +import {dashboardFixture} from './dashboard.js' +import {CLI_TIMEOUT} from './constants.js' import * as toml from '@iarna/toml' import * as path from 'path' 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 @@ -331,104 +329,11 @@ export async function configLink( } } -// --------------------------------------------------------------------------- -// Dev dashboard browser actions — find and 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 - - await navigateToDashboard({browserPage: page, email, orgId: org, searchTerm: appName}) - - // 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}` - } - } - - // 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) - } - - return null -} - -/** - * 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 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) - 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({timeout: BROWSER_TIMEOUT.long}) - if (!(await deleteBtn.isEnabled())) { - await page.reload({waitUntil: 'domcontentloaded'}) - await page.waitForTimeout(BROWSER_TIMEOUT.medium) - await deleteBtn.scrollIntoViewIfNeeded({timeout: BROWSER_TIMEOUT.long}) - if (!(await deleteBtn.isEnabled())) throw new Error('STILL_HAS_INSTALLS') - } - await deleteBtn.click({timeout: 2 * BROWSER_TIMEOUT.long}) - await page.waitForTimeout(BROWSER_TIMEOUT.medium) - - // 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) - } - - // 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) - - // 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 false -} - // --------------------------------------------------------------------------- // Fixture — ensures auth runs before tests. Tests use helper functions directly. // --------------------------------------------------------------------------- -export const appTestFixture = authFixture.extend<{authReady: void}>({ +export const appTestFixture = dashboardFixture.extend<{authReady: void}>({ // Auto-trigger authLogin so the OAuth session is available for all app tests authReady: [ async ({authLogin: _authLogin}: {authLogin: void}, use: () => Promise) => { diff --git a/packages/e2e/setup/auth.ts b/packages/e2e/setup/auth.ts index 48e34d85742..998b81b78cb 100644 --- a/packages/e2e/setup/auth.ts +++ b/packages/e2e/setup/auth.ts @@ -1,4 +1,4 @@ -import {browserFixture} from './browser.js' +import {cliFixture} from './cli.js' import {authStatePaths} from './auth-state.js' import {AuthSetupError} from './auth-diagnostics.js' import {globalLog} from './env.js' @@ -13,9 +13,9 @@ const log = {log: (_ctx: any, msg: string) => globalLog('auth', msg)} * Copies the pre-authenticated session from the remote auth setup into this * worker's isolated XDG directories. * - * Fixture chain: envFixture → cliFixture → browserFixture → authFixture + * Fixture chain: envFixture → cliFixture → authFixture */ -export const authFixture = browserFixture.extend<{}, {authLogin: void}>({ +export const authFixture = cliFixture.extend<{}, {authLogin: void}>({ authLogin: [ async ({env}, use) => { const {xdgEnv: authXdgEnv} = authStatePaths() diff --git a/packages/e2e/setup/browser.ts b/packages/e2e/setup/browser.ts index e18df20d34e..d6e39156aa2 100644 --- a/packages/e2e/setup/browser.ts +++ b/packages/e2e/setup/browser.ts @@ -1,9 +1,5 @@ -import {cliFixture} from './cli.js' -import {authStatePaths} from './auth-state.js' import {BROWSER_TIMEOUT} from './constants.js' -import {addLoadtestHeader} from '../helpers/loadtest-header.js' -import {chromium, type Locator, type Page} from '@playwright/test' -import * as fs from 'fs' +import type {Locator, Page} from '@playwright/test' // --------------------------------------------------------------------------- // Shared browser context type @@ -20,7 +16,7 @@ export interface BrowserContext { /** * 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)`. + * (see `trackMainFrameStatus`). Read via `getLastPageStatus(page)`. * * A WeakMap lets the entry be garbage-collected with the page — no manual * cleanup required. @@ -40,43 +36,6 @@ export function getLastPageStatus(page: Page): number | undefined { return lastMainFrameStatus.get(page) } -// --------------------------------------------------------------------------- -// Fixture -// --------------------------------------------------------------------------- - -/** - * Worker-scoped fixture providing a persistent Playwright browser page. - * - * The browser launches once per worker and stays open for the entire run. - * Downstream fixtures (auth) and tests use `browserPage` for any browser-based - * actions: OAuth login, dashboard navigation, app uninstall/deletion, etc. - * - * Fixture chain: envFixture → cliFixture → browserFixture - */ -export const browserFixture = cliFixture.extend<{}, {browserPage: Page}>({ - browserPage: [ - // eslint-disable-next-line no-empty-pattern - async ({}, use) => { - const browser = await chromium.launch({headless: !process.env.E2E_HEADED}) - const storageStatePath = authStatePaths().storageStatePath - const hasValidStorageState = Boolean( - process.env.E2E_ACCOUNT_EMAIL && process.env.E2E_ACCOUNT_PASSWORD && fs.existsSync(storageStatePath), - ) - const context = await browser.newContext({ - ...(hasValidStorageState ? {storageState: storageStatePath} : {}), - }) - await addLoadtestHeader(context) - context.setDefaultTimeout(BROWSER_TIMEOUT.max) - context.setDefaultNavigationTimeout(BROWSER_TIMEOUT.max) - const page = await context.newPage() - trackMainFrameStatus(page) - await use(page) - await browser.close() - }, - {scope: 'worker'}, - ], -}) - // --------------------------------------------------------------------------- // Browser helpers // --------------------------------------------------------------------------- diff --git a/packages/e2e/setup/dashboard.ts b/packages/e2e/setup/dashboard.ts new file mode 100644 index 00000000000..a0a2608b045 --- /dev/null +++ b/packages/e2e/setup/dashboard.ts @@ -0,0 +1,184 @@ +/* eslint-disable no-await-in-loop */ +import {authFixture} from './auth.js' +import {authStatePaths} from './auth-state.js' +import {requireEnv} from './env.js' +import {loadtestHeaderRecord} from '../helpers/loadtest-header.js' +import {request, type APIRequestContext, type APIResponse} from '@playwright/test' +import {parse, type DefaultTreeAdapterTypes} from 'parse5' +import {setTimeout as delay} from 'node:timers/promises' + +const DASHBOARD_ORIGIN = 'https://dev.shopify.com' +const APP_DELETION_TIMEOUT_MS = 15_000 +type HtmlNode = DefaultTreeAdapterTypes.Node +type HtmlElement = DefaultTreeAdapterTypes.Element + +/** Uses the dashboard's authenticated HTTP routes without launching a browser. */ +export class DevDashboardClient { + private readonly appsUrl: string + + constructor( + private readonly http: Pick, + orgId: string, + ) { + if (!/^\d+$/.test(orgId)) throw new Error('A numeric E2E organization ID is required.') + this.appsUrl = `${DASHBOARD_ORIGIN}/dashboard/${orgId}/apps` + } + + async deleteApp({ + name, + url, + uninstallConfirmed = false, + }: { + name: string + url?: string + /** Allow the dashboard installation count to settle after a successful uninstall. */ + uninstallConfirmed?: boolean + }): Promise { + if (!name.startsWith('E2E-')) throw new Error(`Refusing to delete non-E2E app ${name}.`) + if (url) this.requireAppUrl(url) + + const appUrl = await this.findApp(name) + if (!appUrl) { + if (url && (await this.http.get(`${url}/settings`)).status() !== 404) { + throw new Error(`App ${name} was absent from search but its deletion could not be verified.`) + } + return + } + if (url && url !== appUrl) throw new Error(`App ${name} has an unexpected dashboard ID.`) + + const deadline = Date.now() + APP_DELETION_TIMEOUT_MS + let deletionAccepted = false + do { + const settings = await this.http.get(`${appUrl}/settings`, {timeout: Math.max(1, deadline - Date.now())}) + const document = await readDashboardPage(settings) + const csrfToken = readCsrfToken(document) + const deleted = await this.http.delete(appUrl, { + timeout: Math.max(1, deadline - Date.now()), + headers: {'X-CSRF-Token': csrfToken, Origin: DASHBOARD_ORIGIN, Referer: `${appUrl}/settings`}, + }) + const location = deleted.headers().location + const redirect = + location && URL.canParse(location, DASHBOARD_ORIGIN) ? new URL(location, DASHBOARD_ORIGIN) : undefined + if (deleted.status() === 303 && redirect?.href === this.appsUrl) { + deletionAccepted = true + break + } + if (!uninstallConfirmed || deleted.status() !== 303 || redirect?.href !== `${appUrl}/settings`) { + const target = redirect ? `${redirect.origin}${redirect.pathname}` : 'missing or invalid' + throw new Error(`App ${name} deletion was rejected (HTTP ${deleted.status()}, redirect: ${target}).`) + } + // The dashboard guards deletion with a search index that can lag behind a successful uninstall. + await delay(Math.min(1000, Math.max(0, deadline - Date.now()))) + } while (Date.now() < deadline) + if (!deletionAccepted) { + throw new Error(`App ${name} deletion timed out waiting for the dashboard installation count to reach zero.`) + } + + do { + const response = await this.http.get(`${appUrl}/settings`, {timeout: Math.max(1, deadline - Date.now())}) + if (response.status() === 404) return + await readDashboardPage(response) + await delay(Math.min(1000, Math.max(0, deadline - Date.now()))) + } while (Date.now() < deadline) + throw new Error(`Deletion of app ${name} was not confirmed.`) + } + + private async findApp(name: string): Promise { + let nextUrl: string | undefined = `${this.appsUrl}?search_term=${encodeURIComponent(name)}` + const visited = new Set() + const matches = new Set() + + while (nextUrl) { + if (visited.has(nextUrl) || visited.size >= 50) throw new Error('App search pagination did not complete.') + visited.add(nextUrl) + const document = await readDashboardPage(await this.http.get(nextUrl)) + const links = elements(document).filter((element) => element.tagName === 'a') + const appLinks = links.filter((link) => { + const href = attribute(link, 'href') + return href && new URL(href, DASHBOARD_ORIGIN).href.startsWith(`${this.appsUrl}/`) + }) + if (!appLinks.length && !textContent(document).includes('No apps matched your search.')) { + throw new Error(`Could not verify app search results for ${name}.`) + } + for (const link of appLinks) { + if (textContent(link).trim() !== name) continue + const appUrl = new URL(attribute(link, 'href')!, DASHBOARD_ORIGIN).href + this.requireAppUrl(appUrl) + matches.add(appUrl) + } + const nextLinks = links.filter((link) => attribute(link, 'href')?.includes('next_cursor=')) + if (nextLinks.length > 1) throw new Error('App search returned ambiguous pagination.') + const nextHref = nextLinks[0] && attribute(nextLinks[0], 'href') + nextUrl = nextHref ? new URL(nextHref, DASHBOARD_ORIGIN).href : undefined + if (nextUrl && new URL(nextUrl).origin + new URL(nextUrl).pathname !== this.appsUrl) { + throw new Error('App search returned a URL outside the E2E organization.') + } + } + if (matches.size > 1) throw new Error(`Multiple apps have the E2E name ${name}.`) + return [...matches][0] + } + + private requireAppUrl(url: string): void { + if (!url.startsWith(`${this.appsUrl}/`) || !/^\d+$/.test(url.slice(this.appsUrl.length + 1))) { + throw new Error('App URL must belong to the configured E2E organization.') + } + } +} + +async function readDashboardPage(response: APIResponse): Promise { + if (response.status() !== 200 || !response.headers()['content-type']?.includes('text/html')) { + throw new Error(`Dashboard request failed (HTTP ${response.status()}). Check the E2E session.`) + } + const document = parse(await response.text()) + readCsrfToken(document) + return document +} + +function readCsrfToken(document: HtmlNode): string { + const meta = elements(document).find( + (element) => element.tagName === 'meta' && attribute(element, 'name') === 'csrf-token', + ) + const token = meta && attribute(meta, 'content') + if (!token) throw new Error('Dashboard response has no CSRF token. Check the E2E session.') + return token +} + +function attribute(element: HtmlElement, name: string): string | undefined { + return element.attrs.find((attr) => attr.name === name)?.value +} + +function elements(node: HtmlNode): HtmlElement[] { + const children = 'childNodes' in node ? node.childNodes.flatMap(elements) : [] + return 'tagName' in node ? [node, ...children] : children +} + +function textContent(node: HtmlNode): string { + if ('value' in node) return node.value + return 'childNodes' in node ? node.childNodes.map(textContent).join('') : '' +} + +export const dashboardFixture = authFixture.extend<{}, {dashboard: DevDashboardClient}>({ + dashboard: [ + async ({env, authLogin: _authLogin}, use) => { + requireEnv(env, 'orgId') + const http = await createDashboardRequest(authStatePaths().storageStatePath) + try { + await use(new DevDashboardClient(http, env.orgId)) + } finally { + await http.dispose() + } + }, + {scope: 'worker'}, + ], +}) + +export function createDashboardRequest( + storageState: NonNullable[0]>['storageState'], +) { + return request.newContext({ + storageState, + extraHTTPHeaders: {Accept: 'text/html', ...loadtestHeaderRecord()}, + maxRedirects: 0, + timeout: 20_000, + }) +} diff --git a/packages/e2e/setup/teardown.ts b/packages/e2e/setup/teardown.ts index fceb488111d..8c4c14cc49a 100644 --- a/packages/e2e/setup/teardown.ts +++ b/packages/e2e/setup/teardown.ts @@ -1,15 +1,14 @@ import {uninstallAppWithAdminApi} from './admin-api.js' import {createLogger, e2eSection} from './env.js' -import {findAppOnDevDashboard, deleteAppFromDevDashboard} from './app.js' import type {CLIProcess} from './cli.js' -import type {Page} from '@playwright/test' +import type {DevDashboardClient} from './dashboard.js' const log = createLogger('teardown') interface BaseTeardownCtx { - browserPage: Page + dashboard: Pick appName: string - /** Direct dashboard URL for app cleanup. */ + /** Expected app URL, checked against the app found by its exact name. */ appUrl?: string /** Local app directory used to clean the preview and uninstall through the Admin API. */ appDir?: string @@ -26,6 +25,7 @@ type TeardownCtx = BaseTeardownCtx & export async function teardownAll(ctx: TeardownCtx): Promise { const wCtx = {workerIndex: ctx.workerIndex ?? 0} await ctx.cli?.stopAll() + let uninstallConfirmed = false if (ctx.storeFqdn && ctx.appDir) { const {cli, appDir, storeFqdn} = ctx @@ -36,15 +36,15 @@ export async function teardownAll(ctx: TeardownCtx): Promise { throw new Error(`Failed to clean the dev preview:\n${result.stdout}\n${result.stderr}`) } }) - await attemptRemoteCleanup(ctx, 'app uninstall', () => uninstallAppWithAdminApi({cli, appDir, storeFqdn})) + await attemptRemoteCleanup(ctx, 'app uninstall', async () => { + await uninstallAppWithAdminApi({cli, appDir, storeFqdn}) + uninstallConfirmed = true + }) } e2eSection(wCtx, `Teardown: app ${ctx.appName}`) await attemptRemoteCleanup(ctx, 'app deletion', async () => { - const appUrl = ctx.appUrl ?? (await findAppOnDevDashboard(ctx.browserPage, ctx.appName, ctx.orgId)) - if (appUrl && !(await deleteAppFromDevDashboard(ctx.browserPage, appUrl))) { - throw new Error('App deletion was not confirmed') - } + await ctx.dashboard.deleteApp({name: ctx.appName, url: ctx.appUrl, uninstallConfirmed}) log.log(wCtx, 'app deletion confirmed') }) } diff --git a/packages/e2e/tests/app-deploy.spec.ts b/packages/e2e/tests/app-deploy.spec.ts index 9dc4830c095..bd359220dd5 100644 --- a/packages/e2e/tests/app-deploy.spec.ts +++ b/packages/e2e/tests/app-deploy.spec.ts @@ -81,7 +81,7 @@ function assertActiveVersion(opts: { } test.describe('App deploy', () => { - test('init, deploy, versions list, config link, deploy to secondary', async ({cli, env, browserPage}) => { + test('init, deploy, versions list, config link, deploy to secondary', async ({cli, env, dashboard}) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -148,15 +148,13 @@ test.describe('App deploy', () => { }) const linkOutput = linkResult.stdout + linkResult.stderr expect(linkResult.exitCode, `Step 4 - config link failed:\n${linkOutput}`).toBe(0) - expect(linkOutput, `Step 4 - missing "is now linked to \\"${secondaryAppName}\\""`).toContain( - `is now linked to "${secondaryAppName}"`, - ) const secondaryTomlPath = path.join(appDir, `shopify.app.${secondaryConfig}.toml`) expect( fs.existsSync(secondaryTomlPath), `Step 4 - expected ${secondaryTomlPath} to exist after config link`, ).toBe(true) secondaryAppUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId, configName: secondaryConfig}) + expect(secondaryAppUrl, 'Step 4 - secondary config must link to a different app').not.toBe(primaryAppUrl) // Step 5: Deploy from primary dir to secondary app via --config secondary const secondaryVersionTag = `E2E-v2-${Date.now()}` @@ -187,14 +185,14 @@ test.describe('App deploy', () => { fs.rmSync(parentDir, {recursive: true, force: true}) // Neither app was installed on a store — delete the apps only (no uninstall) await teardownAll({ - browserPage, + dashboard, appName, appUrl: primaryAppUrl, orgId: env.orgId, workerIndex: env.workerIndex, }) await teardownAll({ - browserPage, + dashboard, appName: secondaryAppName, appUrl: secondaryAppUrl, orgId: env.orgId, diff --git a/packages/e2e/tests/app-dev-server.spec.ts b/packages/e2e/tests/app-dev-server.spec.ts index e3586fa45a8..4f7c0c1afd7 100644 --- a/packages/e2e/tests/app-dev-server.spec.ts +++ b/packages/e2e/tests/app-dev-server.spec.ts @@ -7,7 +7,7 @@ import * as fs from 'fs' import * as path from 'path' // eslint-disable-line no-restricted-imports test.describe('App dev server', () => { - test('dev starts, shows ready message, and quits with q', async ({cli, env, browserPage}) => { + test('dev starts, shows ready message, and quits with q', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.devServer test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -56,7 +56,7 @@ test.describe('App dev server', () => { if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, diff --git a/packages/e2e/tests/app-scaffold.spec.ts b/packages/e2e/tests/app-scaffold.spec.ts index a532e2afa10..14888760fb5 100644 --- a/packages/e2e/tests/app-scaffold.spec.ts +++ b/packages/e2e/tests/app-scaffold.spec.ts @@ -8,7 +8,7 @@ import * as fs from 'fs' import * as path from 'path' test.describe('App scaffold', () => { - test('init creates a react-router app and builds', async ({cli, env, browserPage}) => { + test('init creates a react-router app and builds', async ({cli, env, dashboard}) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -51,7 +51,7 @@ test.describe('App scaffold', () => { if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ - browserPage, + dashboard, appName, appUrl, orgId: env.orgId, @@ -61,7 +61,7 @@ test.describe('App scaffold', () => { } }) - test('init creates an extension-only app', async ({cli, env, browserPage}) => { + test('init creates an extension-only app', async ({cli, env, dashboard}) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -89,7 +89,7 @@ test.describe('App scaffold', () => { if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ - browserPage, + dashboard, appName, appUrl, orgId: env.orgId, @@ -99,7 +99,7 @@ test.describe('App scaffold', () => { } }) - test('generates extensions and builds', async ({cli, env, browserPage}) => { + test('generates extensions and builds', async ({cli, env, dashboard}) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -147,7 +147,7 @@ test.describe('App scaffold', () => { if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ - browserPage, + dashboard, appName, appUrl, orgId: env.orgId, diff --git a/packages/e2e/tests/dashboard.spec.ts b/packages/e2e/tests/dashboard.spec.ts new file mode 100644 index 00000000000..5f2e7b11c49 --- /dev/null +++ b/packages/e2e/tests/dashboard.spec.ts @@ -0,0 +1,234 @@ +import {DevDashboardClient} from '../setup/dashboard.js' +import {expect, test, type APIRequestContext, type APIResponse} from '@playwright/test' + +const appsUrl = 'https://dev.shopify.com/dashboard/123/apps' +const appUrl = `${appsUrl}/456` +const appName = 'E2E-test-app' +const csrf = '' +const appLink = `${appName}` + +test('deletes only the exact E2E app with CSRF protection and confirms its absence', async () => { + const calls: string[] = [] + const http = { + async get(url: string) { + calls.push(url) + if (url.includes('search_term=')) return response(200, csrf + appLink) + return calls.length === 2 ? response(200, csrf) : response(404) + }, + async delete(url: string, options: Parameters[1]) { + calls.push(`DELETE ${url}`) + expect(options?.headers).toEqual({ + 'X-CSRF-Token': 'test&token', + Origin: 'https://dev.shopify.com', + Referer: `${appUrl}/settings`, + }) + return response(303, '', {location: '/dashboard/123/apps'}) + }, + } + await new DevDashboardClient(http, '123').deleteApp({name: appName, url: appUrl}) + expect(calls).toEqual([ + `${appsUrl}?search_term=${appName}`, + `${appUrl}/settings`, + `DELETE ${appUrl}`, + `${appUrl}/settings`, + ]) +}) + +for (const [name, html, status, message] of [ + ['login redirect', '', 302, 'Check the E2E session'], + ['server error', '', 503, 'Check the E2E session'], + ['missing CSRF', appLink, 200, 'no CSRF token'], + ['unknown page', `${csrf}

Loading

`, 200, 'Could not verify app search'], + ['duplicate names', `${csrf + appLink}${appName}`, 200, 'Multiple apps'], + [ + 'foreign pagination', + `${csrf + appLink}Next`, + 200, + 'outside the E2E organization', + ], +] as const) { + test(`rejects ${name} before sending deletion`, async () => { + let deletionRequested = false + const http = { + async get() { + return response(status, html) + }, + async delete() { + deletionRequested = true + return response(303) + }, + } + await expect(new DevDashboardClient(http, '123').deleteApp({name: appName})).rejects.toThrow(message) + expect(deletionRequested).toBe(false) + }) +} + +test('does not delete similarly named apps', async () => { + const http = { + async get() { + return response(200, `${csrf}${appName}-other`) + }, + async delete() { + throw new Error('Must not delete another app') + }, + } + await new DevDashboardClient(http, '123').deleteApp({name: appName}) +}) + +test('requires explicit empty search results and verifies a known app URL', async () => { + const http = { + async get(url: string) { + return url.includes('search_term=') ? response(200, `${csrf}

No apps matched your search.

`) : response(404) + }, + async delete() { + throw new Error('App is already absent') + }, + } + await new DevDashboardClient(http, '123').deleteApp({name: appName, url: appUrl}) +}) + +test('rejects unsafe app names and organization URLs before making requests', async () => { + const http = { + async get() { + throw new Error('Must not make a request') + }, + async delete() { + throw new Error('Must not make a request') + }, + } + const dashboard = new DevDashboardClient(http, '123') + await expect(dashboard.deleteApp({name: 'production-app'})).rejects.toThrow('non-E2E') + await expect( + dashboard.deleteApp({name: appName, url: 'https://dev.shopify.com/dashboard/999/apps/456'}), + ).rejects.toThrow('configured E2E organization') +}) + +test('does not treat the remaining-installs redirect as deletion success', async () => { + let deletionRequests = 0 + const http = { + async get() { + return response(200, csrf + appLink) + }, + async delete() { + deletionRequests++ + return response(303, '', {location: `${appUrl}/settings`}) + }, + } + await expect(new DevDashboardClient(http, '123').deleteApp({name: appName})).rejects.toThrow('deletion was rejected') + expect(deletionRequests).toBe(1) +}) + +test('retries a stale installation count after confirmed uninstall and refreshes CSRF', async () => { + let deletionRequests = 0 + const http = { + async get(url: string) { + if (url.includes('search_term=')) return response(200, csrf + appLink) + if (deletionRequests === 2) return response(404) + return response(200, ``) + }, + async delete(url: string, options: Parameters[1]) { + expect(url).toBe(appUrl) + expect(options?.headers?.['X-CSRF-Token']).toBe(`token-${deletionRequests}`) + deletionRequests++ + return response(303, '', {location: deletionRequests === 1 ? `${appUrl}/settings` : appsUrl}) + }, + } + await new DevDashboardClient(http, '123').deleteApp({name: appName, url: appUrl, uninstallConfirmed: true}) + expect(deletionRequests).toBe(2) +}) + +for (const [status, location] of [ + [302, `${appUrl}/settings`], + [503, `${appUrl}/settings`], + [303, `${appsUrl}/789/settings`], + [303, 'https://example.com/dashboard/123/apps/456/settings'], + [303, 'https://accounts.shopify.com/lookup?token=secret-query#secret-fragment'], + [303, ''], + [303, 'http://[invalid'], +] as const) { + test(`does not retry an unexpected deletion response after uninstall (${status}, ${location})`, async () => { + let deletionRequests = 0 + const http = { + async get() { + return response(200, csrf + appLink) + }, + async delete() { + deletionRequests++ + return response(status, '', {location}) + }, + } + const deletion = new DevDashboardClient(http, '123').deleteApp({name: appName, uninstallConfirmed: true}) + await expect(deletion).rejects.toThrow('deletion was rejected') + await expect(deletion).rejects.not.toThrow('secret-query') + await expect(deletion).rejects.not.toThrow('secret-fragment') + expect(deletionRequests).toBe(1) + }) +} + +test('stops retrying when the installation count has not settled after 15 seconds', async () => { + const originalNow = Date.now + let now = originalNow() + let deletionRequests = 0 + try { + Date.now = () => now + const http = { + async get() { + return response(200, csrf + appLink) + }, + async delete() { + deletionRequests++ + now += 15_000 + return response(303, '', {location: `${appUrl}/settings`}) + }, + } + await expect( + new DevDashboardClient(http, '123').deleteApp({name: appName, uninstallConfirmed: true}), + ).rejects.toThrow('timed out waiting for the dashboard installation count to reach zero') + expect(deletionRequests).toBe(1) + } finally { + Object.defineProperty(Date, 'now', {value: originalNow}) + } +}) + +test('stops retrying if the dashboard session is lost after an installation-count redirect', async () => { + let deletionRequests = 0 + const http = { + async get() { + return deletionRequests === 0 ? response(200, csrf + appLink) : response(302) + }, + async delete() { + deletionRequests++ + return response(303, '', {location: `${appUrl}/settings`}) + }, + } + await expect( + new DevDashboardClient(http, '123').deleteApp({name: appName, uninstallConfirmed: true}), + ).rejects.toThrow('Check the E2E session') + expect(deletionRequests).toBe(1) +}) + +test('follows pagination and rejects repeated cursors', async () => { + let requests = 0 + const nextLink = `Next` + const http = { + async get() { + requests++ + return response(200, csrf + appLink + nextLink) + }, + async delete() { + throw new Error('Must finish discovery before deletion') + }, + } + await expect(new DevDashboardClient(http, '123').deleteApp({name: appName})).rejects.toThrow( + 'pagination did not complete', + ) + expect(requests).toBe(2) +}) + +function response(status: number, body = '', headers: Record = {}): APIResponse { + return { + status: () => status, + headers: () => ({'content-type': 'text/html', ...headers}), + text: async () => body, + } as unknown as APIResponse +} diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts index ec4c95bd7d6..49c9ef10ad1 100644 --- a/packages/e2e/tests/dev-hot-reload.spec.ts +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -38,7 +38,7 @@ description = "E2E test trigger" } test.describe('Dev hot reload', () => { - test('editing app config TOML triggers reload', async ({cli, env, browserPage}) => { + test('editing app config TOML triggers reload', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.configReload test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -91,7 +91,7 @@ test.describe('Dev hot reload', () => { if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, @@ -104,7 +104,7 @@ test.describe('Dev hot reload', () => { } }) - test('creating a new extension mid-dev is detected', async ({cli, env, browserPage}) => { + test('creating a new extension mid-dev is detected', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.extensionCreation test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -150,7 +150,7 @@ test.describe('Dev hot reload', () => { if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, @@ -163,7 +163,7 @@ test.describe('Dev hot reload', () => { } }) - test('deleting an extension mid-dev is detected', async ({cli, env, browserPage}) => { + test('deleting an extension mid-dev is detected', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.extensionDeletion test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -215,7 +215,7 @@ test.describe('Dev hot reload', () => { if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, diff --git a/packages/e2e/tests/multi-config-dev.spec.ts b/packages/e2e/tests/multi-config-dev.spec.ts index 1598284a5b2..5e190b59720 100644 --- a/packages/e2e/tests/multi-config-dev.spec.ts +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -19,7 +19,7 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)) const FIXTURE_TOML = fs.readFileSync(path.join(__dirname, '../data/valid-app/shopify.app.toml'), 'utf8') test.describe('Multi-config dev', () => { - test('dev with -c flag loads the named config', async ({cli, env, browserPage}) => { + test('dev with -c flag loads the named config', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.namedConfig test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -100,7 +100,7 @@ extensions_summary = "E2E staging app extensions" if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, @@ -113,7 +113,7 @@ extensions_summary = "E2E staging app extensions" } }) - test('dev without -c flag uses default config', async ({cli, env, browserPage}) => { + test('dev without -c flag uses default config', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.defaultConfig test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -182,7 +182,7 @@ extensions_summary = "E2E staging app extensions" if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, diff --git a/packages/e2e/tests/persistent-stores.spec.ts b/packages/e2e/tests/persistent-stores.spec.ts index ba9da46e980..b46faadb883 100644 --- a/packages/e2e/tests/persistent-stores.spec.ts +++ b/packages/e2e/tests/persistent-stores.spec.ts @@ -6,7 +6,6 @@ import {e2eRunSegment} from '../setup/env.js' import {teardownAll} from '../setup/teardown.js' import {devDashboardAppUrl} from '../setup/app.js' import {cleanupTemporaryStore} from '../setup/temporary-store.js' -import {trackMainFrameStatus} from '../setup/browser.js' import {expect, test, type Page} from '@playwright/test' import {mkdtemp, rm, writeFile} from 'node:fs/promises' import {tmpdir} from 'node:os' @@ -192,11 +191,13 @@ for (const failure of ['none', 'preview', 'uninstall', 'deletion', 'all', 'proce appName: 'E2E-test-app', orgId: '123', storeFqdn: stores[0]!, - appUrl: 'https://dev.shopify.com/dashboard/123/apps/456', - browserPage: deletionPage(() => { - events.push('delete-app') - return fails('deletion') ? 500 : 404 - }), + dashboard: { + async deleteApp({uninstallConfirmed}: {uninstallConfirmed?: boolean}) { + events.push('delete-app') + expect(uninstallConfirmed).toBe(!fails('uninstall')) + if (fails('deletion')) throw new Error('App deletion was rejected') + }, + }, } if (failure === 'process-stop') { await expect(teardownAll(ctx)).rejects.toThrow('Process did not stop') @@ -232,20 +233,3 @@ for (const failure of ['none', 'preview', 'uninstall', 'deletion', 'all', 'proce } }) } - -function deletionPage(status: () => number): Page { - const frame = {} - let onResponse: (response: unknown) => void = () => {} - const page = { - on(_event: string, listener: (response: unknown) => void) { - onResponse = listener - }, - mainFrame: () => frame, - async goto() { - onResponse({frame: () => frame, request: () => ({resourceType: () => 'document'}), status}) - }, - async waitForTimeout() {}, - } as unknown as Page - trackMainFrameStatus(page) - return page -} diff --git a/packages/e2e/tests/toml-config-invalid.spec.ts b/packages/e2e/tests/toml-config-invalid.spec.ts index 0997d9d81c8..b406c6cbdd8 100644 --- a/packages/e2e/tests/toml-config-invalid.spec.ts +++ b/packages/e2e/tests/toml-config-invalid.spec.ts @@ -10,15 +10,16 @@ import {fileURLToPath} from 'url' const __dirname = path.dirname(fileURLToPath(import.meta.url)) const INVALID_TOMLS_DIR = path.join(__dirname, '../data/invalid-tomls') -const invalidTomls = fs.readdirSync(INVALID_TOMLS_DIR).filter((file) => file.endsWith('.toml')) +const invalidTomls = fs + .readdirSync(INVALID_TOMLS_DIR) + .filter((file) => file.endsWith('.toml') && file !== 'bad-syntax.toml') test.describe('TOML config invalid', () => { for (const tomlFile of invalidTomls) { const label = tomlFile.replace('.toml', '') test(`deploy rejects invalid toml: ${label}`, async ({cli, env}) => { - // Invalid TOMLs use a dummy client_id — the CLI rejects them at validation - // before any network request, so no real app is needed. + // Schema validation uses remote specifications; syntax-only coverage runs locally. const appDir = fs.mkdtempSync(path.join(env.tempDir, `invalid-toml-${label}-`)) try { const toml = fs.readFileSync(path.join(INVALID_TOMLS_DIR, tomlFile), 'utf8') diff --git a/packages/e2e/tests/toml-config.spec.ts b/packages/e2e/tests/toml-config.spec.ts index 30b0620c1ee..5f37c78454a 100644 --- a/packages/e2e/tests/toml-config.spec.ts +++ b/packages/e2e/tests/toml-config.spec.ts @@ -13,7 +13,7 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)) const FIXTURE_TOML = fs.readFileSync(path.join(__dirname, '../data/valid-app/shopify.app.toml'), 'utf8') test.describe('TOML config regression', () => { - test('deploy succeeds with fully populated toml', async ({cli, env, browserPage}) => { + test('deploy succeeds with fully populated toml', async ({cli, env, dashboard}) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -40,7 +40,7 @@ test.describe('TOML config regression', () => { if (!process.env.E2E_SKIP_TEARDOWN) { fs.rmSync(parentDir, {recursive: true, force: true}) await teardownAll({ - browserPage, + dashboard, appName, appUrl, orgId: env.orgId, @@ -50,7 +50,7 @@ test.describe('TOML config regression', () => { } }) - test('dev starts with fully populated toml', async ({cli, env, browserPage}) => { + test('dev starts with fully populated toml', async ({cli, env, dashboard}) => { const storeFqdn = env.storeFqdn || E2E_STORES.fullConfig test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -87,7 +87,7 @@ test.describe('TOML config regression', () => { if (!process.env.E2E_SKIP_TEARDOWN) { await teardownAll({ cli, - browserPage, + dashboard, appName, appUrl, appDir, diff --git a/packages/e2e/tests/toml-syntax.spec.ts b/packages/e2e/tests/toml-syntax.spec.ts new file mode 100644 index 00000000000..8ac3e3b5999 --- /dev/null +++ b/packages/e2e/tests/toml-syntax.spec.ts @@ -0,0 +1,23 @@ +/* eslint-disable no-restricted-imports */ +import {cliFixture as test} from '../setup/cli.js' +import {expect} from '@playwright/test' +import {mkdtemp, readFile, rm, writeFile} from 'node:fs/promises' +import {join} from 'node:path' + +test('deploy rejects malformed TOML without authentication', async ({cli, env}) => { + const appDir = await mkdtemp(join(env.tempDir, 'invalid-toml-')) + try { + const toml = await readFile(new URL('../data/invalid-tomls/bad-syntax.toml', import.meta.url), 'utf8') + await writeFile(join(appDir, 'shopify.app.toml'), toml) + await writeFile( + join(appDir, 'package.json'), + JSON.stringify({name: 'invalid-toml', version: '1.0.0', private: true}), + ) + + const result = await cli.exec(['app', 'deploy', '--path', appDir, '--allow-updates', '--allow-deletes']) + expect(result.exitCode).toBe(1) + expect(result.stdout + result.stderr).toContain('Unterminated string') + } finally { + await rm(appDir, {recursive: true, force: true}) + } +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7a1f983b4f9..84d77852a8d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -564,6 +564,9 @@ importers: node-pty: specifier: ^1.0.0 version: 1.1.0 + parse5: + specifier: 7.3.0 + version: 7.3.0 strip-ansi: specifier: ^7.2.0 version: 7.2.0