Skip to content

test(sprites): hoist lockAllAnchors fixture onto a real-locked prototype - #6204

Merged
atomantic merged 2 commits into
atomantic:mainfrom
Bryandero98:feat/hoist-sprite-lock-fixtures
Sep 4, 2026
Merged

test(sprites): hoist lockAllAnchors fixture onto a real-locked prototype#6204
atomantic merged 2 commits into
atomantic:mainfrom
Bryandero98:feat/hoist-sprite-lock-fixtures

Conversation

@Bryandero98

@Bryandero98 Bryandero98 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Addresses CI: cut fixture-construction file I/O in sprites/walk and sprites/atlas (successor to #6004) #6180's lockAllAnchors half: 136 call sites across walk.test.js/atlas.test.js/animationTrackWorkflow.test.js locked a character from byte-identical cached candidate PNGs, each re-running 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.
  • No call site changed except the three that create the record + now pass records through — the fast path lives entirely inside spriteTestFixtures.js.
  • Each suite gets an equivalence test (the acceptance gate CI: cut fixture-construction file I/O in sprites/walk and sprites/atlas (successor to #6004) #6180 asks for, not an extra): builds a character both through the real pipeline (lockAllAnchorsReal, exported unchanged) and through the materialized path, and asserts byte-identical results after id normalization — file names, PNG bytes, the full manifest (every embedded sha256), and the record's chromaKey/status.

Two options for this issue

This is option A of two — see #6207 for option B, which adds a buildFinalizedWalkSet optimization on top of this same lockAllAnchors change. I found that a copy-based materialization of buildFinalizedWalkSet (mirroring this PR's approach) doesn't actually help — that function never calls Sharp (writeWalkFramePng already 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 — same lockAllAnchors change, 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 the buildFinalizedWalkSet change.

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

  • Full server/services/sprites/ suite: 716 passed, 1 skipped, 0 failed (same pass count as main)
  • Both suites' new equivalence tests pass, and caught two real bugs during development (south-anchor sync, partial-lock manifest status) before landing
  • Measured locally (Windows): 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)
  • Windows CI before/after (will follow up with the actual CI numbers once this runs)

🤖 Generated with Claude Code

…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>
# Conflicts:
#	server/services/sprites/walk.test.js
@Bryandero98

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #6205 merged (span narrowing + capSharpThreads) — both touch the same 3 files. No functional changes here beyond the merge; full sprites suite re-verified post-merge (26/26 files, 716/717 tests, 1 pre-existing skip).

@atomantic
atomantic merged commit 91740f8 into atomantic:main Sep 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: cut fixture-construction file I/O in sprites/walk and sprites/atlas (successor to #6004)

2 participants