diff --git a/packages/e2e/README.md b/packages/e2e/README.md index 9163462881a..e8ccdb4d168 100644 --- a/packages/e2e/README.md +++ b/packages/e2e/README.md @@ -44,7 +44,7 @@ pnpm --filter e2e exec playwright test --project remote --shard 1/2 `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. +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. After a failed test, cleanup waits up to one minute for the store to appear. If the store remains absent, cleanup reports an unresolved failure. 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: diff --git a/packages/e2e/setup/temporary-store.ts b/packages/e2e/setup/temporary-store.ts index 34afc8b294b..bbb2fb6a6c8 100644 --- a/packages/e2e/setup/temporary-store.ts +++ b/packages/e2e/setup/temporary-store.ts @@ -4,6 +4,7 @@ import {e2eRunSegment, requireEnv} from './env.js' import {isProtectedStore, readPersistentStores} from './persistent-stores.js' import {deleteDevStoreWithCli} from './store.js' import {randomUUID} from 'node:crypto' +import {setTimeout as delay} from 'node:timers/promises' import type {CLIProcess} from './cli.js' interface TemporaryStore { @@ -54,9 +55,23 @@ export async function cleanupTemporaryStore(options: { cli: Pick orgId: string name: string + /** Wait for provisioning after a failed test before treating an absent store as unresolved cleanup. */ + creationTimeoutMs?: number }): Promise { - const store = await findTemporaryStore(options) - if (!store) return + const deadline = Date.now() + (options.creationTimeoutMs ?? 0) + let store = await findTemporaryStore(options) + while (!store && Date.now() < deadline) { + // eslint-disable-next-line no-await-in-loop + await delay(Math.min(5000, Math.max(0, deadline - Date.now()))) + // eslint-disable-next-line no-await-in-loop + store = await findTemporaryStore(options) + } + if (!store) { + if (options.creationTimeoutMs) { + throw new Error(`Temporary store ${options.name} did not appear before cleanup timed out; cleanup is unresolved.`) + } + return + } const confirmed = await deleteDevStoreWithCli({...options, storeFqdn: store.store}) if (!confirmed) throw new Error(`Deletion of temporary store ${store.store} was not confirmed.`) } @@ -72,9 +87,14 @@ export const temporaryStoreFixture = authFixture.extend<{temporaryStoreName: str await use(name) } finally { // Creation can succeed remotely before the CLI returns its domain. - await cleanupTemporaryStore({cli, orgId: env.orgId, name}) + await cleanupTemporaryStore({ + cli, + orgId: env.orgId, + name, + creationTimeoutMs: testInfo.status === 'passed' ? 0 : CLI_TIMEOUT.short, + }) } }, - {timeout: CLI_TIMEOUT.store + CLI_TIMEOUT.short}, + {timeout: CLI_TIMEOUT.store + 2 * CLI_TIMEOUT.short}, ], }) diff --git a/packages/e2e/tests/persistent-stores.spec.ts b/packages/e2e/tests/persistent-stores.spec.ts index 12b565711e2..9ec1d2ec157 100644 --- a/packages/e2e/tests/persistent-stores.spec.ts +++ b/packages/e2e/tests/persistent-stores.spec.ts @@ -141,6 +141,7 @@ test('protects named persistent stores and configured stores from cleanup', asyn for (const scenario of [ 'owned', + 'delayed', 'absent', 'protected', 'duplicate', @@ -153,6 +154,7 @@ for (const scenario of [ const ownedStore = {name, store: 'e2e-store-local-test.myshopify.com', type: 'dev'} const candidates = { owned: [ownedStore], + delayed: [ownedStore], absent: [{...ownedStore, name: 'another-run'}], protected: [{...ownedStore, store: stores[0]}], duplicate: [ownedStore, {...ownedStore, store: 'another.myshopify.com'}], @@ -161,15 +163,17 @@ for (const scenario of [ unconfirmed: [ownedStore], } const deleted: string[] = [] + let listings = 0 const cli = { async exec(args: string[]) { if (args[1] === 'list') { + listings++ return { exitCode: 0, stderr: '', stdout: JSON.stringify({ organization: {id: scenario === 'wrong-org' ? '456' : '123'}, - stores: candidates[scenario], + stores: scenario === 'delayed' && listings === 1 ? [] : candidates[scenario], truncated: scenario === 'truncated', }), } @@ -183,8 +187,13 @@ for (const scenario of [ } }, } - const cleanup = cleanupTemporaryStore({cli, orgId: '123', name}) - if (scenario === 'owned' || scenario === 'absent') { + const cleanup = cleanupTemporaryStore({ + cli, + orgId: '123', + name, + creationTimeoutMs: scenario === 'absent' ? 0 : 1000, + }) + if (scenario === 'owned' || scenario === 'absent' || scenario === 'delayed') { await cleanup } else { const errors = { @@ -196,10 +205,44 @@ for (const scenario of [ } await expect(cleanup).rejects.toThrow(errors[scenario]) } - expect(deleted).toEqual(scenario === 'owned' || scenario === 'unconfirmed' ? [ownedStore.store] : []) + expect(deleted).toEqual( + scenario === 'owned' || scenario === 'delayed' || scenario === 'unconfirmed' ? [ownedStore.store] : [], + ) + expect(listings).toBe(scenario === 'delayed' ? 2 : 1) }) } +test('reports unresolved cleanup when a store does not finish provisioning within the wait', async () => { + const cli = { + async exec(args: string[]) { + expect(args.slice(0, 2)).toEqual(['store', 'list']) + return {exitCode: 0, stdout: JSON.stringify({organization: {id: '123'}, stores: []}), stderr: ''} + }, + } + + await expect( + cleanupTemporaryStore({cli, orgId: '123', name: 'e2e-store-delayed', creationTimeoutMs: 1000}), + ).rejects.toThrow('Temporary store e2e-store-delayed did not appear before cleanup timed out; cleanup is unresolved.') +}) + +test('preserves listing errors while waiting for a store to finish provisioning', async () => { + let listings = 0 + const cli = { + async exec(args: string[]) { + expect(args.slice(0, 2)).toEqual(['store', 'list']) + listings++ + return listings === 1 + ? {exitCode: 0, stdout: JSON.stringify({organization: {id: '123'}, stores: []}), stderr: ''} + : {exitCode: 1, stdout: '', stderr: 'Authentication failed'} + }, + } + + await expect( + cleanupTemporaryStore({cli, orgId: '123', name: 'e2e-store-delayed', creationTimeoutMs: 1000}), + ).rejects.toThrow('Authentication failed') + expect(listings).toBe(2) +}) + test('legacy store cleanup requires an explicit empty state', async () => { const page = (empty: boolean) => ({ diff --git a/packages/organizations/src/cli/services/dev/create-dev-store.test.ts b/packages/organizations/src/cli/services/dev/create-dev-store.test.ts index eb32b2ab673..2e5b9bfb79c 100644 --- a/packages/organizations/src/cli/services/dev/create-dev-store.test.ts +++ b/packages/organizations/src/cli/services/dev/create-dev-store.test.ts @@ -1,5 +1,6 @@ import {createDevStoreJsonOutputSchema} from './types.js' import {createDevStore} from './create-dev-store.js' +import {PollStoreCreation} from '../../api/graphql/business-platform-organizations/generated/poll_store_creation.js' import {describe, expect, test, vi, beforeEach} from 'vitest' import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform' @@ -51,6 +52,81 @@ beforeEach(() => { }) describe('createDevStore', () => { + test.each([undefined, null])('waits for an initially missing creation record (%s)', async (storeCreation) => { + vi.mocked(businessPlatformOrganizationsRequestDoc) + .mockResolvedValueOnce(defaultMutationResult) + .mockResolvedValueOnce({organization: {id: '123', storeCreation}}) + .mockResolvedValueOnce({organization: {id: '123', storeCreation: {status: 'AWAITING_CORE_STORE_READY'}}}) + .mockResolvedValueOnce({organization: {id: '123', storeCreation: {status: 'COMPLETE'}}}) + vi.mocked(sleep).mockImplementation(async () => { + expect(outputResult).not.toHaveBeenCalled() + }) + + const domain = await createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic', json: true}) + + expect(domain).toBe('test-store.myshopify.com') + expect(sleep).toHaveBeenCalledTimes(2) + expect(businessPlatformOrganizationsRequestDoc).toHaveBeenCalledTimes(4) + for (const [request] of vi.mocked(businessPlatformOrganizationsRequestDoc).mock.calls.slice(1)) { + expect(request).toMatchObject({query: PollStoreCreation, variables: {shopDomain: domain}}) + } + expect(JSON.parse(vi.mocked(outputResult).mock.calls[0]![0] as string).store.domain).toBe(domain) + }) + + test('stops waiting for a missing creation record at the existing timeout', async () => { + let now = 0 + const clock = vi.spyOn(Date, 'now').mockImplementation(() => now) + vi.mocked(businessPlatformOrganizationsRequestDoc) + .mockResolvedValueOnce(defaultMutationResult) + .mockResolvedValue({organization: {id: '123', storeCreation: null}}) + vi.mocked(sleep).mockImplementation(async (seconds) => { + now += seconds * 1000 + }) + + try { + await expect( + createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic', json: true}), + ).rejects.toThrow('Store creation timed out after 5 minutes.') + expect(now).toBeGreaterThanOrEqual(5 * 60_000) + expect(now).toBeLessThanOrEqual(5 * 60_000 + 2000) + expect(outputResult).not.toHaveBeenCalled() + } finally { + clock.mockRestore() + } + }) + + test.each([undefined, null])('does not retry a missing organization (%s)', async (organization) => { + vi.mocked(businessPlatformOrganizationsRequestDoc) + .mockResolvedValueOnce(defaultMutationResult) + .mockResolvedValueOnce({organization}) + + await expect(createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic'})).rejects.toThrow( + 'Unable to determine store creation status.', + ) + expect(sleep).not.toHaveBeenCalled() + }) + + test.each(['FAILED', 'TIMED_OUT', 'USER_ERROR'])('does not retry terminal creation status %s', async (status) => { + vi.mocked(businessPlatformOrganizationsRequestDoc) + .mockResolvedValueOnce(defaultMutationResult) + .mockResolvedValueOnce({organization: {id: '123', storeCreation: {status}}}) + + await expect(createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic'})).rejects.toThrow( + `Store creation failed with status: ${status}`, + ) + expect(sleep).not.toHaveBeenCalled() + }) + + test('preserves unrelated polling errors', async () => { + const error = new Error('Authentication failed') + vi.mocked(businessPlatformOrganizationsRequestDoc) + .mockResolvedValueOnce(defaultMutationResult) + .mockRejectedValueOnce(error) + + await expect(createDevStore({name: 'test-store', organization: defaultOrg, plan: 'basic'})).rejects.toBe(error) + expect(sleep).not.toHaveBeenCalled() + }) + test('returns the polled shop domain without rendering output when summary is false', async () => { vi.mocked(businessPlatformOrganizationsRequestDoc) .mockResolvedValueOnce(defaultMutationResult) diff --git a/packages/organizations/src/cli/services/dev/create-dev-store.ts b/packages/organizations/src/cli/services/dev/create-dev-store.ts index db9d5eaa238..ee8959615ae 100644 --- a/packages/organizations/src/cli/services/dev/create-dev-store.ts +++ b/packages/organizations/src/cli/services/dev/create-dev-store.ts @@ -107,10 +107,10 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise