-
Notifications
You must be signed in to change notification settings - Fork 0
test(e2e): Playwright smoke gate — boot/auth + full escrow money path (A–F) #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
86b42c4
0abe43b
548f5c6
7b46402
38def32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| name: e2e-smoke | ||
| on: | ||
| pull_request: | ||
| workflow_dispatch: | ||
| jobs: | ||
| smoke: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: pnpm/action-setup@v4 | ||
| with: { version: 10 } | ||
| - uses: actions/setup-node@v4 | ||
| with: { node-version: 20, cache: pnpm } | ||
| - run: pnpm install --frozen-lockfile | ||
| - run: pnpm exec playwright install --with-deps chromium | ||
| - run: pnpm test:e2e | ||
| env: | ||
| E2E_BASE_URL: ${{secrets.E2E_BASE_URL}} | ||
| VERCEL_AUTOMATION_BYPASS_SECRET: ${{secrets.VERCEL_AUTOMATION_BYPASS_SECRET}} | ||
| - uses: actions/upload-artifact@v4 | ||
| if: success() || failure() | ||
| with: | ||
| name: playwright-report | ||
| path: playwright-report/ | ||
| retention-days: 7 | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,3 +49,6 @@ scratch/ | |
| !tsconfig*.json | ||
| !jsconfig.json | ||
|
|
||
|
|
||
| test-results/ | ||
| playwright-report/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { test as base, expect } from "@playwright/test" | ||
|
|
||
| // Extends the base test so every test collects console + page errors. | ||
| // Assert `expect(consoleErrors).toEqual([])` at the end of each test. | ||
| export const test = base.extend<{ consoleErrors: string[] }>({ | ||
| consoleErrors: async ({ page }, use) => { | ||
| const errors: string[] = [] | ||
| page.on("console", (msg) => { | ||
| if (msg.type() === "error") { | ||
| console.error("[Browser Error]", msg.text()) | ||
| errors.push(msg.text()) | ||
| } | ||
| }) | ||
| page.on("pageerror", (err) => errors.push(err.message)) | ||
| // eslint-disable-next-line react-hooks/rules-of-hooks -- Playwright fixture 'use', not a React Hook | ||
| await use(errors) | ||
| }, | ||
| }) | ||
|
|
||
| export { expect } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| import { Page, expect } from "@playwright/test" | ||
|
|
||
| export const DEMO = { | ||
| client: "+919876500001", | ||
| worker: "+919876500011", | ||
| admin: "+919876500099", | ||
| } as const | ||
|
|
||
| export const DEMO_OTP = "123456" | ||
|
|
||
| // Phone-OTP login (DEMO_MODE bypass code). TODO: align test ids/urls. | ||
| export async function login(page: Page, phone: string) { | ||
| await page.goto("/login") | ||
|
|
||
| await page.getByTestId("phone-input").fill(phone.replace(/^\+91/, "")) | ||
| await page.getByTestId("send-otp").click() | ||
|
|
||
| await expect(page).toHaveURL(/\/login\/verify/) | ||
|
|
||
| await page.getByTestId("otp-input").pressSequentially(DEMO_OTP, { delay: 50 }) | ||
|
Comment on lines
+15
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: These selectors don't exist in the current login UI ( Severity Level: Critical 🚨- ❌ All auth-dependent Playwright tests fail before OTP entry.
- ❌ Smoke gate cannot validate boot/auth paths at all.
- ⚠️ CI signal for login regressions becomes unusable.Steps of Reproduction ✅1. Run the Playwright suite (e.g. `pnpm playwright test e2e/smoke.spec.ts`), which uses
the shared `login()` helper from `e2e/helpers.ts:12-22`.
2. In test `"phone-OTP login works for client and worker"` at `e2e/smoke.spec.ts:12-17`,
`login(page, DEMO.client)` is called, which navigates to `/login` and executes
`page.getByTestId("phone-input")` and `page.getByTestId("send-otp")` at
`e2e/helpers.ts:15-16`.
3. The login form implementation in `src/app/login/phone-form.tsx:39-58` renders the phone
input as `<Input id="phone" ... {...form.register("phone")}>` with no
`data-testid="phone-input"` or `"send-otp"` attributes, and
`src/app/login/verify/verify-form.tsx:71-120` similarly renders the OTP slots without
`data-testid="otp-input"`.
4. Because these `data-testid` attributes do not exist anywhere in the codebase (confirmed
by a project-wide grep for `data-testid="phone-input"`, `data-testid="send-otp"`, and
`data-testid="otp-input"`), Playwright cannot resolve these locators and the first
`login()` call times out, causing all tests that rely on `login()` to fail before
authentication.Fix in Cursor | Fix in VSCode Claude (Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** e2e/helpers.ts
**Line:** 15:20
**Comment:**
*Api Mismatch: These selectors don't exist in the current login UI (`phone-form.tsx` and `verify-form.tsx` have no matching `data-testid` attributes), so `login()` will time out before authentication starts. Update the helper to target existing stable selectors or add matching test IDs in the login forms.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
|
|
||
| await expect(page).toHaveURL(/\/(client|worker|admin|onboarding)/) | ||
| } | ||
|
|
||
| export async function logout(page: Page) { | ||
| await page.locator("a[href$='/account']").click() | ||
| await page.getByTestId("sign-out").click() | ||
| await page.getByTestId("sign-out-confirm").click() | ||
|
Comment on lines
+27
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The account UI currently has no Severity Level: Critical 🚨- ❌ Any test that switches users fails on logout.
- ❌ Escrow path tests B–F cannot complete end-to-end.
- ⚠️ Prevents validating multi-role session handling.Steps of Reproduction ✅1. Run `pnpm playwright test e2e/smoke.spec.ts`; tests call the shared `logout()` helper
from `e2e/helpers.ts:25-30` whenever switching between client and worker users (e.g.
`"phone-OTP login works for client and worker"` at `e2e/smoke.spec.ts:12-17`, Section
B/C/E/F tests).
2. `logout(page)` first clicks the account navigation link via
`page.locator("a[href$='/account']").click()` at `e2e/helpers.ts:26`, which matches
`/client/account` and `/worker/account` links defined in
`src/components/nav/client-nav-shell.tsx:10-12` and
`src/components/nav/worker-nav-shell.tsx:10-12`.
3. Once on the account page (`src/app/client/account/page.tsx:4-22` or
`src/app/worker/account/page.tsx:6-23`), the sign-out UI is rendered by `SignOutButton`
(`src/components/account/sign-out-button.tsx:21-69`), which exposes a `<Button>` and an
`AlertDialogAction` with visible text "Sign out" but no `data-testid="sign-out"` or
`data-testid="sign-out-confirm"`.
4. A repo-wide grep finds no `data-testid="sign-out"` or `"sign-out-confirm"` anywhere, so
`page.getByTestId("sign-out")` and `"sign-out-confirm"` at `e2e/helpers.ts:27-28` always
fail, causing every test that logs out to error at the first logout call.Fix in Cursor | Fix in VSCode Claude (Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** e2e/helpers.ts
**Line:** 27:28
**Comment:**
*Api Mismatch: The account UI currently has no `sign-out` / `sign-out-confirm` test IDs, so this logout helper cannot find its targets and will fail every flow that switches users. Align selectors with the actual `SignOutButton` dialog controls or add those test IDs.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
| await expect(page).toHaveURL(/\/login/) | ||
|
Comment on lines
+15
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Architect Review — CRITICAL The Playwright login/logout and wallet top-up helpers depend on data-testid selectors (phone-input, send-otp, otp-input, sign-out, sign-out-confirm, add-money, topup-amount, topup-confirm) that are not present anywhere in the current UI components, so the core smoke tests cannot drive these flows and will fail. Suggestion: Add matching data-testid attributes to the login/verify forms, sign-out dialog, and TopUpDialog components, or update the helpers/tests to use stable role/name-based locators that match the existing markup. Fix in Cursor | Fix in VSCode Claude (Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** e2e/helpers.ts
**Line:** 15:29
**Comment:**
*CRITICAL: The Playwright login/logout and wallet top-up helpers depend on data-testid selectors (phone-input, send-otp, otp-input, sign-out, sign-out-confirm, add-money, topup-amount, topup-confirm) that are not present anywhere in the current UI components, so the core smoke tests cannot drive these flows and will fail.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
| } | ||
|
|
||
| // Reads a money value rendered with data-testid="wallet-available" / "wallet-locked". | ||
| export async function balance(page: Page, which: "available" | "locked") { | ||
| const raw = await page.getByTestId(`wallet-${which}`).innerText() | ||
| return Number(raw.replace(/[^0-9.]/g, "")) | ||
| } | ||
|
|
||
| // Waits for a specific RPC to return 2xx (proves the money write happened). | ||
| export async function expectRpcOk(page: Page, urlPart: string, act: () => Promise<void>) { | ||
| const [res] = await Promise.all([ | ||
| page.waitForResponse((r) => r.url().includes(urlPart) && r.request().method() === "POST"), | ||
| act(), | ||
| ]) | ||
| expect(res.status(), `${urlPart} should return 2xx`).toBeLessThan(400) | ||
| return res | ||
| } | ||
|
Comment on lines
+39
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The helper is fully implemented and documented but Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
| /** | ||
| * Drive the 6-step PostJobForm wizard as the currently-logged-in client. | ||
| * Navigates to /client/jobs/new, fills all required fields, submits. | ||
| * Returns the newly created job's ID extracted from the redirect URL. | ||
| */ | ||
| export async function postJob( | ||
| page: Page, | ||
| opts: { title?: string; budget?: string; milestoneTitle?: string } = {}, | ||
| ): Promise<string> { | ||
| const title = opts.title ?? "E2E test job" | ||
| const budget = opts.budget ?? "1000" | ||
| const milestoneTitle = opts.milestoneTitle ?? "Complete work" | ||
|
|
||
| await page.getByTestId("post-job").click() | ||
| await expect(page).toHaveURL(/\/client\/jobs\/new/) | ||
|
|
||
| // Step 1: Basics | ||
| await page.getByTestId("job-title").fill(title) | ||
| await page.getByRole("combobox").click() | ||
| await page.getByRole("option").first().click() | ||
| await page.locator("#description").fill("E2E smoke test job for escrow flow testing") | ||
| await page.getByRole("button", { name: "Next", exact: true }).click() | ||
|
|
||
| // Step 2: Location | ||
| await page.locator("#location_text").fill("Banjara Hills, Hyderabad") | ||
| await page.getByRole("button", { name: "Next", exact: true }).click() | ||
|
|
||
| // Step 3: Budget | ||
| await page.locator("#total_budget").fill(budget) | ||
| await page.getByRole("button", { name: "Next", exact: true }).click() | ||
|
|
||
| // Step 4: Milestones | ||
| await page.locator("#ms-title-0").fill(milestoneTitle) | ||
| await page.getByTestId("milestone-amount-0").fill(budget) | ||
| await page.getByRole("button", { name: "Next", exact: true }).click() | ||
|
|
||
| // Step 5: Materials — skip | ||
| await page.getByRole("button", { name: "Next", exact: true }).click() | ||
|
|
||
| // Step 6: Review → Submit | ||
| await page.getByTestId("job-submit").click() | ||
| await expect(page).toHaveURL(/\/client\/jobs\/[0-9a-f-]+$/, { timeout: 15_000 }) | ||
|
|
||
| return page.url().split("/").pop()! | ||
| } | ||
|
|
||
| /** | ||
| * Worker applies to a job (navigates to /worker/jobs/:id, opens modal, fills bid). | ||
| */ | ||
| export async function applyToJob(page: Page, jobId: string, bid: string = "1000") { | ||
| await page.goto(`/worker/jobs/${jobId}`) | ||
| await page.getByTestId("apply-job").click() | ||
| await page.locator("#bid_amount").fill(bid) | ||
| await page.locator("#eta_days").fill("7") | ||
| await page.getByRole("button", { name: /submit application/i }).click() | ||
| await expect(page.getByRole("button", { name: /submit application/i })).toBeHidden({ | ||
| timeout: 15_000, | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * Client accepts first applicant on a job (navigates to /client/jobs/:id). | ||
| * After accept, the page redirects to /milestones. | ||
| */ | ||
| export async function acceptApplicant(page: Page, jobId: string) { | ||
| await page.goto(`/client/jobs/${jobId}`) | ||
| await page.getByTestId("accept-applicant").click() | ||
| await expect(page).toHaveURL(/\/client\/jobs\/[0-9a-f-]+\/milestones/, { timeout: 15_000 }) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Harden workflow actions: pin by commit SHA and disable checkout credential persistence.
Line 9/10/12/20 use tag refs (
@v4) instead of immutable SHAs, and checkout doesn’t setpersist-credentials: false. This weakens CI supply-chain posture.Suggested hardening pattern
Also applies to: 20-20
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 9-9: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 9-9: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 10-10: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 12-12: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Source: Linters/SAST tools