Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ryancbahan
changed the title
ryancbahan
force-pushed
the
e2e-testing
branch
from
ad0ad81 to
a55826d
Compare
New packages/e2e/ package with Playwright test runner, node-pty for interactive CLI testing, and isolated XDG environments. Includes: - Package config (package.json, tsconfig, playwright.config, project.json) - Fixtures: env (XDG isolation, auth tokens), cli-process (execa + PTY) - Helpers: strip-ansi, wait-for-port, file-edit - Smoke tests validating both execa and PTY execution paths
ryancbahan
force-pushed
the
e2e-testing
branch
from
a55826d to
c19eb9e
Compare
Coverage report
Test suite run success3778 tests passing in 1448 suites. Report generated by 🧪jest coverage report action from c96d6b9 |
| @@ -0,0 +1,19 @@ | |||
| # 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= | |||
There was a problem hiding this comment.
For initial E2E tests, we won't create new apps but instead use existing ones. We should 100% evolve this, but we need a starting point to monitor and observe stability of this job.
![binks-code-reviewer[bot]](https://avatars.githubusercontent.com/in/2788732?s=60&v=4){kind=small}
| waiter.reject(new Error(`Process exited (code ${code}) while waiting for output: "${waiter.text}"`)) | ||
| } | ||
| outputWaiters.length = 0 | ||
| }) |
There was a problem hiding this comment.
waitForExit() only supports a single waiter; subsequent calls can deadlock
The implementation stores only one exitResolve callback. If waitForExit() is called twice before the process exits, the second call overwrites exitResolve, leaving the first promise unresolved forever. Evidence: single-slot let exitResolve: ((code: number) => void) | undefined; waitForExit assigns exitResolve = (code) => { ... resolve(code) } overwriting any prior waiter; onExit calls only the last assigned exitResolve.
|
🤖 Code Review · #projects-dev-ai for questions ✅ Complete - 1 findings 📋 History✅ 1 findings |
1 participant
WHY are these changes introduced?
The CLI team manually runs a ~30-step QA checklist before every release. This is time-consuming and error-prone.
Cucumber suite tests CLI commands non-interactively but cannot handle interactive CLI prompts, long-running dev servers, auth, or browser verification. We should have better e2e test infra that allows us to automate QA, as well as provide better regression testing for CLI features. This is now possible with Shopify infra. for automating auth. We can leverage well-established packages Playwright and Node-pty for drivers/runners.
WHAT is this pull request doing?
Introduces a new
packages/e2e/package with Playwright + node-pty infrastructure for automated end-to-end testing of the full CLI workflow.Package setup:
package.json,tsconfig.json,project.json,playwright.config.tspnpm-workspace.yamlupdated to allowlistnode-ptynative buildsFixtures:
env.ts— worker-scoped XDG directory isolation and env var managementcli-process.ts— CLI execution via execa (non-interactive) and node-pty (interactive PTY with output matching, key sending, and process lifecycle)Helpers:
strip-ansi.ts,wait-for-port.ts,file-edit.tsSmoke tests:
smoke.spec.ts— validates execa path (shopify version)smoke-pty.spec.ts— validates PTY path (shopify versionvia node-pty)Subsequent PRs in this stack add OAuth login, app scaffold/deploy/dev tests, and CI integration.
How to test your changes?
Measuring impact
n/a - this is test infrastructure