Skip to content

Add Playwright test infrastructure#29

Merged
plx merged 1 commit intomainfrom
plx/add-playwright-tests
Mar 15, 2026
Merged

Add Playwright test infrastructure#29
plx merged 1 commit intomainfrom
plx/add-playwright-tests

Conversation

@plx
Copy link
Owner

@plx plx commented Mar 15, 2026

Summary

  • Add Playwright end-to-end test suite with 4 spec files covering navigation, accessibility, content, and responsive design
  • Multi-browser testing: Chromium, Firefox, WebKit, plus mobile (Pixel 5, iPhone 12)
  • Add npm scripts and justfile commands for running tests in various modes
  • Update .gitignore for Playwright artifacts

Dependencies

This PR should be merged after the following PRs, as the tests validate the state they establish:

Design decisions

  • Dropped the comprehensive sitemap-walker test from PR Add dedicated QA script for website testing #22 — it required an xml2js dependency, and the core test suites already cover structural checks on all major pages
  • Tests auto-build and start a preview server via webServer config
  • No CI workflow integration yet — can be added once the test suite is validated locally

Test plan

  • Run just setup to install Playwright browsers
  • Run just qa to execute full test suite
  • Verify tests pass across all browsers
  • Run just qa-headed to visually verify

🤖 Generated with Claude Code

Add end-to-end test suite covering navigation, accessibility,
content rendering, and responsive design. Tests run across
Chromium, Firefox, WebKit, and mobile viewports.

Includes:
- playwright.config.ts with multi-browser + mobile setup
- Navigation tests (page loads, routing, 404, consistency)
- Accessibility tests (heading structure, alt text, links, lang, skip link, duplicate IDs)
- Content tests (page content, RSS, sitemap, external link security)
- Responsive tests (horizontal scroll, font size, touch targets, reflow)
- npm scripts (qa, qa:headed, qa:ui, qa:debug, qa:report, qa:codegen)
- justfile commands (qa, qa-headed, qa-ui, qa-debug, qa-report, qa-codegen, setup)
- Updated .gitignore for Playwright artifacts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

PR Validation ✅

All checks passed! Ready for review.

Checks Performed:

✓ Linting
✓ Spell check (source)
✓ Type checking & Build
✓ Spell check (HTML)
✓ Internal link validation
✓ Artifact upload

This is a complete dry-run of the deployment process, ensuring your changes will deploy successfully when merged.

@plx plx merged commit b9688ce into main Mar 15, 2026
3 checks passed
@plx plx deleted the plx/add-playwright-tests branch March 15, 2026 18:58
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebefa43ba7

ℹ️ 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".

const skipLink = page.locator("a.skip-link, a[href=\"#main-content\"]").first();
const skipLinkExists = await skipLink.count() > 0;

if (skipLinkExists) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert skip link presence before validating its target

This test is named skip to content link exists, but it only performs assertions inside if (skipLinkExists), so a page with no skip link passes without failure. That creates a false negative in the accessibility suite and can let a regression of a core keyboard-navigation affordance ship undetected.

Useful? React with 👍 / 👎.

forbidOnly: !!process.env.CI,
retries: process.env.CI ? 2 : 0,
workers: process.env.CI ? 1 : undefined,
reporter: process.env.CI ? "github" : "list",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Configure an HTML reporter for qa:report

The project exposes qa:report (playwright show-report), but this config sets the reporter to list locally and github in CI, which do not generate the HTML report that show-report serves (npx playwright show-report --help says it "show HTML report"). In this setup, npm run qa:report fails with "No report found" unless users manually override reporters, so the documented report workflow is broken by default.

Useful? React with 👍 / 👎.

@claude
Copy link

claude bot commented Mar 15, 2026

PR 29 Playwright Test Infrastructure Review

Overall this is a solid foundation for end-to-end testing. Multi-browser coverage is good and the webServer auto-build approach ensures tests run against a real production build.

ISSUES TO ADDRESS:

  1. setup recipe uses npm install instead of npm ci -- The existing install recipe uses npm ci to enforce the lockfile, but setup uses npm install. Should use npm ci for consistency.

  2. Responsive tests run on all 5 browser projects -- responsive.spec.ts calls page.setViewportSize() manually but tests run on all 5 projects including mobile device profiles that already set their own viewport. This creates ~15 redundant variants and silently overrides device viewports. Add test.use({ browserName: 'chromium' }) at the top to scope to desktop only.

  3. Skip-to-content test is a no-op -- In accessibility.spec.ts the skip link assertions are inside if (skipLinkExists) so the test always passes even with no skip link. Assert it unconditionally if a skip link is required, or add a comment explaining it is optional.

  4. Deprecated page.click() API -- navigation.spec.ts uses the deprecated page.click(selector) in four places. Prefer page.locator(selector).click().

  5. Missing trailing newline in justfile -- The PR fixed the missing newline for validate but introduced a new one at the bottom (qa-codegen has no trailing newline).

MINOR NOTES:

  • /sitemap-0.xml is hardcoded in content.spec.ts; could break if Astro changes the filename.
  • webServer.url is hardcoded to localhost:4321 and does not track BASE_URL unlike the baseURL setting.
  • qa:codegen also hardcodes localhost.

POSITIVES:

  • Good use of forbidOnly: !!process.env.CI to catch accidental test.only in CI.
  • --ignore-snapshots on the default qa script is sensible before a baseline exists.
  • Heading hierarchy check correctly allows going up levels while rejecting skipped-level descents.
  • External link security check (noopener + _blank) is a nice addition.

Happy to approve once items 1-3 are addressed; 4-5 are easy fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant