Skip to content

Commit 64ab746

Browse files
phyllis-sy-wuphyllis-sy-wu
committed
E2E: automatic teardown polish
1 parent 9ff8224 commit 64ab746

9 files changed

Lines changed: 92 additions & 96 deletions

‎packages/e2e/setup/app.ts‎

Lines changed: 57 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -225,70 +225,70 @@ export async function findAppOnDevDashboard(page: Page, appName: string, orgId?:
225225
return null
226226
}
227227

228-
/** Delete an app from its dev dashboard settings page. Returns true if deleted, false if not. */
228+
/**
229+
* Delete an app from its dev dashboard settings page. Returns true if deleted.
230+
* Mirrors the resilience patterns from scripts/cleanup-apps.ts:deleteApp:
231+
* - Outer 3-attempt retry loop around the whole flow (500/502 recovery)
232+
* - scrollIntoViewIfNeeded on the "Delete app" button
233+
* - Conditional "type DELETE" confirmation (some orgs/modals require it)
234+
* - Verify via settings-page reload → expect 404
235+
*/
229236
export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Promise<boolean> {
230-
// Step 1: Navigate to settings page
231-
await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
232-
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
233-
await refreshIfPageError(page)
234-
235-
// Step 2: Wait for "Delete app" button to be enabled, then click (retry with error check)
236-
const deleteAppBtn = page.locator('button:has-text("Delete app")').first()
237237
for (let attempt = 1; attempt <= 3; attempt++) {
238-
if (await refreshIfPageError(page)) continue
239-
const isDisabled = await deleteAppBtn.getAttribute('disabled').catch(() => 'true')
240-
if (!isDisabled) break
241-
await page.reload({waitUntil: 'domcontentloaded'})
242-
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
243-
}
244-
245-
// Click the delete button — if it's not found, the page didn't load properly
246-
const deleteClicked = await deleteAppBtn
247-
.click({timeout: BROWSER_TIMEOUT.long})
248-
.then(() => true)
249-
.catch(() => false)
250-
if (!deleteClicked) return false
251-
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
252-
253-
// Step 3: Click confirm "Delete" in the modal (retry step 2+3 if not visible)
254-
// The dev dashboard modal has a submit button with class "critical" inside a form
255-
const confirmAppBtn = page.locator('button.critical[type="submit"]')
256-
for (let attempt = 1; attempt <= 3; attempt++) {
257-
if (await confirmAppBtn.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) break
258-
if (attempt === 3) return false
259-
// Retry: re-click the delete button to reopen modal
260-
await page.keyboard.press('Escape')
261-
await page.waitForTimeout(BROWSER_TIMEOUT.short)
262-
const retryClicked = await deleteAppBtn
263-
.click({timeout: BROWSER_TIMEOUT.long})
264-
.then(() => true)
265-
.catch(() => false)
266-
if (!retryClicked) return false
267-
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
268-
}
238+
try {
239+
await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
240+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
269241

270-
const urlBefore = page.url()
271-
const confirmClicked = await confirmAppBtn
272-
.click({timeout: BROWSER_TIMEOUT.long})
273-
.then(() => true)
274-
.catch(() => false)
275-
if (!confirmClicked) return false
276-
277-
// Wait for page to navigate away after deletion
278-
try {
279-
await page.waitForURL((url) => url.toString() !== urlBefore, {timeout: BROWSER_TIMEOUT.max})
280-
// eslint-disable-next-line no-catch-all/no-catch-all
281-
} catch (_err) {
282-
// URL didn't change — check if page error occurred during redirect
283-
if (await refreshIfPageError(page)) {
284-
// After refresh, 404 means the app was deleted (settings page no longer exists)
242+
// 404 before we even click = app already gone. 500/502 = throw to retry.
285243
const bodyText = (await page.textContent('body')) ?? ''
286244
if (bodyText.includes('404: Not Found')) return true
287-
return false
245+
if (bodyText.includes('500: Internal Server Error') || bodyText.includes('502 Bad Gateway')) {
246+
throw new Error('Server error loading app settings page')
247+
}
248+
await refreshIfPageError(page)
249+
250+
// Wait for "Delete app" button to be enabled. Button can be below the
251+
// fold, so scrollIntoView before each check.
252+
const deleteBtn = page.locator('button:has-text("Delete app")').first()
253+
for (let i = 1; i <= 5; i++) {
254+
await deleteBtn.scrollIntoViewIfNeeded()
255+
const isDisabled = await deleteBtn.getAttribute('disabled')
256+
if (!isDisabled) break
257+
await page.waitForTimeout(BROWSER_TIMEOUT.long)
258+
await page.reload({waitUntil: 'domcontentloaded'})
259+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
260+
}
261+
262+
await deleteBtn.click({timeout: BROWSER_TIMEOUT.max})
263+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
264+
265+
// Some confirmation modals require typing "DELETE". Fill if present.
266+
const confirmInput = page.locator('input[type="text"]').last()
267+
if (await confirmInput.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) {
268+
await confirmInput.fill('DELETE')
269+
await page.waitForTimeout(BROWSER_TIMEOUT.short)
270+
}
271+
272+
// Confirm button is the second "Delete app" button on the page (inside modal).
273+
const confirmBtn = page.locator('button:has-text("Delete app")').last()
274+
await confirmBtn.click({timeout: BROWSER_TIMEOUT.long})
275+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
276+
277+
// Verify by reloading settings — 404 means deleted.
278+
await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
279+
await page.waitForTimeout(BROWSER_TIMEOUT.short)
280+
const afterText = (await page.textContent('body')) ?? ''
281+
if (afterText.includes('404: Not Found')) return true
282+
if (afterText.includes('500: Internal Server Error') || afterText.includes('502 Bad Gateway')) {
283+
throw new Error('Server error verifying app deletion')
284+
}
285+
// eslint-disable-next-line no-catch-all/no-catch-all
286+
} catch (_err) {
287+
if (attempt === 3) return false
288+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
288289
}
289-
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
290290
}
291-
return page.url() !== urlBefore
291+
return false
292292
}
293293

294294
// ---------------------------------------------------------------------------

‎packages/e2e/setup/store.ts‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,20 +167,16 @@ export async function uninstallAppFromStore(page: Page, storeSlug: string, appNa
167167
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
168168
}
169169

170-
// Verify: check the specific app is gone
171-
const check = async () =>
172-
page
173-
.locator(`span:has-text("${appName}"):not([class*="Polaris"])`)
174-
.first()
175-
.isVisible({timeout: BROWSER_TIMEOUT.medium})
176-
.catch(() => false)
177-
178-
if (!(await check())) return true
179-
180-
// If still visible — reload and check again
170+
// Verify: reload and confirm the app is no longer listed
181171
await page.reload({waitUntil: 'domcontentloaded'})
182172
await page.waitForTimeout(BROWSER_TIMEOUT.long)
183-
return !(await check())
173+
await dismissDevConsole(page)
174+
const stillVisible = await page
175+
.locator(`span:has-text("${appName}"):not([class*="Polaris"])`)
176+
.first()
177+
.isVisible({timeout: BROWSER_TIMEOUT.medium})
178+
.catch(() => false)
179+
return !stillVisible
184180
}
185181

186182
/** Check if the current page shows the empty state (zero apps installed). Caller must navigate first. */
@@ -219,7 +215,7 @@ export async function deleteStore(page: Page, storeSlug: string): Promise<boolea
219215
}
220216
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
221217

222-
const modal = page.locator('.Polaris-Modal-Dialog__Modal')
218+
const modal = page.locator('.Polaris-Modal-Dialog__Modal:has-text("Review before deleting store")')
223219

224220
// Step 2: Check the confirmation checkbox (retry step 1+2 if fails)
225221
for (let attempt = 1; attempt <= 3; attempt++) {

‎packages/e2e/tests/app-deploy.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ test.describe('App deploy', () => {
4141
expect(listResult.exitCode, `versions list failed:\n${listOutput}`).toBe(0)
4242
expect(listOutput).toContain(versionTag)
4343
} finally {
44-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
45-
if (!process.env.E2E_SKIP_CLEANUP) {
44+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
45+
if (!process.env.E2E_SKIP_TEARDOWN) {
4646
fs.rmSync(parentDir, {recursive: true, force: true})
4747
await teardownAll({
4848
browserPage,

‎packages/e2e/tests/app-dev-server.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ test.describe('App dev server', () => {
4949
const exitCode = await dev.waitForExit(CLI_TIMEOUT.short)
5050
expect(exitCode, `dev exited with non-zero code. Output:\n${dev.getOutput()}`).toBe(0)
5151
} finally {
52-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
53-
if (!process.env.E2E_SKIP_CLEANUP) {
52+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
53+
if (!process.env.E2E_SKIP_TEARDOWN) {
5454
fs.rmSync(parentDir, {recursive: true, force: true})
5555
await teardownAll({
5656
browserPage,

‎packages/e2e/tests/app-scaffold.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ test.describe('App scaffold', () => {
4545
`buildApp failed:\nstdout: ${buildResult.stdout}\nstderr: ${buildResult.stderr}`,
4646
).toBe(0)
4747
} finally {
48-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
49-
if (!process.env.E2E_SKIP_CLEANUP) {
48+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
49+
if (!process.env.E2E_SKIP_TEARDOWN) {
5050
fs.rmSync(parentDir, {recursive: true, force: true})
5151
await teardownAll({
5252
browserPage,
@@ -80,8 +80,8 @@ test.describe('App scaffold', () => {
8080
expect(fs.existsSync(initResult.appDir)).toBe(true)
8181
expect(fs.existsSync(path.join(initResult.appDir, 'shopify.app.toml'))).toBe(true)
8282
} finally {
83-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
84-
if (!process.env.E2E_SKIP_CLEANUP) {
83+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
84+
if (!process.env.E2E_SKIP_TEARDOWN) {
8585
fs.rmSync(parentDir, {recursive: true, force: true})
8686
await teardownAll({
8787
browserPage,
@@ -138,8 +138,8 @@ test.describe('App scaffold', () => {
138138
`buildApp failed:\nstdout: ${buildResult.stdout}\nstderr: ${buildResult.stderr}`,
139139
).toBe(0)
140140
} finally {
141-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
142-
if (!process.env.E2E_SKIP_CLEANUP) {
141+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
142+
if (!process.env.E2E_SKIP_TEARDOWN) {
143143
fs.rmSync(parentDir, {recursive: true, force: true})
144144
await teardownAll({
145145
browserPage,

‎packages/e2e/tests/dev-hot-reload.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ test.describe('Dev hot reload', () => {
8383
proc.kill()
8484
}
8585
} finally {
86-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
87-
if (!process.env.E2E_SKIP_CLEANUP) {
86+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
87+
if (!process.env.E2E_SKIP_TEARDOWN) {
8888
fs.rmSync(parentDir, {recursive: true, force: true})
8989
await teardownAll({
9090
browserPage,
@@ -135,8 +135,8 @@ test.describe('Dev hot reload', () => {
135135
proc.kill()
136136
}
137137
} finally {
138-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
139-
if (!process.env.E2E_SKIP_CLEANUP) {
138+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
139+
if (!process.env.E2E_SKIP_TEARDOWN) {
140140
fs.rmSync(parentDir, {recursive: true, force: true})
141141
await teardownAll({
142142
browserPage,
@@ -193,8 +193,8 @@ test.describe('Dev hot reload', () => {
193193
proc.kill()
194194
}
195195
} finally {
196-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
197-
if (!process.env.E2E_SKIP_CLEANUP) {
196+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
197+
if (!process.env.E2E_SKIP_TEARDOWN) {
198198
fs.rmSync(parentDir, {recursive: true, force: true})
199199
await teardownAll({
200200
browserPage,

‎packages/e2e/tests/multi-config-dev.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ include_config_on_deploy = true
8484
proc.kill()
8585
}
8686
} finally {
87-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
88-
if (!process.env.E2E_SKIP_CLEANUP) {
87+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
88+
if (!process.env.E2E_SKIP_TEARDOWN) {
8989
fs.rmSync(parentDir, {recursive: true, force: true})
9090
await teardownAll({
9191
browserPage,
@@ -156,8 +156,8 @@ api_version = "2025-01"
156156
proc.kill()
157157
}
158158
} finally {
159-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
160-
if (!process.env.E2E_SKIP_CLEANUP) {
159+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
160+
if (!process.env.E2E_SKIP_TEARDOWN) {
161161
fs.rmSync(parentDir, {recursive: true, force: true})
162162
await teardownAll({
163163
browserPage,

‎packages/e2e/tests/toml-config-invalid.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ test.describe('TOML config invalid', () => {
3636
expect(result.exitCode, `expected deploy to fail for ${label}, but it succeeded:\n${output}`).not.toBe(0)
3737
expect(output.toLowerCase(), `expected error output for ${label}:\n${output}`).toMatch(/error|invalid|failed/)
3838
} finally {
39-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
40-
if (!process.env.E2E_SKIP_CLEANUP) {
39+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
40+
if (!process.env.E2E_SKIP_TEARDOWN) {
4141
fs.rmSync(appDir, {recursive: true, force: true})
4242
}
4343
}

‎packages/e2e/tests/toml-config.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ test.describe('TOML config regression', () => {
3535
const output = result.stdout + result.stderr
3636
expect(result.exitCode, `deploy failed:\n${output}`).toBe(0)
3737
} finally {
38-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
39-
if (!process.env.E2E_SKIP_CLEANUP) {
38+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
39+
if (!process.env.E2E_SKIP_TEARDOWN) {
4040
fs.rmSync(parentDir, {recursive: true, force: true})
4141
await teardownAll({
4242
browserPage,
@@ -77,8 +77,8 @@ test.describe('TOML config regression', () => {
7777
proc.kill()
7878
}
7979
} finally {
80-
// E2E_SKIP_CLEANUP=1 skips cleanup for debugging. Run `pnpm test:e2e-cleanup` afterward.
81-
if (!process.env.E2E_SKIP_CLEANUP) {
80+
// E2E_SKIP_TEARDOWN=1 skips teardown for debugging. Run cleanup scripts afterward.
81+
if (!process.env.E2E_SKIP_TEARDOWN) {
8282
fs.rmSync(parentDir, {recursive: true, force: true})
8383
await teardownAll({
8484
browserPage,

0 commit comments

Comments
 (0)