Set up Playwright for local, manual e2e testing (LS-2335) - #20
Set up Playwright for local, manual e2e testing (LS-2335)#20brandonmarshal wants to merge 2 commits into
Conversation
Install Playwright via the official create-playwright scaffold shape (@playwright/test, dotenv, @types/node) and configure all 3 browser projects (Chromium, Firefox, WebKit). baseURL is read from a local, gitignored .env so each developer points tests at their own environment — no CI wiring, since there's no shared environment that reflects a PR's own code before merge; tests run manually via `npx playwright test`. Add a real spec (tests/specs/work-archive.spec.ts) exercising all 6 generic assertion helpers from tests/helpers/assertions.ts (LS-2244) against the live Work Archive template: section order, category-card count/parts, hero link href, related-routes grid reflow at mobile, and stats-grid divider styling.
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Configure Playwright execution package.json, playwright.config.ts, .gitignore, CHANGELOG.md |
Added Playwright tooling, .env loading, BASE_URL support, Chromium/Firefox/WebKit projects, CI execution settings, report and trace handling, ignored artifacts, and changelog coverage. |
Add reusable page assertions tests/helpers/assertions.ts |
Added assertions for section order, element counts, card parts, link URLs, responsive columns, and computed CSS properties. |
Cover Work templates tests/specs/work-archive.spec.ts, tests/specs/work-single.spec.ts |
Added Work Archive checks for structure, cards, links, responsive layout, and divider styling. Added a skipped Work Single specification with equivalent coverage targets. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Sequence Diagram(s)
sequenceDiagram
participant PlaywrightTest
participant PlaywrightConfig
participant Browser
participant WorkTemplate
PlaywrightTest->>PlaywrightConfig: Load BASE_URL and browser project
PlaywrightConfig->>Browser: Start Chromium, Firefox, or WebKit
PlaywrightTest->>WorkTemplate: Navigate to Work Archive path
WorkTemplate-->>PlaywrightTest: Render sections, cards, links, and layout
PlaywrightTest->>WorkTemplate: Inspect text, counts, URLs, columns, and styles
Suggested reviewers: ashleyshaw
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: setting up Playwright for local, manual end-to-end testing. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/ls-2335-set-up-playwright-testing-and-write-generic-assertion
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@playwright.config.ts`:
- Line 34: Update the Playwright configuration’s baseURL assignment to validate
that process.env.BASE_URL is present and non-empty before the suite starts, and
fail clearly during configuration when it is missing. Preserve the existing
baseURL behavior for valid values.
- Line 12: Update the Playwright configuration’s dotenv path resolution to
support Node.js 20.0.0 by deriving the directory from import.meta.url with
fileURLToPath and path.dirname instead of using import.meta.dirname; add or
reuse the required imports while preserving the existing .env path.
In `@tests/specs/work-archive.spec.ts`:
- Around line 43-50: Update the hero CTA assertion in the test using
WORK_ARCHIVE_URL as the source of truth: resolve the expected href with new
URL() so WORK_ARCHIVE_PATH is respected and trailing slashes in baseURL cannot
produce a double slash. Keep the existing “Explore case studies” link
verification unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: ffe2f40e-284b-4139-b1ec-67cf8cfcdc4c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.gitignoreCHANGELOG.mdpackage.jsonplaywright.config.tstests/helpers/assertions.tstests/specs/work-archive.spec.tstests/specs/work-single.spec.ts
There was a problem hiding this comment.
🟡 Changes recommended
The Playwright config is not compatible with the repo’s stated Node support range, and the Work Archive spec hardcodes /work/ despite supporting an override path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds a local Playwright test harness to the theme repo to support manual, developer-run end-to-end checks against live environments, plus initial assertion helpers and template specs.
Changes:
- Adds Playwright configuration (multi-browser projects, html reporter) and loads
BASE_URLfrom a local.env. - Introduces reusable Playwright assertion helpers and a real “Work Archive” spec using them.
- Updates Node devDependencies and ignores Playwright output directories; documents the addition in the changelog.
File summaries
| File | Description |
|---|---|
playwright.config.ts |
Adds Playwright test configuration and dotenv loading for local base URL. |
tests/helpers/assertions.ts |
Adds generic assertion helpers used by e2e specs. |
tests/specs/work-archive.spec.ts |
Adds a real, running spec for the Work Archive template using the helpers. |
tests/specs/work-single.spec.ts |
Adds an illustrative (skipped) spec demonstrating intended helper usage. |
package.json |
Adds Playwright + dotenv + Node types to devDependencies. |
package-lock.json |
Locks Playwright/dotenv dependency tree. |
CHANGELOG.md |
Notes the addition of Playwright e2e testing setup. |
.gitignore |
Ignores Playwright reports/output directories. |
Review details
- Files reviewed: 6/8 changed files
- Comments generated: 5
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
- Use fileURLToPath(import.meta.url) instead of import.meta.dirname in playwright.config.ts so the dotenv loader works across the full declared engines.node range (>=20.0.0), not just Node 20.11+ - Fail fast with a clear error if BASE_URL isn't set, instead of a confusing "Invalid URL" failure inside a test - Fix work-archive.spec.ts's hero CTA test: rename it to match what it actually checks, and resolve the expected href from WORK_ARCHIVE_URL via new URL() so it respects the WORK_ARCHIVE_PATH override and can't produce a double slash - Fix the same relative-href bug in the skipped work-single.spec.ts example, so it doesn't get copied forward when that template exists
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (5)
tests/helpers/assertions.ts:30
expectSectionOrderusesindexOf()without a start position, so if a marker string appears earlier on the page as well (or markers repeat), the helper can pass/fail incorrectly. Searching for each marker after the previous match makes the ordering assertion reliable.
const bodyText = (await page.locator('body').innerText()).toLowerCase();
const positions = markers.map((marker) => bodyText.indexOf(marker.toLowerCase()));
tests/helpers/assertions.ts:62
expectLinkHrefcan throw a strict-mode error if more than one link partially matcheslinkText(becauseexact: false). Making the helper assert a single match first keeps failures clearer and prevents Playwright strictness errors.
export async function expectLinkHref(page: Page, linkText: string, expectedHref: string) {
const link = page.getByRole('link', { name: linkText, exact: false });
await expect(link).toHaveAttribute('href', expectedHref);
}
tests/helpers/assertions.ts:90
gridTemplateColumns.split(' ')is not a reliable way to count grid tracks because computed values can include spaces inside functions (e.g.minmax(0px, 1fr)), which inflates the count and will be browser-dependent. Split the track list on top-level whitespace instead.
if (display === 'grid') {
const columns = await grid.evaluate(
(el) => getComputedStyle(el).gridTemplateColumns.split(' ').length
);
tests/helpers/assertions.ts:96
- In the flex/columns branch, if the grid has no child elements then
tops[0]isundefinedand the failure message becomes misleading (it will compare againstundefined). Add an explicit assertion that the grid has at least one child first.
// Flex/columns layout: count distinct items sharing the first row's top offset.
const tops: number[] = await grid.evaluate((el) =>
[...el.children].map((child) => Math.round(child.getBoundingClientRect().top))
);
const firstRowCount = tops.filter((t) => t === tops[0]).length;
tests/specs/work-single.spec.ts:22
- This file is entirely skipped via
test.describe.skip, so it will never exercise the helpers but will still appear as a permanently skipped suite in local runs. If it’s only meant as an example, consider switching to a normaldescribewith a singletest.skip(true, ...)inside so the report shows an explicit reason (and it’s harder to forget this is intentionally inactive).
// Skipped: work-single doesn't exist yet, and these selectors/URL are illustrative
// guesses, not real markup. Un-skip once the template is built and selectors are
// swapped for the real ones.
test.describe.skip('Work Single template', () => {
test('renders sections in order', async ({ page }) => {
ZaredRogers
left a comment
There was a problem hiding this comment.
Looks good @brandonmarshal
Summary
create-playwrightscaffold defaults (verified directly against the real installer output):@playwright/test,dotenv,@types/node, all 3 browser projects (Chromium, Firefox, WebKit),reporter: 'html',forbidOnly, CI-awareretries/workersbaseURLis read from a local, gitignored.env(not hardcoded) so each developer points tests at their own environmentnpx playwright testtests/specs/work-archive.spec.ts, using the 6 generic assertion helpers fromtests/helpers/assertions.ts(LS-2244) against the live Work Archive templateTest plan
npx playwright testpasses locally against the Work Archive templatecreate-playwrightoutput.envconfirmed gitignored, never committedLS-2335
Summary by CodeRabbit
Tests
Chores