E2E: migrate tests to use new infra by phyllis-sy-wu · Pull Request #7162 · Shopify/cli

phyllis-sy-wu · 2026-04-02T12:51:26Z

WHY are these changes introduced?

This is a follow-up to #7147 (E2E infra refactor). That PR migrated the app tests (scaffold, deploy, dev-server) to create fresh apps from scratch. This PR completes the migration by converting the remaining 4 test files that still depended on pre-provisioned apps via SHOPIFY_FLAG_CLIENT_ID.

After this PR, all E2E tests are self-contained — each test creates its own app, runs its assertions, and cleans up. No test depends on a pre-provisioned app or a fixed SHOPIFY_FLAG_CLIENT_ID.

WHAT is this pull request doing?

TOML helpers in setup/app.ts:

extractClientId(appDir) — reads client_id from shopify.app.toml using @iarna/toml parser (same parser cli-kit uses internally via decodeToml)
injectFixtureToml(appDir, template, name) — patches client_id and name into fixture TOML using @shopify/toml-patch (updateTomlValues), preserving comments and formatting

TOML editing in tests:

dev-hot-reload.spec.ts — scopes edit now uses updateTomlValues from @shopify/toml-patch instead of string replace

Shared timeout constants (setup/constants.ts):

TEST_TIMEOUT — per-test Playwright timeouts: default (3 min), long (10 min)
CLI_TIMEOUT — CLI command execution: short (1 min), medium (3 min), long (5 min)
BROWSER_TIMEOUT — browser interactions: short (1s), medium (3s), long (5s), max (60s)
All hardcoded timeouts across setup/, helpers/, and test files replaced with named constants

Migrated tests:

dev-hot-reload.spec.ts — creates fresh app, injects fully populated TOML fixture, tests hot reload (edit scopes, create/delete extensions mid-dev)
multi-config-dev.spec.ts — creates fresh app, injects fixture TOML, creates staging config with same client_id, tests -c flag config selection
toml-config.spec.ts — creates fresh app, injects fixture TOML, tests deploy and dev with fully populated config
toml-config-invalid.spec.ts — uses hardcoded dummy client_id (CLI rejects invalid TOMLs at validation before any API call, so no real app needed)

Cleaned up:

Deleted setup/toml-app.ts — the tomlAppFixture is no longer used
Removed SHOPIFY_FLAG_CLIENT_ID and E2E_SECONDARY_CLIENT_ID from CI workflow
Stripped SHOPIFY_FLAG_CLIENT_ID globally in env.ts so it can't leak from local .env files
Removed clientId, secondaryClientId, partnersToken from E2EEnv interface
Removed FRESH_APP_ENV from setup/app.ts
Updated TOML fixtures — data/valid-app/shopify.app.toml uses valid placeholder values patched at runtime by updateTomlValues
Global timeout tuned to 20 min (CI runs at ~16-19 min)

How to test your changes?

DEBUG=1 pnpm --filter e2e exec playwright test

Ensure SHOPIFY_FLAG_CLIENT_ID is NOT in .env (no longer used).

Measuring impact

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

phyllis-sy-wu · 2026-04-02T12:51:40Z

E2E: QA - app basic flow #7211
E2E: cleanup utility #7171
E2E: migrate tests to use new infra #7162 👈 (View in Graphite)
E2E: infra refactor + app tests update #7147
main

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

Copilot

Pull request overview

Completes the E2E infra migration by converting the remaining TOML/dev-related E2E specs to create and clean up fresh apps at runtime, removing reliance on pre-provisioned apps and SHOPIFY_FLAG_CLIENT_ID.

Changes:

Migrates the remaining E2E specs to use appTestFixture + createApp() and runtime TOML injection.
Adds extractClientId() / injectFixtureToml() helpers and removes the legacy toml-app fixture.
Updates TOML fixtures and CI workflow to remove SHOPIFY_FLAG_CLIENT_ID / E2E_SECONDARY_CLIENT_ID.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/e2e/tests/toml-config.spec.ts	Creates fresh apps per test, injects full TOML fixture, deploys/devs, cleans up via dashboard.
packages/e2e/tests/toml-config-invalid.spec.ts	Stops injecting real client id; uses dummy client id in invalid TOMLs.
packages/e2e/tests/multi-config-dev.spec.ts	Creates fresh app, injects TOML fixture, adds staging config, validates `-c` config selection behavior.
packages/e2e/tests/dev-hot-reload.spec.ts	Creates fresh app, injects TOML fixture, verifies dev hot-reload via TOML edits and extension create/delete.
packages/e2e/setup/toml-app.ts	Deletes the old pre-provisioned TOML app fixture.
packages/e2e/setup/env.ts	Simplifies `E2EEnv` and `requireEnv()` (drops client id / partners token fields).
packages/e2e/setup/app.ts	Adds TOML helpers; removes `FRESH_APP_ENV` env-stripping from CLI helpers.
packages/e2e/data/valid-app/shopify.app.toml	Switches placeholders to `__CLIENT_ID__` / `__NAME__` for runtime injection.
packages/e2e/data/invalid-tomls/*	Replaces `client_id` placeholder with a dummy value.
.github/workflows/tests-pr.yml	Removes `SHOPIFY_FLAG_CLIENT_ID` and `E2E_SECONDARY_CLIENT_ID` from E2E job env.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/e2e/setup/app.ts

github-actions · 2026-04-02T21:35:29Z

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/common/version.d.ts

@@ -1 +1 @@
-export declare const CLI_KIT_VERSION = "3.93.0";
\ No newline at end of file
+export declare const CLI_KIT_VERSION = "3.92.0";
\ No newline at end of file

ryancbahan · 2026-04-08T02:02:21Z

.github/workflows/tests-pr.yml

    if: github.event.pull_request.head.repo.full_name == github.repository
    runs-on: ubuntu-latest
-    timeout-minutes: 15
+    timeout-minutes: 30


why double?

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!

ryancbahan · 2026-04-08T02:04:06Z

packages/e2e/setup/app.ts

-    // Strip CLIENT_ID so the CLI creates a new app instead of linking to a pre-existing one
-    env: {FORCE_COLOR: '0', ...FRESH_APP_ENV},
+    // Disable color output and strip CLIENT_ID to prevent leaking from parent process.env
+    env: {FORCE_COLOR: '0', SHOPIFY_FLAG_CLIENT_ID: undefined},


let's just remove SHOPIFY_FLAG_CLIENT_ID

Thanks for spottig this! Removed, but added SHOPIFY_FLAG_CLIENT_ID: undefined in pe2e/setup/env.ts to avoid errors if SHOPIFY_FLAG_CLIENT_ID is set in local .env

ryancbahan · 2026-04-08T02:05:14Z

packages/e2e/setup/app.ts

+/**
+ * Read the client_id from a shopify.app.toml file.
+ */
+export function extractClientId(appDir: string): string {


won't block on this, but we can use @shopify/toml-patch for this in the same manner that the cli does to extract toml fields. as much as possible we should avoid writing our own parsers

Thanks for the suggestions. Switched extractClientId from regex to @iarna/toml, which parses TOML into a JS object so we can access client_id directly. @shopify/toml-patch only works with TOML strings in/out — it doesn't expose parsed values as JS objects, so it can't be used for reading fields.

ryancbahan · 2026-04-08T02:06:14Z

packages/e2e/setup/app.ts

+}
+
+/**
+ * Overwrite a created app's shopify.app.toml with a fixture TOML template.


same comment re: @shopify/toml-patch

Thanks for the suggestions. Now used toml-patch for injectFixtureToml. The fixture TOML now uses valid placeholder values instead of __CLIENT_ID__/__NAME__ — toml-patch patches them surgically at runtime, preserving comments and formatting.

ryancbahan · 2026-04-08T02:07:02Z

packages/e2e/setup/app.ts

  if (ctx.message) args.push('--message', ctx.message)
  if (ctx.config) args.push('--config', ctx.config)
-  return ctx.cli.exec(args, {env: FRESH_APP_ENV, timeout: 5 * 60 * 1000})
+  return ctx.cli.exec(args, {timeout: 5 * 60 * 1000})


it's probably a good time to register a default timeout and add constants for areas that need more time

Thanks for the suggestions! Now added setup/constants.ts. All magic numbers in e2e are replaced. Few older files are not processed because the files are planned to be removed completely later.

ryancbahan · 2026-04-08T02:09:26Z

packages/e2e/tests/dev-hot-reload.spec.ts

+        // Edit scopes in the TOML to trigger a reload
+        const tomlPath = path.join(appDir, 'shopify.app.toml')
+        const original = fs.readFileSync(tomlPath, 'utf8')
+        fs.writeFileSync(


same comment re: toml patch

Thanks for the suggestions. Updated!

ryancbahan · 2026-04-08T02:10:15Z

packages/e2e/tests/dev-hot-reload.spec.ts

+      }
    } finally {
-      proc.kill()
+      fs.rmSync(parentDir, {recursive: true, force: true})


i think cli also has tmp dir utils for setting up ephemeral temporary directories that we can leverage here

Thanks for the suggestions. Looked into this — cli-kit has inTemporaryDirectory and tempDirectory in @shopify/cli-kit/node/fs. Our env fixture already creates a single worker-scoped temp root (env.tempDir) that holds both XDG dirs and per-test app dirs. This gives us one place to inspect during debugging and one cleanup path on worker teardown. cli-kit's inTemporaryDirectory would scatter dirs across /tmp and always cleans up in its finally block — no way to preserve files for debugging. So keeping the current approach for now.

ryancbahan · 2026-04-08T02:11:16Z

packages/e2e/playwright.config.ts

  reporter: isCI ? [['html', {open: 'never'}], ['list']] : [['list']],
  timeout: 3 * 60 * 1000, // 3 minutes per test
-  globalTimeout: 15 * 60 * 1000, // 15 minutes total
+  globalTimeout: 30 * 60 * 1000, // 30 minutes total


i'd encourage us to increment these in smaller portions, and try to use data when available to inform the decision

Same reply here

ryancbahan

Looking good overall! i think there are a few quality of life things that would be good to take a pass on, then ship. lmk what you think.

phyllis-sy-wu mentioned this pull request Apr 2, 2026

E2E: infra refactor + app tests update #7147

Merged

3 tasks

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from e574bd4 to e55998a Compare April 2, 2026 14:08

phyllis-sy-wu marked this pull request as ready for review April 2, 2026 14:15

phyllis-sy-wu requested a review from a team as a code owner April 2, 2026 14:15

phyllis-sy-wu added the #gsd:49408 Agentic app validation label Apr 2, 2026

phyllis-sy-wu requested a review from Copilot April 2, 2026 15:07

Copilot started reviewing on behalf of phyllis-sy-wu April 2, 2026 15:08 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

packages/e2e/setup/app.ts Show resolved Hide resolved

packages/e2e/setup/app.ts Outdated Show resolved Hide resolved

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch 3 times, most recently from 07eee8f to 91ac6a4 Compare April 2, 2026 16:11

phyllis-sy-wu requested a review from ryancbahan April 2, 2026 16:44

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch 2 times, most recently from 3b4a51b to 1d49241 Compare April 2, 2026 21:33

phyllis-sy-wu force-pushed the psyw-0331-E2E-infra-refactor branch from b931123 to bf1e8b6 Compare April 2, 2026 21:33

phyllis-sy-wu mentioned this pull request Apr 2, 2026

E2E: cleanup utility #7171

Open

3 tasks

phyllis-sy-wu force-pushed the psyw-0331-E2E-infra-refactor branch from bf1e8b6 to de26577 Compare April 2, 2026 22:23

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from 1d49241 to 8be6206 Compare April 2, 2026 22:24

phyllis-sy-wu force-pushed the psyw-0331-E2E-infra-refactor branch from de26577 to 9649836 Compare April 6, 2026 19:42

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from 8be6206 to 3a642b1 Compare April 6, 2026 19:42

phyllis-sy-wu mentioned this pull request Apr 7, 2026

E2E: QA - app basic flow #7211

Open

3 tasks

phyllis-sy-wu force-pushed the psyw-0331-E2E-infra-refactor branch from 9649836 to 8e05272 Compare April 7, 2026 18:41

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from 3a642b1 to a7567f7 Compare April 7, 2026 18:41

Base automatically changed from psyw-0331-E2E-infra-refactor to main April 7, 2026 19:26

E2E: migrate tests to use new infra

7b0cfa8

ryancbahan reviewed Apr 8, 2026

View reviewed changes

e2e env cleanup

0e2a505

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from a7567f7 to 0e2a505 Compare April 8, 2026 16:21

refactor timeouts

fa365b7

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from 04cbb24 to 57c09aa Compare April 9, 2026 06:11

use proper TOML tooling instead of regex for fixture helpers

9aa7b21

phyllis-sy-wu force-pushed the psyw-0401-E2E-migrate-remaining-tests-to-use-new-infra branch from 57c09aa to 9aa7b21 Compare April 9, 2026 14:38

phyllis-sy-wu requested a review from ryancbahan April 9, 2026 15:18

ryancbahan approved these changes Apr 9, 2026

View reviewed changes

Conversation

phyllis-sy-wu commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Measuring impact

Checklist

Uh oh!

phyllis-sy-wu commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 2, 2026

Differences in type declarations

New type declarations

Existing type declarations

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phyllis-sy-wu Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryancbahan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phyllis-sy-wu commented Apr 2, 2026 •

edited

Loading

phyllis-sy-wu commented Apr 2, 2026 •

edited

Loading

phyllis-sy-wu Apr 9, 2026 •

edited

Loading