Skip to content

E2E: migrate tests to use new infra #7162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
phyllis-sy-wu wants to merge 4 commits into
base: main
Choose a base branch
from
+448 −442
Open

E2E: migrate tests to use new infra #7162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b0cfa8
E2E: migrate tests to use new infra
phyllis-sy-wu Apr 2, 2026
0e2a505
e2e env cleanup
phyllis-sy-wu Apr 8, 2026
fa365b7
refactor timeouts
phyllis-sy-wu Apr 8, 2026
57c09aa
use proper TOML tooling instead of regex for fixture helpers
phyllis-sy-wu Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/tests-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ jobs:
name: 'E2E tests'
if: github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest
timeout-minutes: 15
timeout-minutes: 30
Copy link
Copy Markdown
Contributor

@ryancbahan ryancbahan Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why double?

Copy link
Copy Markdown
Contributor Author

@phyllis-sy-wu phyllis-sy-wu Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally, the full suite takes ~12-13 min. In CI it went over the original 15 min limit, 19 min on this PR, mainly due to per-test teardown. To prevent CI from killing the run mid-suite, I increased the timeout. But I agree 30 min might be too much. Will set it to 20 min now.

The follow-up stacked PR #7171 optimizes the teardown process a bit, which should bring CI times down further, will evaluate this again on that PR!

continue-on-error: true
steps:
- uses: actions/checkout@v3
Expand All @@ -241,11 +241,9 @@ jobs:
- name: Run E2E tests
working-directory: packages/e2e
env:
SHOPIFY_FLAG_CLIENT_ID: ${{ secrets.E2E_CLIENT_ID }}
E2E_ACCOUNT_EMAIL: ${{ secrets.E2E_ACCOUNT_EMAIL }}
E2E_ACCOUNT_PASSWORD: ${{ secrets.E2E_ACCOUNT_PASSWORD }}
E2E_STORE_FQDN: ${{ secrets.E2E_STORE_FQDN }}
E2E_SECONDARY_CLIENT_ID: ${{ secrets.E2E_SECONDARY_CLIENT_ID }}
E2E_ORG_ID: ${{ secrets.E2E_ORG_ID }}
run: npx playwright test
- name: Upload Playwright report
Expand Down
9 changes: 2 additions & 7 deletions packages/e2e/.env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
# Required: Client ID of the primary test app (must be in the genghis account's org)
# CI secret: E2E_CLIENT_ID
SHOPIFY_FLAG_CLIENT_ID=

# Required: Genghis account email for browser-based OAuth login
# CI secret: E2E_ACCOUNT_EMAIL
E2E_ACCOUNT_EMAIL=
Expand All @@ -14,6 +10,5 @@ E2E_ACCOUNT_PASSWORD=
# CI secret: E2E_STORE_FQDN
E2E_STORE_FQDN=

# Optional: Client ID of a secondary app for config link tests
# CI secret: E2E_SECONDARY_CLIENT_ID
E2E_SECONDARY_CLIENT_ID=
# Required: dedicated e2e org ID
E2E_ORG_ID=
2 changes: 1 addition & 1 deletion packages/e2e/data/invalid-tomls/bad-syntax.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Invalid TOML: malformed syntax (missing closing quote)
client_id = "__E2E_CLIENT_ID__"
client_id = "1234567890"
name = "Bad Syntax App
application_url = "https://example.com"
embedded = true
2 changes: 1 addition & 1 deletion packages/e2e/data/invalid-tomls/invalid-webhook.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Invalid TOML: bad webhook config (missing required uri)
client_id = "__E2E_CLIENT_ID__"
client_id = "1234567890"
name = "Invalid Webhook App"
application_url = "https://example.com"
embedded = true
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e/data/invalid-tomls/wrong-type.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Invalid TOML: wrong types for known fields
client_id = "__E2E_CLIENT_ID__"
client_id = "1234567890"
name = "Wrong Type App"
application_url = "https://example.com"
embedded = "not-a-boolean"
Expand Down
6 changes: 3 additions & 3 deletions packages/e2e/data/valid-app/shopify.app.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Comprehensive shopify.app.toml for E2E regression testing
# client_id is injected at runtime by the toml-app fixture
client_id = "__E2E_CLIENT_ID__"
name = "E2E TOML Regression Test"
# client_id and name are patched at runtime via injectFixtureToml()
client_id = "placeholder"
name = "placeholder"
application_url = "https://example.com"
embedded = true

Expand Down
11 changes: 7 additions & 4 deletions packages/e2e/helpers/browser-login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {BROWSER_TIMEOUT} from '../setup/constants.js'
import type {Page} from '@playwright/test'

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

try {
// Fill in email
await page.waitForSelector('input[name="account[email]"], input[type="email"]', {timeout: 60_000})
await page.waitForSelector('input[name="account[email]"], input[type="email"]', {timeout: BROWSER_TIMEOUT.max})
await fillSensitive(page, 'input[name="account[email]"], input[type="email"]', email)
await page.locator('button[type="submit"]').first().click()

// Fill in password
await page.waitForSelector('input[name="account[password]"], input[type="password"]', {timeout: 60_000})
await page.waitForSelector('input[name="account[password]"], input[type="password"]', {
timeout: BROWSER_TIMEOUT.max,
})
await fillSensitive(page, 'input[name="account[password]"], input[type="password"]', password)
await page.locator('button[type="submit"]').first().click()

// Handle any confirmation/approval page
await page.waitForTimeout(3000)
await page.waitForTimeout(BROWSER_TIMEOUT.medium)
try {
const btn = page.locator('button[type="submit"]').first()
if (await btn.isVisible({timeout: 5000})) await btn.click()
if (await btn.isVisible({timeout: BROWSER_TIMEOUT.long})) await btn.click()
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (_error) {
// No confirmation page — expected
Expand Down
2 changes: 2 additions & 0 deletions packages/e2e/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
}
},
"devDependencies": {
"@iarna/toml": "2.2.5",
"@playwright/test": "^1.50.0",
"@shopify/toml-patch": "0.3.0",
"@types/node": "18.19.70",
"dotenv": "16.4.7",
"execa": "^7.2.0",
Expand Down
5 changes: 3 additions & 2 deletions packages/e2e/playwright.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable line-comment-position */
import {TEST_TIMEOUT} from './setup/constants.js'
import {config} from 'dotenv'
import {defineConfig} from '@playwright/test'

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

use: {
trace: isCI ? 'on' : 'off',
Expand Down
Loading
Loading