Skip to content

Commit 70ed332

Browse files
test(studio): score edits on GSAP-animated size, scale, rotation and keyframes (#4866)
* test(studio): score edits on GSAP-animated size, scale, rotation, keyframes and from/fromTo * test(studio): settle the edit bench only once no shadow preview is waiting to be swapped in * test(studio): end a drag at its recorded pointer-up, even when no frame painted while held * ci(studio): run the keyframed edit accuracy cases in CI across 20 shards * refactor(studio): split the keyframe render and stray-css checks into small helpers * test(studio): bank the edit bench's smooth verdict as smooth, judged on the worst frame's work * style(studio): name the edit gate's smooth count helper * test(studio): count smooth edit cases only among the accurate ones * test(studio): only keyframed cases wait for the preview swap, so others keep main's undo timing * refactor(studio): keep the edit bench's keyframe check apart from the hidden preview scan * test(studio): check the keyframed cases are in the full grid without pinning its size * test(studio): quarantine the nudge sequence whose redo waits behind preview videos (#4889) * test(studio): bank the 255 keyframed edit cases that pass
1 parent 74e6df4 commit 70ed332

14 files changed

Lines changed: 2050 additions & 1035 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,15 +1013,15 @@ jobs:
10131013

10141014
# Manual editing accuracy in the built Studio, gated against the base branch's baseline.json.
10151015
studio-edit-accuracy:
1016-
name: "Studio: edit accuracy (${{ matrix.shard }}/12)"
1016+
name: "Studio: edit accuracy (${{ matrix.shard }}/20)"
10171017
needs: [changes, build]
10181018
if: github.event_name == 'pull_request' && needs.changes.outputs.edit_accuracy == 'true'
10191019
runs-on: ubuntu-latest
10201020
timeout-minutes: 45
10211021
strategy:
10221022
fail-fast: false
10231023
matrix:
1024-
shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
1024+
shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]
10251025
steps:
10261026
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
10271027
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
@@ -1046,7 +1046,7 @@ jobs:
10461046
run: |
10471047
set -euo pipefail
10481048
bench() { bun run --cwd packages/studio test:edit-accuracy -- --grid full --jobs 2 "$@"; }
1049-
bench --shard "${{ matrix.shard }}/12" --out /tmp/edit-accuracy/run1
1049+
bench --shard "${{ matrix.shard }}/20" --out /tmp/edit-accuracy/run1
10501050
mapfile -t FLIPPED < <(node packages/studio/tests/e2e/edit-accuracy/ratchet.mjs flipped \
10511051
/tmp/base-edit-accuracy.json /tmp/edit-accuracy/run1/results.json)
10521052
if (( ${#FLIPPED[@]} > 0 )); then

‎packages/studio/tests/e2e/edit-accuracy/baseline.json‎

Lines changed: 1565 additions & 965 deletions
Large diffs are not rendered by default.

‎packages/studio/tests/e2e/edit-accuracy/case.mjs‎

Lines changed: 125 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -242,20 +242,44 @@ const previewFrames = (page) =>
242242
.filter((u) => u.includes("/preview"))
243243
.join(" ");
244244

245-
/** Measures once the preview frames and the box have held still for STILL_MS; Studio updates both after a save. */
245+
/** A hidden preview holding the target is a shadow reload not yet promoted: the visible frame is about to go stale. */
246+
async function hiddenTarget(page, selector = "#target") {
247+
for (const f of page.frames().filter((f) => f.url().includes("/preview"))) {
248+
const host = await f.frameElement().catch(() => null);
249+
const shown = await host?.evaluate((e) => e.checkVisibility({ visibilityProperty: true }));
250+
if (shown === false && (await f.$(selector).catch(() => null))) return true;
251+
}
252+
return false;
253+
}
254+
255+
// Keyframed cases only: waiting out the swap lands a later undo in the preview's burst of requests.
256+
export const swapPending = (ctx) => Boolean(ctx.keys) && hiddenTarget(ctx.page, ctx.selector);
257+
258+
/** Restart the stillness window: a pending swap, a changed set of preview frames, or the box moved. */
259+
export const unsettledBy = (start, now) =>
260+
start.pending ||
261+
now.pending ||
262+
now.frames !== start.frames ||
263+
quadDistance(now.m.visible, start.m.visible) >= 0.01;
264+
265+
/** Measures once the shown preview and the box have held still for STILL_MS; Studio updates both after a save. */
246266
// fallow-ignore-next-line complexity
247267
export async function settled(ctx, timeout = 15_000) {
248268
const deadline = Date.now() + timeout;
249-
let start = { m: await measure(ctx), frames: previewFrames(ctx.page) };
269+
const read = async () => ({
270+
m: await measure(ctx),
271+
frames: previewFrames(ctx.page),
272+
pending: await swapPending(ctx),
273+
});
274+
let start = await read();
250275
let now = start;
251276
// Compared with the window's first read, so a drift too slow to show read to read still restarts it.
252-
for (let since = Date.now(); Date.now() - since < STILL_MS; ) {
277+
for (let since = Date.now(); start.pending || Date.now() - since < STILL_MS; ) {
253278
// A preview that never holds still is a Studio defect: the metrics it feeds fail, the rest still count.
254279
if (Date.now() > deadline) return { ...now.m, unsettled: true };
255280
await nextFrame(ctx.page);
256-
now = { m: await measure(ctx), frames: previewFrames(ctx.page) };
257-
if (now.frames !== start.frames || quadDistance(now.m.visible, start.m.visible) >= 0.01)
258-
[start, since] = [now, Date.now()];
281+
now = await read();
282+
if (unsettledBy(start, now)) [start, since] = [now, Date.now()];
259283
}
260284
return now.m;
261285
}
@@ -268,9 +292,14 @@ export async function openStudio(ctx) {
268292
let seek = null;
269293
for (const deadline = Date.now() + 30_000; Date.now() < deadline; await sleep(250)) {
270294
seek = await ctx.page
271-
.evaluate((time) => window.__editBench.call("studio_seek", { time }), PLAYHEAD)
295+
.evaluate((time) => window.__editBench.call("studio_seek", { time }), ctx.playhead)
272296
.catch(String);
273-
if (seek?.ok && seek.duration > 0 && seek.playhead === PLAYHEAD && (await findTarget(ctx.page)))
297+
if (
298+
seek?.ok &&
299+
seek.duration > 0 &&
300+
seek.playhead === ctx.playhead &&
301+
(await findTarget(ctx.page))
302+
)
274303
break;
275304
seek = null;
276305
}
@@ -279,6 +308,75 @@ export async function openStudio(ctx) {
279308
return settled(ctx);
280309
}
281310

311+
async function seekTo(ctx, time) {
312+
const seek = await ctx.page.evaluate(
313+
(t) => window.__editBench.call("studio_seek", { time: t }),
314+
time,
315+
);
316+
if (!seek?.ok || seek.playhead !== time)
317+
throw new Error(`studio_seek ${time}: ${JSON.stringify(seek)}`);
318+
return settled(ctx);
319+
}
320+
321+
/** Each GSAP-animated property's value at every other keyframe time (and the box there), then back to the playhead. */
322+
async function readKeyframes(ctx, keys, withBox = false) {
323+
const at = {};
324+
for (const time of keys.times) {
325+
const m = await seekTo(ctx, time);
326+
const values = await ctx.handles.target.evaluate((el, props) => {
327+
const gsap = el.ownerDocument.defaultView.gsap;
328+
return Object.fromEntries(props.map((p) => [p, Number.parseFloat(gsap.getProperty(el, p))]));
329+
}, keys.props);
330+
at[time] = { values, ...(withBox && { visible: m.visible }) };
331+
}
332+
await seekTo(ctx, ctx.playhead);
333+
return at;
334+
}
335+
336+
// GSAP's own numbers (px, deg, scale): an untouched keyframe reads back exactly.
337+
const KEY_TOLERANCE = 0.01;
338+
339+
/** The largest change of an animated value at a keyframe the edit was not on; NaN (unreadable) fails. */
340+
export function keyframeDrift(before, after) {
341+
let worst = { diff: 0, time: null, prop: null };
342+
for (const [time, b] of Object.entries(before))
343+
for (const [prop, v] of Object.entries(b.values)) {
344+
const diff = Math.abs(after[time].values[prop] - v);
345+
if (!(diff <= worst.diff)) worst = { diff, time: Number(time), prop };
346+
}
347+
return { ...worst, pass: worst.diff <= KEY_TOLERANCE };
348+
}
349+
350+
const declarations = (text = "") =>
351+
Object.fromEntries(
352+
text
353+
.split(";")
354+
.map((d) => d.split(":"))
355+
.filter((d) => d.length > 1)
356+
.map(([k, ...v]) => [k.trim(), v.join(":").trim()]),
357+
);
358+
const capture = (re, text = "") => re.exec(text)?.[1];
359+
const targetCss = (html) => ({
360+
rule: declarations(capture(/#target\s*\{([^}]*)\}/, html)),
361+
inline: declarations(capture(/\bstyle="([^"]*)"/, capture(/(<[^>]*\bid="target"[^>]*>)/, html))),
362+
});
363+
364+
/** Plain CSS the edit wrote for a property GSAP animates: it would override or fight the timeline. */
365+
export function strayCss(original, saved, props) {
366+
const changed = (a, b, file, where) =>
367+
props
368+
.filter((p) => a[p] !== b[p])
369+
.map((p) => `${file} ${where} ${p}: ${a[p] ?? "-"} -> ${b[p] ?? "-"}`);
370+
const stray = Object.keys(original).flatMap((file) => {
371+
const [a, b] = [targetCss(original[file]), targetCss(saved[file])];
372+
return [
373+
...changed(a.rule, b.rule, file, "rule"),
374+
...changed(a.inline, b.inline, file, "inline"),
375+
];
376+
});
377+
return { pass: stray.length === 0, stray };
378+
}
379+
282380
/** Puppeteer presses one key at a time: hold the modifiers around the last key. */
283381
export async function chord(page, keys) {
284382
const [key, ...mods] = keys.split("+").reverse();
@@ -672,7 +770,14 @@ async function nudgeGesture(ctx, pre) {
672770
export async function inStudio({ browser, spec, dir, files, url, evidence }, drive) {
673771
const context = await browser.createBrowserContext();
674772
const page = await context.newPage();
675-
const ctx = { page, dir, files, handles: null };
773+
const ctx = {
774+
page,
775+
dir,
776+
files,
777+
handles: null,
778+
playhead: spec.playhead ?? PLAYHEAD,
779+
keys: spec.keys,
780+
};
676781
const consoleErrors = [];
677782
page.on("pageerror", (e) => consoleErrors.push(e.message));
678783
evidence.shots = {};
@@ -686,10 +791,12 @@ export async function inStudio({ browser, spec, dir, files, url, evidence }, dri
686791
let pre = await openStudio(ctx);
687792
await disableSnap(page);
688793
const zoom = await setZoom(ctx, spec.zoom, pre.map.toScreen(centre(pre.visible)));
794+
// The animated values at the other keyframes, read before anything is selected or edited.
795+
const keysBefore = spec.keys && (await readKeyframes(ctx, spec.keys));
689796
pre = await settled(ctx);
690797
await selectTarget(ctx, pre);
691798
pre = await settled(ctx);
692-
return await drive({ ctx, page, pre, zoom, shoot, consoleErrors });
799+
return await drive({ ctx, page, pre, zoom, shoot, consoleErrors, keysBefore });
693800
} catch (error) {
694801
await shoot("error").catch(() => undefined);
695802
throw error;
@@ -707,7 +814,7 @@ export async function runCase(args) {
707814
// fallow-ignore-next-line complexity
708815
async function measureCase(
709816
{ spec, dir, files, evidence },
710-
{ ctx, page, pre, zoom, shoot, consoleErrors },
817+
{ ctx, page, pre, zoom, shoot, consoleErrors, keysBefore },
711818
control,
712819
) {
713820
const original = readFiles(dir, files);
@@ -753,6 +860,8 @@ async function measureCase(
753860
await page.reload();
754861
const reloaded = await openStudio(ctx);
755862
await shoot("reloaded");
863+
// From the saved file: the other keyframes keep their values, and no animated property gets plain CSS.
864+
const keysAfter = spec.keys && (await readKeyframes(ctx, spec.keys, true));
756865
const quads = Object.fromEntries(
757866
Object.entries({ pre, committed, undone, redone, reloaded }).filter(([, m]) => m),
758867
);
@@ -793,6 +902,11 @@ async function measureCase(
793902
smooth: { ...drive.smooth, control },
794903
unsettled: Object.keys(quads).filter((k) => quads[k].unsettled),
795904
reloaded,
905+
...(spec.keys && {
906+
keys: keyframeDrift(keysBefore, keysAfter),
907+
css: strayCss(original, readFiles(dir, files), spec.keys.css),
908+
keyRender: { time: spec.keys.render, visible: keysAfter[spec.keys.render].visible },
909+
}),
796910
diag: {
797911
...drive.diag,
798912
consoleErrors: consoleErrors.slice(0, 5),

‎packages/studio/tests/e2e/edit-accuracy/grid.mjs‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,76 @@ const AXES = {
2323
zoom: [50, 100, 200],
2424
};
2525

26+
// Timelines with keyframes at 0, 2 and 3 s on the property each animates: `css` is what that property is in CSS.
27+
const T = '"#target"';
28+
const KEYFRAMED = {
29+
size: {
30+
lines: [
31+
`tl.to(${T}, { width: 300, height: 200, duration: 2, ease: "none" }, 0);`,
32+
`tl.to(${T}, { width: 360, height: 240, duration: 1, ease: "none" }, 2);`,
33+
],
34+
props: ["width", "height"],
35+
css: ["width", "height"],
36+
},
37+
scale: {
38+
lines: [
39+
`tl.to(${T}, { scale: 1.25, duration: 2, ease: "none" }, 0);`,
40+
`tl.to(${T}, { scale: 1.5, duration: 1, ease: "none" }, 2);`,
41+
],
42+
props: ["scaleX", "scaleY"],
43+
css: ["scale", "transform"],
44+
},
45+
spin: {
46+
lines: [
47+
`tl.to(${T}, { rotation: 20, duration: 2, ease: "none" }, 0);`,
48+
`tl.to(${T}, { rotation: 40, duration: 1, ease: "none" }, 2);`,
49+
],
50+
props: ["rotation"],
51+
css: ["rotate", "transform"],
52+
},
53+
// A resize meets a keyframes array that also animates width.
54+
keys: {
55+
lines: [
56+
`tl.to(${T}, { keyframes: [{ x: 60, width: 280, duration: 2, ease: "none" }, { x: 120, width: 320, duration: 1, ease: "none" }] }, 0);`,
57+
],
58+
props: ["x", "width"],
59+
css: ["left", "top", "translate", "transform", "width"],
60+
},
61+
fromto: {
62+
lines: [
63+
`tl.from(${T}, { x: -60, duration: 2, ease: "none" }, 0);`,
64+
`tl.fromTo(${T}, { x: 0 }, { x: 60, duration: 1, ease: "none" }, 2);`,
65+
],
66+
props: ["x"],
67+
css: ["left", "top", "translate", "transform"],
68+
},
69+
};
70+
const KEY_TIMES = [0, 2, 3];
71+
// On a keyframe the edit changes that keyframe; between two it adds one at the playhead.
72+
const AT = { on: 2, mid: 1 };
73+
74+
function keyframedCases() {
75+
return product({
76+
gsap: Object.keys(KEYFRAMED),
77+
placement: ["px"],
78+
rotation: AXES.rotation,
79+
nesting: AXES.nesting,
80+
zoom: AXES.zoom,
81+
gesture: GESTURES,
82+
at: Object.keys(AT),
83+
}).map((c) => ({
84+
id: `${caseId(c)}-${c.at}`,
85+
...c,
86+
playhead: AT[c.at],
87+
keys: {
88+
times: KEY_TIMES.filter((t) => t !== AT[c.at]),
89+
props: KEYFRAMED[c.gsap].props,
90+
css: KEYFRAMED[c.gsap].css,
91+
render: KEY_TIMES.at(-1),
92+
},
93+
}));
94+
}
95+
2696
const product = (axes) =>
2797
Object.entries(axes).reduce(
2898
(rows, [key, values]) => rows.flatMap((row) => values.map((v) => ({ ...row, [key]: v }))),
@@ -31,14 +101,15 @@ const product = (axes) =>
31101
const caseId = (c) =>
32102
[c.gesture, c.gsap, c.placement, `r${c.rotation}`, c.nesting, `z${c.zoom}`].join("-");
33103

34-
/** `pr` is a smaller slice for CI; its final size is still an open decision. */
104+
/** `pr` is a smaller slice for CI; `keyframes` is the GSAP-animated set alone, which `full` also runs. */
35105
export function buildGrid(kind = "full") {
106+
if (kind === "keyframes") return keyframedCases();
36107
return (
37108
product({ ...AXES, gesture: GESTURES })
38109
// xPercent only exists through GSAP on the target itself.
39110
.filter((c) => c.placement !== "xpercent" || !["none", "idle"].includes(c.gsap))
40111
.map((c) => ({ id: caseId(c), ...c, other: c.gsap === "idle" }))
41-
.concat(dragCases())
112+
.concat(dragCases(), keyframedCases())
42113
.filter((c) => kind !== "pr" || (c.zoom === 100 && c.nesting === "root"))
43114
);
44115
}
@@ -74,6 +145,7 @@ function targetCss(spec) {
74145

75146
// fallow-ignore-next-line complexity
76147
function gsapLines(spec) {
148+
if (KEYFRAMED[spec.gsap]) return KEYFRAMED[spec.gsap].lines;
77149
if (spec.gsap === "idle") return [`tl.to("#other", { x: 120, duration: 4, ease: "none" }, 0);`];
78150
const percent = spec.placement === "xpercent" ? ", xPercent: -50, yPercent: -50" : "";
79151
if (spec.gsap === "hold") return [`gsap.set("#target", { x: 40, y: 20${percent} });`];

0 commit comments

Comments
 (0)