diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 9a2d043e9c4..e09a2569b10 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -252,7 +252,7 @@ jobs: # Playwright — those failures belong to the build lane, not to E2E. needs: [type-check, bundle] runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 40 continue-on-error: true strategy: fail-fast: false @@ -283,7 +283,7 @@ jobs: E2E_STORE_FQDN: ${{ secrets.E2E_STORE_FQDN }} E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }} E2E_LOADTEST_HEADER: ${{ secrets.E2E_LOADTEST_HEADER }} - run: pnpm exec playwright test --project remote --shard ${{ matrix.shard }} + run: pnpm exec playwright test --project remote --shard ${{ matrix.shard }} --global-timeout 1800000 - name: Upload Playwright report uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} diff --git a/packages/e2e/playwright.config.ts b/packages/e2e/playwright.config.ts index c4e6e148c8f..5be9c7e9287 100644 --- a/packages/e2e/playwright.config.ts +++ b/packages/e2e/playwright.config.ts @@ -14,7 +14,7 @@ export default defineConfig({ retries: 0, workers: 10, maxFailures: isCI ? 3 : 0, // Stop early in CI after 3 failures - reporter: isCI ? [['html', {open: 'never'}], ['list']] : [['list']], + reporter: isCI ? [['html', {open: 'never'}], ['list'], ['./retry-summary-reporter.ts']] : [['list']], timeout: TEST_TIMEOUT.default, // Heavy tests override via test.setTimeout() globalTimeout: 20 * 60 * 1000, @@ -31,6 +31,7 @@ export default defineConfig({ 'tests/smoke-pty.spec.ts', 'tests/fixture-toml.spec.ts', 'tests/auth-diagnostics.spec.ts', + 'tests/retry-behavior.spec.ts', ], }, { @@ -39,12 +40,14 @@ export default defineConfig({ }, { name: 'remote', + retries: isCI ? 1 : 0, testMatch: 'tests/*.spec.ts', testIgnore: [ 'tests/smoke.spec.ts', 'tests/smoke-pty.spec.ts', 'tests/fixture-toml.spec.ts', 'tests/auth-diagnostics.spec.ts', + 'tests/retry-behavior.spec.ts', ], dependencies: ['remote-auth'], }, diff --git a/packages/e2e/retry-summary-reporter.ts b/packages/e2e/retry-summary-reporter.ts new file mode 100644 index 00000000000..303086db1f4 --- /dev/null +++ b/packages/e2e/retry-summary-reporter.ts @@ -0,0 +1,75 @@ +import type {FullConfig, Reporter, Suite, TestCase, TestResult} from '@playwright/test/reporter' + +type TestStatus = TestResult['status'] + +export interface RetryTestResult { + title: string + expectedStatus: TestStatus + attempts: {retry: number; status: TestStatus}[] +} + +export interface RetrySummary { + firstAttemptFailures: string[] + passedRetries: string[] + persistentFailures: string[] +} + +export function summarizeRetryTests(tests: RetryTestResult[]): RetrySummary { + const summary: RetrySummary = { + firstAttemptFailures: [], + passedRetries: [], + persistentFailures: [], + } + + for (const test of tests) { + const firstAttempt = test.attempts.find(({retry}) => retry === 0) + if (!firstAttempt || firstAttempt.status === test.expectedStatus) continue + + summary.firstAttemptFailures.push(test.title) + if (test.attempts.some(({retry, status}) => retry > 0 && status === test.expectedStatus)) { + summary.passedRetries.push(test.title) + } else { + summary.persistentFailures.push(test.title) + } + } + + return summary +} + +export default class RetrySummaryReporter implements Reporter { + private remoteTests: TestCase[] = [] + + onBegin(_config: FullConfig, suite: Suite): void { + this.remoteTests = suite.allTests().filter((test) => test.parent.project()?.name === 'remote') + } + + onEnd(): void { + if (this.remoteTests.length === 0) return + + const summary = summarizeRetryTests( + this.remoteTests.map((test) => ({ + title: test.titlePath().join(' > '), + expectedStatus: test.expectedStatus, + attempts: test.results.map(({retry, status}) => ({retry, status})), + })), + ) + + this.printTests('first-attempt-failure', summary.firstAttemptFailures) + this.printTests('passed-retry', summary.passedRetries) + this.printTests('persistent-failure', summary.persistentFailures) + process.stdout.write( + `[e2e][retry-summary] first_attempt_failures=${summary.firstAttemptFailures.length} ` + + `passed_retries=${summary.passedRetries.length} persistent_failures=${summary.persistentFailures.length}\n`, + ) + } + + printsToStdio(): boolean { + return true + } + + private printTests(category: string, tests: string[]): void { + for (const test of tests) { + process.stdout.write(`[e2e][retry-summary] ${category} test=${JSON.stringify(test)}\n`) + } + } +} diff --git a/packages/e2e/setup/env.ts b/packages/e2e/setup/env.ts index 37858cf50e2..47b2002f230 100644 --- a/packages/e2e/setup/env.ts +++ b/packages/e2e/setup/env.ts @@ -140,8 +140,14 @@ export function e2eRunSegment(): string { return runId ? `r${BigInt(runId).toString(36)}a${runAttempt}` : 'local' } -export function e2eAppName(prefix: string): string { - const timestampSegment = Date.now().toString(36) +export function retryScopedTimestamp(retry: number, now = Date.now()): string { + if (!Number.isInteger(retry) || retry < 0) throw new Error('retry must be a non-negative integer') + + return (now + retry).toString(36) +} + +export function e2eAppName(prefix: string, retry = 0): string { + const timestampSegment = retryScopedTimestamp(retry) return `E2E-${E2E_APP_PREFIXES[prefix] ?? prefix}-${e2eRunSegment()}-${timestampSegment}` } diff --git a/packages/e2e/setup/store.ts b/packages/e2e/setup/store.ts index 135aa7e228b..f22599d7f7c 100644 --- a/packages/e2e/setup/store.ts +++ b/packages/e2e/setup/store.ts @@ -2,7 +2,7 @@ import {appTestFixture} from './app.js' import {isVisibleWithin} from './browser.js' import {BROWSER_TIMEOUT, CLI_TIMEOUT} from './constants.js' -import {createLogger, e2eRunSegment, e2eSection, requireEnv} from './env.js' +import {createLogger, e2eRunSegment, e2eSection, requireEnv, retryScopedTimestamp} from './env.js' import type {CLIProcess, ExecResult} from './cli.js' import type {Locator, Page} from '@playwright/test' @@ -13,8 +13,8 @@ const log = createLogger('cli') // --------------------------------------------------------------------------- /** Generate a unique store name for a worker. */ -export function generateStoreName(workerIndex: number): string { - const timestampSegment = Date.now().toString(36) +export function generateStoreName(workerIndex: number, retry = 0): string { + const timestampSegment = retryScopedTimestamp(retry) return `e2e-w${workerIndex}-${e2eRunSegment()}-${timestampSegment}` } @@ -275,7 +275,7 @@ export async function isStoreAppsEmpty(page: Page): Promise { * Tests that don't (scaffold, deploy, commands, smoke) stay on appTestFixture. */ export const storeTestFixture = appTestFixture.extend<{storeFqdn: string}>({ - storeFqdn: async ({cli, env}, use) => { + storeFqdn: async ({cli, env}, use, testInfo) => { requireEnv(env, 'orgId') const wi = env.workerIndex @@ -284,7 +284,7 @@ export const storeTestFixture = appTestFixture.extend<{storeFqdn: string}>({ env.processEnv.SHOPIFY_FLAG_GRAPHIQL_PORT = String(portBase) env.processEnv.SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT = String(portBase + 2) - const storeName = generateStoreName(wi) + const storeName = generateStoreName(wi, testInfo.retry) const fqdn = await createDevStoreWithCli({cli, workerIndex: wi, storeName, orgId: env.orgId}) env.processEnv.SHOPIFY_FLAG_STORE = fqdn // eslint-disable-line require-atomic-updates diff --git a/packages/e2e/tests/app-deploy.spec.ts b/packages/e2e/tests/app-deploy.spec.ts index a8c1c0fac37..aca9a55103d 100644 --- a/packages/e2e/tests/app-deploy.spec.ts +++ b/packages/e2e/tests/app-deploy.spec.ts @@ -81,13 +81,13 @@ function assertActiveVersion(opts: { } test.describe('App deploy', () => { - test('init, deploy, versions list, config link, deploy to secondary', async ({cli, env, browserPage}) => { + test('init, deploy, versions list, config link, deploy to secondary', async ({cli, env, browserPage}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('deploy1') - const secondaryAppName = e2eAppName('deploy2') + const appName = e2eAppName('deploy1', testInfo.retry) + const secondaryAppName = e2eAppName('deploy2', testInfo.retry) let primaryAppUrl: string | undefined let secondaryAppUrl: string | undefined diff --git a/packages/e2e/tests/app-dev-server.spec.ts b/packages/e2e/tests/app-dev-server.spec.ts index 19bbf43ec57..1a1af97d1d9 100644 --- a/packages/e2e/tests/app-dev-server.spec.ts +++ b/packages/e2e/tests/app-dev-server.spec.ts @@ -8,12 +8,12 @@ import * as fs from 'fs' import * as path from 'path' // eslint-disable-line no-restricted-imports test.describe('App dev server', () => { - test('dev starts, shows ready message, and quits with q', async ({cli, env, browserPage, storeFqdn}) => { + test('dev starts, shows ready message, and quits with q', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('dev') + const appName = e2eAppName('dev', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined diff --git a/packages/e2e/tests/app-scaffold.spec.ts b/packages/e2e/tests/app-scaffold.spec.ts index 5a62c8659f2..364a6584a86 100644 --- a/packages/e2e/tests/app-scaffold.spec.ts +++ b/packages/e2e/tests/app-scaffold.spec.ts @@ -8,12 +8,12 @@ import * as fs from 'fs' import * as path from 'path' test.describe('App scaffold', () => { - test('init creates a react-router app and builds', async ({cli, env, browserPage}) => { + test('init creates a react-router app and builds', async ({cli, env, browserPage}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('scaffold') + const appName = e2eAppName('scaffold', testInfo.retry) let appUrl: string | undefined try { @@ -61,12 +61,12 @@ test.describe('App scaffold', () => { } }) - test('init creates an extension-only app', async ({cli, env, browserPage}) => { + test('init creates an extension-only app', async ({cli, env, browserPage}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('ext-only') + const appName = e2eAppName('ext-only', testInfo.retry) let appUrl: string | undefined try { @@ -99,12 +99,12 @@ test.describe('App scaffold', () => { } }) - test('generates extensions and builds', async ({cli, env, browserPage}) => { + test('generates extensions and builds', async ({cli, env, browserPage}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('ext-gen') + const appName = e2eAppName('ext-gen', testInfo.retry) let appUrl: string | undefined try { diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts index f976451aaf5..e6dd086a7ff 100644 --- a/packages/e2e/tests/dev-hot-reload.spec.ts +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -39,12 +39,12 @@ description = "E2E test trigger" } test.describe('Dev hot reload', () => { - test('editing app config TOML triggers reload', async ({cli, env, browserPage, storeFqdn}) => { + test('editing app config TOML triggers reload', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('hot-reload') + const appName = e2eAppName('hot-reload', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined @@ -104,12 +104,12 @@ test.describe('Dev hot reload', () => { } }) - test('creating a new extension mid-dev is detected', async ({cli, env, browserPage, storeFqdn}) => { + test('creating a new extension mid-dev is detected', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('hot-create') + const appName = e2eAppName('hot-create', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined @@ -162,12 +162,12 @@ test.describe('Dev hot reload', () => { } }) - test('deleting an extension mid-dev is detected', async ({cli, env, browserPage, storeFqdn}) => { + test('deleting an extension mid-dev is detected', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('hot-delete') + const appName = e2eAppName('hot-delete', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined diff --git a/packages/e2e/tests/multi-config-dev.spec.ts b/packages/e2e/tests/multi-config-dev.spec.ts index d2559fc783e..04d1a001efa 100644 --- a/packages/e2e/tests/multi-config-dev.spec.ts +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -14,12 +14,12 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)) const FIXTURE_TOML = fs.readFileSync(path.join(__dirname, '../data/valid-app/shopify.app.toml'), 'utf8') test.describe('Multi-config dev', () => { - test('dev with -c flag loads the named config', async ({cli, env, browserPage, storeFqdn}) => { + test('dev with -c flag loads the named config', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('multi-cfg') + const appName = e2eAppName('multi-cfg', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined @@ -107,12 +107,12 @@ extensions_summary = "E2E staging app extensions" } }) - test('dev without -c flag uses default config', async ({cli, env, browserPage, storeFqdn}) => { + test('dev without -c flag uses default config', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('mcfg-def') + const appName = e2eAppName('mcfg-def', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined diff --git a/packages/e2e/tests/retry-behavior.spec.ts b/packages/e2e/tests/retry-behavior.spec.ts new file mode 100644 index 00000000000..9e4a19f81a8 --- /dev/null +++ b/packages/e2e/tests/retry-behavior.spec.ts @@ -0,0 +1,48 @@ +import {summarizeRetryTests} from '../retry-summary-reporter.js' +import {e2eAppName} from '../setup/env.js' +import {generateStoreName} from '../setup/store.js' +import {expect, test} from '@playwright/test' + +test.describe('remote retries', () => { + test('reports first failures, recovered retries, and persistent failures separately', () => { + const summary = summarizeRetryTests([ + { + title: 'passes immediately', + expectedStatus: 'passed', + attempts: [{retry: 0, status: 'passed'}], + }, + { + title: 'passes on retry', + expectedStatus: 'passed', + attempts: [ + {retry: 0, status: 'failed'}, + {retry: 1, status: 'passed'}, + ], + }, + { + title: 'fails persistently', + expectedStatus: 'passed', + attempts: [ + {retry: 0, status: 'timedOut'}, + {retry: 1, status: 'failed'}, + ], + }, + ]) + + expect(summary.firstAttemptFailures).toEqual(['passes on retry', 'fails persistently']) + expect(summary.passedRetries).toEqual(['passes on retry']) + expect(summary.persistentFailures).toEqual(['fails persistently']) + }) + + test('uses the retry index to create distinct app and store names', () => { + const originalNow = Date.now + Date.now = () => 1_786_704_000_000 + + try { + expect(e2eAppName('dev', 0)).not.toBe(e2eAppName('dev', 1)) + expect(generateStoreName(2, 0)).not.toBe(generateStoreName(2, 1)) + } finally { + Date.now = originalNow + } + }) +}) diff --git a/packages/e2e/tests/toml-config.spec.ts b/packages/e2e/tests/toml-config.spec.ts index 3f93816cb95..a47fdf813de 100644 --- a/packages/e2e/tests/toml-config.spec.ts +++ b/packages/e2e/tests/toml-config.spec.ts @@ -14,12 +14,12 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url)) const FIXTURE_TOML = fs.readFileSync(path.join(__dirname, '../data/valid-app/shopify.app.toml'), 'utf8') test.describe('TOML config regression', () => { - test('deploy succeeds with fully populated toml', async ({cli, env, browserPage}) => { + test('deploy succeeds with fully populated toml', async ({cli, env, browserPage}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('toml-deploy') + const appName = e2eAppName('toml-deploy', testInfo.retry) let appUrl: string | undefined try { @@ -51,12 +51,12 @@ test.describe('TOML config regression', () => { } }) - test('dev starts with fully populated toml', async ({cli, env, browserPage, storeFqdn}) => { + test('dev starts with fully populated toml', async ({cli, env, browserPage, storeFqdn}, testInfo) => { test.setTimeout(TEST_TIMEOUT.long) requireEnv(env, 'orgId') const parentDir = fs.mkdtempSync(path.join(env.tempDir, 'app-')) - const appName = e2eAppName('toml-dev') + const appName = e2eAppName('toml-dev', testInfo.retry) let appUrl: string | undefined let appDir: string | undefined