Skip to content

Commit 3f68519

Browse files
phyllis-sy-wuphyllis-sy-wu
committed
refactor timeouts
1 parent 394d994 commit 3f68519

14 files changed

Lines changed: 130 additions & 84 deletions

‎packages/e2e/helpers/browser-login.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {BROWSER_TIMEOUT} from '../setup/constants.js'
12
import type {Page} from '@playwright/test'
23

34
/**
@@ -27,20 +28,22 @@ export async function completeLogin(page: Page, loginUrl: string, email: string,
2728

2829
try {
2930
// Fill in email
30-
await page.waitForSelector('input[name="account[email]"], input[type="email"]', {timeout: 60_000})
31+
await page.waitForSelector('input[name="account[email]"], input[type="email"]', {timeout: BROWSER_TIMEOUT.max})
3132
await fillSensitive(page, 'input[name="account[email]"], input[type="email"]', email)
3233
await page.locator('button[type="submit"]').first().click()
3334

3435
// Fill in password
35-
await page.waitForSelector('input[name="account[password]"], input[type="password"]', {timeout: 60_000})
36+
await page.waitForSelector('input[name="account[password]"], input[type="password"]', {
37+
timeout: BROWSER_TIMEOUT.max,
38+
})
3639
await fillSensitive(page, 'input[name="account[password]"], input[type="password"]', password)
3740
await page.locator('button[type="submit"]').first().click()
3841

3942
// Handle any confirmation/approval page
40-
await page.waitForTimeout(3000)
43+
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
4144
try {
4245
const btn = page.locator('button[type="submit"]').first()
43-
if (await btn.isVisible({timeout: 5000})) await btn.click()
46+
if (await btn.isVisible({timeout: BROWSER_TIMEOUT.long})) await btn.click()
4447
// eslint-disable-next-line no-catch-all/no-catch-all
4548
} catch (_error) {
4649
// No confirmation page — expected

‎packages/e2e/playwright.config.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable line-comment-position */
2+
import {TEST_TIMEOUT} from './setup/constants.js'
23
import {config} from 'dotenv'
34
import {defineConfig} from '@playwright/test'
45

@@ -14,8 +15,8 @@ export default defineConfig({
1415
workers: 1,
1516
maxFailures: isCI ? 3 : 0, // Stop early in CI after 3 failures
1617
reporter: isCI ? [['html', {open: 'never'}], ['list']] : [['list']],
17-
timeout: 3 * 60 * 1000, // 3 minutes per test
18-
globalTimeout: 30 * 60 * 1000, // 30 minutes total
18+
timeout: TEST_TIMEOUT.default, // Heavy tests override via test.setTimeout()
19+
globalTimeout: 20 * 60 * 1000,
1920

2021
use: {
2122
trace: isCI ? 'on' : 'off',

‎packages/e2e/setup/app.ts‎

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable no-restricted-imports, no-await-in-loop */
22
import {authFixture} from './auth.js'
33
import {navigateToDashboard} from './browser.js'
4+
import {CLI_TIMEOUT, BROWSER_TIMEOUT} from './constants.js'
45
import * as path from 'path'
56
import * as fs from 'fs'
67
import type {CLIContext, CLIProcess, ExecResult} from './cli.js'
@@ -41,7 +42,7 @@ export async function createApp(ctx: {
4142

4243
const result = await cli.execCreateApp(args, {
4344
env: {FORCE_COLOR: '0'},
44-
timeout: 5 * 60 * 1000,
45+
timeout: CLI_TIMEOUT.long,
4546
})
4647

4748
let appDir = ''
@@ -111,11 +112,11 @@ export async function generateExtension(
111112
): Promise<ExecResult> {
112113
const args = ['app', 'generate', 'extension', '--name', ctx.name, '--path', ctx.appDir, '--template', ctx.template]
113114
if (ctx.flavor) args.push('--flavor', ctx.flavor)
114-
return ctx.cli.exec(args, {timeout: 5 * 60 * 1000})
115+
return ctx.cli.exec(args, {timeout: CLI_TIMEOUT.long})
115116
}
116117

117118
export async function buildApp(ctx: CLIContext): Promise<ExecResult> {
118-
return ctx.cli.exec(['app', 'build', '--path', ctx.appDir], {timeout: 5 * 60 * 1000})
119+
return ctx.cli.exec(['app', 'build', '--path', ctx.appDir], {timeout: CLI_TIMEOUT.long})
119120
}
120121

121122
export async function deployApp(
@@ -133,7 +134,7 @@ export async function deployApp(
133134
if (ctx.version) args.push('--version', ctx.version)
134135
if (ctx.message) args.push('--message', ctx.message)
135136
if (ctx.config) args.push('--config', ctx.config)
136-
return ctx.cli.exec(args, {timeout: 5 * 60 * 1000})
137+
return ctx.cli.exec(args, {timeout: CLI_TIMEOUT.long})
137138
}
138139

139140
export async function appInfo(ctx: CLIContext): Promise<{
@@ -153,7 +154,7 @@ export async function appInfo(ctx: CLIContext): Promise<{
153154
}
154155

155156
export async function functionBuild(ctx: CLIContext): Promise<ExecResult> {
156-
return ctx.cli.exec(['app', 'function', 'build', '--path', ctx.appDir], {timeout: 3 * 60 * 1000})
157+
return ctx.cli.exec(['app', 'function', 'build', '--path', ctx.appDir], {timeout: CLI_TIMEOUT.medium})
157158
}
158159

159160
export async function functionRun(
@@ -162,13 +163,13 @@ export async function functionRun(
162163
},
163164
): Promise<ExecResult> {
164165
return ctx.cli.exec(['app', 'function', 'run', '--path', ctx.appDir, '--input', ctx.inputPath], {
165-
timeout: 60 * 1000,
166+
timeout: CLI_TIMEOUT.short,
166167
})
167168
}
168169

169170
export async function versionsList(ctx: CLIContext): Promise<ExecResult> {
170171
return ctx.cli.exec(['app', 'versions', 'list', '--path', ctx.appDir, '--json'], {
171-
timeout: 60 * 1000,
172+
timeout: CLI_TIMEOUT.short,
172173
})
173174
}
174175

@@ -178,7 +179,7 @@ export async function configLink(
178179
},
179180
): Promise<ExecResult> {
180181
return ctx.cli.exec(['app', 'config', 'link', '--path', ctx.appDir, '--client-id', ctx.clientId], {
181-
timeout: 2 * 60 * 1000,
182+
timeout: CLI_TIMEOUT.medium,
182183
})
183184
}
184185

@@ -223,7 +224,7 @@ export async function uninstallApp(
223224
const orgId = ctx.orgId ?? (process.env.E2E_ORG_ID ?? '').trim()
224225

225226
await browserPage.goto(`${appUrl}/installs`, {waitUntil: 'domcontentloaded'})
226-
await browserPage.waitForTimeout(3000)
227+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
227228

228229
const rows = await browserPage.locator('table tbody tr').all()
229230
const storeNames: string[] = []
@@ -245,20 +246,20 @@ export async function uninstallApp(
245246
let navigated = false
246247
for (let attempt = 1; attempt <= 3; attempt++) {
247248
await browserPage.goto(dashboardUrl, {waitUntil: 'domcontentloaded'})
248-
await browserPage.waitForTimeout(3000)
249+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
249250

250251
const pageText = (await browserPage.textContent('body')) ?? ''
251252
if (pageText.includes('500') || pageText.includes('Internal Server Error')) continue
252253

253254
const orgButton = browserPage.locator('header button').last()
254-
if (!(await orgButton.isVisible({timeout: 5000}).catch(() => false))) continue
255+
if (!(await orgButton.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) continue
255256
await orgButton.click()
256-
await browserPage.waitForTimeout(1000)
257+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.short)
257258

258259
const storeLink = browserPage.locator('a, button').filter({hasText: storeName}).first()
259-
if (!(await storeLink.isVisible({timeout: 5000}).catch(() => false))) continue
260+
if (!(await storeLink.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) continue
260261
await storeLink.click()
261-
await browserPage.waitForTimeout(3000)
262+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
262263
navigated = true
263264
break
264265
}
@@ -271,41 +272,41 @@ export async function uninstallApp(
271272
// Navigate to store's apps settings page
272273
const storeAdminUrl = browserPage.url()
273274
await browserPage.goto(`${storeAdminUrl.replace(/\/$/, '')}/settings/apps`, {waitUntil: 'domcontentloaded'})
274-
await browserPage.waitForTimeout(5000)
275+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.long)
275276

276277
// Dismiss any Dev Console dialog
277278
const cancelButton = browserPage.locator('button:has-text("Cancel")')
278-
if (await cancelButton.isVisible({timeout: 2000}).catch(() => false)) {
279+
if (await cancelButton.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) {
279280
await cancelButton.click()
280-
await browserPage.waitForTimeout(1000)
281+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.short)
281282
}
282283

283284
// Find the app in the installed list (plain span, not Dev Console's Polaris text)
284285
const appSpan = browserPage.locator(`span:has-text("${appName}"):not([class*="Polaris"])`).first()
285-
if (!(await appSpan.isVisible({timeout: 5000}).catch(() => false))) {
286+
if (!(await appSpan.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) {
286287
allUninstalled = false
287288
continue
288289
}
289290

290291
// Click the ⋯ menu button next to the app name
291292
const menuButton = appSpan.locator('xpath=./following::button[1]')
292293
await menuButton.click()
293-
await browserPage.waitForTimeout(1000)
294+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.short)
294295

295296
// Click "Uninstall" in the dropdown menu
296297
const uninstallOption = browserPage.locator('text=Uninstall').last()
297-
if (!(await uninstallOption.isVisible({timeout: 3000}).catch(() => false))) {
298+
if (!(await uninstallOption.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false))) {
298299
allUninstalled = false
299300
continue
300301
}
301302
await uninstallOption.click()
302-
await browserPage.waitForTimeout(2000)
303+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
303304

304305
// Handle confirmation dialog
305306
const confirmButton = browserPage.locator('button:has-text("Uninstall"), button:has-text("Confirm")').last()
306-
if (await confirmButton.isVisible({timeout: 3000}).catch(() => false)) {
307+
if (await confirmButton.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) {
307308
await confirmButton.click()
308-
await browserPage.waitForTimeout(3000)
309+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
309310
}
310311
// eslint-disable-next-line no-catch-all/no-catch-all
311312
} catch (_err) {
@@ -325,32 +326,32 @@ export async function deleteApp(
325326
const {browserPage, appUrl} = ctx
326327

327328
await browserPage.goto(`${appUrl}/settings`, {waitUntil: 'domcontentloaded'})
328-
await browserPage.waitForTimeout(3000)
329+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
329330

330331
// Retry if delete button is disabled (uninstall propagation delay)
331332
const deleteButton = browserPage.locator('button:has-text("Delete app")').first()
332333
for (let attempt = 1; attempt <= 5; attempt++) {
333334
await deleteButton.scrollIntoViewIfNeeded()
334335
const isDisabled = await deleteButton.getAttribute('disabled')
335336
if (!isDisabled) break
336-
await browserPage.waitForTimeout(5000)
337+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.long)
337338
await browserPage.reload({waitUntil: 'domcontentloaded'})
338-
await browserPage.waitForTimeout(3000)
339+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
339340
}
340341

341-
await deleteButton.click({timeout: 10_000})
342-
await browserPage.waitForTimeout(2000)
342+
await deleteButton.click({timeout: BROWSER_TIMEOUT.long})
343+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
343344

344345
// Handle confirmation dialog — may need to type "DELETE"
345346
const confirmInput = browserPage.locator('input[type="text"]').last()
346-
if (await confirmInput.isVisible({timeout: 3000}).catch(() => false)) {
347+
if (await confirmInput.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) {
347348
await confirmInput.fill('DELETE')
348-
await browserPage.waitForTimeout(500)
349+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.short)
349350
}
350351

351352
const confirmButton = browserPage.locator('button:has-text("Delete app")').last()
352353
await confirmButton.click()
353-
await browserPage.waitForTimeout(3000)
354+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
354355
}
355356

356357
/** Best-effort teardown: find app on dashboard by name, uninstall from all stores, delete. */

‎packages/e2e/setup/auth.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {CLI_TIMEOUT, BROWSER_TIMEOUT} from './constants.js'
12
import {browserFixture} from './browser.js'
23
import {executables} from './env.js'
34
import {stripAnsi} from '../helpers/strip-ansi.js'
@@ -56,7 +57,7 @@ export const authFixture = browserFixture.extend<{}, {authLogin: void}>({
5657
if (process.env.DEBUG === '1') process.stdout.write(data)
5758
})
5859

59-
await waitForText(() => output, 'Open this link to start the auth process', 30_000)
60+
await waitForText(() => output, 'Open this link to start the auth process', CLI_TIMEOUT.short)
6061

6162
const stripped = stripAnsi(output)
6263
const urlMatch = stripped.match(/https:\/\/accounts\.shopify\.com\S+/)
@@ -66,7 +67,7 @@ export const authFixture = browserFixture.extend<{}, {authLogin: void}>({
6667

6768
await completeLogin(browserPage, urlMatch[0], email, password)
6869

69-
await waitForText(() => output, 'Logged in', 60_000)
70+
await waitForText(() => output, 'Logged in', BROWSER_TIMEOUT.max)
7071
try {
7172
ptyProcess.kill()
7273
// eslint-disable-next-line no-catch-all/no-catch-all

‎packages/e2e/setup/browser.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {cliFixture} from './cli.js'
2+
import {BROWSER_TIMEOUT} from './constants.js'
23
import {chromium, type Page} from '@playwright/test'
34

45
// ---------------------------------------------------------------------------
@@ -32,8 +33,8 @@ export const browserFixture = cliFixture.extend<{}, {browserPage: Page}>({
3233
'X-Shopify-Loadtest-Bf8d22e7-120e-4b5b-906c-39ca9d5499a9': 'true',
3334
},
3435
})
35-
context.setDefaultTimeout(60_000)
36-
context.setDefaultNavigationTimeout(60_000)
36+
context.setDefaultTimeout(BROWSER_TIMEOUT.max)
37+
context.setDefaultNavigationTimeout(BROWSER_TIMEOUT.max)
3738
const page = await context.newPage()
3839
await use(page)
3940
await browser.close()
@@ -56,22 +57,22 @@ export async function navigateToDashboard(
5657
const orgId = ctx.orgId ?? (process.env.E2E_ORG_ID ?? '').trim()
5758
const dashboardUrl = orgId ? `https://dev.shopify.com/dashboard/${orgId}/apps` : 'https://dev.shopify.com/dashboard'
5859
await browserPage.goto(dashboardUrl, {waitUntil: 'domcontentloaded'})
59-
await browserPage.waitForTimeout(3000)
60+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
6061

6162
// Handle account picker (skip if email not provided)
6263
if (ctx.email) {
6364
const accountButton = browserPage.locator(`text=${ctx.email}`).first()
64-
if (await accountButton.isVisible({timeout: 5000}).catch(() => false)) {
65+
if (await accountButton.isVisible({timeout: BROWSER_TIMEOUT.medium}).catch(() => false)) {
6566
await accountButton.click()
66-
await browserPage.waitForTimeout(3000)
67+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium)
6768
}
6869
}
6970

7071
// Retry on 500 errors
7172
for (let attempt = 1; attempt <= 3; attempt++) {
7273
const pageText = (await browserPage.textContent('body')) ?? '' // eslint-disable-line no-await-in-loop
7374
if (!pageText.includes('500: Internal Server Error') && !pageText.includes('Internal Server Error')) break
74-
await browserPage.waitForTimeout(3000) // eslint-disable-line no-await-in-loop
75+
await browserPage.waitForTimeout(BROWSER_TIMEOUT.medium) // eslint-disable-line no-await-in-loop
7576
await browserPage.reload({waitUntil: 'domcontentloaded'}) // eslint-disable-line no-await-in-loop
7677
}
7778
}

‎packages/e2e/setup/cli.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable no-console */
2+
import {CLI_TIMEOUT} from './constants.js'
23
import {envFixture, executables} from './env.js'
34
import {stripAnsi} from '../helpers/strip-ansi.js'
45
import {execa, type Options as ExecaOptions} from 'execa'
@@ -47,8 +48,7 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
4748

4849
const cli: CLIProcess = {
4950
async exec(args, opts = {}) {
50-
// 3 min default
51-
const timeout = opts.timeout ?? 3 * 60 * 1000
51+
const timeout = opts.timeout ?? CLI_TIMEOUT.medium
5252
const execaOpts: ExecaOptions = {
5353
cwd: opts.cwd,
5454
env: {...env.processEnv, ...opts.env},
@@ -70,8 +70,7 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
7070
},
7171

7272
async execCreateApp(args, opts = {}) {
73-
// 5 min default for scaffolding
74-
const timeout = opts.timeout ?? 5 * 60 * 1000
73+
const timeout = opts.timeout ?? CLI_TIMEOUT.long
7574
const execaOpts: ExecaOptions = {
7675
cwd: opts.cwd,
7776
env: {...env.processEnv, ...opts.env},
@@ -153,7 +152,7 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
153152
const spawned: SpawnedProcess = {
154153
ptyProcess,
155154

156-
waitForOutput(text: string, timeoutMs = 3 * 60 * 1000) {
155+
waitForOutput(text: string, timeoutMs = CLI_TIMEOUT.medium) {
157156
// Check if already in output (raw or stripped)
158157
if (stripAnsi(output).includes(text) || output.includes(text)) {
159158
return Promise.resolve()
@@ -194,7 +193,7 @@ export const cliFixture = envFixture.extend<{cli: CLIProcess}>({
194193
ptyProcess.write(`${line}\r`)
195194
},
196195

197-
waitForExit(timeoutMs = 60 * 1000) {
196+
waitForExit(timeoutMs = CLI_TIMEOUT.short) {
198197
if (exitCode !== undefined) {
199198
return Promise.resolve(exitCode)
200199
}

‎packages/e2e/setup/constants.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// ---------------------------------------------------------------------------
2+
// Shared timeout constants for E2E tests
3+
// ---------------------------------------------------------------------------
4+
5+
/** Per-test timeouts (Playwright test.setTimeout) */
6+
export const TEST_TIMEOUT = {
7+
/** 3 min — default per-test timeout (set in playwright.config.ts) */
8+
default: 3 * 60_000,
9+
/** 10 min — tests with create + deploy + teardown */
10+
long: 10 * 60_000,
11+
} as const
12+
13+
/** CLI command execution timeouts */
14+
export const CLI_TIMEOUT = {
15+
/** 1 min — quick commands (versions list, function run, app info) */
16+
short: 1 * 60_000,
17+
/** 3 min — standard commands (deploy, build, config link) */
18+
medium: 3 * 60_000,
19+
/** 5 min — slow commands (create app, scaffold + npm install) */
20+
long: 5 * 60_000,
21+
} as const
22+
23+
/** Browser interaction timeouts */
24+
export const BROWSER_TIMEOUT = {
25+
/** 1s — brief pause between UI interactions */
26+
short: 1_000,
27+
/** 3s — page settle, element visibility checks */
28+
medium: 3_000,
29+
/** 5s — slow page loads (store admin, heavy pages) */
30+
long: 5_000,
31+
/** 60s — browser-level default for any Playwright action */
32+
max: 60_000,
33+
} as const

0 commit comments

Comments
 (0)