test: add Playwright e2e testing framework with core scenario coverage

[wyuc]

Closes #230

Summary

• Set up Playwright e2e testing infrastructure (playwright.config.ts, fixtures, Page Object Models)
• Implement MockApi class using page.route() to intercept API calls with fixture data (zero source code invasion)
• Add 3 test specs covering the main happy-path user flow:
  • home-to-generation: Home page loads → type requirement → click "Enter Classroom" → navigate to generation-preview
  • generation-flow: Mock SSE outline streaming + scene content/actions APIs → generation pipeline completes → auto-redirect to classroom
  • classroom-interaction: Seed IndexedDB with stage + scenes → classroom loads → sidebar shows scenes → switch scenes
• Add e2e CI job to GitHub Actions (Chromium only, production build)

Test plan

• pnpm test:e2e — all 3 specs pass locally (11.2s)
• pnpm check — Prettier clean
• pnpm lint — no new errors
• CI e2e job runs on this PR

🤖 Generated with Claude Code

[cosarah]

Code Review: Playwright E2E Testing Framework

Overall this is a well-structured PR that introduces a clean Playwright e2e testing infrastructure with good separation of concerns (fixtures, Page Object Models, test data). The zero-source-code-invasion approach using page.route() for API mocking is solid. A few issues to address:

---

Bug: Fragile / Incorrect Selector

e2e/tests/classroom-interaction.spec.ts:162

await expect(page.getByRole('heading', { name: '光反应' })).toBeVisible();

Classroom page and scene-sidebar.tsx do not render any <h1>–<h6> or role="heading" element for the current scene name. The scene title in the sidebar is a plain <span class="...truncate...">. This assertion will likely fail at runtime.

Suggestion: Either change the assertion to match the actual sidebar markup (e.g. check which scene card has an "active" class), or add a heading element to the classroom page to display the current scene title.

---

Bug: Fragile CSS-class Selectors

e2e/pages/classroom.page.ts:12

this.sidebarScenes = page.locator('.overflow-y-auto.space-y-2 > div');

Targeting Tailwind utility class names is brittle — they can change during routine styling work. Consider adding data-testid attributes to the sidebar container and scene items, e.g. data-testid="scene-list" and data-testid="scene-item".

Same applies to span.truncate (line 28) for scene titles.

---

Suggestion: Duplicated Constants

SETTINGS_STORAGE and defaultTheme are defined in multiple test files (classroom-interaction.spec.ts, generation-flow.spec.ts, home-to-generation.spec.ts). Consider extracting them into a shared fixture file like e2e/fixtures/test-data/constants.ts to keep things DRY and make future schema changes easier.

---

Suggestion: setupGenerationMocks double-mocks serverProviders

e2e/fixtures/mock-api.ts:70-75

setupGenerationMocks() calls mockServerProviders(), but the base.ts fixture already calls mockServerProviders() on every test via the mockApi fixture. Calling it twice is harmless but misleading — either remove it from setupGenerationMocks() or document that it's idempotent.

---

Minor: CI Job — Missing pnpm test in existing lint job

The diff adds a Unit Tests step (pnpm test) to the existing lint job. This is fine but the job name is "Lint / Type Check" — it may be worth updating the name to reflect that it now also runs unit tests, or splitting it into its own job.

---

Minor: Hardcoded Port

playwright.config.ts:11 — baseURL: 'http://localhost:3002'

Port 3002 is fine as a convention, but if another service is running on that port, tests will silently connect to the wrong server. The webServer.env.PORT is set to match, which is good — just worth noting in a comment or README.

---

Summary

Area	Verdict
Architecture	✅ Good — fixtures, POM, test-data separation
API Mocking	✅ Clean page.route() approach
Test Coverage	✅ Covers core happy-path flow
Selectors	⚠️ Needs fix — heading assertion will fail; CSS-class selectors are fragile
DRY	⚠️ Minor — duplicated constants
CI Integration	✅ Good — artifact upload on failure

Main blocker: fix the getByRole('heading') assertion that doesn't match actual markup.

🤖 Generated with Claude Code

[DataCrumbsAI]

Review Feedback Addressed

Thanks for the thorough review @cosarah! All must-fix and should-fix items have been implemented. Here's a summary of what changed:

Bug Fixes (Must Fix)

1. Wrong heading selector (classroom-interaction.spec.ts:162)

• Added data-testid="scene-title-heading" to the <h1> in components/header.tsx
• Updated ClassroomPage to expose sceneTitleHeading = page.getByTestId('scene-title-heading')
• Test assertion updated to await expect(classroom.sceneTitleHeading).toContainText('光反应') — explicit and independent of ARIA role assumptions

2. Fragile CSS-class selectors (classroom.page.ts:12)

• Added data-testid="scene-list" to the scenes list container in components/stage/scene-sidebar.tsx
• Added data-testid="scene-item" to each scene item <div>
• Added data-testid="scene-item-title" to the title <span>
• ClassroomPage now uses page.locator('[data-testid="scene-item"]') and getByTestId('scene-item-title') — no more Tailwind class dependency

Refactoring (Should Fix)

3. Duplicated constants

• Extracted SETTINGS_STORAGE and defaultTheme into e2e/fixtures/test-data/constants.ts
• All three test files now import from the shared module

4. Double-mocked serverProviders

• Removed the redundant this.mockServerProviders() call from setupGenerationMocks() — the base fixture already covers it

---

Commit: ed35ad3 on DataCrumbsAI/OpenMAIC@worktree-playwright-e2e

Note: The minor items (CI job name, port comment) are acknowledged — they can be addressed as a follow-up if desired.

🤖 Generated with Claude Code

[wyuc]

Thanks for the review!

Already fixed in 9fcba8b:

• CSS-class selectors → replaced with data-testid attributes
• Duplicated constants → extracted to shared helpers
• Double mockServerProviders() → removed from setupGenerationMocks()

Re: getByRole('heading') concern — the current scene title is rendered as <h1> in components/header.tsx:97, so the assertion is correct. CI confirms it passes.

[cosarah]

Re-review

Previous round feedback all addressed, confirmed working. LGTM overall — clean architecture, solid mock strategy.

One thing to fix before merge:

e2e/ should be excluded from tsconfig.json

tsconfig.json currently includes all **/*.ts files, which pulls e2e tests into Next.js compilation. As the test suite grows and uses Node/Playwright-specific APIs, this will cause false type errors.

Fix: add "e2e" to the exclude array in tsconfig.json, or create a dedicated e2e/tsconfig.json.

🤖 Generated with Claude Code

[cosarah]

LGTM