Include mutation flow in local E2E#107
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Playwright Hosted Data-FlowOutcome: success This optional check runs the mutation-backed profile flow against a configured hosted dev/staging target with isolated E2E test data. |
Playwright Data-Flow PreviewOutcome: success Captured flow:
Artifacts include screenshots, traces, and recorded video for the flow run. |
Playwright Public Screenshot PreviewOutcome: success Screenshots: all public route checks passed on desktop and mobile. Full screenshot set is available in the artifact. Pixel diff baselines are handled by the separate Playwright Image Diff check. |
Playwright Image DiffOutcome: success Changed screenshot baselines: none in this PR. This check compares public route screenshots against committed baselines. Inline images show only added or modified baseline PNGs. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Greptile SummaryThis PR enables the mutation-backed
Confidence Score: 4/5Safe to merge; the change only affects local developer E2E runs and leaves hosted, smoke, and production modes untouched. The logic is sound — localE2eHelperEnv correctly gates on !hostedBaseURL so credential defaults never leak into hosted or production runs. The one nit is a dead-code ?? 'true' fallback that is unreachable but harmless. Everything else — the grep-invert adjustment, the env spreading order, and the escape-hatch via explicit VRDEX_ENABLE_E2E_HELPERS=false — behaves as intended. No files require special attention beyond the minor redundancy in playwright.config.mjs. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[pnpm test:e2e] --> B{PLAYWRIGHT_BASE_URL set?}
B -- No / Local --> C[hostedBaseURL = undefined]
B -- Yes / Hosted --> D[hostedBaseURL = URL]
C --> E["e2eHelpersEnabled = VRDEX_ENABLE_E2E_HELPERS ?? 'true'"]
D --> F["e2eHelpersEnabled = VRDEX_ENABLE_E2E_HELPERS ?? undefined"]
E --> G["localE2eHelperEnv = {VRDEX_ENABLE_E2E_HELPERS, VRDEX_E2E_BROWSER_TOKEN ?? 'local-playwright-token', VRDEX_E2E_CONVEX_SECRET ?? 'local-convex-e2e-secret'}"]
F --> H["localE2eHelperEnv = {}"]
G --> I[sharedEnv spread into webServer.env]
H --> I
I --> J[Convex dev server]
I --> K[Next dev server]
K --> L{grep-invert pattern}
L -- test:e2e --> M["exclude @visual | @snapshot (includes @flow ✓)"]
L -- test:e2e:hosted:smoke --> N["exclude @visual | @flow | @snapshot (production-safe only)"]
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/web/playwright.config.mjs:25
The `?? "true"` fallback on this line is unreachable. In the non-hosted branch, `e2eHelpersEnabled` is always `process.env.VRDEX_ENABLE_E2E_HELPERS ?? "true"` — a string, never `null` or `undefined` — so the nullish coalescing here never fires. Dropping it makes the intent clearer.
```suggestion
VRDEX_ENABLE_E2E_HELPERS: e2eHelpersEnabled,
```
Reviews (1): Last reviewed commit: "Include mutation flow in local E2E" | Re-trigger Greptile |
| const localE2eHelperEnv = hostedBaseURL | ||
| ? {} | ||
| : { | ||
| VRDEX_ENABLE_E2E_HELPERS: e2eHelpersEnabled ?? "true", |
There was a problem hiding this comment.
The
?? "true" fallback on this line is unreachable. In the non-hosted branch, e2eHelpersEnabled is always process.env.VRDEX_ENABLE_E2E_HELPERS ?? "true" — a string, never null or undefined — so the nullish coalescing here never fires. Dropping it makes the intent clearer.
| VRDEX_ENABLE_E2E_HELPERS: e2eHelpersEnabled ?? "true", | |
| VRDEX_ENABLE_E2E_HELPERS: e2eHelpersEnabled, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/playwright.config.mjs
Line: 25
Comment:
The `?? "true"` fallback on this line is unreachable. In the non-hosted branch, `e2eHelpersEnabled` is always `process.env.VRDEX_ENABLE_E2E_HELPERS ?? "true"` — a string, never `null` or `undefined` — so the nullish coalescing here never fires. Dropping it makes the intent clearer.
```suggestion
VRDEX_ENABLE_E2E_HELPERS: e2eHelpersEnabled,
```
How can I resolve this? If you propose a fix, please make it concise.
What changed
@flowprofile submission journey inpnpm test:e2eby excluding only visual/snapshot suites.Why
Closes a concrete issue #100 acceptance gap: the normal local E2E command should include at least one mutation-backed journey, not only public render checks.
Testing
node --check apps/web/playwright.config.mjsgit diff --checkpnpm test:e2epnpm --filter web lintpnpm --filter web typecheckRisk
Local-only Playwright startup behavior changes. Hosted and production smoke modes still require explicit
PLAYWRIGHT_BASE_URL; hosted smoke still excludes@flow, and production E2E helpers remain blocked by the route guard.