From 0edc788b02b3742c9cd0e9af6403280c1c36f748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 19 Oct 2026 15:48:09 +0200 Subject: [PATCH] Use fixed stores for CLI E2E tests --- .github/workflows/tests-pr.yml | 67 +----- docs/cli/testing-strategy.md | 2 +- packages/e2e/.env.example | 4 + packages/e2e/README.md | 50 +++++ packages/e2e/playwright.config.ts | 4 +- packages/e2e/setup/admin-api.ts | 14 +- packages/e2e/setup/app.ts | 20 +- packages/e2e/setup/cli.ts | 44 ++-- packages/e2e/setup/constants.ts | 11 + packages/e2e/setup/env.ts | 14 +- packages/e2e/setup/store.ts | 78 +------ packages/e2e/setup/teardown.ts | 189 ++++------------- packages/e2e/setup/temporary-store.ts | 80 +++++++ packages/e2e/tests/app-deploy.spec.ts | 4 +- packages/e2e/tests/app-dev-server.spec.ts | 10 +- packages/e2e/tests/app-scaffold.spec.ts | 6 +- packages/e2e/tests/dev-hot-reload.spec.ts | 20 +- packages/e2e/tests/multi-config-dev.spec.ts | 21 +- packages/e2e/tests/persistent-stores.spec.ts | 212 ++++++++++++++++++- packages/e2e/tests/smoke-pty.spec.ts | 7 + packages/e2e/tests/store-lifecycle.spec.ts | 51 +++++ packages/e2e/tests/toml-config.spec.ts | 12 +- 22 files changed, 568 insertions(+), 352 deletions(-) create mode 100644 packages/e2e/setup/temporary-store.ts create mode 100644 packages/e2e/tests/store-lifecycle.spec.ts diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 9a2d043e9c4..3da014cea94 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -252,8 +252,14 @@ jobs: # Playwright — those failures belong to the build lane, not to E2E. needs: [type-check, bundle] runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 45 continue-on-error: true + env: + E2E_ACCOUNT_EMAIL: ${{ secrets.E2E_ACCOUNT_EMAIL }} + E2E_ACCOUNT_PASSWORD: ${{ secrets.E2E_ACCOUNT_PASSWORD }} + E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }} + E2E_LOADTEST_HEADER: ${{ secrets.E2E_LOADTEST_HEADER }} + E2E_SHARD: ${{ matrix.shard }} strategy: fail-fast: false matrix: @@ -276,13 +282,8 @@ jobs: - name: Rebuild node-pty run: pnpm rebuild node-pty - name: Run E2E tests + id: e2e working-directory: packages/e2e - env: - E2E_ACCOUNT_EMAIL: ${{ secrets.E2E_ACCOUNT_EMAIL }} - E2E_ACCOUNT_PASSWORD: ${{ secrets.E2E_ACCOUNT_PASSWORD }} - E2E_STORE_FQDN: ${{ secrets.E2E_STORE_FQDN }} - E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }} - E2E_LOADTEST_HEADER: ${{ secrets.E2E_LOADTEST_HEADER }} run: pnpm exec playwright test --project remote --shard ${{ matrix.shard }} - name: Upload Playwright report uses: actions/upload-artifact@v4 @@ -304,58 +305,12 @@ jobs: !packages/e2e/test-results/**/trace.zip retention-days: 14 - e2e-cleanup: - name: 'Cleanup current-run E2E resources' - needs: e2e-tests - if: ${{ always() && needs.e2e-tests.result != 'skipped' }} - runs-on: ubuntu-latest - timeout-minutes: 20 - continue-on-error: true - steps: - - uses: actions/checkout@v6 - with: - repository: ${{ github.repository }} - ref: ${{ github.sha }} - fetch-depth: 1 - - name: Setup deps - uses: ./.github/actions/setup-cli-deps - with: - node-version: ${{ env.PLAYWRIGHT_NODE_VERSION }} - - name: Install Playwright Chromium - run: pnpm exec playwright install chromium - working-directory: packages/e2e - - name: Build CLI for cleanup auth - run: pnpm nx run cli:build - - name: Prime cleanup auth state - continue-on-error: true - env: - E2E_ACCOUNT_EMAIL: ${{ secrets.E2E_ACCOUNT_EMAIL }} - E2E_ACCOUNT_PASSWORD: ${{ secrets.E2E_ACCOUNT_PASSWORD }} - E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }} - E2E_LOADTEST_HEADER: ${{ secrets.E2E_LOADTEST_HEADER }} - run: pnpm --filter e2e exec tsx scripts/prime-browser-auth.ts - name: Cleanup current-run E2E apps - if: ${{ always() }} - continue-on-error: true - env: - E2E_ACCOUNT_EMAIL: ${{ secrets.E2E_ACCOUNT_EMAIL }} - E2E_ACCOUNT_PASSWORD: ${{ secrets.E2E_ACCOUNT_PASSWORD }} - E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }} - E2E_LOADTEST_HEADER: ${{ secrets.E2E_LOADTEST_HEADER }} - run: | - RUN_TOKEN=$(node -e "process.stdout.write(BigInt(process.env.GITHUB_RUN_ID).toString(36))") - pnpm --filter e2e exec tsx scripts/cleanup-apps.ts --pattern "r${RUN_TOKEN}" - - name: Cleanup current-run E2E stores - if: ${{ always() }} - continue-on-error: true - env: - E2E_ACCOUNT_EMAIL: ${{ secrets.E2E_ACCOUNT_EMAIL }} - E2E_ACCOUNT_PASSWORD: ${{ secrets.E2E_ACCOUNT_PASSWORD }} - E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }} - E2E_LOADTEST_HEADER: ${{ secrets.E2E_LOADTEST_HEADER }} + if: ${{ always() && steps.e2e.outcome != 'skipped' }} run: | RUN_TOKEN=$(node -e "process.stdout.write(BigInt(process.env.GITHUB_RUN_ID).toString(36))") - pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --pattern "r${RUN_TOKEN}" + SHARD_NUMBER=${E2E_SHARD%/*} + pnpm --filter e2e exec tsx scripts/cleanup-apps.ts --pattern "r${RUN_TOKEN}a${GITHUB_RUN_ATTEMPT}s${SHARD_NUMBER}-" type-diff: if: github.event.pull_request.head.repo.full_name == github.repository diff --git a/docs/cli/testing-strategy.md b/docs/cli/testing-strategy.md index 11d079cbdf8..9312e3ec172 100644 --- a/docs/cli/testing-strategy.md +++ b/docs/cli/testing-strategy.md @@ -95,7 +95,7 @@ For Vitest problems, see [our troubleshooting page](troubleshooting.md) or the [ End-to-end tests live under `packages/e2e` and are implemented using [Playwright](https://playwright.dev/). They test full user journeys by invoking the CLI and verifying outputs. Run them with `pnpm test:e2e`. -See the [E2E setup guide](../../packages/e2e/README.md) for legacy resource cleanup. +See the [E2E setup guide](../../packages/e2e/README.md) for persistent store configuration, CI isolation, and cleanup. ## GitHub Actions diff --git a/packages/e2e/.env.example b/packages/e2e/.env.example index b9b336403ee..016658fa241 100644 --- a/packages/e2e/.env.example +++ b/packages/e2e/.env.example @@ -10,6 +10,10 @@ E2E_ACCOUNT_PASSWORD= # CI secret: E2E_ORG_ID E2E_ORG_ID= +# Optional: use this development store instead of each test's fixed assignment. +# The test account must have access. Leave empty to use setup/constants.ts. +E2E_STORE_FQDN= + # Required: full X-Shopify-Loadtest- header name # CI secret: E2E_LOADTEST_HEADER E2E_LOADTEST_HEADER= diff --git a/packages/e2e/README.md b/packages/e2e/README.md index 7d90ce63432..49176abcd82 100644 --- a/packages/e2e/README.md +++ b/packages/e2e/README.md @@ -6,6 +6,56 @@ Run local tests without Shopify credentials: pnpm --filter e2e exec playwright test --project local ``` +## Persistent development stores + +App tests reuse existing stores. They never create or delete a store. Each test creates its own app and local files, then stops its dev process and attempts to remove its preview, uninstall its app, and delete the app. Stores can contain apps left by previous tests. + +Create dedicated development stores once in the E2E organization and give the E2E account access. Use the `no-delete-e2e-` name prefix: the store cleanup script refuses to delete these stores, including in `--force` mode. + +The store domains are defined in `E2E_STORES` in `setup/constants.ts`. Each of the seven dev cases selects a named store directly; store 8 is spare. Adding a test requires an explicit assignment, with no worker or shard allocation. + +Copy `.env.example` to `.env` and supply the account, organization, and load-test header. Set `E2E_STORE_FQDN` to override the fixed assignments for local development. The test account must have access to the selected store. + +Concurrent runs can use the same stores because each test creates a separate app and cleans only its own app and preview. Do not reuse the same app across concurrent runs. Keep local CLI processes in separate directories and use separate ports when running multiple suites on one machine. + +## 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. + +## CI setup + +Keep `E2E_ACCOUNT_EMAIL`, `E2E_ACCOUNT_PASSWORD`, `E2E_ORG_ID`, and `E2E_LOADTEST_HEADER` in Actions secrets. Store configuration does not require a repository variable. + +CI passes `--shard 1/2` or `--shard 2/2` to Playwright and runs four workers per shard. Store assignment does not depend on the shard or worker count. E2E jobs do not lock the stores across runs. + +App names and cleanup patterns include the run, attempt, and shard. Cleanup cannot select another run's apps or delete the fixed development stores. + +To select one shard locally: + +```sh +pnpm --filter e2e exec playwright test --project remote --shard 1/2 +``` + +## Store lifecycle coverage + +`tests/store-lifecycle.spec.ts` is the one exception to store reuse. It creates one temporary development store with `store create dev --json`, checks it with `store list --json`, and deletes it with `store delete --force --json`. It requires confirmed deletion and checks that the store is absent from the active store list. It runs once across the two remote shards and needs one free store slot in the E2E organization. + +The lifecycle commands do not use a browser. They use the CLI session from shared authentication. The temporary store has an `e2e-store-` name with the run marker and a random suffix. Cleanup runs even when the test fails or `E2E_SKIP_TEARDOWN=1`, with a separate timeout. It finds only the exact test name and refuses to delete protected stores. The test report includes the temporary name for recovery if the runner stops before cleanup or provisioning finishes late. + +Run only this test with: + +```sh +pnpm --filter e2e exec playwright test tests/store-lifecycle.spec.ts --project remote --workers 1 +``` + +## Failed or cancelled runs + +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. + +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. + ## Old store cleanup `cleanup-stores.ts` remains available for old temporary stores. It excludes the `no-delete-e2e-` prefix and all stores configured in `E2E_STORE_FQDNS` or `E2E_STORE_FQDN`. Full and delete modes use five browser workers and the existing `store delete` behavior. Force mode sends API deletion requests with twenty workers and skips app checks. Confirmed deletions and requests still awaiting confirmation are reported separately. Force deletion can leave app installation records that prevent app deletion until the backend clears them. diff --git a/packages/e2e/playwright.config.ts b/packages/e2e/playwright.config.ts index f213aa9980a..aed340bbdcc 100644 --- a/packages/e2e/playwright.config.ts +++ b/packages/e2e/playwright.config.ts @@ -12,11 +12,11 @@ export default defineConfig({ fullyParallel: true, forbidOnly: isCI, retries: 0, - workers: 10, + workers: 4, maxFailures: isCI ? 3 : 0, // Stop early in CI after 3 failures reporter: isCI ? [['html', {open: 'never'}], ['list']] : [['list']], timeout: TEST_TIMEOUT.default, // Heavy tests override via test.setTimeout() - globalTimeout: 20 * 60 * 1000, + globalTimeout: 30 * 60 * 1000, use: { trace: isCI ? 'on' : 'off', diff --git a/packages/e2e/setup/admin-api.ts b/packages/e2e/setup/admin-api.ts index d7c0b83a70d..13a8060d48a 100644 --- a/packages/e2e/setup/admin-api.ts +++ b/packages/e2e/setup/admin-api.ts @@ -25,7 +25,7 @@ interface AdminApiUninstallCtx { * 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. + * Throws on failure so teardown can report the app that needs cleanup. */ export async function uninstallAppWithAdminApi(ctx: AdminApiUninstallCtx): Promise { const clientId = extractClientId(ctx.appDir) @@ -55,7 +55,10 @@ async function mintAppAccessTokenWithRetry(storeFqdn: string, clientId: string, } async function fetchClientSecret(ctx: AdminApiUninstallCtx): Promise { - const result = await ctx.cli.exec(['app', 'env', 'show', '--path', ctx.appDir], {timeout: CLI_TIMEOUT.short}) + const result = await ctx.cli.exec(['app', 'env', 'show', '--path', ctx.appDir], { + timeout: CLI_TIMEOUT.short, + quiet: true, + }) if (result.exitCode !== 0) { throw new Error(`app env show failed (exit ${result.exitCode}): ${result.stderr}`) } @@ -69,6 +72,7 @@ async function fetchClientSecret(ctx: AdminApiUninstallCtx): Promise { async function mintAppAccessToken(storeFqdn: string, clientId: string, clientSecret: string): Promise { const response = await fetch(`https://${storeFqdn}/admin/oauth/access_token`, { method: 'POST', + signal: AbortSignal.timeout(CLI_TIMEOUT.short), headers: {'Content-Type': 'application/x-www-form-urlencoded', ...loadtestHeaderRecord()}, body: new URLSearchParams({ grant_type: 'client_credentials', @@ -92,6 +96,7 @@ async function mintAppAccessToken(storeFqdn: string, clientId: string, clientSec async function runAppUninstall(storeFqdn: string, accessToken: string): Promise { const response = await fetch(`https://${storeFqdn}/admin/api/${ADMIN_API_VERSION}/graphql.json`, { method: 'POST', + signal: AbortSignal.timeout(CLI_TIMEOUT.short), headers: { 'Content-Type': 'application/json', 'X-Shopify-Access-Token': accessToken, @@ -111,7 +116,10 @@ async function runAppUninstall(storeFqdn: string, accessToken: string): Promise< if (payload.errors) { throw new Error(`appUninstall returned errors: ${JSON.stringify(payload.errors)}`) } - const userErrors = payload.data?.appUninstall?.userErrors ?? [] + const userErrors = payload.data?.appUninstall?.userErrors + if (!Array.isArray(userErrors)) { + throw new Error('appUninstall returned an invalid response; uninstall was not confirmed') + } if (userErrors.length > 0) { throw new Error(`appUninstall returned user errors: ${userErrors.map((error) => error.message).join(', ')}`) } diff --git a/packages/e2e/setup/app.ts b/packages/e2e/setup/app.ts index 1f8a3b94fb4..97315519632 100644 --- a/packages/e2e/setup/app.ts +++ b/packages/e2e/setup/app.ts @@ -111,10 +111,22 @@ export function extractClientId(appDir: string, configName?: string): string { } /** - * Build the direct Dev Dashboard URL for an app from its local TOML client_id. + * Resolve the numeric app ID used by Dev Dashboard routes, which differ from client IDs. */ -export function devDashboardAppUrl(appDir: string, orgId: string, configName?: string): string { - return `https://dev.shopify.com/dashboard/${orgId}/apps/${extractClientId(appDir, configName)}` +export async function devDashboardAppUrl(ctx: { + cli: Pick + appDir: string + orgId: string + configName?: string +}): Promise { + const args = ['app', 'info', '--json', '--path', ctx.appDir] + if (ctx.configName) args.push('--config', ctx.configName) + const result = await ctx.cli.exec(args, {quiet: true}) + if (result.exitCode !== 0) throw new Error(`Can't resolve the app's dashboard URL: ${result.stderr}`) + const info = JSON.parse(result.stdout) as {remoteApp?: {id?: string}} + const appId = info.remoteApp?.id?.match(/^(?:gid:\/\/shopify\/App\/)?(\d+)$/)?.[1] + if (!appId) throw new Error("app info didn't return a valid numeric app ID") + return `https://dev.shopify.com/dashboard/${ctx.orgId}/apps/${appId}` } /** @@ -328,7 +340,7 @@ export async function findAppOnDevDashboard(page: Page, appName: string, orgId?: const org = orgId ?? (process.env.E2E_ORG_ID ?? '').trim() const email = process.env.E2E_ACCOUNT_EMAIL - await navigateToDashboard({browserPage: page, email, orgId: org}) + await navigateToDashboard({browserPage: page, email, orgId: org, searchTerm: appName}) // Scan current page + pagination for the app while (true) { diff --git a/packages/e2e/setup/cli.ts b/packages/e2e/setup/cli.ts index ad1793d171f..a81817bc858 100644 --- a/packages/e2e/setup/cli.ts +++ b/packages/e2e/setup/cli.ts @@ -40,11 +40,16 @@ export interface SpawnedProcess { export interface CLIProcess { /** Execute a CLI command non-interactively via execa */ - exec(args: string[], opts?: {cwd?: string; env?: NodeJS.ProcessEnv; timeout?: number}): Promise + exec( + args: string[], + opts?: {cwd?: string; env?: NodeJS.ProcessEnv; timeout?: number; quiet?: boolean}, + ): Promise /** Execute the create-app binary non-interactively via execa */ execCreateApp(args: string[], opts?: {cwd?: string; env?: NodeJS.ProcessEnv; timeout?: number}): Promise /** Spawn an interactive CLI command via node-pty */ spawn(args: string[], opts?: {cwd?: string; env?: NodeJS.ProcessEnv}): Promise + /** Stop all owned interactive processes before resetting their remote state. */ + stopAll(): Promise } /** @@ -59,9 +64,9 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ // When DEBUG=1, tee the subprocess streams to the parent so the CLI's // info/success boxes and progress messages appear live, while still // letting execa buffer and return the captured output. - const runExeca = (bin: string, args: string[], execaOpts: ExecaOptions) => { + const runExeca = (bin: string, args: string[], execaOpts: ExecaOptions, quiet = false) => { const subprocess = execa('node', [bin, ...args], execaOpts) - if (process.env.DEBUG === '1') { + if (process.env.DEBUG === '1' && !quiet) { subprocess.stdout?.pipe(process.stdout, {end: false}) subprocess.stderr?.pipe(process.stderr, {end: false}) } @@ -69,6 +74,10 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ } const cli: CLIProcess = { + async stopAll() { + for (const proc of spawnedProcesses) proc.kill() + await Promise.all(spawnedProcesses.map((proc) => proc.waitForExit(CLI_TIMEOUT.short))) + }, async exec(args, opts = {}) { const timeout = opts.timeout ?? CLI_TIMEOUT.medium const execaOpts: ExecaOptions = { @@ -81,7 +90,7 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ cliLog.log(env, `exec: node ${executables.cli}`) cliLog.log(env, args.join(' ')) - const result = await runExeca(executables.cli, args, execaOpts) + const result = await runExeca(executables.cli, args, execaOpts, opts.quiet) return { stdout: result.stdout ?? '', @@ -154,17 +163,20 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ }) let exitCode: number | undefined - let exitResolve: ((code: number) => void) | undefined + const exitListeners = new Set<(code: number) => void>() ptyProcess.onExit(({exitCode: code}) => { exitCode = code - if (exitResolve) { - exitResolve(code) - } + for (const listener of exitListeners) listener(code) + exitListeners.clear() // Reject any remaining output waiters. reject() removes each waiter // from outputWaiters, so iterate over a snapshot to avoid skipping. for (const waiter of [...outputWaiters]) { - waiter.reject(new Error(`Process exited (code ${code}) while waiting for output: "${waiter.text}"`)) + waiter.reject( + new Error( + `Process exited (code ${code}) while waiting for output: "${waiter.text}"\n\nCaptured output:\n${stripAnsi(output)}`, + ), + ) } }) @@ -247,17 +259,20 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ return new Promise((resolve, reject) => { const timer = setTimeout(() => { + exitListeners.delete(onExit) reject(new Error(`Timed out after ${timeoutMs}ms waiting for process exit`)) }, timeoutMs) - exitResolve = (code) => { + const onExit = (code: number) => { clearTimeout(timer) resolve(code) } + exitListeners.add(onExit) }) }, kill() { + if (exitCode !== undefined) return try { ptyProcess.kill() // eslint-disable-next-line no-catch-all/no-catch-all @@ -276,11 +291,10 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({ }, } - await use(cli) - - // Teardown: kill all spawned processes - for (const proc of spawnedProcesses) { - proc.kill() + try { + await use(cli) + } finally { + await cli.stopAll() } }, }) diff --git a/packages/e2e/setup/constants.ts b/packages/e2e/setup/constants.ts index 4d098f63efa..ac277d555a4 100644 --- a/packages/e2e/setup/constants.ts +++ b/packages/e2e/setup/constants.ts @@ -1,3 +1,14 @@ +export const E2E_STORES = { + devServer: 'no-delete-e2e-reusable-store-1.myshopify.com', + configReload: 'no-delete-e2e-reusable-store-2.myshopify.com', + extensionCreation: 'no-delete-e2e-reusable-store-3.myshopify.com', + extensionDeletion: 'no-delete-e2e-reusable-store-4.myshopify.com', + namedConfig: 'no-delete-e2e-reusable-store-5.myshopify.com', + defaultConfig: 'no-delete-e2e-reusable-store-6.myshopify.com', + fullConfig: 'no-delete-e2e-reusable-store-7.myshopify.com', + spare: 'no-delete-e2e-reusable-store-8.myshopify.com', +} as const + // --------------------------------------------------------------------------- // Shared timeout constants for E2E tests // --------------------------------------------------------------------------- diff --git a/packages/e2e/setup/env.ts b/packages/e2e/setup/env.ts index 37858cf50e2..47b5152924d 100644 --- a/packages/e2e/setup/env.ts +++ b/packages/e2e/setup/env.ts @@ -8,7 +8,7 @@ const __filename = fileURLToPath(import.meta.url) const __dirname = path.dirname(__filename) export interface E2EEnv { - /** Dev store FQDN (e.g. cli-e2e-test.myshopify.com) */ + /** Optional store override for local runs. */ storeFqdn: string /** Dedicated e2e org ID for fresh-app tests (empty string if not set) */ orgId: string @@ -134,10 +134,11 @@ const E2E_APP_PREFIXES: Record = { /** * Generate a short E2E app name that can be clustered by GitHub Actions run. */ -export function e2eRunSegment(): string { - const runId = process.env.GITHUB_RUN_ID - const runAttempt = process.env.GITHUB_RUN_ATTEMPT ?? '1' - return runId ? `r${BigInt(runId).toString(36)}a${runAttempt}` : 'local' +export function e2eRunSegment(env: NodeJS.ProcessEnv = process.env): string { + const runId = env.GITHUB_RUN_ID + const runAttempt = env.GITHUB_RUN_ATTEMPT ?? '1' + const shardNumber = env.E2E_SHARD?.split('/')[0] ?? '1' + return runId ? `r${BigInt(runId).toString(36)}a${runAttempt}s${shardNumber}` : 'local' } export function e2eAppName(prefix: string): string { @@ -175,6 +176,7 @@ export const envFixture = base.extend<{testSection: void}, {env: E2EEnv}>({ process.platform === 'win32' ? 'cloudflared.exe' : 'cloudflared', ) + const portBase = 3457 + workerInfo.parallelIndex * 10 const processEnv: NodeJS.ProcessEnv = { ...process.env, ...xdgEnv, @@ -184,6 +186,8 @@ export const envFixture = base.extend<{testSection: void}, {env: E2EEnv}>({ NODE_OPTIONS: '', CI: '1', SHOPIFY_FLAG_CLIENT_ID: undefined, + SHOPIFY_FLAG_GRAPHIQL_PORT: String(portBase), + SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT: String(portBase + 2), } if (storeFqdn) { diff --git a/packages/e2e/setup/store.ts b/packages/e2e/setup/store.ts index 5dff5c3d50f..9939c60a4f3 100644 --- a/packages/e2e/setup/store.ts +++ b/packages/e2e/setup/store.ts @@ -1,54 +1,21 @@ /* eslint-disable no-await-in-loop */ -import {appTestFixture} from './app.js' import {isVisibleWithin} from './browser.js' import {BROWSER_TIMEOUT, CLI_TIMEOUT} from './constants.js' -import {createLogger, e2eRunSegment, e2eSection, requireEnv} from './env.js' import {isProtectedStore, readPersistentStores} from './persistent-stores.js' import type {CLIProcess, ExecResult} from './cli.js' import type {Locator, Page} from '@playwright/test' -const log = createLogger('cli') - // --------------------------------------------------------------------------- -// Dev store provisioning +// Legacy store cleanup // --------------------------------------------------------------------------- -/** Generate a unique store name for a worker. */ -export function generateStoreName(workerIndex: number): string { - const timestampSegment = Date.now().toString(36) - return `e2e-w${workerIndex}-${e2eRunSegment()}-${timestampSegment}` -} - -interface WorkerCtx { - workerIndex: number -} - interface StoreCommandJson { store?: { - domain?: unknown deletionRequested?: unknown deletionConfirmed?: unknown } } -/** Create a development store with the CLI and return its FQDN. */ -export async function createDevStoreWithCli( - ctx: WorkerCtx & {cli: CLIProcess; storeName: string; orgId: string}, -): Promise { - e2eSection(ctx, `Setup: store ${ctx.storeName}`) - log.log(ctx, 'creating store') - - const result = await ctx.cli.exec( - ['store', 'create', 'dev', '--name', ctx.storeName, '--organization-id', ctx.orgId, '--plan', 'basic', '--json'], - {timeout: CLI_TIMEOUT.store}, - ) - assertCommandSucceeded('create development store', result) - - const storeFqdn = parseStoreDomain(result, 'create development store') - log.log(ctx, `store created ${storeFqdn}`) - return storeFqdn -} - /** Request development store deletion with the CLI and report whether the CLI confirmed it. */ export async function deleteDevStoreWithCli(options: { cli: Pick @@ -86,15 +53,6 @@ function assertCommandSucceeded(action: string, result: ExecResult): void { } } -function parseStoreDomain(result: ExecResult, action: string): string { - const output = parseJsonOutput(result, action) - const domain = output.store?.domain - if (typeof domain !== 'string' || !domain.endsWith('.myshopify.com')) { - throw new Error(`CLI returned an invalid store domain:\n${result.stdout}`) - } - return domain -} - function parseJsonOutput(result: ExecResult, action: string): StoreCommandJson { try { return JSON.parse(result.stdout) as StoreCommandJson @@ -265,37 +223,3 @@ async function dismissOpenPopover(page: Page, modal: Locator): Promise { export async function isStoreAppsEmpty(page: Page): Promise { return isVisibleWithin(page.locator('text=Add apps to your store'), BROWSER_TIMEOUT.medium) } - -// --------------------------------------------------------------------------- -// Fixture — per-test dev store for tests that need `app dev` -// --------------------------------------------------------------------------- - -/** - * Test-scoped fixture that creates a fresh dev store per test. - * - * Each test gets its own isolated store — no shared state between tests. - * Store + app cleanup is handled by teardownAll() in the test's finally block. - * - * Fixture chain: envFixture → cliFixture → browserFixture → authFixture → appTestFixture → storeTestFixture - * - * Tests that need a dev store (app dev, hot reload, multi-config dev) use this fixture. - * Tests that don't (scaffold, deploy, commands, smoke) stay on appTestFixture. - */ -export const storeTestFixture = appTestFixture.extend<{storeFqdn: string}>({ - storeFqdn: async ({cli, env}, use) => { - requireEnv(env, 'orgId') - const wi = env.workerIndex - - // Unique ports per worker to avoid EADDRINUSE when running in parallel - const portBase = 3457 + wi * 10 - env.processEnv.SHOPIFY_FLAG_GRAPHIQL_PORT = String(portBase) - env.processEnv.SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT = String(portBase + 2) - - const storeName = generateStoreName(wi) - const fqdn = await createDevStoreWithCli({cli, workerIndex: wi, storeName, orgId: env.orgId}) - - env.processEnv.SHOPIFY_FLAG_STORE = fqdn // eslint-disable-line require-atomic-updates - - await use(fqdn) - }, -}) diff --git a/packages/e2e/setup/teardown.ts b/packages/e2e/setup/teardown.ts index fcfb0aa32f5..fceb488111d 100644 --- a/packages/e2e/setup/teardown.ts +++ b/packages/e2e/setup/teardown.ts @@ -1,21 +1,17 @@ -/* 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 {BROWSER_TIMEOUT} from './constants.js' -import {uninstallAppFromStore, deleteDevStoreWithCli, isStoreAppsEmpty, dismissDevConsole} from './store.js' +import {findAppOnDevDashboard, deleteAppFromDevDashboard} from './app.js' import type {CLIProcess} from './cli.js' import type {Page} from '@playwright/test' -const log = createLogger('browser') +const log = createLogger('teardown') interface BaseTeardownCtx { browserPage: Page appName: string - /** Direct Dev Dashboard app URL. Prefer this when available to avoid slow org-wide pagination. */ + /** Direct dashboard URL for app cleanup. */ appUrl?: string - /** Local app directory. When present, uninstall goes through the Admin API instead of the store admin UI. */ + /** Local app directory used to clean the preview and uninstall through the Admin API. */ appDir?: string workerIndex?: number } @@ -24,159 +20,46 @@ type TeardownCtx = BaseTeardownCtx & ({storeFqdn: string; orgId: string; cli: CLIProcess} | {storeFqdn?: undefined; orgId?: string; cli?: CLIProcess}) /** - * 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) - * - * App-only flow: - * Phase 3 only + * Stop owned processes, then attempt each remote cleanup step independently. + * Remote cleanup failures are reported without preventing reuse of the store. */ 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') - } - - // Phase 2: Delete store - log.log(wCtx, 'deleting store') - let storeDeletionRequested = false - let safeToDelete = false + await ctx.cli?.stopAll() - // 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') - } + if (ctx.storeFqdn && ctx.appDir) { + const {cli, appDir, storeFqdn} = ctx + e2eSection(wCtx, `Teardown: app ${ctx.appName} on ${storeFqdn}`) + await attemptRemoteCleanup(ctx, 'preview removal', async () => { + const result = await cli.exec(['app', 'dev', 'clean', '--path', appDir, '--store', storeFqdn, '--json']) + if (result.exitCode !== 0 || JSON.parse(result.stdout).status !== 'success') { + throw new Error(`Failed to clean the dev preview:\n${result.stdout}\n${result.stderr}`) } - // 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}`) - } - - 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') - } - } - - // Gate: confirm the store has zero apps before attempting delete app. - if (!uninstalled) { - log.log(wCtx, 'skipping app delete — uninstall failed, run `pnpm test:e2e-cleanup-apps` after') - return - } + }) + await attemptRemoteCleanup(ctx, 'app uninstall', () => uninstallAppWithAdminApi({cli, appDir, storeFqdn})) } - // Phase 3: Delete app from dev dashboard e2eSection(wCtx, `Teardown: app ${ctx.appName}`) - log.log(wCtx, 'deleting app') - let appDeleted = false - let stillHasInstalls = false - for (let attempt = 1; attempt <= 3; attempt++) { - try { - const appUrl = 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`) - // 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}`) + 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') } - } - if (!appDeleted && !stillHasInstalls) { - log.error(wCtx, 'app deletion failed after 3 attempts') + log.log(wCtx, 'app deletion confirmed') + }) +} + +async function attemptRemoteCleanup(ctx: TeardownCtx, phase: string, cleanup: () => Promise): Promise { + try { + await cleanup() + // eslint-disable-next-line no-catch-all/no-catch-all -- Remote cleanup failures must not prevent later cleanup or store reuse. + } catch (error) { + const store = ctx.storeFqdn ? ` on ${ctx.storeFqdn}` : '' + // eslint-disable-next-line no-console -- Cleanup failures must be visible even when DEBUG is disabled. + console.warn( + `[e2e][w${ctx.workerIndex ?? 0}][cleanup] ${phase} failed for ${ctx.appName}${store}: ${ + error instanceof Error ? error.message : String(error) + }. Recover this app with cleanup-apps.ts --pattern "${ctx.appName}".`, + ) } } diff --git a/packages/e2e/setup/temporary-store.ts b/packages/e2e/setup/temporary-store.ts new file mode 100644 index 00000000000..34afc8b294b --- /dev/null +++ b/packages/e2e/setup/temporary-store.ts @@ -0,0 +1,80 @@ +import {authFixture} from './auth.js' +import {CLI_TIMEOUT} from './constants.js' +import {e2eRunSegment, requireEnv} from './env.js' +import {isProtectedStore, readPersistentStores} from './persistent-stores.js' +import {deleteDevStoreWithCli} from './store.js' +import {randomUUID} from 'node:crypto' +import type {CLIProcess} from './cli.js' + +interface TemporaryStore { + store: string + name: string + type: string + plan?: string +} + +export async function findTemporaryStore({ + cli, + orgId, + name, +}: { + cli: Pick + orgId: string + name: string +}): Promise { + if (!name.startsWith('e2e-store-')) throw new Error(`Invalid temporary store name: ${name}`) + + const result = await cli.exec(['store', 'list', '--organization-id', orgId, '--type', 'dev', '--json'], { + timeout: CLI_TIMEOUT.short, + }) + if (result.exitCode !== 0) { + throw new Error(`Could not list temporary stores:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`) + } + const output = JSON.parse(result.stdout) + if (output.organization?.id !== orgId || output.notice || !Array.isArray(output.stores)) { + throw new Error(`Could not verify the store list for organization ${orgId}.`) + } + + const matches: TemporaryStore[] = output.stores.filter((store: TemporaryStore) => store.name === name) + if (matches.length > 1) throw new Error(`Multiple stores have the temporary name ${name}.`) + const store = matches[0] + if (!store) { + if (output.truncated) throw new Error(`The store list is truncated; unable to verify whether ${name} exists.`) + return undefined + } + + const [domain] = readPersistentStores({E2E_STORE_FQDN: store.store}) + if (store.type !== 'dev' || !domain || isProtectedStore(domain, readPersistentStores(process.env, false))) { + throw new Error(`Refusing to use ${store.store} as a temporary store.`) + } + return store +} + +export async function cleanupTemporaryStore(options: { + cli: Pick + orgId: string + name: string +}): Promise { + const store = await findTemporaryStore(options) + if (!store) return + const confirmed = await deleteDevStoreWithCli({...options, storeFqdn: store.store}) + if (!confirmed) throw new Error(`Deletion of temporary store ${store.store} was not confirmed.`) +} + +export const temporaryStoreFixture = authFixture.extend<{temporaryStoreName: string}>({ + temporaryStoreName: [ + async ({cli, env, authLogin: _authLogin}, use, testInfo) => { + requireEnv(env, 'orgId') + const name = `e2e-store-${e2eRunSegment()}-${randomUUID().slice(0, 8)}` + await testInfo.attach('temporary-store', {body: name, contentType: 'text/plain'}) + + try { + await use(name) + } finally { + // Creation can succeed remotely before the CLI returns its domain. + await cleanupTemporaryStore({cli, orgId: env.orgId, name}) + } + }, + {timeout: CLI_TIMEOUT.store + CLI_TIMEOUT.short}, + ], +}) diff --git a/packages/e2e/tests/app-deploy.spec.ts b/packages/e2e/tests/app-deploy.spec.ts index a8c1c0fac37..9dc4830c095 100644 --- a/packages/e2e/tests/app-deploy.spec.ts +++ b/packages/e2e/tests/app-deploy.spec.ts @@ -105,7 +105,7 @@ test.describe('App deploy', () => { }) expect(initResult.exitCode, `Step 1 - primary app init failed:\n${initResult.stderr}`).toBe(0) const appDir = initResult.appDir - primaryAppUrl = devDashboardAppUrl(appDir, env.orgId) + primaryAppUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) // Step 2: Deploy with a tagged version const versionTag = `E2E-v1-${Date.now()}` @@ -156,7 +156,7 @@ test.describe('App deploy', () => { fs.existsSync(secondaryTomlPath), `Step 4 - expected ${secondaryTomlPath} to exist after config link`, ).toBe(true) - secondaryAppUrl = devDashboardAppUrl(appDir, env.orgId, secondaryConfig) + secondaryAppUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId, configName: secondaryConfig}) // Step 5: Deploy from primary dir to secondary app via --config secondary const secondaryVersionTag = `E2E-v2-${Date.now()}` diff --git a/packages/e2e/tests/app-dev-server.spec.ts b/packages/e2e/tests/app-dev-server.spec.ts index 19bbf43ec57..e3586fa45a8 100644 --- a/packages/e2e/tests/app-dev-server.spec.ts +++ b/packages/e2e/tests/app-dev-server.spec.ts @@ -1,14 +1,14 @@ -import {createApp, devDashboardAppUrl} from '../setup/app.js' +import {createApp, devDashboardAppUrl, appTestFixture as test} from '../setup/app.js' import {teardownAll} from '../setup/teardown.js' -import {CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' +import {E2E_STORES, CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' import {e2eAppName, requireEnv} from '../setup/env.js' -import {storeTestFixture as test} from '../setup/store.js' import {expect} from '@playwright/test' 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, storeFqdn}) => { + test('dev starts, shows ready message, and quits with q', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.devServer test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -31,7 +31,7 @@ test.describe('App dev server', () => { 0, ) appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) // Step 2: Start dev server via PTY, targeting the worker's store const dev = await cli.spawn(['app', 'dev', '--path', appDir], { diff --git a/packages/e2e/tests/app-scaffold.spec.ts b/packages/e2e/tests/app-scaffold.spec.ts index 5a62c8659f2..a532e2afa10 100644 --- a/packages/e2e/tests/app-scaffold.spec.ts +++ b/packages/e2e/tests/app-scaffold.spec.ts @@ -33,7 +33,7 @@ test.describe('App scaffold', () => { const initOutput = initResult.stdout + initResult.stderr expect(initOutput).toContain('is ready for you to build!') const appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) // Step 2: Verify the app directory was created with expected files expect(fs.existsSync(appDir)).toBe(true) @@ -81,7 +81,7 @@ test.describe('App scaffold', () => { expect(initResult.exitCode, `createApp failed:\nstdout: ${initResult.stdout}\nstderr: ${initResult.stderr}`).toBe( 0, ) - appUrl = devDashboardAppUrl(initResult.appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir: initResult.appDir, orgId: env.orgId}) expect(fs.existsSync(initResult.appDir)).toBe(true) expect(fs.existsSync(path.join(initResult.appDir, 'shopify.app.toml'))).toBe(true) } finally { @@ -121,7 +121,7 @@ test.describe('App scaffold', () => { 0, ) const appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) const extensionConfigs = [ {name: 'test-subscription-link', template: 'subscription_link_extension'}, diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts index f976451aaf5..ec4c95bd7d6 100644 --- a/packages/e2e/tests/dev-hot-reload.spec.ts +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -1,10 +1,9 @@ /* eslint-disable no-console */ /* eslint-disable no-restricted-imports */ -import {createApp, devDashboardAppUrl, injectFixtureToml} from '../setup/app.js' +import {appTestFixture as test, createApp, devDashboardAppUrl, injectFixtureToml} from '../setup/app.js' import {teardownAll} from '../setup/teardown.js' -import {CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' +import {E2E_STORES, CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' import {e2eAppName, requireEnv} from '../setup/env.js' -import {storeTestFixture as test} from '../setup/store.js' import {updateTomlValues} from '@shopify/toml-patch' import {expect} from '@playwright/test' import * as fs from 'fs' @@ -39,7 +38,8 @@ description = "E2E test trigger" } test.describe('Dev hot reload', () => { - test('editing app config TOML triggers reload', async ({cli, env, browserPage, storeFqdn}) => { + test('editing app config TOML triggers reload', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.configReload test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -52,7 +52,7 @@ test.describe('Dev hot reload', () => { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -104,7 +104,8 @@ test.describe('Dev hot reload', () => { } }) - test('creating a new extension mid-dev is detected', async ({cli, env, browserPage, storeFqdn}) => { + test('creating a new extension mid-dev is detected', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.extensionCreation test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -117,7 +118,7 @@ test.describe('Dev hot reload', () => { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -162,7 +163,8 @@ test.describe('Dev hot reload', () => { } }) - test('deleting an extension mid-dev is detected', async ({cli, env, browserPage, storeFqdn}) => { + test('deleting an extension mid-dev is detected', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.extensionDeletion test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -175,7 +177,7 @@ test.describe('Dev hot reload', () => { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) injectFixtureToml(appDir, FIXTURE_TOML, appName) diff --git a/packages/e2e/tests/multi-config-dev.spec.ts b/packages/e2e/tests/multi-config-dev.spec.ts index d2559fc783e..1598284a5b2 100644 --- a/packages/e2e/tests/multi-config-dev.spec.ts +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -1,10 +1,15 @@ /* eslint-disable no-console */ /* eslint-disable no-restricted-imports */ -import {createApp, devDashboardAppUrl, extractClientId, injectFixtureToml} from '../setup/app.js' +import { + appTestFixture as test, + createApp, + devDashboardAppUrl, + extractClientId, + injectFixtureToml, +} from '../setup/app.js' import {teardownAll} from '../setup/teardown.js' -import {CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' +import {E2E_STORES, CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' import {e2eAppName, requireEnv} from '../setup/env.js' -import {storeTestFixture as test} from '../setup/store.js' import {expect} from '@playwright/test' import * as fs from 'fs' import * as path from 'path' @@ -14,7 +19,8 @@ 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, storeFqdn}) => { + test('dev with -c flag loads the named config', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.namedConfig test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -27,7 +33,7 @@ test.describe('Multi-config dev', () => { const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) // Inject the fully populated TOML as the default config injectFixtureToml(appDir, FIXTURE_TOML, appName) @@ -107,7 +113,8 @@ extensions_summary = "E2E staging app extensions" } }) - test('dev without -c flag uses default config', async ({cli, env, browserPage, storeFqdn}) => { + test('dev without -c flag uses default config', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.defaultConfig test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -120,7 +127,7 @@ extensions_summary = "E2E staging app extensions" const initResult = await createApp({cli, parentDir, name: appName, template: 'none', orgId: env.orgId}) expect(initResult.exitCode, `createApp failed:\nstderr: ${initResult.stderr}`).toBe(0) appDir = initResult.appDir - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) injectFixtureToml(appDir, FIXTURE_TOML, appName) const clientId = extractClientId(appDir) diff --git a/packages/e2e/tests/persistent-stores.spec.ts b/packages/e2e/tests/persistent-stores.spec.ts index 0d37f315c39..ba9da46e980 100644 --- a/packages/e2e/tests/persistent-stores.spec.ts +++ b/packages/e2e/tests/persistent-stores.spec.ts @@ -1,10 +1,21 @@ -import {readPersistentStores, isProtectedStore} from '../setup/persistent-stores.js' +/* eslint-disable no-restricted-imports, no-console */ +import {isProtectedStore, readPersistentStores} from '../setup/persistent-stores.js' +import {E2E_STORES} from '../setup/constants.js' import {deleteDevStoreWithCli, isStoreAppsEmpty} from '../setup/store.js' +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' +import {join} from 'node:path' +import type {CLIProcess} from '../setup/cli.js' -const stores = Array.from({length: 6}, (_, index) => `no-delete-e2e-reusable-store-${index + 1}.myshopify.com`) +const stores = Object.values(E2E_STORES) -test('reads a pool and supports one local store', () => { +test('reads protected store domains and supports the local override', () => { expect(readPersistentStores({E2E_STORE_FQDNS: ' ONE.myshopify.com,\ntwo.myshopify.com '})).toEqual([ 'one.myshopify.com', 'two.myshopify.com', @@ -14,8 +25,34 @@ test('reads a pool and supports one local store', () => { expect(() => readPersistentStores({})).toThrow('existing development-store domains') }) +test('rejects invalid store protection lists', () => { + expect(() => readPersistentStores({E2E_STORE_FQDNS: 'one.myshopify.com,ONE.myshopify.com'})).toThrow('duplicate') + expect(() => readPersistentStores({E2E_STORE_FQDNS: 'https://one.myshopify.com'})).toThrow('Invalid E2E store domain') +}) + +test('keeps app cleanup scoped to the run, attempt, and shard', () => { + const env = {GITHUB_RUN_ID: '123', GITHUB_RUN_ATTEMPT: '1'} + expect(e2eRunSegment({...env, E2E_SHARD: '1/2'})).toBe('r3fa1s1') + expect(e2eRunSegment({...env, E2E_SHARD: '2/2'})).toBe('r3fa1s2') + expect(e2eRunSegment({...env, GITHUB_RUN_ATTEMPT: '2', E2E_SHARD: '1/2'})).toBe('r3fa2s1') +}) + +test('uses the numeric remote app ID for cleanup links', async () => { + const commands: string[][] = [] + const cli = { + async exec(args: string[]) { + commands.push(args) + return {exitCode: 0, stdout: '{"remoteApp":{"id":"gid://shopify/App/456"}}', stderr: ''} + }, + } + expect(await devDashboardAppUrl({cli, appDir: '/app', orgId: '123', configName: 'secondary'})).toBe( + 'https://dev.shopify.com/dashboard/123/apps/456', + ) + expect(commands).toEqual([['app', 'info', '--json', '--path', '/app', '--config', 'secondary']]) +}) + test('protects named persistent stores and configured stores from cleanup', async () => { - expect(isProtectedStore(stores[0]!)).toBe(true) + expect(stores.every((store) => isProtectedStore(store))).toBe(true) expect(isProtectedStore('other.myshopify.com', ['other.myshopify.com'])).toBe(true) expect(isProtectedStore('e2e-w0-old.myshopify.com', stores)).toBe(false) let invoked = false @@ -31,6 +68,67 @@ test('protects named persistent stores and configured stores from cleanup', asyn expect(invoked).toBe(false) }) +for (const scenario of [ + 'owned', + 'absent', + 'protected', + 'duplicate', + 'truncated', + 'wrong-org', + 'unconfirmed', +] as const) { + test(`temporary store cleanup checks ownership and deletion (${scenario})`, async () => { + const name = 'e2e-store-local-test' + const ownedStore = {name, store: 'e2e-store-local-test.myshopify.com', type: 'dev'} + const candidates = { + owned: [ownedStore], + absent: [{...ownedStore, name: 'another-run'}], + protected: [{...ownedStore, store: stores[0]}], + duplicate: [ownedStore, {...ownedStore, store: 'another.myshopify.com'}], + truncated: [], + 'wrong-org': [ownedStore], + unconfirmed: [ownedStore], + } + const deleted: string[] = [] + const cli = { + async exec(args: string[]) { + if (args[1] === 'list') { + return { + exitCode: 0, + stderr: '', + stdout: JSON.stringify({ + organization: {id: scenario === 'wrong-org' ? '456' : '123'}, + stores: candidates[scenario], + truncated: scenario === 'truncated', + }), + } + } + expect(args.slice(0, 2)).toEqual(['store', 'delete']) + deleted.push(args[args.indexOf('--store') + 1]!) + return { + exitCode: 0, + stderr: '', + stdout: JSON.stringify({store: {deletionRequested: true, deletionConfirmed: scenario !== 'unconfirmed'}}), + } + }, + } + const cleanup = cleanupTemporaryStore({cli, orgId: '123', name}) + if (scenario === 'owned' || scenario === 'absent') { + await cleanup + } else { + const errors = { + protected: 'Refusing to use', + duplicate: 'Multiple stores', + truncated: 'The store list is truncated', + 'wrong-org': 'Could not verify the store list', + unconfirmed: 'was not confirmed', + } + await expect(cleanup).rejects.toThrow(errors[scenario]) + } + expect(deleted).toEqual(scenario === 'owned' || scenario === 'unconfirmed' ? [ownedStore.store] : []) + }) +} + test('legacy store cleanup requires an explicit empty state', async () => { const page = (empty: boolean) => ({ @@ -45,3 +143,109 @@ test('legacy store cleanup requires an explicit empty state', async () => { expect(await isStoreAppsEmpty(page(true))).toBe(true) expect(await isStoreAppsEmpty(page(false))).toBe(false) }) + +for (const failure of ['none', 'preview', 'uninstall', 'deletion', 'all', 'process-stop'] as const) { + test(`attempts owned app cleanup without blocking the store (${failure})`, async () => { + const appDir = await mkdtemp(join(tmpdir(), 'e2e-store-cleanup-')) + const originalFetch = globalThis.fetch + const originalWarn = console.warn + const warnings: string[] = [] + console.warn = (message: string) => warnings.push(message) + const events: string[] = [] + const fails = (phase: string) => failure === phase || failure === 'all' + try { + await writeFile(join(appDir, 'shopify.app.toml'), 'client_id = "test-client"\n') + const cli: CLIProcess = { + async stopAll() { + events.push('stop') + if (failure === 'process-stop') throw new Error('Process did not stop') + }, + async exec(args) { + if (args.slice(0, 3).join(' ') === 'app dev clean') { + events.push('preview') + expect(args).toContain(stores[0]) + return {exitCode: fails('preview') ? 1 : 0, stdout: '{"status":"success"}', stderr: ''} + } + if (args.slice(0, 3).join(' ') === 'app env show') { + return {exitCode: 0, stdout: 'SHOPIFY_API_SECRET=test-secret', stderr: ''} + } + throw new Error(`Unexpected cleanup command: ${args.join(' ')}`) + }, + async execCreateApp() { + throw new Error('Cleanup must not create an app') + }, + async spawn() { + throw new Error('Cleanup must not start a process') + }, + } + globalThis.fetch = async (url, options) => { + expect(options?.signal).toBeInstanceOf(AbortSignal) + if (String(url).endsWith('/admin/oauth/access_token')) return Response.json({access_token: 'test-token'}) + expect(String(url)).toBe(`https://${stores[0]}/admin/api/2026-04/graphql.json`) + expect(JSON.parse(String(options?.body)).query).toContain('appUninstall') + events.push('uninstall') + return Response.json(fails('uninstall') ? {} : {data: {appUninstall: {userErrors: []}}}) + } + const ctx = { + cli, + appDir, + 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 + }), + } + if (failure === 'process-stop') { + await expect(teardownAll(ctx)).rejects.toThrow('Process did not stop') + expect(events).toEqual(['stop']) + expect(warnings).toEqual([]) + return + } + + await teardownAll(ctx) + expect(events).toEqual(['stop', 'preview', 'uninstall', 'delete-app']) + const expectedWarnings = {none: 0, preview: 1, uninstall: 1, deletion: 1, all: 3} + expect(warnings).toHaveLength(expectedWarnings[failure]) + for (const warning of warnings) { + expect(warning).toContain('E2E-test-app') + expect(warning).toContain(stores[0]) + expect(warning).toContain('cleanup-apps.ts --pattern "E2E-test-app"') + expect(warning).not.toContain('test-secret') + expect(warning).not.toContain('test-token') + } + if (fails('preview')) expect(warnings.some((warning) => warning.includes('preview removal failed'))).toBe(true) + if (fails('uninstall')) expect(warnings.some((warning) => warning.includes('app uninstall failed'))).toBe(true) + if (fails('deletion')) expect(warnings.some((warning) => warning.includes('app deletion failed'))).toBe(true) + + events.length = 0 + await teardownAll({...ctx, appName: 'E2E-next-app'}) + expect(events).toEqual(['stop', 'preview', 'uninstall', 'delete-app']) + } finally { + // eslint-disable-next-line require-atomic-updates + globalThis.fetch = originalFetch + // eslint-disable-next-line require-atomic-updates + console.warn = originalWarn + await rm(appDir, {recursive: true, force: true}) + } + }) +} + +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/smoke-pty.spec.ts b/packages/e2e/tests/smoke-pty.spec.ts index 1c6f2dc5c02..15b6a722061 100644 --- a/packages/e2e/tests/smoke-pty.spec.ts +++ b/packages/e2e/tests/smoke-pty.spec.ts @@ -8,4 +8,11 @@ test.describe('PTY smoke test', () => { expect(code, `shopify version (PTY) failed. Output:\n${proc.getOutput()}`).toBe(0) expect(proc.getOutput()).toMatch(/\d+\.\d+\.\d+/) }) + + test('stops a process while another caller waits for its exit', async ({cli}) => { + const proc = await cli.spawn(['version']) + const exiting = proc.waitForExit() + await cli.stopAll() + await expect(exiting).resolves.toEqual(expect.any(Number)) + }) }) diff --git a/packages/e2e/tests/store-lifecycle.spec.ts b/packages/e2e/tests/store-lifecycle.spec.ts new file mode 100644 index 00000000000..fa57c95aaa4 --- /dev/null +++ b/packages/e2e/tests/store-lifecycle.spec.ts @@ -0,0 +1,51 @@ +import {temporaryStoreFixture as test, findTemporaryStore} from '../setup/temporary-store.js' +import {CLI_TIMEOUT} from '../setup/constants.js' +import {deleteDevStoreWithCli} from '../setup/store.js' +import {expect} from '@playwright/test' + +test('creates and deletes a development store through the CLI', async ({cli, env, temporaryStoreName}) => { + test.setTimeout(2 * CLI_TIMEOUT.store + 2 * CLI_TIMEOUT.short) + + const result = await cli.exec( + [ + 'store', + 'create', + 'dev', + '--name', + temporaryStoreName, + '--organization-id', + env.orgId, + '--plan', + 'basic', + '--country', + 'US', + '--no-demo-data', + '--json', + ], + {timeout: CLI_TIMEOUT.store}, + ) + expect(result.exitCode, `Store creation failed:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`).toBe(0) + const created = JSON.parse(result.stdout) + expect(created).toMatchObject({ + store: { + name: temporaryStoreName, + domain: expect.stringMatching(/^[a-z0-9][a-z0-9-]*\.myshopify\.com$/), + plan: 'basic', + country: 'US', + demoData: false, + }, + organization: {id: env.orgId}, + }) + + const findStore = () => findTemporaryStore({cli, orgId: env.orgId, name: temporaryStoreName}) + await expect.poll(findStore, {timeout: CLI_TIMEOUT.short, intervals: [5000]}).toMatchObject({ + store: created.store.domain, + name: temporaryStoreName, + type: 'dev', + plan: 'basic', + }) + + const confirmed = await deleteDevStoreWithCli({cli, orgId: env.orgId, storeFqdn: created.store.domain}) + expect(confirmed, 'The CLI must confirm deletion, not only accept the request.').toBe(true) + await expect.poll(findStore, {timeout: CLI_TIMEOUT.short, intervals: [5000]}).toBeUndefined() +}) diff --git a/packages/e2e/tests/toml-config.spec.ts b/packages/e2e/tests/toml-config.spec.ts index 3f93816cb95..30b0620c1ee 100644 --- a/packages/e2e/tests/toml-config.spec.ts +++ b/packages/e2e/tests/toml-config.spec.ts @@ -1,10 +1,9 @@ /* eslint-disable no-console */ /* eslint-disable no-restricted-imports */ -import {createApp, devDashboardAppUrl, injectFixtureToml} from '../setup/app.js' +import {appTestFixture as test, createApp, devDashboardAppUrl, injectFixtureToml} from '../setup/app.js' import {teardownAll} from '../setup/teardown.js' -import {CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' +import {E2E_STORES, CLI_TIMEOUT, TEST_TIMEOUT} from '../setup/constants.js' import {e2eAppName, requireEnv} from '../setup/env.js' -import {storeTestFixture as test} from '../setup/store.js' import {expect} from '@playwright/test' import * as fs from 'fs' import * as path from 'path' @@ -29,7 +28,7 @@ test.describe('TOML config regression', () => { // Overwrite with fully populated TOML fixture (injects the real client_id) injectFixtureToml(appDir, FIXTURE_TOML, appName) - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) const result = await cli.exec(['app', 'deploy', '--path', appDir, '--allow-updates', '--allow-deletes'], { timeout: CLI_TIMEOUT.long, @@ -51,7 +50,8 @@ test.describe('TOML config regression', () => { } }) - test('dev starts with fully populated toml', async ({cli, env, browserPage, storeFqdn}) => { + test('dev starts with fully populated toml', async ({cli, env, browserPage}) => { + const storeFqdn = env.storeFqdn || E2E_STORES.fullConfig test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') @@ -66,7 +66,7 @@ test.describe('TOML config regression', () => { appDir = initResult.appDir injectFixtureToml(appDir, FIXTURE_TOML, appName) - appUrl = devDashboardAppUrl(appDir, env.orgId) + appUrl = await devDashboardAppUrl({cli, appDir, orgId: env.orgId}) const proc = await cli.spawn(['app', 'dev', '--path', appDir], {env: {CI: '', SHOPIFY_FLAG_STORE: storeFqdn}})