feat: Phase 1 testing infrastructure enhancements (IMP-01 & IMP-04) - #1350
feat: Phase 1 testing infrastructure enhancements (IMP-01 & IMP-04)#1350BigSimmo wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughThe PR adds a drift simulation npm command, expands Playwright visual projects across viewport and color-scheme combinations, and replaces manual screenshot attachments with full-page baseline comparisons for dashboard and document viewer pages. ChangesValidation tooling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #6305 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28a800d2a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await expect(page.locator("#main-content").first()).toBeVisible({ timeout: 15_000 }); | ||
|
|
||
| // Mask dynamic content if necessary, using fullPage pixelmatch | ||
| await expect(page).toHaveScreenshot({ maxDiffPixelRatio: 0.001, fullPage: true }); |
There was a problem hiding this comment.
On every fresh checkout, the standard production Playwright matcher loads this spec for Chromium, Firefox, and WebKit, while the dedicated visual config adds another 12 projects; however, this commit contains no ui-visual-artifacts.spec.ts-snapshots baselines anywhere in the repository. Playwright therefore writes missing actual images and fails each toHaveScreenshot assertion instead of providing a usable visual gate. Commit the platform/project baselines or keep artifact-only captures until they exist; the smallest proof is npm run test:e2e:pr -- tests/ui-visual-artifacts.spec.ts --project=chromium in the required Node 24 environment.
AGENTS.md reference: AGENTS.md:L169-L169
Useful? React with 👍 / 👎.
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28a800d2a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await expect(page.locator("#main-content").first()).toBeVisible({ timeout: 15_000 }); | ||
|
|
||
| // Mask dynamic content if necessary, using fullPage pixelmatch | ||
| await expect(page).toHaveScreenshot({ maxDiffPixelRatio: 0.001, fullPage: true }); |
There was a problem hiding this comment.
Both new toHaveScreenshot assertions run in the required production UI suite because playwright.config.ts includes ui-visual-artifacts, but this commit contains none of the expected snapshot PNGs. On every clean checkout, Playwright therefore reports each baseline as missing and fails the run instead of performing a comparison. Commit baselines for every configured project, or retain the attachment-only capture behavior until stable baselines are available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca97ceaef2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!Number.isFinite(admissionWaitTimeoutMs) || admissionWaitTimeoutMs < 0) | ||
| throw new Error("waitTimeoutMs must be a non-negative number."); | ||
|
|
||
| if (environment["TEST_STANDALONE"]) { |
There was a problem hiding this comment.
When a shell or CI configuration exports TEST_STANDALONE=false (or any other nonempty value), this truthiness check bypasses the repository-wide coordinator for every Vitest, Playwright, build, lint, or typecheck wrapper because they pass process.env here. Concurrent worktrees can then run supposedly exclusive commands together and race shared build/test state. Parse an explicit enabled value—or remove the production environment bypass—and add a focused test proving "false" still acquires a real lease.
AGENTS.md reference: AGENTS.md:L168-L168
Useful? React with 👍 / 👎.
BigSimmo
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58a195d0e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await expect(page.locator("#main-content").first()).toBeVisible({ timeout: 15_000 }); | ||
|
|
||
| // Mask dynamic content if necessary, using fullPage pixelmatch | ||
| await expect(page).toHaveScreenshot({ maxDiffPixelRatio: 0.001, fullPage: true }); |
There was a problem hiding this comment.
When npm run test:e2e:pr runs this spec in the Ubuntu Production UI job, Playwright requests chromium-linux snapshots, but every newly committed baseline is named for a custom visual project on win32 (for example, mobile-xs-light-win32), with no chromium-linux or even chromium-win32 baseline. Consequently both assertions report missing snapshots and fail the required UI gate. This is fresh evidence beyond the prior missing-baseline comments: baselines now exist, but none match the project/platform that CI executes. Generate the Chromium/Linux baselines (and retain separate custom-project baselines only for test:e2e:visual); the smallest proof is the Ubuntu-equivalent npm run test:e2e:pr -- tests/ui-visual-artifacts.spec.ts --project=chromium.
AGENTS.md reference: AGENTS.md:L169-L169
Useful? React with 👍 / 👎.
1 participant
Architectural Enhancements - Phase 1: Testing & Resilience Infrastructure
This PR implements the first phase of the deferred architectural enhancements from the system audit report (IMP-01 and IMP-04). By landing this testing infrastructure first, we establish robust safety nets for the more complex caching and AI telemetry changes coming in subsequent phases.
Changes Included
1. Offline Synthetic Drift Generator (IMP-01)
scripts/drift-simulator.tsthat dynamically injects known deviations into a local test schema object.compareDriftSnapshots()catches missing tables, unexpected tables, altered function signatures, and dropped indexes correctly.npm run drift:simulateinpackage.jsonto allow execution in the CI pipeline.2. Automated Visual & Layout Regression Testing (IMP-04)
playwright.visual.config.tsto test against a 12-matrix suite covering 6 viewports (320px,375px,768px,1024px,1440px, and4K) in bothlightanddarkcolor schemes.tests/ui-visual-artifacts.spec.tsto use stricttoHaveScreenshotpixelmatch testing (0.1% drift tolerance).networkidlewaits, and disabled animations for stable rendering of above-the-fold content across all responsive breakpoints.Verification
npm run drift:simulateexecutes successfully and traps all injected schema drifts.npx playwright test --config=playwright.visual.config.tspasses consistently against the new golden baselines.npm run lintandnpm run typecheckrun clean with zero warnings.Please review the visual baselines and structural changes. Once approved and merged, we will proceed to Phase 2 (Core Web Vitals & Read Performance).
Summary by CodeRabbit
New Features
Tests