E2E: automatic teardown polish

[phyllis-sy-wu]

WHY are these changes introduced?

Improves reliability of per-test teardown. Before these changes, CI runs commonly left 2–3 orphaned stores/apps per full 5-worker run, and a failed click could burn 5–7 minutes in a single phase before the outer attempt gave up.

WHAT is this pull request doing?

Four coordinated fixes in the teardown path. All three "do-the-thing" helpers (uninstallAppFromStore, deleteStore, deleteAppFromDevDashboard) now own their own retry + verification, and teardown.ts becomes the orchestrator.

---

1. deleteAppFromDevDashboard — sync with cleanup-apps.ts:deleteApp (setup/app.ts)

Teardown's app-delete helper and the standalone cleanup script had drifted apart; cleanup-apps had absorbed fixes that teardown was missing. Brought them back to parity:

• Outer 3-attempt retry around the whole flow — 500/502 triggers retry instead of giving up.
• Early 404 short-circuit — app already gone = return true immediately (no URL-change wait).
• scrollIntoViewIfNeeded on the "Delete app" button — it sits below the fold on some viewport sizes.
• isEnabled() gate instead of the brittle getAttribute('disabled') check (the latter returns "" for <button disabled>, making !isDisabled truthy — a silent false positive).
• Single reload as a fallback for propagation lag (not a 5× polling loop). click({timeout: 2 * BROWSER_TIMEOUT.long}) auto-wait handles the rest at ~100ms granularity.
• Optional DELETE confirmation input (some modals require typing it).
• Semantic button:has-text("Delete app").last() for the modal confirm.
• Verify via settings-page reload → expect 404: Not Found (not a URL-change wait).

404 short-circuit — before/after

Before: clicks confirm, then waitForURL(url !== urlBefore) for up to 60s — if the page never navigates away (e.g. the delete silently succeeded but left us on the same URL), the helper burns the full timeout and returns false.

Expand to see code

const urlBefore = page.url()
await confirmAppBtn.click(...)
await page.waitForURL((url) => url.toString() !== urlBefore, {timeout: BROWSER_TIMEOUT.max})
return page.url() !== urlBefore

delete-app-automatic-redirect.mov

After: reloads {appUrl}/settings after confirm — a 404: Not Found body is the definitive "app is gone" signal, and short-circuits in seconds instead of waiting 60s for a URL change that may never come.

Expand to see code

// Verify: reload settings — 404 = deleted.
await page.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
await page.waitForTimeout(BROWSER_TIMEOUT.short)
const afterText = (await page.textContent('body')) ?? ''
if (afterText.includes('404: Not Found')) return true
if (afterText.includes('500') || afterText.includes('502')) throw new Error('...') // → retry

delete-app-force-refresh.mov

---

2. deleteStore — full-flow retry with silent-failure detection (setup/store.ts)

Before: the function had three independent step-level retries (navigate, checkbox, confirm) that couldn't recover from silent click failures. The admin's "Delete store" confirm button sometimes silently fails and redirects to the store home (/store/{slug}) instead of /access_account — and the old code would then waitForURL(/access_account/) for the full 60s budget before returning false.

After: full flow wrapped in an outer 3-attempt retry, each attempt re-navigates to planUrl at the top (guarantees a clean start — even if the previous attempt landed on store home). Verification matches either URL and returns fast in both cases.

for (let attempt = 1; attempt <= 3; attempt++) {
  try {
    // Always re-navigate at the top of each attempt
    await page.goto(planUrl, {waitUntil: 'load'})
    if (page.url().includes('access_account')) return true

    // Open modal with inner 3× click retry (first click often refreshes the page)
    ...
    await confirmButton.click({force: true})

    // Wait for EITHER access_account (success) OR store home (silent failure)
    await page.waitForURL(
      (url) => url.toString().includes('access_account')
            || url.pathname.match(/^\/store\/[^/]+\/?$/) !== null,
      {timeout: BROWSER_TIMEOUT.max},
    )
    if (page.url().includes('access_account')) return true
    throw new Error('Store deletion not confirmed') // → outer catch → retry
  } catch (_err) {
    if (attempt === 3) return false
  }
}

Also scopes the modal selector by title (matching cleanup-stores.ts):

page.locator('.Polaris-Modal-Dialog__Modal:has-text("Review before deleting store")')

…to avoid matching stale/hidden modals left over from previous interactions.

---

3. uninstallAppFromStore — always reload before verify (setup/store.ts)

The old check-stale-DOM-first pattern could return success before the uninstall propagated. Now unconditional: reload, dismiss Dev Console, then check.

Before:

// Check stale DOM first
if (!(await check())) return true
// Only reload as fallback
await page.reload(...)
return !(await check())

After:

await page.reload({waitUntil: 'domcontentloaded'})
await page.waitForTimeout(BROWSER_TIMEOUT.long)
await dismissDevConsole(page)
const stillVisible = await page.locator(...).isVisible(...)
return !stillVisible

---

4. teardown.ts — orchestrator simplification

With each helper now owning its own retry + verification, the orchestrator becomes simpler and more targeted:

• Phase 1 (uninstall) — 3× retry around uninstallAppFromStore.
• Phase 2 (delete store) — single call; deleteStore retries internally. Pre-flight check: navigate to /settings/apps, reload once if stale, verify apps are empty before attempting delete.
• Phase 3 (delete app) — skipped when Phase 1 failed. The dev dashboard's Delete button stays perma-disabled while installs exist, so polling it would just burn time; cleanup-apps.ts reaps the orphan instead. If uninstall succeeded, runs 3× findAppOnDevDashboard + deleteAppFromDevDashboard.

Combined effect: worst-case teardown time drops from ~20 minutes (3 × 5–7min per phase) to a bounded few minutes per phase, and the "uninstall-fail → waste 5–7min polling a disabled button" failure mode is gone entirely.

Fastest CI run times observed: 6m 32s (shard 1/2) and 4m 50s (shard 2/2).

---

5. Rename E2E_SKIP_CLEANUP → E2E_SKIP_TEARDOWN

The env var controls per-test teardown, not the standalone cleanup scripts. Renamed across all 7 test specs for clarity:

E2E_SKIP_TEARDOWN=1 DEBUG=1 pnpm --filter e2e exec playwright test

---

How to test your changes?

1. Run E2E tests and confirm teardown completes cleanly:

   DEBUG=1 pnpm --filter e2e exec playwright test

2. After a full run, verify 0 leftovers:

   pnpm --filter e2e exec tsx scripts/cleanup-apps.ts --list
   pnpm --filter e2e exec tsx scripts/cleanup-stores.ts --list

3. Verify skip flag works:

   E2E_SKIP_TEARDOWN=1 DEBUG=1 pnpm --filter e2e exec playwright test app-deploy
   # → apps and stores should remain on the dashboard after the test

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've considered possible documentation changes
• I've considered analytics changes to measure impact
• The change is user-facing, so I've added a changelog entry with pnpm changeset add

[phyllis-sy-wu]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• E2E - QA: init, deploy, versions list, config link, 2nd deploy #7367
• E2E: automatic teardown polish #7358 👈 (View in Graphite)
• E2E: improve CLI log format #7364
• E2E: utility - cleanup all #7357
• E2E: utility - cleanup apps #7356
• E2E: utility - cleanup stores #7355
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.