diff --git a/AGENTS.md b/AGENTS.md index 8910fb828..f2b8073ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,7 +150,7 @@ PR titles use Conventional Commits because GitHub uses them as merge subjects. T - Put code and tests in the established owning domain; inspect nearby structure before adding files. - `bun run check:arch` enforces boundaries: use public workspace exports, keep Core framework-neutral, keep app services out of views/shared UI, and keep property-panel internals scoped to that panel. -- Tests belong in `tests/e2e/**/*.spec.ts` (browser UI/visual), `tests/figma/**/*.spec.ts` (Figma automation), `tests/engine/**/*.test.ts` (engine/unit), `tests/helpers/**` (shared helpers), or an established package-local test location. Mirror source domains where practical and test behavior/contracts, not source text. Never commit temporary/profile specs. +- Follow the canonical [testing architecture](packages/docs/development/testing.md): package-local tests mirror source domains; central app tests mirror `src/app/**`; central integration requires a genuinely cross-owner contract. E2E follows user workflows; native and Figma acceptance remain explicit exceptions. Existing `tests/engine/**` domains migrate together with runner discovery—do not create competing homes or undiscovered suites. Owner-local helpers/fixtures stay local; only genuinely shared support is central. Specs use domain drivers/probes, not scattered Window/store traversal or unrestricted evaluator wrappers. Test contracts, not source text; never commit temporary/profile specs. ### File and folder naming diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f239a2c6d..d70b4f6f3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -75,15 +75,9 @@ Keep package boundaries and public exports intact. Keep pull requests focused: e ## Tests -Place tests in the established layer and mirror the source domain where practical: +Follow the [testing architecture](packages/docs/development/testing.md) for ownership, helpers, fixtures, browser adapters and migration rules. Package-local tests mirror their source domains; central integration is reserved for genuinely cross-owner contracts. E2E follows user workflows rather than implementation files. -- `tests/e2e/**/*.spec.ts` — browser UI and visual behavior. -- `tests/figma/**/*.spec.ts` — Figma automation. -- `tests/engine/**/*.test.ts` — engine and unit behavior. -- `tests/helpers/**` — shared test utilities. -- Package-local `tests/**` — standalone package coverage where that structure already exists. - -Test behavior and stable contracts, not source text or implementation details. Before adding a test file or helper, inspect nearby tests and follow their existing structure. +Existing `tests/engine/**` coverage moves domain-by-domain with runner discovery, not opportunistically during feature work. Extend the existing home until that migration; do not create a duplicate suite. Test behavior and stable contracts, not source text. During iteration, run focused checks for the changed contract rather than the complete suite after every edit. ### Test selectors diff --git a/packages/docs/development/testing.md b/packages/docs/development/testing.md index 0720355ab..efc585297 100644 --- a/packages/docs/development/testing.md +++ b/packages/docs/development/testing.md @@ -1,119 +1,101 @@ # Testing -## Overview +## Ownership and location -| Type | Framework | Command | Location | -| --------------------- | ---------- | -------------------- | --------------- | -| E2E visual regression | Playwright | `bun run test` | `tests/e2e/` | -| Figma CDP reference | Playwright | `bun run test:figma` | `tests/figma/` | -| Storybook components | Playwright | `bun run test:storybook` | `tests/e2e/storybook/` | -| Unit tests | bun:test | `bun run test:unit` | `tests/engine/` | +Place a test with the source that owns its contract. Choose the runtime separately: using a browser does not automatically make a test application E2E, and importing several packages does not automatically make it cross-package integration. -## E2E Visual Regression +| Contract | Canonical location | +| --------------------------------------- | -------------------------------------------------------------- | +| Package unit and in-process integration | `packages//tests//**/*.test.ts` | +| App services and state | `tests/app//**/*.test.ts` | +| Cross-package/system integration | `tests/integration//` | +| Application workflows and visuals | `tests/e2e//**/*.spec.ts` | +| Native WebView and shell delivery | `tests/e2e/native/**/*.spec.ts` | +| Figma compatibility acceptance | `tests/figma/**/*.spec.ts` | +| Private tool contracts | `tools//tests/` | -Playwright creates shapes on the canvas and compares screenshots against baseline PNGs. +For example, `packages/core/src/canvas/text/prepared.ts` maps to `packages/core/tests/canvas/text/prepared.test.ts`; `src/app/demo/viewport.ts` maps to `tests/app/demo/viewport.test.ts`. A substantial module may have a matching test directory rather than one large test file. Application E2E, external-system acceptance and large contract suites are intentional exceptions to file-for-file mirroring. + +Core tests using SceneGraph still belong to Core. Central integration is for contracts without a single owning implementation, such as interoperability between published packages. Name that contract explicitly; do not use integration as a miscellaneous bucket. + +**Migration status:** much existing coverage remains under `tests/engine/**`, alongside package-local suites. The table is the agreed destination, not a claim that migration or runner support is complete. Until a domain migrates, extend its existing suite rather than create a second home. Move a domain together with its discovery/shard configuration and imports. Do not move files into an undiscovered directory. The repository-wide migration is separate from feature work. + +## Test purpose + +- Unit tests cover isolated rules and state transitions. +- Package integration tests exercise real collaborators owned by that package. +- Browser integration tests cover browser CSS, WebGL, fonts and DOM behavior where that runtime matters. They need not launch the whole app if the contract can be exercised independently. +- E2E tests exercise application workflows, accessibility and representative visual integration. +- Native tests answer whether the real WebView or shell delivers an interaction. A CPU CanvasKit test is not native desktop acceptance; synthetic composition is not real IME acceptance. +- Benchmarks belong to performance tooling and opt-in runs, not ordinary correctness E2E. Bounded assertions about redundant work may accompany a real interaction, but exhaustive cache policy belongs with the owning implementation. + +Test observable behavior and contracts, not source text. A test claiming a UI update must observe the UI, not merely read a Set after calling an editor action. + +## Fixtures, builders, drivers and probes + +These responsibilities are distinct: + +| Kind | Responsibility | +| -------------- | ---------------------------------------------------------------------------- | +| Runner fixture | Acquire a context/editor/resource and guarantee teardown. | +| Data builder | Construct deterministic plain inputs; no browser, assertions or hidden I/O. | +| UI driver | Perform named user interactions through roles, labels and semantic controls. | +| Probe | Read a bounded typed observation, or install scoped instrumentation. | +| Asset fixture | Immutable document, font, image or reference data. | +| Snapshot | Expected visual output colocated with its spec. | + +Owner-specific builders and support belong under that owner's `tests/helpers//`; owner-specific assets belong under `tests/fixtures/` within the owning package. Central `tests/helpers//` and `tests/fixtures//` are reserved for genuine shared consumers, runner adapters and large shared corpora. Keep existing LFS assets in place until all consumers are mapped; binary moves are not a prerequisite for clarity. Record font licenses and fixture provenance beside the assets. + +Do not import another package's test internals or import test support from production. Share production contracts through public exports. Promote test support only when multiple owners actually need it. Avoid catch-all `utils`, `store` and `test-utils` collections with mixed runtime dependencies. Keep temporary diagnostics and profiling specs in ignored scratch space; extract a small durable regression when an investigation finds a bug. + +Prefer test-runner-owned fixtures, such as Playwright `test.extend`, over implicit shared pages. Serial mode is for an intentionally dependent scenario, not a side effect of importing a helper. A failed screenshot must not silently prevent unrelated contracts from executing. A fixture's housekeeping should not be counted as separate native acceptance. + +## Browser and native boundaries + +Specs should read as fixture setup, user action, observation and assertion. Do not scatter `window.openPencil`, store/renderer traversal, internal module URLs or resource-timing module discovery through specs. + +Keep unavoidable transport access in a small guarded adapter with named domain operations. Separate: + +1. Explicit fixture setup mutations. +2. User interactions through the UI. +3. Read-only serializable observations. +4. Scoped instrumentation with restoration and disposal in `finally`. + +Do not replace global digging with a generic `evaluateEditor(callback)` escape hatch or a giant driver exposing the mutable store. Use public named types and precise result projections; do not maintain inconsistent handcrafted copies of node shapes. A UI test must not call the editor action instead of performing the interaction it claims to test. A renderer test may deliberately use a bounded renderer probe because rendering is its subject. + +Existing global-based helpers are migration debt, not the preferred API. Migrate a complete domain slice rather than adding forwarding-only wrappers. Test-only browser entrypoints must stay out of production artifacts. Do not enlarge production Window declarations for fixtures or counters. Native adapters retain guarded vendor-typed invocation and the test-only Cargo feature; browser and native drivers need not share an implementation or pretend they support the same capabilities. + +## Execution and iteration + +Current commands: + +| Suite | Command | +| ----------------- | ------------------------ | +| Engine/unit | `bun run test:unit` | +| App browser E2E | `bun run test` | +| Storybook browser | `bun run test:storybook` | +| Figma acceptance | `bun run test:figma` | +| Native WebView | `bun run test:native` | + +During implementation, run the affected unit files or one representative browser scenario. Inspect discovery changes without executing the whole suite when reorganizing files. Use the final CI gate after integration; do not rerun full suites for each edit. Report commands actually run, and distinguish focused coverage from full acceptance. + +Visual changes require inspection and committed coverage. Update only a justified affected snapshot, then rerun that test without update mode. Never relax tolerances or regenerate unrelated baselines to turn a failed run green. Browser GPU parity and real glyph coverage complement CPU rendering tests; they are not redundant merely because both compare pixels. + +## Server ownership and worktrees + +The canonical `playwright.config.ts` owns app, Figma and Storybook projects. The `test`, `test:update`, `test:real-llm` and `test:figma` scripts select only the app server; `test:storybook` selects Storybook on port `6017`. Direct Playwright commands start both servers by default. Set `OPENPENCIL_TEST_SERVER=app`, `storybook` or `all`; `--project` selects tests, not servers. + +App tests start Vite from the current checkout and wait for its HTTP URL. Vite owns its MCP companion. Reuse is off by default and always off in CI. Default ports are app `1420` and MCP `7600`; concurrent worktrees need distinct free pairs: ```sh -bun run test # Run tests, compare against baselines -bun run test:update # Regenerate baseline screenshots +OPENPENCIL_TEST_SERVER=app OPENPENCIL_TEST_PORT=1482 OPENPENCIL_TEST_MCP_PORT=7682 \ + bunx playwright test tests/e2e/properties/effect-panel.spec.ts --project=openpencil ``` -### Server ownership and worktrees +Use the configured base URL and same-origin paths, never hard-coded ports in specs or adapters. For intentional debugging against a verified matching server, set `OPENPENCIL_TEST_REUSE_SERVER=1`. HTTP readiness alone does not prove checkout identity, so do not reuse servers for baseline comparisons. Portless remains the preferred interactive preview workflow, separate from managed fixed-port tests. -The canonical `playwright.config.ts` owns app, Figma, and Storybook projects. The `test`, `test:update`, `test:real-llm`, and `test:figma` scripts select only the app server; `test:storybook` selects only Storybook on port `6017`. Direct Playwright commands start both servers by default. Set `OPENPENCIL_TEST_SERVER=app`, `storybook`, or `all` to select servers explicitly; `--project` selects tests, not servers. +## External-system acceptance -App tests start Vite from the current checkout and wait for its HTTP URL. Vite starts and stops its MCP companion. Server reuse is off by default and always off in CI, so a test run cannot silently attach to another checkout on the default port. +Storybook uses its own viewport, device scale and reduced-motion context. Its specs are excluded from app projects. Figma acceptance requires the desktop app's configured CDP connection. -Defaults are app port `1420` and MCP port `7600`. For concurrent worktrees, choose a free, distinct pair: - -```sh -OPENPENCIL_TEST_PORT=1482 OPENPENCIL_TEST_MCP_PORT=7682 \ - bun run test tests/e2e/settings -``` - -The configuration passes the app origin and MCP port to Vite; the companion receives matching CORS configuration and a port-specific socket/discovery directory. Port conflicts fail rather than silently selecting another endpoint. Do not reuse ports across concurrent runs. - -For intentional local debugging against an already-running matching server, set `OPENPENCIL_TEST_REUSE_SERVER=1`. Do not use reuse for baseline comparisons: HTTP readiness does not establish checkout identity. Start a matching custom-port preview with `OPENPENCIL_DEV_ORIGIN=http://localhost:1482 OPENPENCIL_DEV_MCP_PORT=7682 bun run dev --port 1482`. Portless remains the preferred interactive worktree preview workflow, separate from managed fixed-port tests. - -### How It Works - -1. Tests load the editor in a headless browser -2. The editor signals readiness via a `data-ready` HTML attribute -3. Tests create shapes via the editor's API -4. Screenshots are taken and compared against baselines using `toMatchSnapshot` -5. Page is reused across test cases for speed (~2s total) - -### No-Chrome Test Mode - -The editor supports a test mode that hides UI chrome (toolbar, panels) for clean screenshot capture. Activated via URL parameter. - -## Storybook Component Tests - -```sh -bun run test:storybook -bun run test:storybook --list -``` - -The `storybook-chromium` project preserves its own viewport, device scale, screenshot defaults, and reduced-motion browser context. Storybook specs are excluded from the app projects. Snapshot updates use `bun run test:storybook --update-snapshots`. - -## Figma CDP Reference Tests - -A separate Playwright project connects to Figma via Chrome DevTools Protocol to capture reference screenshots for pixel-perfect comparison. - -```sh -bun run figma:debug # Launch Figma with debugging port -bun run test:figma # Connect to Figma, capture references -``` - -Requires Figma desktop app running with `--remote-debugging-port=9222`. - -## Unit Tests - -Engine unit tests use bun:test and target < 50ms execution: - -```sh -bun run test:unit -``` - -Tests cover: - -- Scene graph CRUD operations, parent-child relationships, z-ordering, hit testing -- **Fig-import pipeline** — node type mapping, transforms, fills/strokes/effects, gradients, images, arcs, nested hierarchies (`tests/engine/io/fig/import/legacy/*.test.ts`) -- **Layout computation** — Yoga auto-layout: direction, gap, padding, justify, align, child sizing (fixed/fill/hug), cross-axis sizing, wrap, nested layouts (`tests/engine/layout/`) - -### Writing Unit Tests - -```typescript -import { describe, expect, it } from 'bun:test' -import { SceneGraph } from '@open-pencil/scene-graph' - -describe('SceneGraph', () => { - it('creates and retrieves a node', () => { - const sg = new SceneGraph() - const node = sg.createNode('RECTANGLE', sg.root, { name: 'Test' }) - expect(sg.getNode(node.guid)).toBeDefined() - }) -}) -``` - -## E2E Test Coverage - -| Test file | Scope | -| -------------------------------- | --------------------------------------------------------------- | -| `tests/e2e/layers-panel.spec.ts` | Layers panel tree structure, visibility toggles, selection sync | -| `tests/e2e/visual.spec.ts` | Visual regression screenshots for shapes and rendering | - -## Test Helpers - -| File | Purpose | -| ------------------------- | -------------------------------------- | -| `tests/helpers/canvas.ts` | Canvas setup and interaction utilities | -| `tests/helpers/figma.ts` | Figma CDP connection helpers | - -## Performance Targets - -| Metric | Target | -| --------------------- | ----------------------------- | -| E2E suite total | < 3s | -| Unit test suite total | < 50ms | -| Screenshot comparison | toMatchSnapshot (pixel-level) | +Native tests use an explicit test-only binary, separate application identity and test-owned credentials/profile. Never run acceptance against production secrets or clear user recovery data. Skip unsupported platforms rather than claim coverage. Ordinary WebDriver text input, trusted clipboard events, real IME and persistence across restarts are separate contracts. diff --git a/tests/e2e/native/text-editing.spec.ts b/tests/e2e/native/text-editing.spec.ts index 2ee9965e6..a2d62f3df 100644 --- a/tests/e2e/native/text-editing.spec.ts +++ b/tests/e2e/native/text-editing.spec.ts @@ -9,6 +9,16 @@ import { withNativeEventRecorder } from '#tests/helpers/tauri/event-recorder' describe('native text editing', () => { it('commits ordinary WebView input exactly once', async () => { await createNativeTextFixture('Replace me') + const tabCount = await browser.execute(() => document.querySelectorAll('[role="tab"]').length) + const id = await createNativeTextFixture('Another fixture') + const prepared = await readNativeEditorSnapshot() + assert.equal(prepared.editingTextId, id) + assert.equal(prepared.editingText, 'Another fixture') + assert.equal(prepared.textNodeCount, 1) + assert.equal( + await browser.execute(() => document.querySelectorAll('[role="tab"]').length), + tabCount + ) const textarea = await $('textarea[aria-hidden="true"]') await textarea.waitForExist() @@ -38,18 +48,4 @@ describe('native text editing', () => { ) }) }) - - it('reuses the open document when preparing another text fixture', async () => { - const tabCount = await browser.execute(() => document.querySelectorAll('[role="tab"]').length) - const id = await createNativeTextFixture('Another fixture') - const snapshot = await readNativeEditorSnapshot() - - assert.equal(snapshot.editingTextId, id) - assert.equal(snapshot.editingText, 'Another fixture') - assert.equal(snapshot.textNodeCount, 1) - assert.equal( - await browser.execute(() => document.querySelectorAll('[role="tab"]').length), - tabCount - ) - }) })