test(smoke): make prod smoke deterministic and required (no flakiness) - #295
Conversation
Root-cause fixes for the two flaky flows: - log-habit: the toggle was force-clicked, bypassing Playwright's actionability/stability wait, so it could hit a row mid-detach during the create/log refetch re-render. Drop force, assert the toggle is enabled, and wait for the POST /log response before asserting the aria-label flip (the assertion auto-retries and re-resolves across the refetch). - paywall: the spec only DOM-waited for the radiogroup. Now it waits for the GET /subscriptions/plans response and asserts it succeeded before checking the CTA, so a slow/cold plans fetch is awaited deterministically instead of racing a 60s DOM timeout. Plus a best-effort backend warmup in global setup (habit list + plans) so per-test waits never race a cold Render dyno or an unprimed price catalog. Make the suite a real required check: drop continue-on-error so a smoke failure goes red (the suite is now deterministic, so red == real regression). The failure alert still fires, and the manual Promote Production workflow remains the recovery lever (no flaky auto-rollback). Remove the Vercel-CLI-install and previous-deployment-capture steps left orphaned when auto-rollback was dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR 295 — APPROVE
Summary: This PR hardens the post-deploy smoke suite by replacing force-clicks and blind DOM waits with explicit waitForResponse assertions, adds a best-effort backend warm-up in global setup, and restores the suite as a required (non-continue-on-error) CI check. The fixes directly address the two documented root causes. The diff is clean, well-scoped, and correct.
Findings: Critical: None. High: None. Medium: None.
Dimension walk
Correctness PASS — log-habit.spec.ts: waitForResponse listener registered before toggle.click() (line 27 vs. 32), correct Playwright pattern, no race window. initialState captured before click so before/after comparison is valid. Regex anchors to /log without over-matching. paywall.spec.ts: listener registered before page.goto('/upgrade') so any plans response triggered by navigation is captured. global.setup.ts: warmBackend runs after resetSmokeAccount and before storageState is saved; swallowed warm-up failures cannot produce false greens because spec-level waitForResponse assertions remain authoritative. smoke-prod.yml: VERCEL_TOKEN/ORG_ID/PROJECT_ID all still consumed by the curl call in the deploy-wait step — no orphaned secrets after the Vercel CLI removal.
Dead/stale code PASS — warmBackend exported and consumed in global.setup.ts. No orphaned symbols.
Comment policy PASS — warmBackend JSDoc is a Block comment documenting intent and contract; allowed by no-comments.cjs. No line-comment narration introduced.
No-workaround PASS — removes force: true (the flake source), asserts API call before checking DOM state, awaits plans response before asserting UI. All root causes fixed upstream.
Type safety PASS — warmBackend returns Promise, .catch(() => undefined) typed correctly. No any.
Parity N/A — all changed files are CI infrastructure or apps/web/e2e/; root CLAUDE.md explicitly prohibits mobile E2E. Parity-checker confirmed no gap.
i18n, Contract drift, Security, Backend hard rules — all N/A (diff never touches those surfaces).
What is good: Root-cause fix not a retry; best-effort warm-up cannot produce false greens; clean CI removal with no orphaned env vars; if: failure() is correct after dropping continue-on-error; regex is precise.
Recommendation: All relevant dimensions pass. No Critical or High findings. Approve and merge.
|
There was a problem hiding this comment.
PR Review #295: test(smoke): make prod smoke deterministic and required -- Decision: APPROVE -- Severity: Critical=0, High=0, Medium=0, Low/Info=0 -- All 13 rubric dimensions: Correctness PASS (waitForResponse registered before triggering actions, correct Playwright pattern, no race window). Dead code/Comment/No-workaround/Type safety all PASS. Parity/i18n/Contract/Security/Backend all N/A (diff only touches CI and e2e, exempt from mobile parity). What the diff does: removes unused vercel CLI, fixes two race-condition patterns by moving waitForResponse before the triggering action, promotes prod smoke gate from advisory to required. All changes correct, targeted, and complete. Clean PR. -- Reviewed by claude[bot] via /pr-review rubric
1 participant
Problem
The post-deploy prod smoke was made non-blocking (#294) because two flows flaked. This makes them deterministic and restores the suite as a required check.
Root causes
click({ force: true }), which bypasses Playwright's actionability/stability wait. Aftercreate,useCreateHabit/useLogHabitinvalidate the habits list (onSettled), the Today list re-renders, and the force-click could land on a row mid-detach (or assert on an optimistic flip that the refetch then reverted).radiogroup(60s). On a cold Render dyno / unprimed plans catalog theGET /subscriptions/plansfetch was slow, so it raced the timeout instead of awaiting the actual response.Fixes
force; assert the toggle is enabled;waitForResponseonPOST /api/habits/{id}/logand assert it succeeded before checking thearia-labelflip (the assertion auto-retries + re-resolves the locator across the refetch).waitForResponseonGET /api/subscriptions/plans, assertok(), then check the radiogroup + checkout CTA.warmBackend(habit list + plans) so per-test waits never race a cold dyno or unprimed catalog.Required, not skippable
continue-on-error— a smoke failure now goes red. The suite is deterministic, so red == a real regression in a core flow.Notes
@playwright/testisn't in the local node_modules; CI installs it vianpm ci+npx playwright install).🤖 Generated with Claude Code