Repository navigation
test(recommendations): Playwright E2E smoke suite for the recommendations flow (closes #167) - #394
Conversation
cristim
added
priority/p2
📝 WalkthroughWalkthroughThis PR adds frontend Playwright end-to-end infrastructure, deterministic Recommendations API fixtures, CI execution, and smoke tests covering filtering, loading states, purchase execution, and plan creation. ChangesRecommendations E2E Test Infrastructure
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Recommendations E2E test
participant Browser as Browser application
participant Fixture as mockApi(page)
participant CI as GitHub Actions
CI->>Browser: Serve built frontend
Test->>Browser: Open Opportunities
Browser->>Fixture: Request recommendations and account data
Fixture->>Browser: Return seeded mock responses
Test->>Browser: Apply filters or select recommendation
Browser->>Fixture: Submit purchase or plan request
Fixture->>Browser: Return deterministic response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Rebase attempt against current feat/multicloud-web-frontend: only a frontend/package-lock.json conflict (package.json auto-merged cleanly). Needs a manual |
88b9f83 to
d0b3020
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
Rate Limit Exceeded
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
Action performedReview triggered.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
3ac42fc to
48e5084
Compare
Phase-1 investigation update (2026-07-21)The PR body understates what has already landed on the branch. Recap: What the branch actually contains (6 commits, head
So the spec was written, the CI job was wired, and the follow-up items the original body listed are done. Current CI status on the head commit (all green): Playwright Chromium (Frontend E2E PR), Frontend build (PR), Lint Code, Unit Tests, Integration Tests, E2E Tests, Build Docker Image, Security Scanning, Snyk, Trivy, gosec, all three Terraform validate jobs, both parity jobs, CI Success, pre-commit, both sanity dry runs, and CodeRabbit final review state = SUCCESS. CodeRabbit history: four full review cycles between 2026-06-05 and 2026-07-09, all findings addressed with code (route-precedence, dep-version bumps, lockfile sync, comment-count corrections). Rebase footprint vs Verdict: Proceed as-is. The scaffolding is not redundant (no other Playwright infra exists on Note for reviewers: the PR title still says "scaffolding for E2E smoke (WIP for #167)" but the branch now covers everything issue #167 asks for that is testable outside a live browser session. Recommend the title be updated to |
48e5084 to
4336b51
Compare
|
@coderabbitai review Rebased onto latest |
|
✅ Action performedReview finished.
|
Post-rebase status + recommended next stepsRebased. The branch has been rebased onto
CodeRabbit re-review has been triggered on the rebased head; a
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/tests-e2e/recommendations.spec.ts (1)
42-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTight timing margin between injected delay and assertions may flake in CI.
delayNextRecommendationsFetch(3_000)(line 46) delays the fetch by 3s, and the subsequent assertions (lines 49, 52) rely on the default Playwright expect timeout to catch up. Under CI load, the combined delay + render/network round-trip could exceed the default 5s expect timeout with little margin, causing intermittent flakes.Consider passing an explicit longer timeout to these polling assertions (e.g.
{ timeout: 8_000 }) to decouple the test from the injected delay value.🤖 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 `@frontend/tests-e2e/recommendations.spec.ts` around lines 42 - 67, Increase the timeout for the loading-state assertions in the test “shows loading and zero-result filter states without losing filter controls” to provide margin beyond the 3-second delay and CI rendering/network latency. Apply an explicit timeout such as 8,000 ms to the relevant polling assertions, including the skeleton-row count and recommendations filter live-region checks, without changing the test flow.
🤖 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/frontend-e2e.yml:
- Around line 30-31: Update the Checkout step using actions/checkout to set
persist-credentials to false, preventing the GitHub token from being retained
for subsequent npm and build steps.
---
Nitpick comments:
In `@frontend/tests-e2e/recommendations.spec.ts`:
- Around line 42-67: Increase the timeout for the loading-state assertions in
the test “shows loading and zero-result filter states without losing filter
controls” to provide margin beyond the 3-second delay and CI rendering/network
latency. Apply an explicit timeout such as 8,000 ms to the relevant polling
assertions, including the skeleton-row count and recommendations filter
live-region checks, without changing the test flow.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32945be0-9e4b-4e90-889d-e16f6e7f349d
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/workflows/frontend-e2e.yml.gitignorefrontend/package.jsonfrontend/playwright.config.tsfrontend/tests-e2e/fixtures/recs.tsfrontend/tests-e2e/recommendations.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- .gitignore
- frontend/playwright.config.ts
- frontend/package.json
- frontend/tests-e2e/fixtures/recs.ts
|
|
@coderabbitai review Pushed |
|
✅ Action performedReview finished.
|
|
@coderabbitai full review Rate-limit reset window is now open. This is a Fair-Usage recovery: force a full re-scan of |
|
Your plan includes PR reviews subject to rate limits. Reviews are available now. |
Sets up the Playwright infrastructure for the #160 smoke suite without yet writing the actual smoke spec. The spec itself is the remaining work and is tracked in the draft-PR description. What this lays down: - frontend/package.json: @playwright/test 1.50.x + serve 14.x as devDependencies; `test:e2e` + `test:e2e:install` npm scripts. - frontend/playwright.config.ts: Chromium-only, hermetic (boots `npx serve` against the production webpack bundle in dist/), `page.route`-based API mocking, retain-on-failure trace/screenshot/ video, single worker on CI for predictable artefacts. - frontend/tests-e2e/fixtures/recs.ts (301 lines): canned API fixtures covering 3 providers x 4 services x 2 terms with savings spanning [10, 50, 100, 150, 250, 1200] so numeric filter expressions narrow to known row counts. `mockApi(page)` wires `/api/recommendations`, `/api/recommendations/freshness`, `/api/recommendations/refresh`, `/api/accounts`, `/api/session`, `/api/dashboard`, `/api/plans`, `/api/purchases/execute` and returns a `{ calls }` recorder for per-test assertions. - frontend/jest config: excludes tests-e2e/ from jest's pickup so the unit-test suite stays fast. - .gitignore: ignores playwright-report/, test-results/, tests-e2e/.cache/ — generated per run, not committed. Remaining work (kept as a WIP commit because the predecessor agent hit the stream-idle watchdog before reaching this step): - Write tests-e2e/recommendations.spec.ts that exercises the #160 flow end-to-end: load page, apply a column filter, verify row count narrows correctly, select rows, verify sticky bottom action box surfaces the right summary + dispatches the correct POST. - (Optionally) wire test:e2e into a CI workflow alongside the test:unit job.
Playwright executes page.route handlers in reverse-registration order (last registered = first executed). The catch-all '**/api/**' was registered last, causing it to intercept all requests before the specific handlers could run. Move it to be registered first so specific routes (public-info, auth/me, recommendations, etc.) correctly take precedence. Also correct the RECS fixture comment cardinalities that were out of sync with actual data: - "50..200" (inclusive range per parseNumericFilter): 13 rows, not 9 - "5, 50..200, >1000": 14 rows, not 11 (literal "5" matches no rows; ">1000" adds r04 with savings=1200 to the 13-row set) Bump minimum version bounds for devDependencies to current latest: - @playwright/test: ^1.50.0 -> ^1.60.0 - serve: ^14.2.0 -> ^14.2.6
The previous commit bumped @playwright/test to ^1.60.0 and serve to ^14.2.6 in package.json, but package-lock.json's root package entry still recorded the old ^1.50.0 / ^14.2.0 ranges, which makes `npm ci` fail its package.json/lockfile sync check. The resolved versions were already 1.60.0 / 14.2.6, so this is a metadata-only sync via `npm install --package-lock-only`. Verified: npm ci, npx tsc --noEmit (incl. playwright.config.ts and tests-e2e/fixtures/recs.ts), npm test (77 suites / 2553 tests pass, jest still excludes tests-e2e/), go build ./... after rebasing onto origin/main.
…tions wildcard **/api/recommendations/freshness and .../refresh were registered before **/api/recommendations* in mockApi(). Playwright executes handlers in reverse-registration order, so the wildcard (registered later) was winning and returning the full recs list instead of the freshness/refresh payloads. Move both specific handlers to after the wildcard so they take precedence as intended.
Exercise filtering, loading, selection, purchase, and plan flows through the production frontend bundle. Run the Chromium smoke suite for frontend pull requests and retain browser artifacts when it fails.
Regenerate the frontend lockfile against the dependency versions pinned on main so npm ci can install the rebased Playwright test dependencies.
The Playwright job runs `npm ci`, `npm run build`, and `npx playwright install` — none of these need the actions/checkout token. Setting `persist-credentials: false` scrubs the credential from `.git/config` so the token is not accessible to third-party install scripts that run in the same job. Addresses the CodeRabbit / zizmor `artipacked` finding.
e17114a to
ab4eb36
Compare
|
@coderabbitai full review Rebased onto latest
Please full-review this head. The PR's own contribution (Playwright infra + spec + |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 8 minutes. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
cristim
changed the title
* test(recommendations): scaffolding for E2E smoke (WIP for #167) Sets up the Playwright infrastructure for the #160 smoke suite without yet writing the actual smoke spec. The spec itself is the remaining work and is tracked in the draft-PR description. What this lays down: - frontend/package.json: @playwright/test 1.50.x + serve 14.x as devDependencies; `test:e2e` + `test:e2e:install` npm scripts. - frontend/playwright.config.ts: Chromium-only, hermetic (boots `npx serve` against the production webpack bundle in dist/), `page.route`-based API mocking, retain-on-failure trace/screenshot/ video, single worker on CI for predictable artefacts. - frontend/tests-e2e/fixtures/recs.ts (301 lines): canned API fixtures covering 3 providers x 4 services x 2 terms with savings spanning [10, 50, 100, 150, 250, 1200] so numeric filter expressions narrow to known row counts. `mockApi(page)` wires `/api/recommendations`, `/api/recommendations/freshness`, `/api/recommendations/refresh`, `/api/accounts`, `/api/session`, `/api/dashboard`, `/api/plans`, `/api/purchases/execute` and returns a `{ calls }` recorder for per-test assertions. - frontend/jest config: excludes tests-e2e/ from jest's pickup so the unit-test suite stays fast. - .gitignore: ignores playwright-report/, test-results/, tests-e2e/.cache/ — generated per run, not committed. Remaining work (kept as a WIP commit because the predecessor agent hit the stream-idle watchdog before reaching this step): - Write tests-e2e/recommendations.spec.ts that exercises the #160 flow end-to-end: load page, apply a column filter, verify row count narrows correctly, select rows, verify sticky bottom action box surfaces the right summary + dispatches the correct POST. - (Optionally) wire test:e2e into a CI workflow alongside the test:unit job. * test(e2e/recs): fix catch-all route precedence + update fixture counts Playwright executes page.route handlers in reverse-registration order (last registered = first executed). The catch-all '**/api/**' was registered last, causing it to intercept all requests before the specific handlers could run. Move it to be registered first so specific routes (public-info, auth/me, recommendations, etc.) correctly take precedence. Also correct the RECS fixture comment cardinalities that were out of sync with actual data: - "50..200" (inclusive range per parseNumericFilter): 13 rows, not 9 - "5, 50..200, >1000": 14 rows, not 11 (literal "5" matches no rows; ">1000" adds r04 with savings=1200 to the 13-row set) Bump minimum version bounds for devDependencies to current latest: - @playwright/test: ^1.50.0 -> ^1.60.0 - serve: ^14.2.0 -> ^14.2.6 * test(e2e/recs): sync package-lock root ranges after dep bumps The previous commit bumped @playwright/test to ^1.60.0 and serve to ^14.2.6 in package.json, but package-lock.json's root package entry still recorded the old ^1.50.0 / ^14.2.0 ranges, which makes `npm ci` fail its package.json/lockfile sync check. The resolved versions were already 1.60.0 / 14.2.6, so this is a metadata-only sync via `npm install --package-lock-only`. Verified: npm ci, npx tsc --noEmit (incl. playwright.config.ts and tests-e2e/fixtures/recs.ts), npm test (77 suites / 2553 tests pass, jest still excludes tests-e2e/), go build ./... after rebasing onto origin/main. * fix(e2e/recs): register freshness and refresh routes after recommendations wildcard **/api/recommendations/freshness and .../refresh were registered before **/api/recommendations* in mockApi(). Playwright executes handlers in reverse-registration order, so the wildcard (registered later) was winning and returning the full recs list instead of the freshness/refresh payloads. Move both specific handlers to after the wildcard so they take precedence as intended. * test(recommendations): add Playwright smoke coverage Exercise filtering, loading, selection, purchase, and plan flows through the production frontend bundle. Run the Chromium smoke suite for frontend pull requests and retain browser artifacts when it fails. * fix(e2e): sync lockfile after rebase Regenerate the frontend lockfile against the dependency versions pinned on main so npm ci can install the rebased Playwright test dependencies. * ci(e2e): set persist-credentials false on frontend-e2e checkout The Playwright job runs `npm ci`, `npm run build`, and `npx playwright install` — none of these need the actions/checkout token. Setting `persist-credentials: false` scrubs the credential from `.git/config` so the token is not accessible to third-party install scripts that run in the same job. Addresses the CodeRabbit / zizmor `artipacked` finding.
1 participant
Full Playwright E2E smoke suite for the Recommendations flow (closes #167)
Lands a hermetic, Chromium-only Playwright end-to-end smoke suite for the frontend Recommendations page, plus the CI job that runs it. This started as WIP scaffolding; the spec and CI wiring are now complete, so the PR is no longer a draft.
What this PR adds
frontend/tests-e2e/recommendations.spec.ts(136 lines)frontend/tests-e2e/fixtures/recs.ts(337 lines)[10, 50, 100, 150, 250, 1200], so filter expressions like>100/50..200narrow to known row countsfrontend/playwright.config.tsnpx serveagainst the builtdist/; network-layer API mocking viapage.route; CI-friendly retention.github/workflows/frontend-e2e.ymlfrontend/; Node 24,npm ci+ build +playwright install chromium+npm run test:e2e; uploads the Playwright report as an artifact on failure (SHA-pinned actions)frontend/package.json+frontend/package-lock.json@playwright/test+servedeps;test:e2e/test:e2e:installscriptsfrontendjest configtests-e2e/from jest's pickup so unit runs are unaffected.gitignoreplaywright-report/,test-results/,tests-e2e/.cache/What the suite covers (4 tests in
recommendations.spec.ts)1 selected,across 1 cell), opens the Configure Purchase dialog, submits, and asserts the recordedPOST /api/purchases/executerequest body shape.Design choices
serveand mocks the API at the network layer (page.route) with per-test fixtures - no backend required, fully deterministic.Frontend build (PR)job; runs only whenfrontend/changes.Status: complete (no longer WIP). Currently MERGEABLE/CLEAN.
Summary by CodeRabbit