Skip to content

Commit cdddd21

Browse files
Paul Mulliganclaude
Paul Mulligan
and
claude
committed
fix(scripts): report visual mismatch as a true percentage
Caught during an end-to-end smoke against a live app: visual-diff.js's JSON field mismatchPct is a 0-1 RATIO despite the name, and both consumers appended "%" to it — understating diffs 100x (a 7.47% pixel change printed as "0.0747%"). cross-browser-baseline now carries the value as mismatchRatio and formats real percentages in its log lines and report; regression-test.sh converts before printing. Also applies repo Prettier style to the new files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 439e18a commit cdddd21

8 files changed

Lines changed: 160 additions & 119 deletions

‎scripts/__tests__/baseline-backends.test.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ describe("ci-artifact backend", () => {
8282
});
8383

8484
it("wraps a missing gh CLI in a BackendError", async () => {
85-
const { exec } = recorder({ "gh run list": Object.assign(new Error("ENOENT"), { code: "ENOENT" }) });
85+
const { exec } = recorder({
86+
"gh run list": Object.assign(new Error("ENOENT"), { code: "ENOENT" }),
87+
});
8688
const backend = resolveBackend(config, { execFile: exec });
8789
await expect(backend.fetch({ baselineDir: "/ignored" })).rejects.toThrow(BackendError);
8890
});

‎scripts/__tests__/baseline-manifest.test.js‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,7 @@
22
// manifest (record / sync / verify) for cross-browser screenshot baselines.
33
import { describe, it, expect, beforeAll, afterAll } from "vitest";
44
import { execFileSync } from "child_process";
5-
import {
6-
writeFileSync,
7-
readFileSync,
8-
mkdirSync,
9-
mkdtempSync,
10-
rmSync,
11-
existsSync,
12-
} from "fs";
5+
import { writeFileSync, readFileSync, mkdirSync, mkdtempSync, rmSync, existsSync } from "fs";
136
import { createHash } from "crypto";
147
import { join, dirname } from "path";
158
import { tmpdir } from "os";
@@ -397,12 +390,18 @@ describe("CLI", () => {
397390
const { stdout, exitCode } = runCli([
398391
"verify",
399392
"--json",
400-
"--baseline-dir", baselineDir,
401-
"--manifest", manifestPath,
402-
"--engines", "firefox",
403-
"--host", "container",
404-
"--playwright-version", "1.61.1",
405-
"--image", ENVELOPE.image,
393+
"--baseline-dir",
394+
baselineDir,
395+
"--manifest",
396+
manifestPath,
397+
"--engines",
398+
"firefox",
399+
"--host",
400+
"container",
401+
"--playwright-version",
402+
"1.61.1",
403+
"--image",
404+
ENVELOPE.image,
406405
]);
407406
expect(exitCode).toBe(0);
408407
const parsed = JSON.parse(stdout);
@@ -424,12 +423,18 @@ describe("CLI", () => {
424423
const { stdout, exitCode } = runCli([
425424
"verify",
426425
"--json",
427-
"--baseline-dir", baselineDir,
428-
"--manifest", manifestPath,
429-
"--engines", "firefox",
430-
"--host", "container",
431-
"--playwright-version", "1.61.1",
432-
"--image", ENVELOPE.image,
426+
"--baseline-dir",
427+
baselineDir,
428+
"--manifest",
429+
manifestPath,
430+
"--engines",
431+
"firefox",
432+
"--host",
433+
"container",
434+
"--playwright-version",
435+
"1.61.1",
436+
"--image",
437+
ENVELOPE.image,
433438
]);
434439
expect(exitCode).toBe(1);
435440
const parsed = JSON.parse(stdout);

‎scripts/__tests__/cross-browser-baseline.test.js‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,17 @@ describe("compare (commit backend)", () => {
198198
currents: { "firefox/home/desktop_1440px.png": pngBytes(BLUE) },
199199
});
200200
record(proj);
201-
const { exitCode, json } = runJson(
201+
const { exitCode, json, stderr } = runJson(
202202
["compare", "--json", "--blocking", "--current-dir", proj.currentDir, "--host", "container"],
203203
proj,
204204
);
205205
expect(exitCode).toBe(1);
206206
expect(json.blocking).toBe(true);
207+
// visual-diff.js emits a mismatch RATIO; the CLI must report a true percent
208+
// (a fully-different image is 100%, not "1%").
209+
expect(json.results[0].mismatchRatio).toBe(1);
210+
expect(stderr).toContain("(100.00%)");
211+
expect(readFileSync(proj.reportFile, "utf-8")).toContain("100.00%");
207212
});
208213

209214
it("exits 1 on failures when config sets blocking: true (Phase B teeth)", () => {
@@ -228,10 +233,7 @@ describe("compare (commit backend)", () => {
228233
});
229234
record(proj);
230235
// Rewrite the baseline after recording provenance → sha mismatch.
231-
writeFileSync(
232-
join(proj.baselineDir, "firefox", "home", "desktop_1440px.png"),
233-
tampered,
234-
);
236+
writeFileSync(join(proj.baselineDir, "firefox", "home", "desktop_1440px.png"), tampered);
235237
const { json, exitCode } = runJson(
236238
["compare", "--json", "--current-dir", proj.currentDir, "--host", "container"],
237239
proj,
@@ -257,10 +259,7 @@ describe("compare (commit backend)", () => {
257259
writeFileSync(proj.configPath, JSON.stringify(configRaw, null, 2));
258260

259261
record(proj);
260-
writeFileSync(
261-
join(proj.baselineDir, "firefox", "home", "desktop_1440px.png"),
262-
tampered,
263-
);
262+
writeFileSync(join(proj.baselineDir, "firefox", "home", "desktop_1440px.png"), tampered);
264263
const { json, exitCode } = runJson(
265264
["compare", "--json", "--current-dir", proj.currentDir, "--host", "container"],
266265
proj,
@@ -327,7 +326,16 @@ describe("compare (commit backend)", () => {
327326
});
328327
record(proj);
329328
const { json } = runJson(
330-
["compare", "--json", "--engines", "firefox", "--current-dir", proj.currentDir, "--host", "container"],
329+
[
330+
"compare",
331+
"--json",
332+
"--engines",
333+
"firefox",
334+
"--current-dir",
335+
proj.currentDir,
336+
"--host",
337+
"container",
338+
],
331339
proj,
332340
);
333341
expect(json.results).toHaveLength(1);
@@ -409,10 +417,7 @@ describe("verify", () => {
409417
record(proj);
410418
expect(run(["verify", "--json", "--host", "container"], proj).exitCode).toBe(0);
411419

412-
writeFileSync(
413-
join(proj.baselineDir, "firefox", "home", "desktop_1440px.png"),
414-
pngBytes(BLUE),
415-
);
420+
writeFileSync(join(proj.baselineDir, "firefox", "home", "desktop_1440px.png"), pngBytes(BLUE));
416421
const { exitCode, json } = runJson(["verify", "--json", "--host", "container"], proj);
417422
expect(exitCode).toBe(1);
418423
expect(json.statuses["firefox/home/desktop_1440px.png"]).toBe("modified");
@@ -424,10 +429,7 @@ describe("capture", () => {
424429
"fails with an install hint when Playwright is not resolvable",
425430
() => {
426431
const proj = makeProject({});
427-
const { exitCode, stderr } = run(
428-
["capture", "http://127.0.0.1:9", "--local"],
429-
proj,
430-
);
432+
const { exitCode, stderr } = run(["capture", "http://127.0.0.1:9", "--local"], proj);
431433
expect(exitCode).toBe(2);
432434
expect(stderr).toMatch(/@playwright\/test/);
433435
},
@@ -490,23 +492,24 @@ describe("capture — pinned container wrapping (Phase B)", () => {
490492

491493
it.skipIf(playwrightResolvable)("--local bypasses the container wrapper", () => {
492494
const proj = makeProject({ config: containerConfig });
493-
const { exitCode, stderr } = run(
494-
["capture", "http://127.0.0.1:9", "--local"],
495-
proj,
496-
);
495+
const { exitCode, stderr } = run(["capture", "http://127.0.0.1:9", "--local"], proj);
497496
expect(exitCode).toBe(2);
498497
expect(stderr).toMatch(/@playwright\/test/);
499498
expect(stderr).not.toMatch(/docker run/i);
500499
});
501500

502501
it("rejects an image tag it cannot derive a Playwright version from", () => {
503502
const proj = makeProject({
504-
config: { capture: { mode: "container", image: "mcr.microsoft.com/playwright:latest", waitAfterLoadMs: 0, fullPage: true } },
503+
config: {
504+
capture: {
505+
mode: "container",
506+
image: "mcr.microsoft.com/playwright:latest",
507+
waitAfterLoadMs: 0,
508+
fullPage: true,
509+
},
510+
},
505511
});
506-
const { exitCode, stderr } = run(
507-
["capture", "http://localhost:3000", "--dry-run"],
508-
proj,
509-
);
512+
const { exitCode, stderr } = run(["capture", "http://localhost:3000", "--dry-run"], proj);
510513
expect(exitCode).toBe(2);
511514
expect(stderr).toMatch(/image/i);
512515
});

‎scripts/__tests__/visual-baselines-config.test.js‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ describe("visualBaselines config section", () => {
6060
expect(liveConfig.visualBaselines.backend).toBe("commit");
6161
expect(liveConfig.visualBaselines.storage).toBe("git");
6262
expect(liveConfig.visualBaselines.blocking).toBe(false);
63-
expect(liveConfig.visualBaselines.threshold).toBe(
64-
liveConfig.e2e.crossBrowserDiffThreshold,
65-
);
63+
expect(liveConfig.visualBaselines.threshold).toBe(liveConfig.e2e.crossBrowserDiffThreshold);
6664
expect(liveConfig.visualBaselines.provenance.manifest).toBe(
6765
".claude/visual-qa/baselines/manifest.json",
6866
);
@@ -76,9 +74,7 @@ describe("visualBaselines config section", () => {
7674
expect(result.ok).toBe(false);
7775
expect(result.exitCode).toBe(1);
7876
const flagged = result.schemaErrors.some(
79-
(e) =>
80-
e.path.startsWith("/visualBaselines") &&
81-
e.extras.some((x) => x.includes("bogusKey")),
77+
(e) => e.path.startsWith("/visualBaselines") && e.extras.some((x) => x.includes("bogusKey")),
8278
);
8379
expect(flagged).toBe(true);
8480
});
@@ -89,9 +85,7 @@ describe("visualBaselines config section", () => {
8985
});
9086
const result = validate(config, "bad-backend");
9187
expect(result.ok).toBe(false);
92-
expect(result.schemaErrors.some((e) => e.path === "/visualBaselines/backend")).toBe(
93-
true,
94-
);
88+
expect(result.schemaErrors.some((e) => e.path === "/visualBaselines/backend")).toBe(true);
9589
});
9690

9791
it("rejects an unknown provenance policy", () => {
@@ -100,9 +94,9 @@ describe("visualBaselines config section", () => {
10094
});
10195
const result = validate(config, "bad-policy");
10296
expect(result.ok).toBe(false);
103-
expect(
104-
result.schemaErrors.some((e) => e.path === "/visualBaselines/provenance/policy"),
105-
).toBe(true);
97+
expect(result.schemaErrors.some((e) => e.path === "/visualBaselines/provenance/policy")).toBe(
98+
true,
99+
);
106100
});
107101

108102
it("accepts storage=lfs on the commit backend", () => {
@@ -119,9 +113,7 @@ describe("visualBaselines config section", () => {
119113
});
120114
const result = validate(config, "bad-mode");
121115
expect(result.ok).toBe(false);
122-
expect(
123-
result.schemaErrors.some((e) => e.path === "/visualBaselines/capture/mode"),
124-
).toBe(true);
116+
expect(result.schemaErrors.some((e) => e.path === "/visualBaselines/capture/mode")).toBe(true);
125117
});
126118

127119
it("structural: storage=lfs requires the commit backend", () => {
@@ -131,9 +123,7 @@ describe("visualBaselines config section", () => {
131123
});
132124
const result = validate(config, "lfs-wrong-backend");
133125
expect(result.ok).toBe(false);
134-
expect(
135-
result.structuralIssues.some((e) => e.path === "/visualBaselines/storage"),
136-
).toBe(true);
126+
expect(result.structuralIssues.some((e) => e.path === "/visualBaselines/storage")).toBe(true);
137127
});
138128

139129
it("structural: threshold must match e2e.crossBrowserDiffThreshold", () => {
@@ -157,8 +147,6 @@ describe("visualBaselines config section", () => {
157147
});
158148
const result = validate(config, "browser-superset");
159149
expect(result.ok).toBe(false);
160-
expect(
161-
result.structuralIssues.some((e) => e.path === "/visualBaselines/browsers"),
162-
).toBe(true);
150+
expect(result.structuralIssues.some((e) => e.path === "/visualBaselines/browsers")).toBe(true);
163151
});
164152
});

0 commit comments

Comments
 (0)