From 4e92baf5d073a3700231ecdcf4793035f9ff1a6f Mon Sep 17 00:00:00 2001 From: Viktor Vaczi Date: Fri, 3 Jul 2026 12:43:35 +0200 Subject: [PATCH] test(ysync): repro tests for review bugs 01-07 + v2 items-wire e2e port (miss 11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-07-02 sync review (docs/features/ysync-review, ysync-review branch) found 7 bugs and that the two-tab e2e only exercised the DEAD legacy scalar wire. This lands plan doc 15 in full; results + empirical findings in doc 16. - tests/collab/browser-entry-v2.ts (+build.mjs): the PRODUCTION v2 stack bundled for e2e (connectKicadDoc + attachKicadCollab, kdoc_* keys), with in-page renderActiveDoc/singleSeedRender/driftReport helpers and yjs forced to ONE copy (the two web pnpm workspaces otherwise bundle two instanceof-incompatible instances). - tests/kicad/ysync-two-tab.spec.ts: pl_editor green baseline (A↔B edits, ITEM-level drift silence) + divergent-uuid adopt; bug-01 pcb/ee fresh-room repros (Chromium-only: two kicad_editor tabs exceed Firefox's per-process wasm budget); bug-06 concurrent-seed race; bug-03 Y-half. - tests/kicad/ysync-repros-{pcbnew,eeschema}.spec.ts: bugs 02/03/05 + the bug-04 matrix (anchor-centred fp rotation, pad resize, endpoint drag, symbol rotation, Value-field edit), each with green landed-preconditions; the "local move emits" controls double as headless-emit probes — GREEN on both tools, so every emit-dependent repro is a live test.fail. - wasm/bindings: 7 local-edit test hooks via real commits (CallAfter+COROUTINE) — TestRemoveItem/TestRotateItem (both tools, dispatched in the merged image), TestSetPadSize/TestMoveEndpoint (pcbnew), TestSetFieldText (eeschema). - web/standalone ysync-repros.test.ts: bug-01 units (C++-faithful fake gating emit on ensureBridge) + bug-07a/b (stale DOWN hook, real sheet-manager gap). - web/pcbjam-shared bump: bug-03/06 unit repros. Convention: every repro asserts the CORRECT behavior and is expected-fail (test.fail/it.fails) naming its bug doc; a fix flips it to "unexpected pass", forcing marker removal — the repro becomes the regression test. Every expected failure verified (JSON reporter) to fail at its documented assert. Suite state: 39 passed / 0 failed / 0 flaky / 5 skipped (2 firefox guards, 2 pre-existing legacy two-tab skips, 1 pre-existing roundtrip fixme). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DPfrVhfYgPPgtawjSssZfn --- tests/README.md | 33 ++ tests/collab/browser-entry-v2.ts | 154 ++++++ tests/collab/build.mjs | 25 + tests/kicad/ysync-repros-eeschema.spec.ts | 257 +++++++++ tests/kicad/ysync-repros-pcbnew.spec.ts | 522 ++++++++++++++++++ tests/kicad/ysync-two-tab.spec.ts | 503 +++++++++++++++++ tests/playwright-kicad.config.ts | 6 + wasm/bindings/eeschema_embind.cpp | 119 ++++ wasm/bindings/kicad_editor_embind.cpp | 19 + wasm/bindings/pcbnew_embind.cpp | 142 +++++ web/pcbjam-shared | 2 +- .../src/wasm/collab/ysync-repros.test.ts | 249 +++++++++ 12 files changed, 2030 insertions(+), 1 deletion(-) create mode 100644 tests/collab/browser-entry-v2.ts create mode 100644 tests/kicad/ysync-repros-eeschema.spec.ts create mode 100644 tests/kicad/ysync-repros-pcbnew.spec.ts create mode 100644 tests/kicad/ysync-two-tab.spec.ts create mode 100644 web/standalone/src/wasm/collab/ysync-repros.test.ts diff --git a/tests/README.md b/tests/README.md index b16ada5..c1e9b23 100644 --- a/tests/README.md +++ b/tests/README.md @@ -304,3 +304,36 @@ Button positions (relative to canvas): shim ablated*. **To do:** sweep the real apps (modals, nested fibers, long sleeps, pthread pool) with each shim ablated; if all stay green, drop the shim injection and these pins. Until proven, they stay injected (belt-and-suspenders). + +## Collab e2e — legacy vs v2 bundles, and repro markers + +Two esbuild bundles (`npm run build:collab`, rebuilt by the specs' `beforeAll`): + +- `apps/kicad/collab-bundle.js` — the LEGACY scalar wire (`startCollab` / + `kicadCollabSnapshot/Apply` / `onDelta`). Dead in production (nothing registers + `onDelta`); driven by the pre-existing `*-collab.spec.ts` two-tab tests. Kept only + until the scalar wire is deleted. +- `apps/kicad/collab-bundle-v2.js` — the PRODUCTION v2 "items" stack + (`bindKicadCollab` over `kicadCollabSnapshotItems/ApplyItems/onItems`, Y keys + `kdoc_*`), from `collab/browser-entry-v2.ts`. Driven by `kicad/ysync-two-tab.spec.ts`. + The build aliases `yjs` to ONE copy (the two web pnpm workspaces otherwise bundle two, + and Y types are instanceof-checked). + +### ysync repro tests + +`kicad/ysync-two-tab.spec.ts`, `kicad/ysync-repros-{pcbnew,eeschema}.spec.ts` reproduce +the bugs of the 2026-07-02 Yjs⇄KiCad sync review (`docs/features/ysync-review/` on the +`ysync-review` branch; unit-level repros live in +`web/pcbjam-shared/test/ysync-repros.test.ts` and +`web/standalone/src/wasm/collab/ysync-repros.test.ts`). + +Convention: a repro asserts the CORRECT behavior and is marked expected-fail +(`test.fail()` / vitest `it.fails`) with a comment naming the bug doc. The suite stays +green while the bug is open; fixing the bug flips the repro to "unexpected pass", +forcing the marker's removal — the repro becomes the regression test. Green companion +tests pin each repro's preconditions (harness, apply path, emit path) so an expected +failure can only come from the bug itself. The "local move emits" controls double as +the headless-emit probes gating the emit-dependent repros. + +Follow-up (tracked in review miss 11): once the v2 specs are trusted, un-skip/retire +the legacy two-tab specs together with the legacy wire. diff --git a/tests/collab/browser-entry-v2.ts b/tests/collab/browser-entry-v2.ts new file mode 100644 index 0000000..077152b --- /dev/null +++ b/tests/collab/browser-entry-v2.ts @@ -0,0 +1,154 @@ +// Browser bundle entry for the V2 "items" collab e2e — the PRODUCTION stack +// (ysync 0008): startKicadCollab → connectKicadDoc + bindKicadCollab over +// moduleItemsBridge, Y keys kdoc_*, C++ exports kicadCollabSnapshotItems / +// kicadCollabApplyItems / window.kicadCollab.onItems. +// +// The sibling browser-entry.ts drives the LEGACY scalar wire (startCollab / +// onDelta), which is DEAD in production — nothing registers onDelta; WasmTool +// binds onItems only (ysync-review miss 11). New collab e2e must load THIS +// bundle; the legacy one stays only until the scalar wire is deleted. +// +// Build: npm run build:collab (tests/) → tests/apps/kicad/collab-bundle-v2.js +// +// IMPORTANT (build.mjs): yjs is aliased to ONE physical copy. web/standalone and +// web/pcbjam-shared are separate pnpm workspaces, so without the alias the +// bundle carries two yjs instances (the legacy bundle demonstrably does) — and +// the v2 path breaks on that (Y types are instanceof-checked singletons; same +// reason the standalone vitest config sets `dedupe: ["yjs"]`). +import * as Y from "yjs"; +import { + docDelta, + docToFile, + docToY, + fileToDoc, + isEmptyKicadDelta, + yToDoc, +} from "@pcbjam/shared"; +import { + attachKicadCollab, + connectKicadDoc, + type KicadCollabHandle, + type KicadItemsModule, + type KicadItemsWindow, +} from "../../web/standalone/src/wasm/collab/index"; + +interface StartOpts { + room: string; + /** BroadcastChannel settle window before the seed-vs-adopt decision. */ + settleMs?: number; + /** + * Full file text; when set and the room is EMPTY, the Y.Doc is file-seeded + * from fileToDoc(seedText) — the production first-tab path (and the branch + * bug 01 lives in). Omit to exercise the editor-snapshot / adopt branches. + */ + seedText?: string; + /** + * The ydoc-load path: the editor opened exactly this doc's content, so seed + * only baselines the wasm differ instead of running the adopt apply. + */ + editorMatchesDoc?: boolean; +} + +/** Start the v2 stack; the handle lands on window.__collabV2 for in-page asserts. */ +async function start( + mod: KicadItemsModule, + win: KicadItemsWindow, + opts: StartOpts, +): Promise { + // startKicadCollab's body, split so editorMatchesDoc is reachable (the + // production WasmTool uses the same connect + attach pair for ydoc mode). + const session = await connectKicadDoc({ + provider: { kind: "broadcastchannel", settleMs: opts.settleMs ?? 400 }, + room: opts.room, + }); + const h = attachKicadCollab(mod, win, session, { + seedDoc: opts.seedText ? fileToDoc(opts.seedText) : undefined, + editorMatchesDoc: opts.editorMatchesDoc, + }); + (window as unknown as { __collabV2?: KicadCollabHandle }).__collabV2 = h; +} + +function handle(): KicadCollabHandle { + const h = (window as unknown as { __collabV2?: KicadCollabHandle }).__collabV2; + if (!h) throw new Error("KicadCollabV2: start() has not completed"); + return h; +} + +/** docToFile of the live room doc — THROWS if the doc stopped materializing. */ +function renderActiveDoc(): string { + return docToFile(yToDoc(handle().doc)); +} + +/** What ONE seeder would materialize — the bug-06 reference rendering. */ +function singleSeedRender(seedText: string): string { + const ydoc = new Y.Doc(); + try { + docToY(fileToDoc(seedText), ydoc); + return docToFile(yToDoc(ydoc)); + } finally { + ydoc.destroy(); + } +} + +export interface DriftSummary { + added: string[]; + updated: string[]; + removed: string[]; + layoutChanged: boolean; + metaChanged: boolean; +} + +/** + * The drift-detect convergence oracle, replicating computeDrift's core + * (web/standalone/src/wasm/collab/drift-detect.ts:94-118) from @pcbjam/shared + * primitives only — drift-detect itself pulls `@/lib/api`, so it can't be + * bundled here. Serializes the live model via the tool's save fn, diffs it + * against the room doc; null means editor ≡ doc. + */ +function driftReport(saveFn: string, scratchPath: string): DriftSummary | null { + const w = window as unknown as { + Module: Record void>; + FS: { + readFile(p: string, o: { encoding: "utf8" }): string; + unlink(p: string): void; + }; + }; + w.Module[saveFn]!(scratchPath); + let text: string; + try { + text = w.FS.readFile(scratchPath, { encoding: "utf8" }); + } finally { + try { + w.FS.unlink(scratchPath); + } catch { + /* scratch cleanup is best-effort */ + } + } + const wasmDoc = fileToDoc(text); + const ydocDoc = yToDoc(handle().doc); + const diff = docDelta(ydocDoc, wasmDoc); + const layoutChanged = + JSON.stringify(ydocDoc.layout) !== JSON.stringify(wasmDoc.layout); + const metaChanged = ydocDoc.root !== wasmDoc.root; + if (isEmptyKicadDelta(diff) && !layoutChanged && !metaChanged) return null; + return { + added: diff.added.map((i) => i.uuid), + updated: diff.updated.map((i) => i.uuid), + removed: diff.removed, + layoutChanged, + metaChanged, + }; +} + +declare global { + interface Window { + KicadCollabV2?: { + start: typeof start; + renderActiveDoc: typeof renderActiveDoc; + singleSeedRender: typeof singleSeedRender; + driftReport: typeof driftReport; + }; + } +} + +window.KicadCollabV2 = { start, renderActiveDoc, singleSeedRender, driftReport }; diff --git a/tests/collab/build.mjs b/tests/collab/build.mjs index 9b1d5ce..e8ed4c0 100644 --- a/tests/collab/build.mjs +++ b/tests/collab/build.mjs @@ -23,3 +23,28 @@ await build({ }); console.log("collab bundle built → apps/kicad/collab-bundle.js"); + +// The V2 ("items") bundle — the PRODUCTION collab stack (see browser-entry-v2.ts). +await build({ + entryPoints: [path.join(testsDir, "collab/browser-entry-v2.ts")], + bundle: true, + format: "iife", + outfile: path.join(testsDir, "apps/kicad/collab-bundle-v2.js"), + nodePaths: [path.join(testsDir, "node_modules")], + external: ["y-partyserver/provider", "@hocuspocus/provider"], + alias: { + // web/standalone and web/pcbjam-shared are separate pnpm workspaces, so + // their `yjs` imports resolve to two physical copies. The v2 binding hands + // Y types across that boundary (instanceof-checked), so force ONE copy — + // the tests devDep — exactly like the standalone vitest `dedupe: ["yjs"]`. + yjs: path.join(testsDir, "node_modules/yjs"), + // Lets this entry (which lives under tests/, outside the web workspaces) + // import the shared lib by name, resolving to the SAME source instance the + // standalone collab modules bundle. + "@pcbjam/shared": path.join(testsDir, "../web/pcbjam-shared/src/index.ts"), + }, + logLevel: "info", + target: "es2020", +}); + +console.log("collab v2 bundle built → apps/kicad/collab-bundle-v2.js"); diff --git a/tests/kicad/ysync-repros-eeschema.spec.ts b/tests/kicad/ysync-repros-eeschema.spec.ts new file mode 100644 index 0000000..e6902a2 --- /dev/null +++ b/tests/kicad/ysync-repros-eeschema.spec.ts @@ -0,0 +1,257 @@ +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * eeschema single-tab ysync coverage (docs in docs/features/ysync-review on + * the ysync-review branch) — items-bridge.spec.ts driving style. + * + * Today this file holds the HEADLESS EMIT PROBE (plan phase C): whether a real + * local commit emits the v2 items wire in the headless harness once the + * listener IS registered. The legacy two-tab skip in eeschema-collab.spec.ts + * cites a rationale its own single-page test documents as stale ("predated the + * dyncall-shim fix — apply now works"); the EMIT half was never verified. If + * this probe is green, the bug-01 eeschema two-tab repro in + * ysync-two-tab.spec.ts stays a live test.fail; if red, it becomes test.fixme. + * + * The bug-04 edit matrix (rotate / field-text — plan phase D) lands here once + * the C++ test hooks exist in the wasm build. + */ + +const WIRE1 = "22222222-0000-0000-0000-000000000001"; +const WIRE2 = "22222222-0000-0000-0000-000000000002"; +// A real placed symbol (embedded Device:R) — the bug-04 rotate / field-text +// target: symbol rotation leaves GetPosition() unchanged, and fields are not +// in screen->Items() at all. +const SYM1 = "44444444-0000-0000-0000-000000000001"; + +const SAMPLE_SCH = `(kicad_sch +\t(version 20250114) +\t(generator "eeschema") +\t(generator_version "9.0") +\t(uuid "11111111-1111-1111-1111-111111111111") +\t(paper "A4") +\t(lib_symbols +\t\t(symbol "Device:R" (pin_numbers (hide yes)) (pin_names (offset 0)) (exclude_from_sim no) (in_bom yes) (on_board yes) +\t\t\t(property "Reference" "R" (at 2.032 0 90) (effects (font (size 1.27 1.27)))) +\t\t\t(property "Value" "R" (at 0 0 90) (effects (font (size 1.27 1.27)))) +\t\t\t(symbol "R_0_1" +\t\t\t\t(rectangle (start -1.016 -2.54) (end 1.016 2.54) (stroke (width 0.254) (type default)) (fill (type none))) +\t\t\t) +\t\t\t(symbol "R_1_1" +\t\t\t\t(pin passive line (at 0 3.81 270) (length 1.27) (name "~" (effects (font (size 1.27 1.27)))) (number "1" (effects (font (size 1.27 1.27))))) +\t\t\t\t(pin passive line (at 0 -3.81 90) (length 1.27) (name "~" (effects (font (size 1.27 1.27)))) (number "2" (effects (font (size 1.27 1.27))))) +\t\t\t) +\t\t) +\t) +\t(wire (pts (xy 50.8 50.8) (xy 101.6 50.8)) (stroke (width 0) (type default)) (uuid "${WIRE1}")) +\t(wire (pts (xy 50.8 76.2) (xy 101.6 76.2)) (stroke (width 0) (type default)) (uuid "${WIRE2}")) +\t(symbol (lib_id "Device:R") (at 63.5 63.5 0) (unit 1) (exclude_from_sim no) (in_bom yes) (on_board yes) (dnp no) +\t\t(uuid "${SYM1}") +\t\t(property "Reference" "R1" (at 66.04 62.23 0) (effects (font (size 1.27 1.27)) (justify left))) +\t\t(property "Value" "10k" (at 66.04 64.77 0) (effects (font (size 1.27 1.27)) (justify left))) +\t\t(property "Footprint" "" (at 0 0 0) (effects (font (size 1.27 1.27)) (hide yes))) +\t\t(property "Datasheet" "" (at 0 0 0) (effects (font (size 1.27 1.27)) (hide yes))) +\t\t(pin "1" (uuid "44444444-0000-0000-0000-0000000000a1")) +\t\t(pin "2" (uuid "44444444-0000-0000-0000-0000000000a2")) +\t\t(instances (project "rt" (path "/11111111-1111-1111-1111-111111111111" (reference "R1") (unit 1)))) +\t) +\t(sheet_instances (path "/" (page "1"))) +) +`; + +type FS = { + mkdirTree(p: string): void; + writeFile(p: string, d: string): void; + readFile(p: string, o: { encoding: "utf8" }): string; +}; +type Mod = { + kicadOpenFile(p: string): unknown; + kicadCollabSnapshotItems(): string; + kicadCollabApplyItems(j: string): unknown; + kicadCollabTestMoveFirst(dx: number, dy: number): string; + kicadCollabGetPos(id: string): string; + kicadSaveSchematic(p: string): unknown; +}; + +const BOOT_TIMEOUT = 150000; + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +async function bootOpen(page: Page): Promise { + await page.goto("/kicad/eeschema.html"); + await expect(page.locator("#canvas")).toBeVisible({ timeout: BOOT_TIMEOUT }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: BOOT_TIMEOUT }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Mod }).Module; + return ( + typeof m?.kicadOpenFile === "function" && + typeof m?.kicadCollabSnapshotItems === "function" && + typeof m?.kicadCollabApplyItems === "function" && + typeof m?.kicadCollabTestMoveFirst === "function" && + typeof m?.kicadSaveSchematic === "function" + ); + }, + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.evaluate((content) => { + const w = window as unknown as { FS: FS; Module: Mod }; + try { + w.FS.mkdirTree("/home/kicad/documents"); + } catch { + /* exists */ + } + const p = "/home/kicad/documents/rt.kicad_sch"; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, SAMPLE_SCH); +} + +test.describe("eeschema ysync repros (v2 items wire, single tab)", () => { + test.describe.configure({ timeout: 420000 }); + + test("control: a local move emits the v2 items wire (HEADLESS EMIT PROBE)", async ({ + page, + testLogger, + }) => { + await bootOpen(page); + // snapshotItems: registers the SCHEMATIC_LISTENER (ensureBridge) + + // baselines the differ — what seed()'s non-file-seed branches rely on. + await page.evaluate(() => window.Module.kicadCollabSnapshotItems()); + await page.evaluate(() => { + (window as unknown as { __items: string[] }).__items = []; + (window as unknown as { kicadCollab: object }).kicadCollab = { + onItems: (j: string) => (window as unknown as { __items: string[] }).__items.push(j), + }; + }); + + const movedId = (await page.evaluate(() => + window.Module.kicadCollabTestMoveFirst(200000, 0), + )) as string; + expect(movedId).toMatch(/[0-9a-f-]{36}/); + + // listener → scheduleFlush → flushDiff → emitItems: the moved item's uuid + // must appear in an emitted wire. Phase-C gate for the eeschema bug-01 + // two-tab repro and the phase-D bug-04 matrix. + await expect + .poll( + async () => + await page.evaluate( + () => (window as unknown as { __items: string[] }).__items.join("\n"), + ), + { timeout: 20000, intervals: [400] }, + ) + .toContain(movedId); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + // ── Bug 04 matrix — edits invisible to the scalar-projection differ ──────── + // 04-bug-lossy-change-detection.md. Same shape as the pcbnew matrix: real + // commit via a repro hook → green precondition the edit LANDED → expected- + // fail that it EMITTED. Gated on the wasm build carrying the hooks. + + function saveRead(page: Page): Promise { + return page.evaluate(() => { + const w = window as unknown as { FS: FS; Module: Mod }; + const out = "/home/kicad/documents/probe.kicad_sch"; + w.Module.kicadSaveSchematic(out); + return w.FS.readFile(out, { encoding: "utf8" }); + }); + } + + function emittedWires(page: Page): Promise { + return page.evaluate(() => + (window as unknown as { __items: string[] }).__items.join("\n"), + ); + } + + /** Baseline + capture + hook-presence guard (returns false on stale wasm). */ + async function armed(page: Page, hook: string): Promise { + await bootOpen(page); + const has = await page.evaluate( + (h) => typeof (window as unknown as { Module: Record }).Module[h] === "function", + hook, + ); + if (!has) return false; + await page.evaluate(() => window.Module.kicadCollabSnapshotItems()); + await page.evaluate(() => { + (window as unknown as { __items: string[] }).__items = []; + (window as unknown as { kicadCollab: object }).kicadCollab = { + onItems: (j: string) => (window as unknown as { __items: string[] }).__items.push(j), + }; + }); + return true; + } + + test("an in-place symbol rotation reaches the wire", async ({ page, testLogger }) => { + test.fail(); // bug 04 — GetPosition() unchanged; no orientation in the json + + const ok = await armed(page, "kicadCollabTestRotateItem"); + test.skip(!ok, "wasm build predates the ysync repro hooks"); + + const queued = await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestRotateItem(i: string, d: number): boolean } }) + .Module.kicadCollabTestRotateItem(id, 90), + SYM1, + ); + expect(queued, "rotate hook resolved the symbol").toBe(true); + + // Green precondition: the rotation LANDED (the saved symbol's placement + // gained an angle). + await expect + .poll(async () => /\(at 63.5 63.5 (90|180|270)\)/.test(await saveRead(page)), { + timeout: 15000, + intervals: [400], + }) + .toBe(true); + + // CORRECT: the rotation is broadcast. TODAY: the scalar diff is empty. + await expect + .poll(async () => await emittedWires(page), { timeout: 8000, intervals: [400] }) + .toContain(SYM1); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("a symbol Value field edit reaches the wire", async ({ page, testLogger }) => { + test.fail(); // bug 04 — fields are invisible to the snapshot entirely + + const ok = await armed(page, "kicadCollabTestSetFieldText"); + test.skip(!ok, "wasm build predates the ysync repro hooks"); + + const queued = await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestSetFieldText(i: string, t: string): boolean } }) + .Module.kicadCollabTestSetFieldText(id, "22k"), + SYM1, + ); + expect(queued, "field hook resolved the symbol").toBe(true); + + // Green precondition: the edit LANDED in the model. + await expect + .poll(async () => (await saveRead(page)).includes(`"22k"`), { + timeout: 15000, + intervals: [400], + }) + .toBe(true); + + // CORRECT: the most common schematic edit after moving things is + // broadcast. TODAY: nothing emits — the value edit stays local forever. + await expect + .poll(async () => await emittedWires(page), { timeout: 8000, intervals: [400] }) + .toContain(SYM1); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); +}); diff --git a/tests/kicad/ysync-repros-pcbnew.spec.ts b/tests/kicad/ysync-repros-pcbnew.spec.ts new file mode 100644 index 0000000..3d79f7a --- /dev/null +++ b/tests/kicad/ysync-repros-pcbnew.spec.ts @@ -0,0 +1,522 @@ +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * pcbnew single-tab repros for the C++-side ysync bugs (docs in + * docs/features/ysync-review on the ysync-review branch) — items-bridge.spec.ts + * driving style: the v2 exports are called directly, no Y.Doc/bundle. + * + * REPRO CONVENTION: each repro asserts the CORRECT behavior and is marked + * `test.fail()` with a comment naming the bug doc; fixing the bug flips it to + * "unexpected pass", forcing the marker's removal. Green companion tests pin + * the preconditions so an expected failure can only come from the bug itself. + * Expected-fail polls use short timeouts so they don't burn the clock. + * + * The "local move emits" control doubles as the HEADLESS EMIT PROBE (plan + * phase C): if it is red, the emit-dependent repros here and the bug-01 + * two-tab repros in ysync-two-tab.spec.ts cannot run headless → test.fixme. + */ + +const FP1 = "66666666-0000-0000-0000-000000000001"; +const FP1_TXT = "66666666-0000-0000-0000-0000000000cc"; +const PAD1 = "66666666-0000-0000-0000-0000000000d1"; +const PAD2 = "66666666-0000-0000-0000-0000000000d2"; +const VIA1 = "77777777-0000-0000-0000-000000000001"; +const SEG1 = "88888888-0000-0000-0000-000000000001"; +const SEG2 = "88888888-0000-0000-0000-000000000002"; +const NEW_FP = "99999999-0000-0000-0000-000000000009"; +// Bug-04 rotation target: every child sits EXACTLY on the anchor (rotation +// moves no child's absolute position) and there is no fp_text (whose json +// carries an angle) — so nothing in the scalar projection changes. +const FP2 = "aaaaaaaa-0000-0000-0000-000000000002"; +// Bug-04 endpoint target: a graphic shape (Drawings → position-only json). +const GRL1 = "cccccccc-0000-0000-0000-000000000003"; + +// SAMPLE_PCB + a real net (net 1 "SIG") carried by two pads on the footprint — +// the bug-02 fixture requirement (pad net fidelity through the blob). +const SAMPLE_PCB = `(kicad_pcb +\t(version 20241229) +\t(generator "pcbnew") +\t(generator_version "9.0") +\t(general (thickness 1.6)) +\t(paper "A4") +\t(layers +\t\t(0 "F.Cu" signal) +\t\t(2 "B.Cu" signal) +\t\t(37 "F.SilkS" user) +\t\t(25 "Edge.Cuts" user) +\t) +\t(setup) +\t(net 0 "") +\t(net 1 "SIG") +\t(footprint "TestLib:R" +\t\t(layer "F.Cu") +\t\t(uuid "${FP1}") +\t\t(at 100 100) +\t\t(attr smd) +\t\t(property "Reference" "R1" (at 0 -4.2 0) (layer "F.SilkS") (uuid "66666666-0000-0000-0000-0000000000aa") (effects (font (size 1 1) (thickness 0.15)))) +\t\t(property "Value" "R" (at 0 4.6 0) (layer "F.Fab") (uuid "66666666-0000-0000-0000-0000000000bb") (effects (font (size 1 1) (thickness 0.15)))) +\t\t(fp_text user "HELLO" (at 0 0 0) (layer "F.SilkS") (uuid "${FP1_TXT}") (effects (font (size 1 1) (thickness 0.15)))) +\t\t(pad "1" smd rect (at -1.27 0) (size 1 1) (layers "F.Cu") (net 1 "SIG") (uuid "${PAD1}")) +\t\t(pad "2" smd rect (at 1.27 0) (size 1 1) (layers "F.Cu") (net 1 "SIG") (uuid "${PAD2}")) +\t) +\t(footprint "TestLib:X" +\t\t(layer "F.Cu") +\t\t(uuid "${FP2}") +\t\t(at 120 120) +\t\t(attr smd) +\t\t(property "Reference" "X1" (at 0 0 0) (layer "F.SilkS") (hide yes) (uuid "aaaaaaaa-0000-0000-0000-0000000000ee") (effects (font (size 1 1) (thickness 0.15)))) +\t\t(property "Value" "X" (at 0 0 0) (layer "F.Fab") (hide yes) (uuid "aaaaaaaa-0000-0000-0000-0000000000ff") (effects (font (size 1 1) (thickness 0.15)))) +\t) +\t(gr_line (start 20 20) (end 40 20) (stroke (width 0.1) (type default)) (layer "Edge.Cuts") (uuid "${GRL1}")) +\t(via (at 80 80) (size 1.4) (drill 0.6) (layers "F.Cu" "B.Cu") (net 0) (uuid "${VIA1}")) +\t(segment (start 50.8 50.8) (end 101.6 50.8) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG1}")) +\t(segment (start 50.8 76.2) (end 101.6 76.2) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG2}")) +) +`; + +// A bare footprint blob for the bug-05 "unrelated remote apply" (the proven +// v2 add path — same shape as items-bridge.spec.ts PCB.added). +const NEW_FP_SEXPR = `(footprint "TestLib:C" (layer "F.Cu") (uuid "${NEW_FP}") (at 50 50) (attr smd) (property "Reference" "C1" (at 0 -2 0) (layer "F.SilkS") (uuid "99999999-0000-0000-0000-0000000000aa") (effects (font (size 1 1) (thickness 0.15)))))`; + +type FS = { + mkdirTree(p: string): void; + writeFile(p: string, d: string): void; + readFile(p: string, o: { encoding: "utf8" }): string; +}; +type Mod = { + kicadOpenFile(p: string): unknown; + kicadCollabSnapshotItems(): string; + kicadCollabApplyItems(j: string): unknown; + kicadCollabTestMoveFirst(dx: number, dy: number): string; + kicadCollabGetPos(id: string): string; + kicadSaveBoard(p: string): unknown; +}; + +const BOOT_TIMEOUT = 150000; + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +async function bootOpen(page: Page): Promise { + await page.goto("/kicad/pcbnew-collab.html"); + await expect(page.locator("#canvas")).toBeVisible({ timeout: BOOT_TIMEOUT }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: BOOT_TIMEOUT }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Mod }).Module; + return ( + typeof m?.kicadOpenFile === "function" && + typeof m?.kicadCollabSnapshotItems === "function" && + typeof m?.kicadCollabApplyItems === "function" && + typeof m?.kicadCollabTestMoveFirst === "function" && + typeof m?.kicadSaveBoard === "function" + ); + }, + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.evaluate((content) => { + const w = window as unknown as { FS: FS; Module: Mod }; + try { + w.FS.mkdirTree("/home/kicad/documents"); + } catch { + /* exists */ + } + const p = "/home/kicad/documents/rt.kicad_pcb"; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, SAMPLE_PCB); +} + +function saveRead(page: Page): Promise { + return page.evaluate(() => { + const w = window as unknown as { FS: FS; Module: Mod }; + const out = "/home/kicad/documents/probe.kicad_pcb"; + w.Module.kicadSaveBoard(out); + return w.FS.readFile(out, { encoding: "utf8" }); + }); +} + +/** The footprint's snapshot blob (v2 wire) — FP1 with its children embedded. */ +async function fp1Blob(page: Page): Promise { + const snap = JSON.parse( + await page.evaluate(() => window.Module.kicadCollabSnapshotItems()), + ) as { added: Array<{ sexpr: string }> }; + const blob = snap.added.map((w) => w.sexpr).find((s) => s.includes(FP1)); + expect(blob, "snapshot blob for the footprint").toBeTruthy(); + return blob!; +} + +/** Register the onItems capture (single tab — no binding to preserve). */ +function captureEmits(page: Page): Promise { + return page.evaluate(() => { + (window as unknown as { __items: string[] }).__items = []; + (window as unknown as { kicadCollab: object }).kicadCollab = { + onItems: (j: string) => (window as unknown as { __items: string[] }).__items.push(j), + }; + }); +} + +function emittedWires(page: Page): Promise { + return page.evaluate(() => + (window as unknown as { __items: string[] }).__items.join("\n"), + ); +} + +test.describe("pcbnew ysync repros (v2 items wire, single tab)", () => { + test.describe.configure({ timeout: 420000 }); + + test("precondition: the footprint blob embeds its pad children", async ({ + page, + testLogger, + }) => { + await bootOpen(page); + const blob = await fp1Blob(page); + // Pins the green half of bug 02: the pads ARE in the blob — only their + // nets are stripped. If this test breaks, the bug-02 repro below is + // failing for the wrong reason. + expect(blob).toContain(PAD1); + expect(blob).toContain(PAD2); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("footprint blob preserves pad nets", async ({ page, testLogger }) => { + test.fail(); // bug 02 — blobForItem's pad->SetNetCode(0) loop strips them + + await bootOpen(page); + const blob = await fp1Blob(page); + // CORRECT: peers share the same board/net lineage — identity-by-uuid, not + // a foreign-board paste — so `(net 1 "SIG")` must survive the wire + // (02-bug-footprint-blob-zeroes-pad-nets.md). TODAY: pads go out net-0 and + // the loss propagates to the Y.Doc, peers, and materialized files. + expect(blob).toContain(`(net 1 "SIG")`); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("precondition: applyItems removes a root item (harness proof)", async ({ + page, + testLogger, + }) => { + await bootOpen(page); + expect(await saveRead(page)).toContain(SEG2); + await page.evaluate((seg) => { + window.Module.kicadCollabApplyItems( + JSON.stringify({ added: [], changed: [], removed: [seg] }), + ); + }, SEG2); + // The same remove path + poll the bug-03 repro uses — proven on a ROOT item. + await expect + .poll(async () => (await saveRead(page)).includes(SEG2), { + timeout: 25000, + intervals: [400], + }) + .toBe(false); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("applyItems removes a footprint CHILD by uuid", async ({ page, testLogger }) => { + test.fail(); // bug 03 (receiving half) — the parent-footprint guard skips it + + await bootOpen(page); + expect(await saveRead(page)).toContain(FP1_TXT); + // The wire a peer sends after deleting the fp_text: a bare child removal + // (the emit side lifts adds/changes to a parent re-blob but NOT removals — + // 03-bug-child-removal-dangling-slot.md). + await page.evaluate((uuid) => { + window.Module.kicadCollabApplyItems( + JSON.stringify({ added: [], changed: [], removed: [uuid] }), + ); + }, FP1_TXT); + // CORRECT: the receiver loses the child too. TODAY: doApplyItems skips any + // removed uuid with GetParentFootprint() (pcbnew_embind.cpp:838) → the peer + // KEEPS the field while the sender lost it — permanent divergence. + await expect + .poll(async () => (await saveRead(page)).includes(FP1_TXT), { + timeout: 8000, + intervals: [400], + }) + .toBe(false); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("control: a local move emits the v2 items wire (HEADLESS EMIT PROBE)", async ({ + page, + testLogger, + }) => { + await bootOpen(page); + // snapshotItems: registers the COLLAB_LISTENER (ensureBridge) + baselines + // the differ — the two side effects seed()'s non-file-seed branches rely on. + await page.evaluate(() => window.Module.kicadCollabSnapshotItems()); + await captureEmits(page); + + const movedId = (await page.evaluate(() => + window.Module.kicadCollabTestMoveFirst(2_000_000, 0), + )) as string; + expect(movedId).toMatch(/[0-9a-f-]{36}/); + + // listener → scheduleFlush → flushDiff → emitItems: the moved item's uuid + // must appear in an emitted wire. THIS is the phase-C gate: if red, the + // emit-dependent repros (bug 01 two-tab, bug 05 below) become test.fixme. + await expect + .poll(async () => await emittedWires(page), { timeout: 20000, intervals: [400] }) + .toContain(movedId); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("a local edit committed while a remote apply is queued still reaches the wire", async ({ + page, + testLogger, + }) => { + test.fail(); // bug 05 — the post-apply GLOBAL rebaseline swallows it + + await bootOpen(page); + await page.evaluate(() => window.Module.kicadCollabSnapshotItems()); + await captureEmits(page); + + // Pre-positions of every fixture uuid, so the moved item is verifiable + // whichever item forEachTopItem yields first. + const uuids = [FP1, FP1_TXT, PAD1, PAD2, VIA1, SEG1, SEG2]; + const before: Record = {}; + for (const u of uuids) { + before[u] = await page.evaluate((id) => window.Module.kicadCollabGetPos(id), u); + } + + // ONE JS turn: queue the local move, then the unrelated remote apply. + // CallAfter drain order is FIFO → [move, apply, flush]: the move's commit + // fires the listener (flush queued BEHIND the apply), then the apply's + // global rebaseline() snapshots the model WITH the move already in it, so + // the flush diffs to empty (05-bug-rebaseline-swallows-local-edits.md). + const movedId = (await page.evaluate((fpSexpr) => { + const m = window.Module; + const id = m.kicadCollabTestMoveFirst(3_000_000, 0); + m.kicadCollabApplyItems( + JSON.stringify({ added: [{ sexpr: fpSexpr, parent: null }], changed: [], removed: [] }), + ); + return id; + }, NEW_FP_SEXPR)) as string; + expect(movedId).toMatch(/[0-9a-f-]{36}/); + + // Green preconditions: the apply landed (FIFO ⇒ the move ran before it)… + await expect + .poll(() => page.evaluate((id) => window.Module.kicadCollabGetPos(id), NEW_FP), { + timeout: 15000, + intervals: [300], + }) + .not.toBe(""); + // …and the local move is REAL. + expect( + await page.evaluate((id) => window.Module.kicadCollabGetPos(id), movedId), + "the local move landed on the board", + ).not.toBe(before[movedId]); + + // CORRECT: the concurrent local edit still reaches peers. TODAY: the flush + // runs against the post-apply baseline and emits NOTHING for it. + await expect + .poll(async () => await emittedWires(page), { timeout: 8000, intervals: [400] }) + .toContain(movedId); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + // ── Bug 04 matrix — edits invisible to the scalar-projection differ ──────── + // 04-bug-lossy-change-detection.md: the v2 wire's TRIGGER is still the legacy + // scalar json diff; any edit that doesn't change the projection never emits. + // Each test: real commit via a repro hook (wasm ≥ the ysync-hooks build) → + // green precondition that the edit LANDED → expected-fail that it EMITTED. + // The "local move emits" control above proves the emit harness itself. + + /** Baseline + capture + hook-presence guard (returns false on stale wasm). */ + async function armed(page: Page, hook: string): Promise { + await bootOpen(page); + const has = await page.evaluate( + (h) => typeof (window as unknown as { Module: Record }).Module[h] === "function", + hook, + ); + if (!has) return false; + await page.evaluate(() => window.Module.kicadCollabSnapshotItems()); + await captureEmits(page); + return true; + } + + test("an anchor-centred footprint rotation reaches the wire", async ({ page, testLogger }) => { + test.fail(); // bug 04 — no orientation in the footprint's scalar json + + const ok = await armed(page, "kicadCollabTestRotateItem"); + test.skip(!ok, "wasm build predates the ysync repro hooks"); + + const queued = await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestRotateItem(i: string, d: number): boolean } }) + .Module.kicadCollabTestRotateItem(id, 90), + FP2, + ); + expect(queued, "rotate hook resolved the footprint").toBe(true); + + // Green precondition: the rotation LANDED (the saved footprint gained an + // orientation). FP2's children all sit on the anchor, so no child position + // moved — nothing in the scalar projection changed. + await expect + .poll(async () => /\(at 120 120 (-?[\d.]+)\)/.test(await saveRead(page)), { + timeout: 15000, + intervals: [400], + }) + .toBe(true); + + // CORRECT: the rotation is broadcast (any wire mentioning FP2). TODAY: the + // diff is empty — the rotation exists only on this tab, forever. + await expect + .poll(async () => await emittedWires(page), { timeout: 8000, intervals: [400] }) + .toContain(FP2); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("a pad size edit reaches the wire", async ({ page, testLogger }) => { + test.fail(); // bug 04 — pads are not visited by forEachTopItem at all + + const ok = await armed(page, "kicadCollabTestSetPadSize"); + test.skip(!ok, "wasm build predates the ysync repro hooks"); + + const queued = await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestSetPadSize(i: string, w: number, h: number): boolean } }) + .Module.kicadCollabTestSetPadSize(id, 2_000_000, 2_000_000), + PAD1, + ); + expect(queued, "pad hook resolved the pad").toBe(true); + + // Green precondition: the resize LANDED in the model. + await expect + .poll(async () => (await saveRead(page)).includes("(size 2 2)"), { + timeout: 15000, + intervals: [400], + }) + .toBe(true); + + // CORRECT: the edit is broadcast — as the pad's parent re-blob (liftBlob + // lifts children), so PAD1 appears inside an emitted footprint blob. + // TODAY: nothing emits. + await expect + .poll(async () => await emittedWires(page), { timeout: 8000, intervals: [400] }) + .toContain(PAD1); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("a graphic-shape endpoint drag reaches the wire", async ({ page, testLogger }) => { + test.fail(); // bug 04 — Drawings' json is position-only; GetPosition() is the start + + const ok = await armed(page, "kicadCollabTestMoveEndpoint"); + test.skip(!ok, "wasm build predates the ysync repro hooks"); + + const queued = await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestMoveEndpoint(i: string, dx: number, dy: number): boolean } }) + .Module.kicadCollabTestMoveEndpoint(id, 5_000_000, 0), + GRL1, + ); + expect(queued, "endpoint hook resolved the shape").toBe(true); + + // Green precondition: the reshape LANDED (end 40 20 → 45 20). + await expect + .poll(async () => (await saveRead(page)).includes("(end 45 20)"), { + timeout: 15000, + intervals: [400], + }) + .toBe(true); + + // CORRECT: the reshape is broadcast. TODAY: start (== position) unchanged + // → invisible to the differ. + await expect + .poll(async () => await emittedWires(page), { timeout: 8000, intervals: [400] }) + .toContain(GRL1); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + // ── Bug 03 sending half — child removal must lift to a parent re-blob ───── + test("precondition: TestRemoveItem deletes a footprint child locally", async ({ + page, + testLogger, + }) => { + await bootOpen(page); + const has = await page.evaluate( + () => + typeof (window as unknown as { Module: Record }).Module + .kicadCollabTestRemoveItem === "function", + ); + test.skip(!has, "wasm build predates the ysync repro hooks"); + + const queued = await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestRemoveItem(i: string): boolean } }) + .Module.kicadCollabTestRemoveItem(id), + FP1_TXT, + ); + expect(queued, "remove hook resolved the child").toBe(true); + await expect + .poll(async () => (await saveRead(page)).includes(FP1_TXT), { + timeout: 15000, + intervals: [400], + }) + .toBe(false); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); + + test("a child deletion goes out as the parent's re-blob", async ({ page, testLogger }) => { + test.fail(); // bug 03 (sending half) — flushDiff has no liftBlob for removals + + const ok = await armed(page, "kicadCollabTestRemoveItem"); + test.skip(!ok, "wasm build predates the ysync repro hooks"); + + await page.evaluate( + (id) => + (window as unknown as { Module: { kicadCollabTestRemoveItem(i: string): boolean } }) + .Module.kicadCollabTestRemoveItem(id), + FP1_TXT, + ); + + // Green precondition: the deletion LANDED locally. + await expect + .poll(async () => (await saveRead(page)).includes(FP1_TXT), { + timeout: 15000, + intervals: [400], + }) + .toBe(false); + + // CORRECT: the deletion travels as the parent footprint's re-blob (its new + // body simply lacks the child — the same containment adds/changes use), so + // some emitted wire carries an FP1 blob WITHOUT the child. TODAY nothing of + // the sort goes out — the flushDiff code would send a bare + // `removed:[childUuid]` (03-bug…md), but the headless run shows the child- + // only delete commit does not even trigger a flush (the baseline-snapshot + // tracer fires once, never again) — the sending-side hole is total. + await expect + .poll( + async () => { + const wires = (await page.evaluate( + () => (window as unknown as { __items: string[] }).__items, + )) as string[]; + return wires.some((w) => { + const wire = JSON.parse(w) as { + added?: Array<{ sexpr: string }>; + changed?: Array<{ sexpr: string }>; + }; + return [...(wire.added ?? []), ...(wire.changed ?? [])].some( + (b) => b.sexpr.includes(FP1) && !b.sexpr.includes(FP1_TXT), + ); + }); + }, + { timeout: 8000, intervals: [400] }, + ) + .toBe(true); + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + }); +}); diff --git a/tests/kicad/ysync-two-tab.spec.ts b/tests/kicad/ysync-two-tab.spec.ts new file mode 100644 index 0000000..878ac5a --- /dev/null +++ b/tests/kicad/ysync-two-tab.spec.ts @@ -0,0 +1,503 @@ +import { execSync } from "node:child_process"; +import path from "node:path"; +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * V2 "items" wire two-tab e2e — the PRODUCTION collab stack, end to end + * (ysync-review miss 11): bindKicadCollab + moduleItemsBridge over the kdoc_* + * Slot-model Y.Doc, C++ kicadCollabSnapshotItems / ApplyItems / + * window.kicadCollab.onItems. The pre-existing *-collab.spec.ts two-tab tests + * drive the LEGACY scalar wire, which is dead in production. + * + * pl_editor is the GREEN baseline: its emit is an eager OnModify hook + * (pl_editor_embind.cpp), not the lazily-registered COLLAB_LISTENER, so bug 01 + * does not gate it — which is exactly what makes it fit for validating the + * harness itself. + * + * REPRO CONVENTION (docs/features/ysync-review on the ysync-review branch): + * each repro asserts the CORRECT behavior and is marked `test.fail()` with a + * comment naming the bug doc. Fixing the bug flips it to "unexpected pass", + * forcing the marker's removal — the repro becomes the regression test. + * Expected-fail polls use short timeouts so they don't burn the clock. + */ + +type FS = { + mkdirTree(p: string): void; + writeFile(p: string, d: string): void; + readFile(p: string, o: { encoding: "utf8" }): string; +}; +type Mod = Record unknown>; + +interface ToolCfg { + html: string; + ext: string; + saveFn: string; + fixture: string; + /** Module fns (beyond the v2 bridge pair + saveFn) boot must wait for. */ + fns: string[]; +} + +const BOOT_TIMEOUT = 150000; + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +// ── Fixtures ───────────────────────────────────────────────────────────────── + +const U_TITLE = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; +const U_RECT = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"; +const U_DIVERGENT = "dddddddd-dddd-dddd-dddd-dddddddddddd"; + +const PL: ToolCfg = { + html: "pl_editor.html", + ext: "kicad_wks", + saveFn: "kicadSaveDrawingSheet", + fixture: `(kicad_wks (version 20220228) (generator "pl_editor") (generator_version "9.0") + (setup (textsize 1.5 1.5)(linewidth 0.15)(textlinewidth 0.15) + (left_margin 10)(right_margin 10)(top_margin 10)(bottom_margin 10)) + (rect (uuid "${U_RECT}") (name border) (start 0 0 ltcorner) (end 0 0 rbcorner)) + (tbtext "Title" (uuid "${U_TITLE}") (name title) (pos 100 20 ltcorner) (font (size 2 2))) +) +`, + fns: ["kicadCollabTestAddText"], +}; + +const FP1 = "66666666-0000-0000-0000-000000000001"; +const FP1_TXT = "66666666-0000-0000-0000-0000000000cc"; +const VIA1 = "77777777-0000-0000-0000-000000000001"; +const SEG1 = "88888888-0000-0000-0000-000000000001"; +const SEG2 = "88888888-0000-0000-0000-000000000002"; + +const PCB: ToolCfg = { + html: "pcbnew-collab.html", + ext: "kicad_pcb", + saveFn: "kicadSaveBoard", + fixture: `(kicad_pcb +\t(version 20241229) +\t(generator "pcbnew") +\t(generator_version "9.0") +\t(general (thickness 1.6)) +\t(paper "A4") +\t(layers +\t\t(0 "F.Cu" signal) +\t\t(2 "B.Cu" signal) +\t\t(37 "F.SilkS" user) +\t\t(25 "Edge.Cuts" user) +\t) +\t(setup) +\t(net 0 "") +\t(footprint "TestLib:R" +\t\t(layer "F.Cu") +\t\t(uuid "${FP1}") +\t\t(at 100 100) +\t\t(attr smd) +\t\t(property "Reference" "R1" (at 0 -4.2 0) (layer "F.SilkS") (uuid "66666666-0000-0000-0000-0000000000aa") (effects (font (size 1 1) (thickness 0.15)))) +\t\t(property "Value" "R" (at 0 4.6 0) (layer "F.Fab") (uuid "66666666-0000-0000-0000-0000000000bb") (effects (font (size 1 1) (thickness 0.15)))) +\t\t(fp_text user "HELLO" (at 0 0 0) (layer "F.SilkS") (uuid "${FP1_TXT}") (effects (font (size 1 1) (thickness 0.15)))) +\t) +\t(via (at 80 80) (size 1.4) (drill 0.6) (layers "F.Cu" "B.Cu") (net 0) (uuid "${VIA1}")) +\t(segment (start 50.8 50.8) (end 101.6 50.8) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG1}")) +\t(segment (start 50.8 76.2) (end 101.6 76.2) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG2}")) +) +`, + fns: ["kicadCollabTestMoveFirst", "kicadCollabGetPos"], +}; + +const WIRE1 = "22222222-0000-0000-0000-000000000001"; +const WIRE2 = "22222222-0000-0000-0000-000000000002"; + +const SCH: ToolCfg = { + html: "eeschema.html", + ext: "kicad_sch", + saveFn: "kicadSaveSchematic", + fixture: `(kicad_sch +\t(version 20250114) +\t(generator "eeschema") +\t(generator_version "9.0") +\t(uuid "11111111-1111-1111-1111-111111111111") +\t(paper "A4") +\t(lib_symbols) +\t(wire (pts (xy 50.8 50.8) (xy 101.6 50.8)) (stroke (width 0) (type default)) (uuid "${WIRE1}")) +\t(wire (pts (xy 50.8 76.2) (xy 101.6 76.2)) (stroke (width 0) (type default)) (uuid "${WIRE2}")) +\t(sheet_instances (path "/" (page "1"))) +) +`, + fns: ["kicadCollabTestMoveFirst", "kicadCollabGetPos"], +}; + +// ── Harness plumbing ───────────────────────────────────────────────────────── + +const BUNDLE = path.resolve(__dirname, "../apps/kicad/collab-bundle-v2.js"); + +async function bootOpen(page: Page, cfg: ToolCfg, name: string): Promise { + await page.goto(`/kicad/${cfg.html}`); + await expect(page.locator("#canvas")).toBeVisible({ timeout: BOOT_TIMEOUT }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: BOOT_TIMEOUT }); + await page.waitForFunction( + (fns) => { + const m = (window as unknown as { Module?: Mod }).Module; + return !!m && fns.every((f) => typeof m[f] === "function"); + }, + ["kicadOpenFile", "kicadCollabSnapshotItems", "kicadCollabApplyItems", cfg.saveFn, ...cfg.fns], + { timeout: BOOT_TIMEOUT }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: BOOT_TIMEOUT }, + ); + await page.evaluate( + ({ content, ext, name }) => { + const w = window as unknown as { FS: FS; Module: { kicadOpenFile(p: string): unknown } }; + try { + w.FS.mkdirTree("/home/kicad/documents"); + } catch { + /* exists */ + } + const p = `/home/kicad/documents/${name}.${ext}`; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, + { content: cfg.fixture, ext: cfg.ext, name }, + ); + await expect.poll(() => page.title(), { timeout: 30000 }).toMatch(new RegExp(name, "i")); + await page.addScriptTag({ path: BUNDLE }); +} + +/** Start the v2 stack in a tab (window.KicadCollabV2 from the bundle). */ +function startV2( + page: Page, + opts: { room: string; settleMs?: number; seedText?: string; editorMatchesDoc?: boolean }, +): Promise { + return page.evaluate(async (o) => { + const w = window as unknown as { + KicadCollabV2: { start: (m: unknown, win: unknown, o: unknown) => Promise }; + Module: unknown; + }; + await w.KicadCollabV2.start(w.Module, window, o); + }, opts); +} + +/** Read a tab's current model back as text via save-to-MEMFS. */ +function modelText(page: Page, cfg: ToolCfg): Promise { + return page.evaluate( + ({ saveFn, ext }) => { + const w = window as unknown as { FS: FS; Module: Mod }; + const out = `/home/kicad/documents/_dump.${ext}`; + (w.Module[saveFn] as (p: string) => unknown)(out); + return w.FS.readFile(out, { encoding: "utf8" }); + }, + { saveFn: cfg.saveFn, ext: cfg.ext }, + ); +} + +/** docToFile(yToDoc(room doc)) in-page; { err } instead of throwing. */ +function renderDoc(page: Page): Promise<{ ok?: string; err?: string }> { + return page.evaluate(() => { + const w = window as unknown as { KicadCollabV2: { renderActiveDoc(): string } }; + try { + return { ok: w.KicadCollabV2.renderActiveDoc() }; + } catch (e) { + return { err: String(e) }; + } + }); +} + +/** Item-level drift summary (see browser-entry-v2.ts driftReport). */ +function drift(page: Page, cfg: ToolCfg) { + return page.evaluate( + ({ saveFn, ext }) => { + const w = window as unknown as { + KicadCollabV2: { + driftReport( + f: string, + p: string, + ): { added: string[]; updated: string[]; removed: string[] } | null; + }; + }; + return w.KicadCollabV2.driftReport(saveFn, `/home/kicad/documents/_drift.${ext}`); + }, + { saveFn: cfg.saveFn, ext: cfg.ext }, + ); +} + +function getPos(page: Page, uuid: string): Promise { + return page.evaluate( + (id) => (window as unknown as { Module: { kicadCollabGetPos(i: string): string } }).Module.kicadCollabGetPos(id), + uuid, + ); +} + +test.beforeAll(() => { + // Rebuild both collab bundles so the test always exercises the current stack. + execSync("node collab/build.mjs", { cwd: path.resolve(__dirname, ".."), stdio: "inherit" }); +}); + +// ── pl_editor: the green baseline (harness validation + adopt coverage) ────── + +test.describe("v2 items wire — pl_editor two tabs (green baseline)", () => { + test.describe.configure({ timeout: 420000 }); + + test("fresh room: A file-seeds, edits flow A→B and B→A, item drift silent", async ({ + context, + testLogger, + }) => { + const room = `ysync-v2-pl-fresh-${test.info().workerIndex}`; + const tabA = await context.newPage(); + const tabB = await context.newPage(); + await bootOpen(tabA, PL, "tabA"); + await bootOpen(tabB, PL, "tabB"); + + await startV2(tabA, { room, seedText: PL.fixture }); // fresh room → file-seed branch + await startV2(tabB, { room }); // joins → adopt + + // A→B. pl_editor's emit hook is EAGER (OnModify, registered at module init), + // so bug 01's never-registered-listener hole does not gate this tool. + const uuidA = (await tabA.evaluate(() => + (window as unknown as { Module: { kicadCollabTestAddText(t: string, x: number, y: number): string } }) + .Module.kicadCollabTestAddText("Hello from A", 40, 40), + )) as string; + expect(uuidA).toMatch(/[0-9a-f-]{36}/); + await expect + .poll(async () => await modelText(tabB, PL), { timeout: 20000, intervals: [300] }) + .toContain("Hello from A"); + expect(await modelText(tabB, PL)).toContain(`(uuid "${uuidA}")`); + + // B→A (the adopting side's listener registered via its seed's snapshotItems). + const uuidB = (await tabB.evaluate(() => + (window as unknown as { Module: { kicadCollabTestAddText(t: string, x: number, y: number): string } }) + .Module.kicadCollabTestAddText("Hello from B", 60, 60), + )) as string; + await expect + .poll(async () => await modelText(tabA, PL), { timeout: 20000, intervals: [300] }) + .toContain("Hello from B"); + expect(await modelText(tabA, PL)).toContain(`(uuid "${uuidB}")`); + + // Drift-detect as the convergence oracle (miss 11 §5): ITEM-level silence on + // both tabs. layoutChanged/metaChanged are NOT asserted — non-item state only + // syncs at seed (miss 08) and the writer may normalize preamble formatting. + for (const [tab, label] of [ + [tabA, "tabA"], + [tabB, "tabB"], + ] as const) { + const d = await drift(tab, PL); + expect(d?.added ?? [], `${label} drift added`).toEqual([]); + expect(d?.updated ?? [], `${label} drift updated`).toEqual([]); + expect(d?.removed ?? [], `${label} drift removed`).toEqual([]); + } + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + await tabA.close(); + await tabB.close(); + }); + + test("cold divergent copy: the joiner adopts the doc's identity", async ({ + context, + testLogger, + }) => { + const room = `ysync-v2-pl-adopt-${test.info().workerIndex}`; + const tabA = await context.newPage(); + const tabB = await context.newPage(); + await bootOpen(tabA, PL, "tabA"); + await startV2(tabA, { room, seedText: PL.fixture }); + + // B cold-opened a never-saved copy: same content, DIFFERENT title uuid. + const divergent: ToolCfg = { ...PL, fixture: PL.fixture.replace(U_TITLE, U_DIVERGENT) }; + await bootOpen(tabB, divergent, "tabB"); + await startV2(tabB, { room }); // populated room → adopt (doc authority) + + // Doc roots applied, local-only roots removed — the editor takes the doc's uuids. + await expect + .poll(async () => await modelText(tabB, PL), { timeout: 20000, intervals: [300] }) + .toContain(U_TITLE); + expect(await modelText(tabB, PL)).not.toContain(U_DIVERGENT); + + const d = await drift(tabB, PL); + expect(d?.added ?? [], "adopted tab drift added").toEqual([]); + expect(d?.removed ?? [], "adopted tab drift removed").toEqual([]); + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + await tabA.close(); + await tabB.close(); + }); +}); + +// ── Bug 01 — the fresh-room seeding tab cannot send (eeschema + pcbnew) ────── +// 01-bug-first-tab-listener-never-registered.md: seed()'s file-seed branch +// never calls snapshotItems(), so ensureBridge() never registers the C++ +// COLLAB_LISTENER on the seeding tab — its local edits are never emitted. The +// joiner uses editorMatchesDoc (the ydoc-load baseline-only path) so this test +// isolates the SEEDER's emit half. Gated on the headless-emit probe in +// ysync-repros-{pcbnew,eeschema}.spec.ts: if that control is red, flip these +// to test.fixme (the harness can't drive the tool's emit at all). + +for (const [cfg, label] of [ + [PCB, "pcbnew"], + [SCH, "eeschema"], +] as const) { + test.describe(`v2 items wire — ${label} fresh room (bug 01 repro)`, () => { + test.describe.configure({ timeout: 420000 }); + + test(`${label}: the seeding tab's local edit must reach the joiner`, async ({ + context, + testLogger, + }) => { + test.fail(); // bug 01 — first tab never registers the C++ change listener + // TWO kicad_editor instances exceed Firefox's per-content-process wasm + // budget (the 2nd tab's #canvas never appears, even serial/isolated — + // same SpiderMonkey wall playwright-kicad.config.ts documents for x86 + // CI, hit at 2× on ARM). V8 handles it: runs on chromium-ci in CI and + // --project=chromium locally. + test.skip( + test.info().project.name === "firefox", + "two kicad_editor tabs exceed Firefox's per-process wasm budget", + ); + + const room = `ysync-v2-${label}-bug01-${test.info().workerIndex}`; + const tabA = await context.newPage(); + const tabB = await context.newPage(); + await bootOpen(tabA, cfg, "tabA"); + await bootOpen(tabB, cfg, "tabB"); + + await startV2(tabA, { room, seedText: cfg.fixture }); // fresh → FILE-SEED branch + await startV2(tabB, { room, editorMatchesDoc: true }); // ydoc-load style joiner + + // Pre-positions of every fixture root, so the moved item is verifiable + // whichever item the tool's forEachTopItem yields first. + const uuids = [...cfg.fixture.matchAll(/\(uuid "([0-9a-f-]{36})"\)/g)].map((m) => m[1]!); + const before: Record = {}; + for (const u of uuids) before[u] = await getPos(tabA, u); + + const movedId = (await tabA.evaluate(() => + (window as unknown as { Module: { kicadCollabTestMoveFirst(dx: number, dy: number): string } }) + .Module.kicadCollabTestMoveFirst(2_000_000, 0), + )) as string; + expect(movedId).toMatch(/[0-9a-f-]{36}/); + + // The local edit is REAL (green precondition — the move landed on A). + await expect + .poll(() => getPos(tabA, movedId), { timeout: 15000, intervals: [300] }) + .not.toBe(before[movedId]); + const posA = await getPos(tabA, movedId); + + // CORRECT: the v2 loop broadcasts it (listener → flushDiff → onItems → Y → + // peer applyItems). TODAY: A has no listener (file-seed skipped + // snapshotItems) → nothing is ever emitted → B never converges. + await expect + .poll(() => getPos(tabB, movedId), { timeout: 8000, intervals: [400] }) + .toBe(posA); + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + await tabA.close(); + await tabB.close(); + }); + }); +} + +// ── Bug 06 — concurrent first-seed duplicates kdoc_layout ──────────────────── +// 06-bug-concurrent-seed-duplicates-layout.md: seed-vs-adopt is client-side +// check-then-act; two tabs opening the same fresh room inside the settle +// window both file-seed, and the two kdoc_layout inserts BOTH survive the +// Y.Array merge. The deterministic repro is the unit test +// (web/pcbjam-shared/test/ysync-repros.test.ts); this is the real-window +// trigger, skipped on runs where the race happens not to fire. + +test.describe("v2 items wire — concurrent seed (bug 06 repro)", () => { + test.describe.configure({ timeout: 420000 }); + + test("both tabs seed a fresh room at once: the room must materialize the single-seed output", async ({ + context, + testLogger, + }) => { + test.fail(); // bug 06 — concurrent first-seed duplicates kdoc_layout + + const room = `ysync-v2-pl-race-${test.info().workerIndex}`; + const tabA = await context.newPage(); + const tabB = await context.newPage(); + await bootOpen(tabA, PL, "tabA"); + await bootOpen(tabB, PL, "tabB"); + + // Equal settle windows, started together: both pass ydocHasState("empty"). + await Promise.all([ + startV2(tabA, { room, seedText: PL.fixture, settleMs: 400 }), + startV2(tabB, { room, seedText: PL.fixture, settleMs: 400 }), + ]); + + // Let the CRDT converge (not part of the repro — both docs must agree). + await expect + .poll( + async () => { + const [a, b] = await Promise.all([renderDoc(tabA), renderDoc(tabB)]); + return !!a.ok && a.ok === b.ok; + }, + { timeout: 15000, intervals: [300] }, + ) + .toBe(true); + + const merged = (await renderDoc(tabA)).ok!; + const single = (await tabA.evaluate( + (txt) => + (window as unknown as { KicadCollabV2: { singleSeedRender(t: string): string } }) + .KicadCollabV2.singleSeedRender(txt), + PL.fixture, + )) as string; + + // If one tab happened to see the other's seed first (race not triggered), + // the run is inconclusive — skip rather than "pass unexpectedly" and turn + // CI red while the bug is still open. The unit repro is the deterministic one. + test.skip(merged === single, "seed race did not trigger this run — inconclusive"); + + // CORRECT: same file, same room → the single-seed materialization. + // TODAY: every root slot + preamble form is doubled, permanently. + expect(merged).toBe(single); + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + await tabA.close(); + await tabB.close(); + }); +}); + +// ── Bug 03 (Y half) — bare child removal poisons the room doc ──────────────── +// 03-bug-child-removal-dangling-slot.md: the C++ emit for a child-only delete +// is `{removed:[childUuid]}` with no parent re-blob (pcbnew_embind.cpp +// flushDiff's removed loop). applyDeltaToY drops the item but leaves the +// parent's `{item}` slot dangling → the room stops materializing. + +test.describe("v2 items wire — pcbnew bare child removal (bug 03 Y-half repro)", () => { + test.describe.configure({ timeout: 420000 }); + + test("a bare child removal must keep the room materializable", async ({ + context, + testLogger, + }) => { + test.fail(); // bug 03 — dangling {item} slot in the parent's Y body + + const room = `ysync-v2-pcb-bug03-${test.info().workerIndex}`; + const tabA = await context.newPage(); + await bootOpen(tabA, PCB, "tabA"); + await startV2(tabA, { room, seedText: PCB.fixture }); // file-seed: fp + children in kdoc_items + + // Simulate exactly the wire the C++ sends for "delete the fp_text child" + // through the binding's REAL hook (moduleItemsBridge registered it). + await tabA.evaluate((uuid) => { + const w = window as unknown as { kicadCollab: { onItems(j: string): void } }; + w.kicadCollab.onItems(JSON.stringify({ added: [], changed: [], removed: [uuid] })); + }, FP1_TXT); + + // CORRECT: the room still materializes, without the child. TODAY: + // renderItem throws `missing item ${FP1_TXT}` through the dangling slot. + const r = await renderDoc(tabA); + expect(r.err, "room must still materialize (docToFile)").toBeUndefined(); + expect(r.ok!).not.toContain(FP1_TXT); + expect(r.ok!).toContain(FP1); // the parent footprint survives + + expect(hasAbort(testLogger), "no WASM abort").toBe(false); + await tabA.close(); + }); +}); diff --git a/tests/playwright-kicad.config.ts b/tests/playwright-kicad.config.ts index 0e76142..72feb19 100644 --- a/tests/playwright-kicad.config.ts +++ b/tests/playwright-kicad.config.ts @@ -116,6 +116,12 @@ const BIG_MODULE_SPECS = [ // only boots the (small) occ_service module but shares the harness page. "**/occ-export.spec.ts", "**/occ-probe.spec.ts", + // ysync v2-wire repros/coverage: the two-tab file boots pcbnew + eeschema + // (pl_editor cases ride along — only the browser changes), the repro files + // boot pcbnew-collab.html / eeschema.html — same V8 routing. + "**/ysync-two-tab.spec.ts", + "**/ysync-repros-pcbnew.spec.ts", + "**/ysync-repros-eeschema.spec.ts", ]; // Runtime-perf specs run ONLY on the Chromium 'perf' project below: they need diff --git a/wasm/bindings/eeschema_embind.cpp b/wasm/bindings/eeschema_embind.cpp index 214f0f5..d2741c7 100644 --- a/wasm/bindings/eeschema_embind.cpp +++ b/wasm/bindings/eeschema_embind.cpp @@ -1020,6 +1020,120 @@ std::string schCollabGetPos( std::string aId ) } +// ── ysync-review repro hooks ───────────────────────────────────────────────── +// Local-edit test hooks for the ysync-review repro e2e (docs/features/ +// ysync-review on the ysync-review branch): each drives a REAL SCH_COMMIT via +// CallAfter + COROUTINE (the doApply wrapping), so the SCHEMATIC_LISTENER → +// flushDiff emit path runs exactly as for a UI edit. Each returns false when +// the uuid doesn't resolve, letting the spec distinguish "hook missed the +// item" from "differ missed the edit" (bug 04). + +// Delete an item by uuid via a real SCH_COMMIT. +bool schCollabTestRemoveItem( std::string aId ) +{ + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return false; + + SCH_SHEET_PATH path; + SCH_ITEM* item = fr->Schematic().ResolveItem( KIID( wxString::FromUTF8( aId.c_str() ) ), + &path, /*allowNull*/ true ); + + if( !item ) + return false; + + SCH_SCREEN* screen = path.LastScreen(); + + fr->CallAfter( [fr, item, screen]() { + COROUTINE cor( [fr, item, screen]( int ) -> int + { + SCH_COMMIT commit( fr ); + commit.Remove( item, screen ); + commit.Push( wxT( "Collab test remove" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + +// Rotate an item in place (aDeg snapped to 90° CCW steps) — bug 04: a symbol's +// GetPosition() is unchanged by an in-place rotation and its json carries no +// orientation, so the rotation is invisible to the scalar differ. +bool schCollabTestRotateItem( std::string aId, double aDeg ) +{ + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return false; + + SCH_SHEET_PATH path; + SCH_ITEM* item = fr->Schematic().ResolveItem( KIID( wxString::FromUTF8( aId.c_str() ) ), + &path, /*allowNull*/ true ); + + if( !item ) + return false; + + SCH_SCREEN* screen = path.LastScreen(); + int steps = ( (int) ( aDeg / 90.0 + ( aDeg >= 0 ? 0.5 : -0.5 ) ) % 4 + 4 ) % 4; + + fr->CallAfter( [fr, item, screen, steps]() { + COROUTINE cor( [fr, item, screen, steps]( int ) -> int + { + SCH_COMMIT commit( fr ); + commit.Modify( item, screen ); + + for( int i = 0; i < steps; ++i ) + item->Rotate( item->GetPosition(), /*aRotateCCW*/ true ); + + commit.Push( wxT( "Collab test rotate" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + +// Set a symbol's Value field text — bug 04: fields live inside the symbol (not +// in screen->Items()) and the symbol json carries no field text, so the most +// common schematic edit after moving things never syncs. +bool schCollabTestSetFieldText( std::string aId, std::string aText ) +{ + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return false; + + SCH_SHEET_PATH path; + SCH_ITEM* item = fr->Schematic().ResolveItem( KIID( wxString::FromUTF8( aId.c_str() ) ), + &path, /*allowNull*/ true ); + + if( !item || item->Type() != SCH_SYMBOL_T ) + return false; + + SCH_SYMBOL* sym = static_cast( item ); + SCH_SCREEN* screen = path.LastScreen(); + wxString text = wxString::FromUTF8( aText.c_str() ); + + fr->CallAfter( [fr, sym, screen, text]() { + COROUTINE cor( [fr, sym, screen, text]( int ) -> int + { + SCH_COMMIT commit( fr ); + commit.Modify( sym, screen ); + sym->SetValueFieldText( text ); + commit.Push( wxT( "Collab test field text" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + + // Programmatically save the in-memory schematic to a .kicad_sch file, without // driving the Save As dialog — eeschema's analogue of pl_editor's // kicadSaveDrawingSheet. Serializes the root sheet via the same SCH_IO_KICAD_SEXPR @@ -1085,6 +1199,8 @@ void kicadSaveSchematic( std::string path ) EMSCRIPTEN_BINDINGS(eeschema) { // Programmatic save of the in-memory schematic (round-trip tests, README §A). function("kicadSaveSchematic", &kicadSaveSchematic); + // eeschema-only ysync-review repro hook (name not shared with pcbnew). + function("kicadCollabTestSetFieldText", &schCollabTestSetFieldText); #ifndef KICAD_MERGED_EMBIND // JS names ALSO registered by pcbnew_embind.cpp — in the merged image these are @@ -1099,6 +1215,9 @@ EMSCRIPTEN_BINDINGS(eeschema) { function("kicadCollabSnapshotItems", &schCollabSnapshotItems); function("kicadCollabTestMoveFirst", &schCollabTestMoveFirst); function("kicadCollabGetPos", &schCollabGetPos); + // ysync-review repro hooks shared with pcbnew (dispatched when merged). + function("kicadCollabTestRemoveItem", &schCollabTestRemoveItem); + function("kicadCollabTestRotateItem", &schCollabTestRotateItem); #endif // !KICAD_MERGED_EMBIND } #endif diff --git a/wasm/bindings/kicad_editor_embind.cpp b/wasm/bindings/kicad_editor_embind.cpp index 6e29fbe..5446d27 100644 --- a/wasm/bindings/kicad_editor_embind.cpp +++ b/wasm/bindings/kicad_editor_embind.cpp @@ -43,6 +43,8 @@ std::string pcbCollabSnapshot(); std::string pcbCollabSnapshotItems(); std::string pcbCollabTestMoveFirst( int aDx, int aDy ); std::string pcbCollabGetPos( std::string aId ); +bool pcbCollabTestRemoveItem( std::string aId ); +bool pcbCollabTestRotateItem( std::string aId, double aDeg ); bool schEditorActive(); void schCollabApply( std::string aJson ); @@ -51,6 +53,8 @@ std::string schCollabSnapshot(); std::string schCollabSnapshotItems(); std::string schCollabTestMoveFirst( int aDx, int aDy ); std::string schCollabGetPos( std::string aId ); +bool schCollabTestRemoveItem( std::string aId ); +bool schCollabTestRotateItem( std::string aId, double aDeg ); // Programmatically open a project file in the running editor frame, without UI @@ -118,6 +122,17 @@ static std::string collabGetPos( std::string aId ) return pcbEditorActive() ? pcbCollabGetPos( aId ) : schCollabGetPos( aId ); } +static bool collabTestRemoveItem( std::string aId ) +{ + return pcbEditorActive() ? pcbCollabTestRemoveItem( aId ) : schCollabTestRemoveItem( aId ); +} + +static bool collabTestRotateItem( std::string aId, double aDeg ) +{ + return pcbEditorActive() ? pcbCollabTestRotateItem( aId, aDeg ) + : schCollabTestRotateItem( aId, aDeg ); +} + EMSCRIPTEN_BINDINGS(kicad_editor) { // Programmatic file open (preferred over UI automation from the web app). @@ -131,6 +146,10 @@ EMSCRIPTEN_BINDINGS(kicad_editor) { function("kicadCollabSnapshotItems", &collabSnapshotItems); function("kicadCollabTestMoveFirst", &collabTestMoveFirst); function("kicadCollabGetPos", &collabGetPos); + // ysync-review repro hooks (shared names; per-editor-only hooks — pad size, + // endpoint, field text — flow from the per-editor blocks unchanged). + function("kicadCollabTestRemoveItem", &collabTestRemoveItem); + function("kicadCollabTestRotateItem", &collabTestRotateItem); } #endif // __EMSCRIPTEN__ diff --git a/wasm/bindings/pcbnew_embind.cpp b/wasm/bindings/pcbnew_embind.cpp index 4773511..eb60625 100644 --- a/wasm/bindings/pcbnew_embind.cpp +++ b/wasm/bindings/pcbnew_embind.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -1149,6 +1150,141 @@ std::string kicadCollabTestItemBlob( std::string aId ) return ""; } +// ── ysync-review repro hooks ───────────────────────────────────────────────── +// Local-edit test hooks for the ysync-review repro e2e (docs/features/ +// ysync-review on the ysync-review branch): each drives a REAL BOARD_COMMIT on +// the app main stack inside a COROUTINE fiber (the collabTestMove wrapping — +// virtual item mutators mis-dispatch off the fiber stack), so the +// COLLAB_LISTENER → flushDiff emit path runs exactly as for a UI edit. Each +// returns false when the uuid doesn't resolve, letting the spec distinguish +// "hook missed the item" from "differ missed the edit" (bug 04). + +// Resolve a live board item by uuid, or null (shared by the hooks below). +static BOARD_ITEM* testResolve( PCB_EDIT_FRAME* aFrame, const std::string& aId ) +{ + if( !aFrame ) + return nullptr; + + return aFrame->GetBoard()->ResolveItem( KIID( wxString::FromUTF8( aId.c_str() ) ), + /*allowNullptr*/ true ); +} + +// Delete an item by uuid. With a footprint CHILD uuid this is the bug-03 +// sending half (the UI's fp-text delete): the emit must lift to a parent +// re-blob; today it goes out as a bare child removal. +bool pcbCollabTestRemoveItem( std::string aId ) +{ + PCB_EDIT_FRAME* fr = pcbFrame(); + BOARD_ITEM* item = testResolve( fr, aId ); + + if( !item ) + return false; + + fr->CallAfter( [fr, item]() { + COROUTINE cor( [fr, item]( int ) -> int + { + BOARD_COMMIT commit( fr ); + commit.Remove( item ); + commit.Push( wxT( "Collab test remove" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + +// Rotate an item about its OWN anchor — bug 04: the scalar json carries no +// orientation for footprints, so an anchor-centred rotation is invisible to +// the differ unless a child's absolute position happens to move. +bool pcbCollabTestRotateItem( std::string aId, double aDeg ) +{ + PCB_EDIT_FRAME* fr = pcbFrame(); + BOARD_ITEM* item = testResolve( fr, aId ); + + if( !item ) + return false; + + fr->CallAfter( [fr, item, aDeg]() { + COROUTINE cor( [fr, item, aDeg]( int ) -> int + { + BOARD_COMMIT commit( fr ); + commit.Modify( item ); + item->Rotate( item->GetPosition(), + EDA_ANGLE( aDeg, DEGREES_T ) ); + commit.Push( wxT( "Collab test rotate" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + +// Resize a pad (the pad-properties dialog edit) — bug 04: pads are not visited +// by forEachTopItem at all, so the edit never reaches either wire. +bool pcbCollabTestSetPadSize( std::string aId, int aW, int aH ) +{ + PCB_EDIT_FRAME* fr = pcbFrame(); + BOARD_ITEM* item = testResolve( fr, aId ); + + if( !item || item->Type() != PCB_PAD_T ) + return false; + + PAD* pad = static_cast( item ); + + fr->CallAfter( [fr, pad, aW, aH]() { + COROUTINE cor( [fr, pad, aW, aH]( int ) -> int + { + BOARD_COMMIT commit( fr ); + commit.Modify( pad ); + pad->SetSize( PADSTACK::ALL_LAYERS, VECTOR2I( aW, aH ) ); + commit.Push( wxT( "Collab test pad size" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + +// Drag a track/shape END point only — bug 04: Drawings' json is position-only +// and GetPosition() is the START, so an end-point reshape of a graphic shape +// is invisible (tracks DO carry endpoints — the visible control case). +bool pcbCollabTestMoveEndpoint( std::string aId, int aDx, int aDy ) +{ + PCB_EDIT_FRAME* fr = pcbFrame(); + BOARD_ITEM* item = testResolve( fr, aId ); + + if( !item || ( !isTrackType( item->Type() ) && item->Type() != PCB_SHAPE_T ) ) + return false; + + fr->CallAfter( [fr, item, aDx, aDy]() { + COROUTINE cor( [fr, item, aDx, aDy]( int ) -> int + { + BOARD_COMMIT commit( fr ); + commit.Modify( item ); + + if( isTrackType( item->Type() ) ) + { + auto* t = static_cast( item ); + t->SetEnd( t->GetEnd() + VECTOR2I( aDx, aDy ) ); + } + else + { + auto* s = static_cast( item ); + s->SetEnd( s->GetEnd() + VECTOR2I( aDx, aDy ) ); + } + + commit.Push( wxT( "Collab test endpoint" ) ); + return 0; + } ); + cor.Call( 0 ); + } ); + + return true; +} + // Wrapper to return footprints as vector for JS iteration std::vector Board_GetFootprints(BOARD* board) { if (!board) return {}; @@ -1218,6 +1354,9 @@ EMSCRIPTEN_BINDINGS(pcbnew) { function("kicadSaveBoard", &kicadSaveBoard); // pcbnew-only test helper (no eeschema counterpart — name is not shared). function("kicadCollabTestItemBlob", &kicadCollabTestItemBlob); + // pcbnew-only ysync-review repro hooks (names not shared with eeschema). + function("kicadCollabTestSetPadSize", &pcbCollabTestSetPadSize); + function("kicadCollabTestMoveEndpoint", &pcbCollabTestMoveEndpoint); #ifndef KICAD_MERGED_EMBIND // JS names ALSO registered by eeschema_embind.cpp — in the merged image these are @@ -1232,6 +1371,9 @@ EMSCRIPTEN_BINDINGS(pcbnew) { function("kicadCollabSnapshotItems", &pcbCollabSnapshotItems); function("kicadCollabTestMoveFirst", &pcbCollabTestMoveFirst); function("kicadCollabGetPos", &pcbCollabGetPos); + // ysync-review repro hooks shared with eeschema (dispatched when merged). + function("kicadCollabTestRemoveItem", &pcbCollabTestRemoveItem); + function("kicadCollabTestRotateItem", &pcbCollabTestRotateItem); #endif // !KICAD_MERGED_EMBIND } #endif diff --git a/web/pcbjam-shared b/web/pcbjam-shared index 067170e..2eca2c3 160000 --- a/web/pcbjam-shared +++ b/web/pcbjam-shared @@ -1 +1 @@ -Subproject commit 067170e50f1c2ba19ffceb9815dcf5d34dc9e800 +Subproject commit 2eca2c31a2dd7a89e8a4a919beb65f465d9e6d0f diff --git a/web/standalone/src/wasm/collab/ysync-repros.test.ts b/web/standalone/src/wasm/collab/ysync-repros.test.ts new file mode 100644 index 0000000..038c041 --- /dev/null +++ b/web/standalone/src/wasm/collab/ysync-repros.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, it, vi } from "vitest"; +import * as Y from "yjs"; +import { + fileToDoc, + itemsWireToDelta, + kicadItemsMap, + parseItemsWireDelta, + renderItem, + type KicadItem, +} from "@pcbjam/shared"; + +// Bug 07b exercises the REAL sheet-manager + REAL kicad-binding + REAL yjs; only +// the provider connect ("./index" → connectKicadDoc) is faked so the async gap +// between destroy(old binding) and bind(new room) can be held open. +const { connectKicadDoc } = vi.hoisted(() => ({ connectKicadDoc: vi.fn() })); +vi.mock("./index", () => ({ connectKicadDoc })); + +import { + bindKicadCollab, + type KicadItemsBridge, + type KicadItemsModule, + type KicadItemsWindow, +} from "./kicad-binding"; +import { createSheetCollabManager } from "./sheet-manager"; +import type { ProviderConfig } from "./provider"; + +/** + * Reproduction tests for the 2026-07-02 ysync review — runtime-binding bugs + * (docs/features/ysync-review on the ysync-review worktree/branch). + * + * Convention: each repro asserts the CORRECT behavior and is marked `it.fails` + * with a comment naming the bug doc. The suite stays green while the bug is + * open; fixing the bug flips the test to "unexpected pass", which forces the + * marker's removal — the repro then becomes the regression test. + */ + +/** + * A C++-FAITHFUL fake of the v2 items bridge. In the real wasm the + * COLLAB_LISTENER — the only thing that turns local commits into emits — is + * registered lazily by ensureBridge(), which is reached ONLY through the + * snapshot entry points (pcbnew_embind.cpp:729 → :975/:998, + * eeschema_embind.cpp:578 → :909/:957). The plain FakeEditor in + * kicad-binding.test.ts cannot see that side effect (which is exactly why + * bug 01 shipped); this fake models it: local edits EMIT only after + * snapshotItems() has run at least once. + */ +class CppFaithfulEditor implements KicadItemsBridge { + store: Record = {}; + applied: string[] = []; // raw JSON of every applyItems call + snapshotCalls = 0; + /** The captured emit hook — fire directly to simulate a raw C++ emit. */ + emitCb: ((json: string) => void) | null = null; + + snapshotItems(): string { + this.snapshotCalls++; // ensureBridge(): registers the C++ change listener + const roots = Object.entries(this.store) + .filter(([, it]) => it.parent === null) + .map(([uuid]) => ({ + sexpr: renderItem({ items: this.store }, uuid), + parent: null, + })); + return JSON.stringify({ added: roots, changed: [], removed: [] }); + } + + applyItems(json: string): void { + this.applied.push(json); + this.applyToStore(json); // no emit — remote applies must not echo + } + + onItems(cb: (json: string) => void): void { + this.emitCb = cb; + } + + /** A local user edit: mutates the model; emits ONLY if the listener exists. */ + localUpsert( + sexpr: string, + parent: string | null = null, + kind: "added" | "changed" = "changed", + ): void { + const json = JSON.stringify({ [kind]: [{ sexpr, parent }] }); + this.applyToStore(json); + if (this.snapshotCalls > 0) this.emitCb?.(json); + } + + private applyToStore(json: string): void { + const delta = itemsWireToDelta(parseItemsWireDelta(json), this.store); + for (const it of [...delta.added, ...delta.updated]) { + const { uuid, ...item } = it; + this.store[uuid] = item; + } + for (const uuid of delta.removed) delete this.store[uuid]; + } +} + +/** Two Y.Docs joined by relaying updates (stand-in for any provider). */ +function pair(): { a: Y.Doc; b: Y.Doc } { + const a = new Y.Doc(); + const b = new Y.Doc(); + a.on("update", (u: Uint8Array) => Y.applyUpdate(b, u, "relay")); + b.on("update", (u: Uint8Array) => Y.applyUpdate(a, u, "relay")); + return { a, b }; +} + +const FILE = `(kicad_pcb + (version 20241229) + (footprint "lib:R" (layer "F.Cu") (uuid "fp-1") (at 10 10) + (pad "1" smd (at 0 0) (uuid "pad-1"))) +)`; + +// ── Bug 01 — file-seed never registers the C++ change listener ─────────────── +// 01-bug-first-tab-listener-never-registered.md: seed()'s file-seed branch +// (fresh room + seedDoc) runs docToY and returns WITHOUT bridge.snapshotItems(), +// so ensureBridge() never runs on the seeding tab — its local edits are never +// emitted (it receives peers' edits but cannot send). Every other seed branch +// calls snapshotItems() and is fine. + +describe("bug 01 — first-ever tab (file-seed branch) never registers the C++ listener", () => { + function freshRoomFileSeed() { + const { a, b } = pair(); + const edA = new CppFaithfulEditor(); + const edB = new CppFaithfulEditor(); + const bindA = bindKicadCollab(a, edA); + const bindB = bindKicadCollab(b, edB); + const seedDoc = fileToDoc(FILE); + Object.assign(edA.store, seedDoc.items); // A's editor opened the same file + bindA.seed(seedDoc); // fresh room → the file-seed branch + bindB.seed(); // B joins → adopts the doc + return { edA, edB }; + } + + it.fails("the file-seed branch calls snapshotItems (the listener-registration contract)", () => { + const { edA } = freshRoomFileSeed(); + // The one-line fix's contract: like the editorMatchesDoc branch, the + // file-seed branch must call snapshotItems() for its SIDE EFFECTS + // (ensureBridge listener registration + differ baseline). TODAY: 0 calls. + expect(edA.snapshotCalls).toBeGreaterThan(0); + }); + + it.fails("a local edit on the seeding tab reaches the joining peer", () => { + const { edA, edB } = freshRoomFileSeed(); + edA.localUpsert(`(segment (start 0 0) (end 1 1) (uuid "seg-new"))`, null, "added"); + // TODAY: A's listener was never registered → the edit is never emitted → + // the peer never receives it (the first-session "seeder can't send" hole). + expect(edB.store["seg-new"]).toBeDefined(); + }); + + it("control: the ADOPTING peer's edits flow back (the asymmetry IS the bug)", () => { + const { edA, edB } = freshRoomFileSeed(); + // B's adopt branch called snapshotItems() → B's listener exists → B→A works. + edB.localUpsert(`(segment (start 2 2) (end 3 3) (uuid "seg-b"))`, null, "added"); + expect(edA.store["seg-b"]).toBeDefined(); + }); +}); + +// ── Bug 07a — destroy() leaves the DOWN hook attached ──────────────────────── +// 07-bug-sheet-switch-stale-down-hook.md: KicadBinding.destroy() only +// unobserves the UP side (items.unobserveDeep); the DOWN hook registered via +// bridge.onItems() is never unregistered, so a C++ emit after destroy() still +// writes into the (now supposedly detached) doc. + +describe("bug 07a — destroy() leaves the DOWN hook (onItems) attached", () => { + it.fails("an emit after destroy() must not write into the doc", () => { + const doc = new Y.Doc(); + const ed = new CppFaithfulEditor(); + const binding = bindKicadCollab(doc, ed); + binding.seed(); // empty room, empty editor — snapshot branch registers all hooks + binding.destroy(); + + // The C++ side keeps emitting through the captured hook (in the real app: + // window.kicadCollab.onItems still points at this binding's closure). + ed.emitCb?.( + JSON.stringify({ + added: [{ sexpr: `(segment (start 9 9) (end 8 8) (uuid "seg-ghost"))`, parent: null }], + changed: [], + removed: [], + }), + ); + + // CORRECT: a destroyed binding is inert both ways. TODAY: the stale hook + // runs applyDeltaToY and the doc gains the item. + expect(kicadItemsMap(doc).get("seg-ghost")).toBeUndefined(); + }); +}); + +// ── Bug 07b — the sheet-switch gap routes edits into the OLD room ──────────── +// 07-bug-sheet-switch-stale-down-hook.md: doSwitch destroys the old binding, +// then AWAITS ensureRoom (a full connect round-trip for a cold room) before +// bindKicadCollab re-registers onItems. In that gap window.kicadCollab.onItems +// still points at the old binding's closure — and C++ has already rebaselined +// to the new sheet, so a local edit emits a new-sheet diff into the OLD room. + +describe("bug 07b — sheet-switch gap: stale onItems writes into the old sheet's room", () => { + it.fails("an emit during a cold-room switch gap must not land in the old doc", async () => { + const docs: Y.Doc[] = []; + let releaseB!: () => void; + const gateB = new Promise((resolve) => (releaseB = resolve)); + let connects = 0; + connectKicadDoc.mockReset(); + connectKicadDoc.mockImplementation(async () => { + const idx = connects++; + if (idx === 1) await gateB; // sheet B's room is COLD: hold the connect open + const doc = new Y.Doc(); + docs.push(doc); + return { doc, provider: { destroy: () => {} } }; + }); + + const win: KicadItemsWindow = {}; + const mod: KicadItemsModule = { + // The editor's model is empty — seed()'s snapshot branch stays a no-op. + kicadCollabSnapshotItems: () => JSON.stringify({ added: [], changed: [], removed: [] }), + kicadCollabApplyItems: () => {}, + }; + const m = createSheetCollabManager({ + mod, + win, + projectId: "P", + provider: { kind: "none" } satisfies ProviderConfig, + seedDocForPath: () => undefined, + log: () => {}, + }); + + await m.switchTo("a.kicad_sch"); // binds sheet A; onItems → A's closure + const aDoc = docs[0]!; + + const switching = m.switchTo("b.kicad_sch"); // destroys A's binding, awaits the cold connect + await vi.waitFor(() => expect(connects).toBe(2)); // we are inside the gap + + // C++ has already rebaselined to sheet B (OnSchSheetChanged fired before the + // JS switch completed); a local edit in the gap emits a B-scoped diff — + // through the STALE hook, which still writes into sheet A's doc. + win.kicadCollab!.onItems!( + JSON.stringify({ + added: [{ sexpr: `(wire (pts (xy 0 0) (xy 10 0)) (uuid "wire-b"))`, parent: null }], + changed: [], + removed: [], + }), + ); + + releaseB(); + await switching; + + // CORRECT: the old sheet's room never receives the new sheet's items (no + // cross-room contamination; peers on room A must not gain sheet B's wire). + // TODAY: the stale closure applied it to aDoc. + expect(kicadItemsMap(aDoc).get("wire-b")).toBeUndefined(); + + m.destroy(); + }); +});