Add Shopify checkout Playwright integration test#461
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
WalkthroughThe changes introduce a multi-project Playwright configuration separating frontend and Shopify integration test execution, add a comprehensive Shopify checkout test spec with live and mock operation modes, and expand the release checklist with explicit Shopify integration verification steps and GitHub Actions secrets documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as shopify-checkout.spec.ts
participant Config as playwright.config.ts
participant Storefront as Shopify Storefront
participant Helper as Backend Helper
participant Supabase as Supabase Admin
Test->>Test: Determine mode (live/mock)
alt Live Mode
Test->>Storefront: Construct client with tokens
Test->>Test: Create checkout payload
Test->>Storefront: Send test checkout
Storefront->>Helper: Forward checkout request
Helper-->>Storefront: Process & return
Storefront-->>Test: Checkout response
Test->>Supabase: Upsert module config
Test->>Supabase: Poll order record (retry)
Supabase-->>Test: Ingestion complete
Test->>Test: Assert identifiers & status
else Mock Mode
Test->>Test: Initialize in-memory storefront
Test->>Test: Create mock Supabase client
Test->>Test: Execute checkout locally
Test->>Test: Simulate ingestion
Test->>Test: Assert identifiers & status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes span heterogeneous modifications across configuration, test infrastructure, and documentation. The playwright.config.ts restructuring involves conditional logic, dynamic project configuration, NODE_PATH manipulation, and helper functions requiring careful validation. The shopify-checkout.spec.ts introduces a substantial new test suite with dual operation modes (live/mock), external API integration, polling logic with retry/timeout semantics, mock implementations, and interface definitions—each requiring separate reasoning. The documentation update is straightforward but contextualizes the overall scope. Combined complexity and file diversity warrant mid-to-high review intensity. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while (Date.now() < deadline) { | ||
| const response = await fetch(`${supabaseUrl}/rest/v1/${DEFAULT_ORDER_TABLE}?order_id=eq.${encodeURIComponent(orderId)}`, { | ||
| method: 'GET', |
There was a problem hiding this comment.
The Supabase polling loop calls GET ${supabaseUrl}/rest/v1/${DEFAULT_ORDER_TABLE}?order_id=eq.${…} but omits a select query parameter. PostgREST requires select on GET requests; without it the endpoint responds with HTTP 406 and this path always throws before reading any order rows. As a result the integration test will fail even when the order has been ingested. Add ?select=* (or the specific columns) to the request so the poller can return the row when it exists.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/release-checklist.md (1)
36-39: Gate must also include the newshopify-integrationjob.Update this bullet to explicitly require
shopify-integrationto pass (in addition toperformance-budgetandobservability-budgets) to keep the gate consistent with Section 1.- - All CI jobs, including `performance-budget` and `observability-budgets`, pass. + - All CI jobs pass, including `performance-budget`, `observability-budgets`, and `shopify-integration`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/release-checklist.md(1 hunks)playwright.config.ts(3 hunks)tests/shopify-checkout.spec.ts(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
tests/shopify-checkout.spec.ts
[error] 85-85: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
| - Confirm the **shopify-integration** Playwright job passes. It executes `tests/shopify-checkout.spec.ts`, which provisions Shopify configuration, drives a Storefront test checkout, and waits for the backend to ingest the order. Configure the following GitHub Actions secrets so the run can talk to Shopify and Supabase: | ||
| - `SHOPIFY_STORE_DOMAIN` | ||
| - `SHOPIFY_STOREFRONT_ACCESS_TOKEN` | ||
| - `SHOPIFY_STOREFRONT_HELPER_URL` | ||
| - `SHOPIFY_STOREFRONT_HELPER_TOKEN` (if the helper requires bearer auth) | ||
| - `SHOPIFY_TEST_VARIANT_ID` | ||
| - `SHOPIFY_TEST_CURRENCY` (defaults to `USD` if omitted) | ||
| - `SUPABASE_URL` | ||
| - `SUPABASE_SERVICE_ROLE_KEY` | ||
| - Optional: `SHOPIFY_MODULE_CONFIG_TABLE` and `SHOPIFY_ORDER_TABLE` when the Supabase schema uses custom table names. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Harden secrets handling for CI (service-role key).
Document safeguards for SUPABASE_SERVICE_ROLE_KEY and other tokens:
- Use GitHub Environments with required reviewers and secret scoping to
shopify-integrationonly. - Ensure logs/artifacts never echo secrets; mask them and avoid attaching request/response bodies that may include auth headers.
- Prefer short‑lived tokens if possible; rotate on a schedule and after test failures/incidents.
Adding these points here prevents accidental leakage during integration runs.
🤖 Prompt for AI Agents
In docs/release-checklist.md around lines 8–17, the checklist currently lists
GitHub Actions secrets for the shopify-integration job but lacks CI
secret-handling safeguards; update this section to document concrete protections
for SUPABASE_SERVICE_ROLE_KEY and other tokens by: add bullets instructing use
of GitHub Environments scoped to the shopify-integration workflow with required
reviewers, restrict secret access to the job only, ensure masking of secrets and
avoid logging or artifacting request/response bodies that may contain auth
headers, recommend short‑lived tokens plus scheduled rotation and rotation after
failures/incidents, and optionally call out using separate least-privilege
service role keys for CI versus production.
| @@ -1,7 +1,9 @@ | |||
| import path from 'path'; | |||
| import { Module } from 'module'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make NODE_PATH handling cross‑platform and fix private API typing.
- Use
path.delimiterinstead of hardcoded:for Windows support. Module._initPaths()is private and not typed; cast toanyor avoid it.- Prefer
node:specifier for core module import.
Apply this diff:
-import { Module } from 'module';
+import { Module } from 'node:module';
@@
-const nodePathEntries = [path.resolve(__dirname, 'frontend/node_modules'), process.env.NODE_PATH].filter(Boolean) as string[];
+const nodePathEntries = [path.resolve(__dirname, 'frontend/node_modules'), process.env.NODE_PATH]
+ .filter(Boolean) as string[];
if (nodePathEntries.length > 0) {
- process.env.NODE_PATH = nodePathEntries.join(':');
- Module._initPaths();
+ process.env.NODE_PATH = nodePathEntries.join(path.delimiter);
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ (Module as any)._initPaths?.();
}Alternatively, avoid NODE_PATH entirely by using TS path aliases + bundler resolution.
Also applies to: 34-38
🤖 Prompt for AI Agents
In playwright.config.ts around lines 2 and 34-38, the code imports the private
Module API and assumes POSIX NODE_PATH separator; change the import to use the
core specifier (import Module from 'node:module' or import * as Module from
'node:module') and replace hardcoded ':' with path.delimiter from the path
module to make NODE_PATH handling cross‑platform; avoid directly calling/typing
Module._initPaths() by casting Module to any when invoking it or refactor to not
call it at all (e.g., rebuild NODE_PATH into process.env.NODE_PATH or rely on TS
path aliases + bundler), and ensure any modifications to NODE_PATH
append/prepend using path.delimiter consistently for Windows support.
| use: { | ||
| ...sharedUseOptions, | ||
| baseURL: process.env.BASE_URL || 'http://localhost:3000', | ||
| env: frontendEnv, | ||
| }, |
There was a problem hiding this comment.
use.env is not a supported Playwright option (TS/type error and no effect).
This will fail type‑checking under PlaywrightTestConfig and won’t set process.env for tests. Tests already read process.env.* directly, and webServer.env correctly seeds Next’s dev server.
- Remove
envunderusefor both projects. - Keep env only where supported (
webServer.env). - Also avoid
actionTimeout: 0; prefer a bounded value to prevent hangs.
Apply this diff:
@@
- use: {
- ...sharedUseOptions,
- baseURL: process.env.BASE_URL || 'http://localhost:3000',
- env: frontendEnv,
- },
+ use: {
+ ...sharedUseOptions,
+ baseURL: process.env.BASE_URL || 'http://localhost:3000',
+ },
@@
- use: {
- ...sharedUseOptions,
- actionTimeout: 0,
- baseURL: process.env.BACKEND_BASE_URL || 'http://localhost:8787',
- env: {
- SHOPIFY_STORE_DOMAIN: process.env.SHOPIFY_STORE_DOMAIN || '',
- SHOPIFY_STOREFRONT_ACCESS_TOKEN: process.env.SHOPIFY_STOREFRONT_ACCESS_TOKEN || '',
- SHOPIFY_STOREFRONT_HELPER_URL: process.env.SHOPIFY_STOREFRONT_HELPER_URL || '',
- SHOPIFY_STOREFRONT_HELPER_TOKEN: process.env.SHOPIFY_STOREFRONT_HELPER_TOKEN || '',
- SHOPIFY_TEST_VARIANT_ID: process.env.SHOPIFY_TEST_VARIANT_ID || '',
- SHOPIFY_TEST_CURRENCY: process.env.SHOPIFY_TEST_CURRENCY || 'USD',
- SHOPIFY_MODULE_CONFIG_TABLE: process.env.SHOPIFY_MODULE_CONFIG_TABLE || 'shopify_module_configurations',
- SHOPIFY_ORDER_TABLE: process.env.SHOPIFY_ORDER_TABLE || 'shopify_orders',
- SUPABASE_URL: process.env.SUPABASE_URL || '',
- SUPABASE_SERVICE_ROLE_KEY: process.env.SUPABASE_SERVICE_ROLE_KEY || '',
- NODE_PATH: [path.resolve(__dirname, 'frontend/node_modules'), process.env.NODE_PATH]
- .filter(Boolean)
- .join(':'),
- },
- },
+ use: {
+ ...sharedUseOptions,
+ actionTimeout: process.env.CI ? 30_000 : 45_000,
+ baseURL: process.env.BACKEND_BASE_URL || 'http://localhost:8787',
+ },Also applies to: 67-86
🤖 Prompt for AI Agents
In playwright.config.ts around lines 55–59 (and similarly lines 67–86), the
configuration incorrectly sets use.env (unsupported by Playwright types and has
no effect) and sets actionTimeout: 0; remove the env property from each use
block so you only seed environment variables via webServer.env, and replace
actionTimeout: 0 with a reasonable bounded timeout (e.g., a nonzero millisecond
value) to avoid potential test hangs.
| memoryState: async ({}, use) => { | ||
| const state: InMemoryState = { orders: new Map() }; | ||
| await use(state); | ||
| }, | ||
| moduleConfig: async ({ memoryState }, use) => { |
There was a problem hiding this comment.
Fix Biome error: avoid empty object pattern in fixture param.
Replace the empty destructuring with an unused identifier.
- memoryState: async ({}, use) => {
+ memoryState: async (_ctx, use) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| memoryState: async ({}, use) => { | |
| const state: InMemoryState = { orders: new Map() }; | |
| await use(state); | |
| }, | |
| moduleConfig: async ({ memoryState }, use) => { | |
| memoryState: async (_ctx, use) => { | |
| const state: InMemoryState = { orders: new Map() }; | |
| await use(state); | |
| }, | |
| moduleConfig: async ({ memoryState }, use) => { |
🧰 Tools
🪛 Biome (2.1.2)
[error] 85-85: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🤖 Prompt for AI Agents
In tests/shopify-checkout.spec.ts around lines 85 to 89, the fixture uses an
empty object destructuring in the parameter list (async ({}, use) => ...) which
triggers a Biome lint/error; replace the empty destructuring with an unused
identifier (for example async (_context, use) => or async (_ , use) =>) so the
signature is valid and the fixture behavior remains unchanged.
| const client: ShopifyStorefrontClient = { | ||
| async performTestCheckout(payload) { | ||
| const requestBody = { | ||
| email: payload.email, | ||
| lineItems: payload.lineItems, | ||
| shippingAddress: payload.shippingAddress, | ||
| currencyCode: moduleConfig.currencyCode, | ||
| note: payload.note ?? `Playwright checkout for ${testInfo.title}`, | ||
| }; | ||
|
|
||
| const response = await fetch(new URL('/test-checkout', storefrontHelperUrl).toString(), { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'X-Shopify-Storefront-Access-Token': storefrontAccessToken, | ||
| ...(helperAuthToken ? { Authorization: `Bearer ${helperAuthToken}` } : {}), | ||
| }, | ||
| body: JSON.stringify(requestBody), | ||
| }); |
There was a problem hiding this comment.
Add request timeouts to external calls (prevent hung CI) — prefer Playwright request or a small timeout helper.
Current fetch calls have no timeout; network stalls can hang the test despite project timeouts.
Option A (preferred): use APIRequestContext (test.request)
- Inject
requestinto fixtures and replacefetchwithrequest.post/getincludingtimeout.
Option B: add fetchWithTimeout and use it here.
Diff (Option B – minimal change):
@@
- const response = await fetch(new URL('/test-checkout', storefrontHelperUrl).toString(), {
+ const response = await fetchWithTimeout(new URL('/test-checkout', storefrontHelperUrl).toString(), {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-Shopify-Storefront-Access-Token': storefrontAccessToken,
...(helperAuthToken ? { Authorization: `Bearer ${helperAuthToken}` } : {}),
},
body: JSON.stringify(requestBody),
+ timeoutMs: 60_000,
});
@@
- const response = await fetch(`${supabaseUrl}/rest/v1/${DEFAULT_MODULE_CONFIG_TABLE}`, {
+ const response = await fetchWithTimeout(`${supabaseUrl}/rest/v1/${DEFAULT_MODULE_CONFIG_TABLE}`, {
method: 'POST',
headers: {
...headers,
Prefer: 'resolution=merge-duplicates',
},
body: JSON.stringify(payload),
+ timeoutMs: 30_000,
});
@@
- const response = await fetch(`${supabaseUrl}/rest/v1/${DEFAULT_ORDER_TABLE}?order_id=eq.${encodeURIComponent(orderId)}`, {
+ const response = await fetchWithTimeout(`${supabaseUrl}/rest/v1/${DEFAULT_ORDER_TABLE}?order_id=eq.${encodeURIComponent(orderId)}`, {
method: 'GET',
- headers,
+ headers,
+ timeoutMs: 30_000,
});Add this helper near delay:
+async function fetchWithTimeout(input: RequestInfo, init: (RequestInit & { timeoutMs?: number }) = {}): Promise<Response> {
+ const { timeoutMs = 60_000, ...rest } = init;
+ const ac = new AbortController();
+ const id = setTimeout(() => ac.abort(), timeoutMs);
+ try {
+ return await fetch(input, { ...rest, signal: ac.signal });
+ } finally {
+ clearTimeout(id);
+ }
+}If you’d like Option A, I can provide a patch wiring request into the fixtures.
Also applies to: 196-204, 217-235
🤖 Prompt for AI Agents
In tests/shopify-checkout.spec.ts around lines 121 to 139, the test's external
POST using fetch has no timeout and can hang CI; replace this fetch with a
Playwright APIRequestContext call (test.request.post) configured with a timeout,
or if you choose the minimal change implement and use a fetchWithTimeout helper
that wraps fetch with an abort controller and a short default timeout, then call
that helper here with an appropriate timeout value; ensure headers/body remain
the same and propagate any thrown errors so the test fails fast.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @shayancoin. * #461 (comment) The following files were modified: * `playwright.config.ts`
…463) Docstrings generation was requested by @shayancoin. * #461 (comment) The following files were modified: * `playwright.config.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f66b85ea208330869c8b3cdf5d2582
Summary by CodeRabbit
Documentation
Tests