Skip to content

Commit f4d2f79

Browse files
phyllis-sy-wuphyllis-sy-wu
committed
E2E: automatic teardown polish
1 parent 24cc2d9 commit f4d2f79

9 files changed

Lines changed: 44 additions & 54 deletions

‎packages/e2e/setup/app.ts‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -267,28 +267,22 @@ export async function deleteAppFromDevDashboard(page: Page, appUrl: string): Pro
267267
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
268268
}
269269

270-
const urlBefore = page.url()
271270
const confirmClicked = await confirmAppBtn
272271
.click({timeout: BROWSER_TIMEOUT.long})
273272
.then(() => true)
274273
.catch(() => false)
275274
if (!confirmClicked) return false
276275

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)
285-
const bodyText = (await page.textContent('body')) ?? ''
286-
if (bodyText.includes('404: Not Found')) return true
287-
return false
288-
}
289-
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
276+
// Verify deletion: reload the settings page and check for 404
277+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
278+
await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
279+
await page.waitForTimeout(BROWSER_TIMEOUT.short)
280+
const bodyText = (await page.textContent('body')) ?? ''
281+
if (bodyText.includes('404: Not Found')) return true
282+
if (bodyText.includes('500: Internal Server Error') || bodyText.includes('502 Bad Gateway')) {
283+
throw new Error('Server error verifying app deletion')
290284
}
291-
return page.url() !== urlBefore
285+
return false
292286
}
293287

294288
// ---------------------------------------------------------------------------

‎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)