test(sprites): hoist lockAllAnchors fixture onto a real-locked prototype - #6204
Merged
atomantic merged 2 commits intoSep 4, 2026
Merged
Conversation
…ype (atomantic#6180) 136 call sites across walk.test.js/atlas.test.js/animationTrackWorkflow.test.js locked a character from byte-identical cached candidate PNGs, so each one re-ran the real Sharp normalize + chroma-key-selection pipeline from scratch. `lockAllAnchors` now locks ONE real prototype per TEST_ROOT (at the full anchor set, since every caller's `directions` is a subset of it) and materializes every other character by recursive-copying only the artifacts a caller actually asked for, id-normalizing the manifest, and resetting any anchor outside the requested set to the exact seeded-pending shape a fresh partial lock would leave (mirroring unlockReferenceAnchorImpl's own reset). No call site changed except the three that build the record + pass `records` through (`characterWithLockedAnchors`, atlas.test.js's `lockAllAnchors(id)`, animationTrackWorkflow.test.js's `characterWithEastAnchor`) — the fast path lives entirely inside spriteTestFixtures.js. Each suite gets an equivalence test (the acceptance gate, not an extra, per the issue): it builds a character both ways — through `lockAllAnchorsReal` (exported unchanged) and through the materialized path — and asserts the result is byte-identical after id normalization: file names, PNG bytes, the full manifest (including every embedded sha256), and the record's chromaKey/status. The partial-set case is the one this test actually earns its keep on: an early version of the reset logic left the `south` anchor entry wrongly reset to pending (it's synced from mainReference, never locked through its own `target`) and left a partial manifest's `status` at 'complete' instead of 'in-progress' — both caught by this test, not by eye. Measured locally (Windows, before/after, `npx vitest run <file>`): walk.test.js: ~29.4s -> ~19.7-21.4s tests phase (~30% faster) atlas.test.js: ~24.4s -> ~20.2-21.3s tests phase (~15% faster) atlas.test.js's own numbers are lower than walk.test.js's because it only has 19 lockAllAnchors calls (all at the full 9-anchor set) vs walk.test.js's 117 (mostly 1-3 anchors) -- fewer calls to save Sharp work on, even though each one touches more anchors. buildFinalizedWalkSet (atlas.test.js's other named fixture cost per the issue, ~32% of its suite time) is untouched here: it never calls Sharp (writeWalkFramePng already caches the encoded buffer), so a copy-based materialization of it doesn't help -- see the companion PR for what does. Full sprites/ suite: 716 passed, 1 skipped, 0 failed (unchanged pass count). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
# Conflicts: # server/services/sprites/walk.test.js
Contributor
Author
|
Rebased onto |
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
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.
Summary
lockAllAnchorshalf: 136 call sites acrosswalk.test.js/atlas.test.js/animationTrackWorkflow.test.jslocked a character from byte-identical cached candidate PNGs, each re-running the real Sharp normalize + chroma-key-selection pipeline from scratch.lockAllAnchorsnow locks ONE real prototype perTEST_ROOT(at the full anchor set, since every caller'sdirectionsis a subset of it) and materializes every other character by recursive-copying only the artifacts a caller actually asked for, id-normalizing the manifest, and resetting any anchor outside the requested set to the exact seeded-pending shape a fresh partial lock would leave.recordsthrough — the fast path lives entirely insidespriteTestFixtures.js.lockAllAnchorsReal, exported unchanged) and through the materialized path, and asserts byte-identical results after id normalization — file names, PNG bytes, the full manifest (every embeddedsha256), and the record'schromaKey/status.Two options for this issue
This is option A of two — see #6207 for option B, which adds a
buildFinalizedWalkSetoptimization on top of this samelockAllAnchorschange. I found that a copy-based materialization ofbuildFinalizedWalkSet(mirroring this PR's approach) doesn't actually help — that function never calls Sharp (writeWalkFramePngalready caches the encoded buffer), so copying files isn't cheaper than the cached-buffer writes it already does. #6207 instead parallelizes that function's 8 independent per-direction writes, which is a real win with much less mechanism. I'd merge #6207 over this one — samelockAllAnchorschange, plus a real additional win, at lower risk (no new copy/rewrite logic, so no new class of drift to worry about). This PR stands alone as the smaller, more conservative option if you'd rather not take thebuildFinalizedWalkSetchange.Only #6207 carries "Closes #6180" — merging either is fine, but I didn't want both auto-closing the issue out from under the other.
Test plan
server/services/sprites/suite: 716 passed, 1 skipped, 0 failed (same pass count asmain)walk.test.js~29.4s → ~19.7-21.4s tests phase (~30% faster);atlas.test.js~24.4s → ~20.2-21.3s tests phase (~15% faster)⏱before/after (will follow up with the actual CI numbers once this runs)🤖 Generated with Claude Code