Setup E2E testing#158
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 4 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds Playwright E2E tests and supporting Spree backend bootstrapping. Includes a guest checkout golden-path test, Docker Compose backend, bootstrap scripts that seed data and mint an API key, Playwright config and npm scripts, CI e2e job, .gitignore updates, and README testing docs. ChangesE2E Testing Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Playwright as Playwright(Test)
participant Next as NextDev(Server)
participant Spree as Spree(Backend)
participant Stripe as Stripe(Gateway)
Playwright->>Next: navigate storefront, add to cart, open checkout
Playwright->>Next: submit contact & address forms
Next->>Spree: create/confirm order, fetch shipping rates
Playwright->>Stripe: complete Payment Element (iframe)
Stripe->>Spree: payment capture/notification
Spree->>Next: order completed
Next->>Playwright: render /order-placed/ with order number
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
README.md (1)
188-188: 💤 Low valueConsider splitting this sentence for improved readability.
This 100+ word sentence packs the entire E2E infrastructure description—Playwright, Docker Compose, services,
@spree/cli, and dev dependencies—into a single clause with nested parentheticals. Breaking it into 2-3 shorter sentences would improve scannability.♻️ Suggested refactor
-End-to-end tests run through Playwright against a real Spree backend booted in Docker. The compose file at `e2e-backend/docker-compose.yml` ships Postgres + Redis + the official `ghcr.io/spree/spree:5.4.3.1` image — no `create-spree-app` setup required. Seeding and API-key creation go through the official [`@spree/cli`](https://spreecommerce.org/docs/developer/cli/quickstart) (`spree seed`, `spree sample-data`, `spree api-key create`), installed as a dev dependency. +End-to-end tests run through Playwright against a real Spree backend booted in Docker. The compose file at `e2e-backend/docker-compose.yml` ships Postgres, Redis, and the official `ghcr.io/spree/spree:5.4.3.1` image—no `create-spree-app` setup required. + +Seeding and API-key creation use the official [`@spree/cli`](https://spreecommerce.org/docs/developer/cli/quickstart) (`spree seed`, `spree sample-data`, `spree api-key create`), installed as a dev dependency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 188, Split the long single sentence describing E2E tests into 2–3 shorter sentences: one stating that end-to-end tests run with Playwright against a real Spree backend booted via Docker, one describing the compose file (e2e-backend/docker-compose.yml) and included services (Postgres, Redis, ghcr.io/spree/spree:5.4.3.1), and one explaining seeding and API-key creation via the `@spree/cli` commands (spree seed, spree sample-data, spree api-key create) and that `@spree/cli` is a dev dependency; keep the same facts and references but break at natural clause boundaries for readability.e2e/checkout.spec.ts (1)
77-102: ⚡ Quick winAdd explicit return types to helper functions.
fillAddressandsafeFillshould declare return types (Promise<void>) to match strict TS guidance.Suggested fix
-async function fillAddress(page: Page) { +async function fillAddress(page: Page): Promise<void> { ... } -async function safeFill(locator: Locator, value: string) { +async function safeFill(locator: Locator, value: string): Promise<void> { ... }As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions, use 'satisfies' for type checking object literals, and avoid 'any' (use 'unknown' instead)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/checkout.spec.ts` around lines 77 - 102, Update the function signatures to include explicit return types: change async function fillAddress(page: Page) to async function fillAddress(page: Page): Promise<void> and async function safeFill(locator: Locator, value: string) to async function safeFill(locator: Locator, value: string): Promise<void>; this ensures both helper functions (fillAddress and safeFill) explicitly declare Promise<void> as their return type to satisfy strict TypeScript checking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 55-56: The workflow uses mutable action tags for actions/checkout
and actions/setup-node and omits checkout credential hardening; update each
actions/checkout step to include persist-credentials: false and replace both
actions/checkout@v4 and actions/setup-node@v4 with their corresponding full
commit SHAs (pin to exact commit IDs) so the workflow is immutably pinned and
checkout does not persist GITHUB_TOKEN credentials to subsequent steps.
In `@e2e/checkout.spec.ts`:
- Around line 87-95: The current check only tests visibility and then blindly
calls selectOption on stateSelect, which fails if the field renders as a
disabled <select> (loading) or as a text <input>; fix by inspecting the first
locator's actual element type and disabled state before acting: call
stateSelect.first() to get the locator/element handle, verify isVisible(), then
use elementHandle.evaluate(...) or locator.evaluate to read tagName and disabled
attribute; if tagName === 'SELECT' and not disabled call selectOption({ index: 1
}) on that locator, otherwise if tagName === 'INPUT' call fill(...) (or
type(...)) with a valid value for the test; ensure you still catch errors from
isVisible() as before.
---
Nitpick comments:
In `@e2e/checkout.spec.ts`:
- Around line 77-102: Update the function signatures to include explicit return
types: change async function fillAddress(page: Page) to async function
fillAddress(page: Page): Promise<void> and async function safeFill(locator:
Locator, value: string) to async function safeFill(locator: Locator, value:
string): Promise<void>; this ensures both helper functions (fillAddress and
safeFill) explicitly declare Promise<void> as their return type to satisfy
strict TypeScript checking.
In `@README.md`:
- Line 188: Split the long single sentence describing E2E tests into 2–3 shorter
sentences: one stating that end-to-end tests run with Playwright against a real
Spree backend booted via Docker, one describing the compose file
(e2e-backend/docker-compose.yml) and included services (Postgres, Redis,
ghcr.io/spree/spree:5.4.3.1), and one explaining seeding and API-key creation
via the `@spree/cli` commands (spree seed, spree sample-data, spree api-key
create) and that `@spree/cli` is a dev dependency; keep the same facts and
references but break at natural clause boundaries for readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 226f72fe-d4dd-4b0d-8fcd-057787b995fe
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/ci.yml.gitignoreREADME.mde2e-backend/.enve2e-backend/docker-compose.ymle2e/checkout.spec.tspackage.jsonplaywright.config.tsscripts/e2e/bootstrap-spree.shscripts/e2e/dev-with-env.sh
- package-lock.json: regenerated from scratch — previous version had a legacy/v3 format mix that npm ci rejected with EUSAGE, breaking all four CI jobs on PR #158. - .github/workflows/ci.yml: set persist-credentials: false on every actions/checkout step (CodeRabbit security comment). - e2e/checkout.spec.ts: state/province field renders as one of enabled <select>, disabled <select> (loading), or text <input> in AddressFormFields. Wait out the disabled state, then dispatch on tagName (CodeRabbit flakiness comment).
CI on Linux failed loading platform-specific optional dependencies (@biomejs/cli-linux-x64, @rolldown/binding-linux-x64-gnu, @parcel/watcher-linux-x64-glibc) because the previous lockfile was regenerated on macOS-arm64 and omitted the Linux entries. Regenerate with `npm install --package-lock-only --force --os=linux --cpu=x64 --libc=glibc` so all platform variants are recorded. Local node_modules still installs only the host-relevant bindings, but `npm ci` on any platform now resolves correctly. Also exclude package-lock.json from the biome pre-commit hook so future lockfile-only commits don't fail on biome's "No files were processed" error (biome ignores lockfiles by default).
E2E test was clicking Add-to-Cart then immediately navigating to /cart, racing the cart-cookie write. Spree logs from the failed CI run confirm the line item was created server-side, but the /cart page rendered before the cookie was visible, so the Checkout link never appeared. Switch to using the cart drawer's own "Checkout" link (the drawer auto-opens after addItem succeeds, so its presence is the natural synchronization point). Scope the locator to the drawer role=dialog to avoid matching the header's "Open cart" affordance. Also pin every actions/* reference to its full commit SHA with the v-tag in a trailing comment, addressing the second half of CodeRabbit's recurring security comment on this workflow.
|
Actionable comments posted: 0 |
Two issues surfaced from the previous CI run's Playwright trace: 1. Email field has no <label>/aria-label — only `placeholder`. Switch from getByLabel(/email address/i) to getByPlaceholder. Other address fields use aria-label so they continue to work with getByLabel. 2. Country dropdown defaults to Canada (alphabetically before US), but our test data (NY, ZIP 10001, US phone) is US-specific. Select "United States" explicitly before filling the rest.
The Spree storefront checkout is a single page with auto-save, not a stepped flow. Previous trace confirmed: - No "Continue to Delivery" button exists (only "Place Order" at the bottom) - Address persists on container blur (handleContainerBlur), not on a click - Shipping methods + payment methods only appear once address is saved - State dropdown had Alabama selected (index:1) instead of New York, which may not match the cart's ZIP/city heuristics Changes: - Drop the imaginary "Continue to Delivery" click. - Click the "Shipping Method" heading after filling the form to take focus out of the address container and trigger auto-save. - Pick "New York" state by label (matches city + ZIP), fall back to index:1. - Check the policy-agreement checkbox before clicking Place Order. - Bump shipping-rate wait to 30s since the address roundtrip + rate recalculation can be slow on first load.
The vanilla Spree 5.4.3.1 image ships with no payment gateway, so the storefront's checkout shows "No payment methods available" and the test never reaches the Stripe iframe. Spree's Admin API for creating payment methods only landed in 5.5 (unreleased), so we fall back to a bin/rails runner snippet that creates a SpreeStripe::Gateway directly. - Publishable key (pk_test_TYooMQ…) stays hardcoded; it's the public documented sample. - Secret key is read from the STRIPE_SECRET_KEY env var. GitHub push protection flags any test-secret literal regardless of provenance, so the script fails fast if the env var is unset. CI consumes it as a repository secret; locally export the matching sample from https://docs.stripe.com/keys or your own test pair. - save!(validate: false) skips the Stripe API roundtrip during create (the validate_secret_key hook); the actual PaymentIntent creation during checkout still hits Stripe for real, exercising the path. - Idempotent: re-running matches on the unique "E2E Stripe" name. Once Spree 5.5 ships with the Admin API endpoint, this can be replaced with a curl POST to /admin/payment_methods.
Stripe no longer publishes a working sample secret key (the old docs sk_test_… returns 401 api_key_expired), and the docs key that does still work belongs to a different account than the previously hardcoded pk_test_TYooMQ… publishable key — a mismatched pair fails PaymentIntent confirmation. Both keys now come from the environment: in CI, STRIPE_SECRET_KEY as a repository secret and STRIPE_PUBLISHABLE_KEY as a repository variable, scoped to the bootstrap step only so npm scripts and third-party actions never see the secret. The E2E job skips itself (true "skipped" status) on fork and dependabot PRs, where repository secrets are never exposed; same-repo runs fail loudly up front when the keys are missing instead of dying mid-bootstrap. The bootstrap script now validates both keys' full shape and passes them to Rails through the container environment instead of interpolating them into the Ruby heredoc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous run surfaced two compounding issues:
1. frameLocator('iframe[name^="__privateStripeFrame"]').first() matched
the Express Checkout ("Pay with Link") widget, which renders above the
card form and shares the name prefix — the card-number fill then waited
on an iframe that never contains it. Target the Payment Element by its
stable accessible title ("Secure payment input frame") and the fields
by role/name from the accessibility snapshot instead of placeholders.
2. Playwright's default 30s per-test timeout covers the entire checkout
walk against dev-mode Next.js + dockerized Spree on a CI runner, so
each retry died mid-flow at a different step (drawer click, product
navigation) — the three attempts failing in three places was budget
exhaustion, not three bugs. Raise the test timeout to 120s.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three failure modes from the last run's traces:
1. Multiple Stripe iframes carry title="Secure payment input frame" — an
accessory frame mounts lazily before or after the real card form, so a
title-based frameLocator either hits a strict-mode violation (attempt
1) or silently fills the wrong frame, leaving the real card field
empty ("Your card number is incomplete", retry 1). Resolve the frame
that actually contains the Card number field and assert the digits
landed before paying.
2. The US card form has its own required ZIP field the test never filled
("Your ZIP is invalid") — fill it when present.
3. The cart drawer re-renders as the cart revalidates, detaching the
Checkout link mid-click indefinitely (retry 2 burned its full 120s on
one click). Navigate to the link's href instead of clicking it.
Also: the expiry field's accessible name varies across Payment Element
mounts, so use its stable MM / YY placeholder.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Quality pass over the branch (no behavior changes intended): - checkout.spec.ts: replace the MutationObserver/tag-dispatch state-field machinery with selectOption's built-in auto-wait (country is pinned to US, so the field is always a <select> with New York present); drop the safeFill helper (fill() already auto-waits); drop the home-page detour and its vacuous title assertion; drop the waitForURL that goto() already guarantees; hoist the frame count out of the scan loop. - bootstrap-spree.sh: replace the hand-rolled curl polling loop with curl --retry; fix the stale idempotency comment; document why the gateway saves with validate: false. - docker-compose.yml: inline the single-consumer YAML anchors; stop publishing Postgres/Redis host ports nothing uses (also removes the LAN-exposed trust-auth surface); fix the stale usage comment. - ci.yml: key the Playwright browser cache on the Playwright version instead of the whole lockfile so unrelated dependency bumps don't evict it; merge the two failure-artifact uploads into one. - dev-with-env.sh: exec npm run dev instead of re-spelling the dev command, so the port/flags live only in package.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores