Testing infra: flushPointerMoves seam + TestEditor polish + clipboard DI#44
Merged
Conversation
Called on every pointer move from the pen cursor computed. Iterates contour points inline (no points() generator allocation) and uses squared-distance comparison (no Math.sqrt).
Public method that drains any pending pointer-move immediately, bypassing rAF. Cancels the scheduled frame to avoid a double flush. Migrates the four rAF-stub blocks in ToolManager.test.ts to use the seam. Adds a targeted test locking in the contract: flush is synchronous and cancels the pending frame. Enables TestEditor.pointerMove to drive the full tool pipeline (gesture → tool event → state signal → cursor effect → hover update) without callers needing to stub requestAnimationFrame.
Calls toolManager.flushPointerMoves() after dispatching so the full pipeline (gesture → tool event → state signal → cursor effect → hover update) completes before pointerMove returns. Callers no longer need to stub requestAnimationFrame to observe the effect of a move. Adds a TestEditor.test.ts with a single contract assertion: two sequential pointerMove calls both register distinct mouse positions — an rAF-coalesced implementation would only retain the latest.
Drops hand-built ToolEvent factories (makeDragStart/Drag/DragEnd/DragCancel) and the transition(state, event) direct-call pattern. New Hand.test.ts drives the real pointer pipeline through TestEditor and asserts on the observable side effect: viewport pan. Three tests cover what users can trigger: - drag pans the viewport by the screen delta - escape mid-drag returns to ready and subsequent moves do not pan - pointer hover in ready state does not pan Per testing-strategy.md scope item #3.
Drops hand-built ToolEvent factories (makeDragStart/Drag/DragEnd/DragCancel) and transition(state, event) direct calls. New Shape.test.ts drives the real pointer pipeline and asserts on the observable side effect: contours added (or not added) to the glyph. Three tests cover what users can trigger: - drag then release commits a closed 4-point rectangle contour - escape mid-drag discards the preview without committing - drag smaller than the 3-unit minimum does not commit Per testing-strategy.md scope item #3.
Covers the runner's priority-resolution contract using stub steps: - runPointPipeline: no-match passthrough, point-to-point short-circuit (wins over closer metrics candidate and prevents later steps from running), closest-candidate fallback when no p2p match - runRotatePipeline: no-match passthrough, first-match semantics Stubs the step interface directly so the runner is tested in isolation from the real snap strategies in steps.ts. Per snapping-tests.md.
Covers the bridge's session contract against the real Rust engine (shift-node via createBridge): - no session / null \$glyph at construction - startEditSession populates \$glyph and reports the glyph name - endEditSession clears both - repeat start for the same glyph is a no-op (reference preserved) - switching glyphs replaces the Glyph instance - \$glyph notifies subscribers on session start Doesn't assert on the \$glyph-vs-data-change distinction — that contract is currently inconsistent between the bridge code and the reactive docs, and needs its own cleanup before being locked in by tests. Per bridge-tests.md.
Drives editor.copy/cut/paste through the real command pipeline and asserts on the resulting glyph state. Stubs window.electronAPI with an in-memory buffer so the Electron IPC round-trip works in the Node test environment. Four tests: - copy on empty selection returns false - copy + paste duplicates the selection (validates offset + PasteCommand path) - cut removes the selected points from the glyph - paste with nothing on the clipboard is a no-op Per clipboard-tests.md scope item 1 (tool test layer).
Sweeps the branch's test files for vi.spyOn / mock.calls.length / counter
assertions — the banned pattern per CLAUDE.md Testing ("Assert on state,
not mock calls") and the cleanup history (5f2f503, fa8a829, bd07e9d).
Changes:
ToolManager.test.ts
- Delete "flushPointerMoves drains..." spy+cancelAnimationFrame test
(added in this branch, pure mock-count anti-pattern)
- Delete "deduplicates pointer move (no force)" — dedup is an optimization
with no user-observable consequence; counting handleEvent calls is the
only way to verify it, which is the pattern we ban
- Delete "processes pointer move when force: true" — same reason
- Rewrite "onActivate callback" and "onReturn callback" to observe a
closure flag instead of vi.spyOn assertion
SnapPipelineRunner.test.ts
- Drop "applied: string[]" execution tracking from the two priority tests;
the returned result's source/delta already encodes which step won
NativeBridge.test.ts
- Delete the "$glyph signal notifies subscribers" counter test;
redundant with the other lifecycle tests that assert $glyph.peek() changes
All remaining tests (40 across 7 files) assert on observable state: tool
state, pan, glyph contours, point count, returned values.
Replaces the direct window.electronAPI.clipboard* calls inside the Clipboard class with an injected ClipboardAdapter boundary. Production wires electronClipboardAdapter through the Editor constructor; tests pass an in-memory fake (done in the next commit). This matches VSCode's IClipboardService + TestClipboardService pattern and aligns with the existing "real class + fake boundary" convention already used for the NativeBridge seam in TestEditor. Changes: - Add ClipboardAdapter interface to lib/clipboard/types.ts - Add electronClipboardAdapter (production impl) in electronAdapter.ts — throws loudly if window.electronAPI is missing rather than silently dropping ops - ClipboardDeps.adapter required; Clipboard.#write / #read go through it - Editor constructor accepts optional clipboardAdapter override (defaults to electronClipboardAdapter), mirroring the existing bridge override pattern - Re-export ClipboardAdapter and electronClipboardAdapter from the clipboard barrel No behavior change — existing Clipboard.test.ts still stubs the global and passes. The next commit drops that stub in favor of DI.
Replaces the stubElectronClipboard + vi.stubGlobal("window", ...) hack
with DI. TestEditor now constructs a FakeClipboardAdapter internally and
passes it to super(), and exposes the in-memory buffer as
editor.clipboardBuffer for direct assertion.
The test file is pure — no vi imports, no afterEach, no stubbing. Each
test drives the editor through its public API and reads observable state.
Also adds a new assertion ("copy writes a shift/glyph-data payload") that
verifies the serialized format without a round-trip, using clipboardBuffer
directly. This replaces the implicit "copy returned true" check with
something load-bearing.
5 tests total.
Two changes in one commit since they interact: 1. Rename - ClipboardAdapter → SystemClipboard (the interface) - electronClipboardAdapter → electronSystemClipboard (prod impl) - ClipboardDeps.adapter → ClipboardDeps.systemClipboard (the field) - Editor option .clipboardAdapter → .systemClipboard - electronAdapter.ts → electronSystemClipboard.ts "Adapter" was a pattern name, not a domain name. "SystemClipboard" says what the thing is — the OS-level clipboard — and avoids collision with the Clipboard orchestrator class. 2. Drop the default-fallback in Editor's constructor Previously: systemClipboard was optional on the Editor constructor, defaulting to electronSystemClipboard when not provided. That's a test-hatch hanging off production code — the optional param only exists for tests. Now systemClipboard is required. The one production callsite (store.ts) passes electronSystemClipboard explicitly. TestEditor passes its InMemorySystemClipboard. No fallback path, no "what happens if this is omitted" ambiguity.
Fourth acceptance box for clipboard-tests.md — verifies that DEFAULT_PASTE_OFFSET compounds across repeated pastes (the #pasteCount counter on the Clipboard class). After one copy + two pastes the three resulting contours are at x-offsets 0 / +20 / +40 from the original. Asserts on sorted x-extents so the test is independent of the contour array's insertion order (pasteContours prepends on the Rust side).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-submits the work from #43 against `main` — that PR merged into its intermediate base (`firmclaw/domain-model-sweep`) which landed on main before the stack caught up, so the testing-infra commits never made it to main. Same commits, clean diff against main now.
Summary
Tickets moved
Test plan