Skip to content

Commit 6b3ad09

Browse files
committed
fix(producer): tighten chunk-boundary test gates + narrow VIDEO_EXT indexing
Address @vanceingalls and @miguel-heygen review findings on #852: 1. Asymmetric soft-skip — only the N=1 plan+render+assemble call was wrapped in the host-Chrome-failure catch; an SwiftShader / cold-Chrome flake on the N=4 call would hard-fail instead of soft-skip. Factor a local runRender() helper and wrap both calls. 2. Vacuously-passing length assertion — 'expect(framesOne.length).toBe( framesFour.length)' passes when both runs produce 0 frames. Pin the absolute count (EXPECTED_FRAME_COUNT = 60) so a regression that identically truncates both renders shows red. 3. CDN version drift — anime-boundary loaded gsap@3.14.2 from jsdelivr while every other boundary fixture loaded 3.12.2 from cdnjs. Unify on cdnjs@3.12.2 so the next reader doesn't have to wonder why one fixture diverges. (gsap is an empty duration-driver in all six fixtures so the version was never load-bearing — but the divergence reads as intentional and isn't.) 4. VIDEO_EXT type narrowing — the lookup is Record<"mp4"|"mov"|"webm"> but outputFormat includes "png-sequence". The isPngSequence ternary short-circuits before png-sequence can reach the indexing site, but TS can't narrow through that. Add an explicit cast at the indexing site (not the lookup definition — over-widening to include "png-sequence": undefined would defeat the existence guarantee). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent bd21d00 commit 6b3ad09

3 files changed

Lines changed: 43 additions & 26 deletions

File tree

packages/producer/src/regression-harness.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,12 +722,19 @@ async function runTestSuite(
722722
// png-sequence output is a directory (basename = "frames"); encoded video
723723
// formats produce a single file (basename = "output.<ext>"). One lookup
724724
// covers both shapes for the in-temp render and the on-disk baseline.
725+
// `VIDEO_EXT` is intentionally typed against only the encoded-video set —
726+
// the `isPngSequence` ternary below short-circuits before `outputFormat`
727+
// can be `"png-sequence"`, but TS can't narrow through that, so we
728+
// assert the narrowing at the indexing site rather than over-widening
729+
// the lookup table.
725730
const VIDEO_EXT: Record<"mp4" | "mov" | "webm", string> = {
726731
mp4: ".mp4",
727732
mov: ".mov",
728733
webm: ".webm",
729734
};
730-
const outputBasename = isPngSequence ? "frames" : `output${VIDEO_EXT[outputFormat]}`;
735+
const outputBasename = isPngSequence
736+
? "frames"
737+
: `output${VIDEO_EXT[outputFormat as "mp4" | "mov" | "webm"]}`;
731738
const renderedOutputPath = join(tempRoot, outputBasename);
732739

733740
// Snapshot files stored in test's output/ directory. For png-sequence the

packages/producer/src/services/distributed/chunkBoundary.test.ts

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ const HOST_CHROME_FAILURE_PATTERNS =
3737
// assemble pipeline so no `output/` baseline is required.
3838
const ADAPTERS = ["gsap", "anime", "three", "lottie", "css", "waapi"] as const;
3939

40+
// Every adapter fixture is a 2-second composition at 30fps. Pin the absolute
41+
// count so a regression that produces fewer frames in both runs (e.g. a
42+
// probe stage that reads duration as 0s) doesn't pass vacuously.
43+
const EXPECTED_FRAME_COUNT = 60;
44+
4045
let runRoot: string;
4146
let testsDistributedDir: string;
4247

@@ -122,30 +127,30 @@ describe("per-adapter chunk-boundary byte equality", () => {
122127
mkdirSync(workOne, { recursive: true });
123128
mkdirSync(workFour, { recursive: true });
124129

125-
let outOne: string;
126-
try {
127-
outOne = await planAndAssemble({
128-
projectDir,
129-
workDir: workOne,
130-
chunkSize: 60,
131-
});
132-
} catch (err) {
133-
const message = err instanceof Error ? err.message : String(err);
134-
if (HOST_CHROME_FAILURE_PATTERNS.test(message)) {
135-
console.warn(
136-
`[chunkBoundary.test] skipping ${adapter} — host Chrome can't render. ` +
137-
"Docker harness covers the contract. Diagnostic:",
138-
message.slice(0, 240),
139-
);
140-
return;
130+
// Soft-skip when host Chrome can't render. Wrap *both* renders —
131+
// cold-Chrome / SwiftShader flakes happen on the second render
132+
// as readily as the first, and a hard-fail on the N=4 path would
133+
// diverge from the rest of the harness's soft-skip convention.
134+
const runRender = async (workDir: string, chunkSize: number): Promise<string | null> => {
135+
try {
136+
return await planAndAssemble({ projectDir, workDir, chunkSize });
137+
} catch (err) {
138+
const message = err instanceof Error ? err.message : String(err);
139+
if (HOST_CHROME_FAILURE_PATTERNS.test(message)) {
140+
console.warn(
141+
`[chunkBoundary.test] skipping ${adapter} — host Chrome can't render. ` +
142+
"Docker harness covers the contract. Diagnostic:",
143+
message.slice(0, 240),
144+
);
145+
return null;
146+
}
147+
throw err;
141148
}
142-
throw err;
143-
}
144-
const outFour = await planAndAssemble({
145-
projectDir,
146-
workDir: workFour,
147-
chunkSize: 15,
148-
});
149+
};
150+
const outOne = await runRender(workOne, 60);
151+
if (outOne === null) return;
152+
const outFour = await runRender(workFour, 15);
153+
if (outFour === null) return;
149154

150155
// Per-frame byte equality across the two frames directories. A
151156
// boundary regression in the adapter's seek-determinism would
@@ -157,7 +162,12 @@ describe("per-adapter chunk-boundary byte equality", () => {
157162
const framesFour = readdirSync(outFour)
158163
.filter((n) => n.toLowerCase().endsWith(".png"))
159164
.sort();
160-
expect(framesOne.length).toBe(framesFour.length);
165+
// Pin the absolute count, not just equality between the two runs.
166+
// Otherwise a regression that truncates BOTH renders identically
167+
// (e.g. a probe stage that misreads duration as 0s) would pass
168+
// vacuously — `0 === 0` is true.
169+
expect(framesOne.length).toBe(EXPECTED_FRAME_COUNT);
170+
expect(framesFour.length).toBe(EXPECTED_FRAME_COUNT);
161171
expect(framesOne).toEqual(framesFour);
162172
for (let i = 0; i < framesOne.length; i++) {
163173
const frameName = framesOne[i];

packages/producer/tests/distributed/anime-boundary/src/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<head>
44
<meta charset="utf-8" />
55
<title>chunk-boundary: anime.js</title>
6-
<script src="https://cdn.jsdelivr.net/npm/gsap@3.14.2/dist/gsap.min.js"></script>
6+
<script src="https://cdnjs.cloudflare.com/ajax/libs/gsap/3.12.2/gsap.min.js"></script>
77
<script src="https://cdn.jsdelivr.net/npm/animejs@4.0.2/lib/anime.iife.min.js"></script>
88
<style>
99
body,

0 commit comments

Comments
 (0)