test(e2e): expand the Playwright suite to cover the primary browser flows - #2
Merged
Conversation
…lows New specs against the canned Playwright backend (real routers, in-memory store, deterministic canned pricing): - portfolios: create, add trade, price (canned PV), valuations list cross-check, remove trade, delete with confirmation - trades: create/update/delete in place, and the 409 delete guard for a product still referenced by a trade - product_builder: every builder template loads, debug dump renders, save and load-back round-trip - models: BS create/update/delete, Dupire wrong-shape 422 banner, model delete guard - dashboard: entity count cards, recent-run status badges (failed runs route-mocked on top of the real list response), valuation details expand 11 -> 21 e2e tests. The CI e2e job now uploads the Playwright report and traces as an artifact on failure. README describes the suite's scope.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 36 |
| Duplication | 19 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Biome's Qwik-specific useQwikValidLexicalScope rule fires on the new e2e specs; this is a React project, so the rule is a false positive. Exclude frontend/tests from the Biome engine, mirroring the Bandit exclusion for backend tests.
There was a problem hiding this comment.
Pull request overview
Expands the frontend Playwright E2E suite to cover additional primary browser workflows (dashboard, products/product builder, models, trades, portfolios) against the canned Playwright test backend, and improves CI debuggability by uploading Playwright reports/traces when failures occur.
Changes:
- Added new Playwright E2E specs covering CRUD flows, valuation lifecycles, delete-guard behavior, and dashboard/valuations UI assertions.
- Documented the expanded E2E coverage in the README.
- Updated CI to upload Playwright HTML report + trace artifacts on E2E failures.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates test documentation to describe expanded E2E coverage and CI artifact behavior. |
| frontend/tests/e2e/trades.spec.ts | Adds trade lifecycle coverage (create/update via API PUT/delete) and product delete-guard assertions. |
| frontend/tests/e2e/product_builder.spec.ts | Adds coverage for loading templates, debug dump rendering, and save/load round-trip behavior. |
| frontend/tests/e2e/portfolios.spec.ts | Adds end-to-end portfolio lifecycle coverage including pricing and valuation list verification. |
| frontend/tests/e2e/models.spec.ts | Adds model CRUD coverage, Dupire validation error coverage, and model delete-guard coverage. |
| frontend/tests/e2e/dashboard.spec.ts | Adds dashboard and valuations-page coverage including synthetic failed-run badge/details verification. |
| .github/workflows/ci.yml | Uploads Playwright report and trace artifacts on CI E2E failures (7-day retention). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+38
to
+39
| // Seeds product + model + trade via the API, prices the trade, and polls until | ||
| // the canned backend settles the run. Returns the valuation id. |
Owner
Author
There was a problem hiding this comment.
Fixed in 71d9df1: seedCompletedTradeValuation now returns pending.id (typed Promise<string>), so the comment is accurate and future callers can use the id. Verified locally — the dashboard spec passes (2 passed).
The comment claimed the helper returns the valuation id but the function returned nothing. Return pending.id (typed Promise<string>) so the comment is true and future callers can use it. Addresses Copilot review on PR #2.
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
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
The existing CI e2e job ran 11 browser tests concentrated on valuation and Curve Lab. This PR grows the suite to 21 tests covering the pages that previously had no browser-level coverage, all running against the canned Playwright backend (real FastAPI routers, in-memory store, deterministic canned pricing at unit PV 8.0):
CI integration
The existing
Browser smoke (Playwright)job runs the whole suite vianpm run test:e2e(no workflow restructuring needed). Added one step: on failure the job uploadsfrontend/playwright-reportandfrontend/test-results(traces) as a 7-day artifact, so e2e failures are debuggable without rerunning locally.Verification
DAL_PLAYWRIGHT_TEST_BACKEND=1 npm run test:e2e: 21 passed, three consecutive local runs (2.5 min each).npm run buildgreen;check_consistency.pygreen.One known residual flake risk is documented in the suite:
valuation.spec.tsasserts on the newest/valuationsrow, so run-producing specs gate on its UI-created trade to keep ordering deterministic; CI'sretries: 1cushions the rest.