test(e2e): hermetic Playwright browser suite for visibility states#65
Merged
Conversation
Adds the browser-rendering half of the per-field visibility coverage.
The server-side filtering math is unit-tested (visibility.test.ts,
profiles-list.test.ts, profile-by-username.test.ts); this suite asserts
the BROWSER paints the right state given a canned API response —
something the source-regex tests can only approximate.
Approach (per the agreed design): hermetic + mocked API.
- Serves the production build via `astro preview`; no deploy, no
Cosmos, no real OAuth.
- page.route() intercepts /.auth/me, /api/get-roles,
/api/profile-by-username and /api/profiles with per-test canned
responses.
- The SWA `/find/* → /find/profile/index.html` rewrite doesn't exist
in astro preview, so the suite emulates it: fulfils any
/find/<username> navigation with the built profile-detail HTML.
The page's client script then reads window.location.pathname to
extract the username exactly as it would behind the real rewrite.
Coverage (10 tests):
detail page — public profile renders all fields; bio stripped →
hidden (not blank paragraph); skills stripped → section hidden;
403 → private state + @username; 404 → not-found state + @username;
500 → error state
directory — "you're listed" banner when principal matches a row;
"want to be listed" when not; case-insensitive listed match;
both banners hidden on empty directory (empty-state owns the CTA)
CI wiring (the gating requirement):
- Runs as a STEP in the `validate` job (hermetic — nothing to wait
for post-deploy). `npx playwright install --with-deps chromium`
then `npm run test:e2e:browser`.
- dev_deploy already `needs: validate`, so a failing browser test
now blocks the deploy (CD). Header comment updated to map each
gate (dependency/security, unit, e2e) to the CD-blocking
relationship; the merge-blocking half is the existing branch-
protection "Validate required check" setting.
Plumbing:
- playwright.config.ts: testDir e2e-browser/, chromium project,
webServer runs `npm run preview`, CI retries=2.
- vitest.config.ts: e2e-browser/** added to exclude so the main
`npm run test:run` doesn't grab the .spec.ts files (they import
@playwright/test, not vitest).
- package.json: test:e2e:browser script + @playwright/test devDep.
- .gitignore: playwright artifact dirs.
Note: the spec derives __dirname from import.meta.url — the repo is
type:module and Playwright's ESM loader (unlike vitest) provides no
__dirname shim. Caught locally before push.
Local verification limit: the sandbox can't download the chromium
binary (network-restricted), so the assertions first truly execute in
CI. Everything up to browser launch is verified locally: config valid,
webServer boots, all 10 tests collected, spec loads clean, and the
full non-browser gate (lint, 485 unit tests, format, build, API
typecheck/build, audits) is green.
https://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
There was a problem hiding this comment.
Pull request overview
Adds a hermetic Playwright browser E2E suite to validate visibility-/status-dependent UI rendering, and wires it into CI so it gates the dev deploy (and therefore CD) alongside existing lint/audit/unit/build checks.
Changes:
- Exclude the new Playwright
.spec.tssuite from Vitest collection and add a dedicated Playwright config. - Add
e2e-browser/visibility.spec.tswith mocked/.auth/me+/api/*routes and an emulated/find/*rewrite forastro preview. - Run Playwright (including browser install) in the
validatejob before deploy, and add Playwright artifacts to.gitignore.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Excludes e2e-browser/** from Vitest so Playwright specs aren’t executed by vitest run. |
| playwright.config.ts | New Playwright configuration pointing at e2e-browser/ and using astro preview as the webServer. |
| package.json | Adds test:e2e:browser script and @playwright/test dev dependency. |
| package-lock.json | Locks Playwright-related dependencies (@playwright/test, playwright, playwright-core). |
| e2e-browser/visibility.spec.ts | New hermetic browser E2E suite covering profile/detail and directory visibility states via route mocking. |
| .gitignore | Ignores Playwright test artifacts (test-results/, reports, etc.). |
| .github/workflows/azure-static-web-apps.yml | Installs Playwright Chromium and runs the browser suite as part of the validate gate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three polish findings on the Playwright suite: 1. Route handler didn't return the fulfill/continue promise. serveDetailPageAtAnyUsername's handler called route.fulfill() / route.continue() fire-and-forget. A handler that returns before the route resolves is a known Playwright flakiness source. Now returns the promise so Playwright awaits it. 2. Raw ENOENT on a fresh/cleaned checkout. The spec reads dist/find/profile/index.html at module-load time. Without a build, that threw a bare ENOENT during test discovery. Wrapped in try/catch with an actionable message pointing at `npm run build` / `npm run test:e2e:browser`. 3. test:e2e:browser didn't guarantee the build. Script is now `npm run build && playwright test` so a local run on a fresh checkout works without a separate build step. In CI the build already runs in the validate job's "Build frontend" step, so the workflow's e2e step calls `npx playwright test` directly to skip the redundant rebuild — documented inline so the CI/local divergence is intentional, not accidental. No assertion changes — the 10 tests passed in CI on the prior commit; these are robustness + DX improvements. https://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
The previous push of `3417053` (Copilot polish: route promise return, ENOENT error message, build-first script) did not produce check runs on this PR head — a CI registration hiccup, not a code issue. The prior commit `85db218` had all four checks green including the browser e2e assertions. This empty commit nudges the workflow to run again so the polish commit gets the green pipeline it would otherwise have had. https://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
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.
What this PR delivers
Adds the browser-rendering half of the per-field visibility coverage, plus wires it into CI as a deploy-gating step (your requirement: dependency/security + unit + e2e all gate CD).
The server-side filtering math is already unit-tested. This suite asserts the browser paints the right state given a canned API response — the thing source-regex tests can only approximate.
Approach (per the agreed design): hermetic + mocked API. Serves the production build via
astro preview;page.route()intercepts/.auth/me,/api/get-roles,/api/profile-by-username,/api/profiles. The SWA/find/* → /find/profile/index.htmlrewrite (absent inastro preview) is emulated by fulfilling/find/<username>navigations with the built detail HTML, so the page'swindow.location.pathnameusername extraction runs exactly as in prod.Coverage (10 tests):
@username; 404 → not-found state +@username; 500 → error stateCI wiring — runs as a step in
validate(hermetic, pre-deploy).dev_deploy needs: validate, so a failing browser test now blocks the deploy. The header comment maps each gate (dependency/security, unit, e2e) to the CD-blocking relationship; the merge-blocking half is the existing branch-protection "Validate required check" setting.The sandbox I authored this in can't download the Chromium binary (network-restricted), so the browser assertions first truly execute in this PR's CI run. Everything up to browser launch is locally verified: Playwright config valid, webServer boots, all 10 tests collected, spec loads clean (the
__dirname-in-ESM fix was caught locally), and the full non-browser gate is green (lint, 485 unit tests, format, build, API typecheck/build, audits). Please watch the Validate job's Playwright step on this PR — if any assertion needs a tweak (timing, selector), I'll iterate.Verified by
e2e-browser/visibility.spec.ts— 10 Playwright tests covering every visibility/status render state (see Coverage above). These execute for the first time in this PR'svalidatejob — the local gate verified collection + config + spec load, not the assertions themselves (sandbox can't fetch the browser binary).npm run test:run— 485 unit tests pass; confirms the new.spec.tsfiles are correctly excluded from the vitest run (still 485, no Playwright import errors)Constitution compliance
validatejob (browser install + 10 hermetic tests); acceptable for a gating suitePlatform constraints
/find/*rewrite doesn't exist inastro preview, so the hermetic suite emulates it viapage.routefulfillment. Noted in the spec's header comment.Operator action items
Validateremains the required status check onmain(it now includes the browser e2e step — no settings change needed if it was already required; just flagging that the gate's scope grew).Test plan
https://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
Generated by Claude Code