diff --git a/integration_tests/pages/ProjectListPage.ts b/integration_tests/pages/ProjectListPage.ts index 0bb3a9b2..c1611851 100644 --- a/integration_tests/pages/ProjectListPage.ts +++ b/integration_tests/pages/ProjectListPage.ts @@ -17,6 +17,10 @@ export class ProjectListPage extends BasePage { name: "Image comparison library", }); + // the rendering-noise filter: part of each library's config section + noiseFilter = this.dialog.getByLabel("Ignore rendering noise"); + noiseThreshold = this.dialog.locator("#blurAmplitudeThreshold"); + // the engine's own fields, below the library select vlmProvider = this.dialog.getByRole("combobox", { name: "VLM Provider" }); ollamaModel = this.dialog.getByRole("combobox", { name: "Model" }); diff --git a/integration_tests/test/projectList.spec.ts b/integration_tests/test/projectList.spec.ts index f2b6af6d..88cd9185 100644 --- a/integration_tests/test/projectList.spec.ts +++ b/integration_tests/test/projectList.spec.ts @@ -1,7 +1,11 @@ -import { expect } from "@playwright/test"; +import { expect, Page, Request } from "@playwright/test"; import { test } from "fixtures"; import { TEST_PROJECT } from "~client/_test/test.data.helper"; -import { ODIFF_DEFAULT_CONFIG } from "~client/constants/project"; +import { + LOOKSSAME_DEFAULT_CONFIG, + ODIFF_DEFAULT_CONFIG, + PIXELMATCH_DEFAULT_CONFIG, +} from "~client/constants/project"; import { ImageComparison } from "~client/types/imageComparison"; import { API_URL, @@ -152,3 +156,174 @@ test("edits a project", async ({ projectListPage, page }) => { await expect(projectListPage.project("Renamed project")).toBeVisible(); await expect(projectListPage.project(project.name)).toHaveCount(0); }); + +// The rendering-noise filter is switched on in the library's config, where a +// library that supports it offers it; the backend applies it to every library. +const savedConfig = (request: Request) => + JSON.parse(request.postDataJSON().imageComparisonConfig); + +const waitForSave = (page: Page, method: "POST" | "PUT") => + page.waitForRequest( + (request) => + request.url() === `${API_URL}/projects` && request.method() === method, + ); + +test("creates a project with the rendering-noise filter on", async ({ + projectListPage, + page, +}) => { + await mockSaveProject(page); + + await projectListPage.addBtn.click(); + await projectListPage.name.fill("New project"); + await projectListPage.mainBranch.fill("main"); + await projectListPage.selectImageComparison(ImageComparison.lookSame); + await projectListPage.noiseFilter.check(); + await projectListPage.noiseThreshold.fill("60"); + + const [request] = await Promise.all([ + waitForSave(page, "POST"), + projectListPage.modal.confirmBtn.click(), + ]); + + expect(savedConfig(request)).toEqual({ + ...JSON.parse(LOOKSSAME_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 60, + }); +}); + +// the threshold means nothing with the filter off, so it is only offered on +test("offers the threshold only while the filter is on", async ({ + projectListPage, +}) => { + await projectListPage.addBtn.click(); + await expect(projectListPage.noiseThreshold).toHaveCount(0); + + await projectListPage.noiseFilter.check(); + // the backend's default, until changed + await expect(projectListPage.noiseThreshold).toHaveValue("80"); + + await projectListPage.noiseFilter.uncheck(); + await expect(projectListPage.noiseThreshold).toHaveCount(0); +}); + +for (const [threshold, message] of [ + ["129", "Enter at most 128"], + ["-1", "Enter at least 0"], + ["", "Enter a threshold"], +]) { + test(`does not save a rendering-noise threshold of "${threshold}"`, async ({ + projectListPage, + page, + }) => { + const creates: string[] = []; + page.on("request", (request) => { + if ( + request.url() === `${API_URL}/projects` && + request.method() === "POST" + ) { + creates.push(request.url()); + } + }); + + await projectListPage.addBtn.click(); + await projectListPage.name.fill("New project"); + await projectListPage.mainBranch.fill("main"); + await projectListPage.noiseFilter.check(); + await projectListPage.noiseThreshold.fill(threshold); + await projectListPage.modal.confirmBtn.click(); + + await expect(projectListPage.dialog.getByText(message)).toBeVisible(); + await expect(projectListPage.dialog).toBeVisible(); + expect(creates).toEqual([]); + }); +} + +// switching swaps in the new library's defaults; the filter is not one of the +// library's own options and would otherwise be silently switched off +test("keeps the rendering-noise filter when the library changes", async ({ + projectListPage, + page, +}) => { + await mockSaveProject(page); + + await projectListPage.addBtn.click(); + await projectListPage.name.fill("New project"); + await projectListPage.mainBranch.fill("main"); + await projectListPage.noiseFilter.check(); + await projectListPage.noiseThreshold.fill("50"); + await projectListPage.selectImageComparison(ImageComparison.odiff); + + await expect(projectListPage.noiseFilter).toBeChecked(); + await expect(projectListPage.noiseThreshold).toHaveValue("50"); + const [request] = await Promise.all([ + waitForSave(page, "POST"), + projectListPage.modal.confirmBtn.click(), + ]); + expect(savedConfig(request)).toEqual({ + ...JSON.parse(ODIFF_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 50, + }); +}); + +// switching provider rebuilds the VLM config from scratch +test("keeps the rendering-noise filter when the VLM provider changes", async ({ + projectListPage, +}) => { + await projectListPage.addBtn.click(); + await projectListPage.selectImageComparison(ImageComparison.vlm); + await projectListPage.noiseFilter.check(); + await projectListPage.noiseThreshold.fill("65"); + + await projectListPage.selectVlmProvider("Google Gemini"); + + await expect(projectListPage.noiseFilter).toBeChecked(); + await expect(projectListPage.noiseThreshold).toHaveValue("65"); +}); + +test("turns the rendering-noise filter on for an existing project", async ({ + projectListPage, + page, +}) => { + await mockSaveProject(page); + + await projectListPage.editBtn.click(); + await expect(projectListPage.noiseFilter).not.toBeChecked(); + await projectListPage.noiseFilter.check(); + await projectListPage.noiseThreshold.fill("70.5"); + + const [request] = await Promise.all([ + waitForSave(page, "PUT"), + projectListPage.modal.confirmBtn.click(), + ]); + + expect(savedConfig(request)).toEqual({ + ...JSON.parse(PIXELMATCH_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 70.5, + }); +}); + +test("opens a project with the filter on as it was saved", async ({ + projectListPage, + page, +}) => { + await mockGetProjects(page, [ + { + ...project, + imageComparisonConfig: JSON.stringify({ + ...JSON.parse(PIXELMATCH_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 42, + }), + }, + ]); + await page.reload(); + + await projectListPage.editBtn.click(); + + await expect(projectListPage.noiseFilter).toBeChecked(); + await expect(projectListPage.noiseThreshold).toHaveValue("42"); +}); diff --git a/integration_tests/test/screens.spec.ts b/integration_tests/test/screens.spec.ts index b2a8e07c..de9789a5 100644 --- a/integration_tests/test/screens.spec.ts +++ b/integration_tests/test/screens.spec.ts @@ -234,6 +234,22 @@ const VIEWS: [name: string, body: Body][] = [ }, ], + [ + "edit project dialog with the rendering-noise filter on", + async ({ projectListPage, shot }) => { + await projectListPage.editBtn.click(); + await projectListPage.noiseFilter.check(); + // the switch keeps its tooltip open while it has the focus: take it off, + // then bring the threshold the switch revealed into the dialog's view + await projectListPage.dialog.getByRole("heading").first().click(); + await projectListPage.noiseThreshold.scrollIntoViewIfNeeded(); + + await expect(projectListPage.noiseThreshold).toHaveValue("80"); + await expect(projectListPage.page.getByRole("tooltip")).toHaveCount(0); + await shot("edit-project-noise-filter"); + }, + ], + [ "delete project confirmation", async ({ projectListPage, shot }) => { diff --git a/integration_tests/test/screens.spec.ts-snapshots/create-project-dark.png b/integration_tests/test/screens.spec.ts-snapshots/create-project-dark.png index cb024a4d..216db7be 100644 Binary files a/integration_tests/test/screens.spec.ts-snapshots/create-project-dark.png and b/integration_tests/test/screens.spec.ts-snapshots/create-project-dark.png differ diff --git a/integration_tests/test/screens.spec.ts-snapshots/create-project-light.png b/integration_tests/test/screens.spec.ts-snapshots/create-project-light.png index 97591642..77695985 100644 Binary files a/integration_tests/test/screens.spec.ts-snapshots/create-project-light.png and b/integration_tests/test/screens.spec.ts-snapshots/create-project-light.png differ diff --git a/integration_tests/test/screens.spec.ts-snapshots/edit-project-noise-filter-dark.png b/integration_tests/test/screens.spec.ts-snapshots/edit-project-noise-filter-dark.png new file mode 100644 index 00000000..fca66c59 Binary files /dev/null and b/integration_tests/test/screens.spec.ts-snapshots/edit-project-noise-filter-dark.png differ diff --git a/integration_tests/test/screens.spec.ts-snapshots/edit-project-noise-filter-light.png b/integration_tests/test/screens.spec.ts-snapshots/edit-project-noise-filter-light.png new file mode 100644 index 00000000..dd2e658f Binary files /dev/null and b/integration_tests/test/screens.spec.ts-snapshots/edit-project-noise-filter-light.png differ diff --git a/src/components/ProjectForm/BlurAmplitudeFields.tsx b/src/components/ProjectForm/BlurAmplitudeFields.tsx new file mode 100644 index 00000000..d0b0d455 --- /dev/null +++ b/src/components/ProjectForm/BlurAmplitudeFields.tsx @@ -0,0 +1,74 @@ +import { FormControlLabel, Switch } from "@mui/material"; +import React from "react"; +import { TextValidator } from "react-material-ui-form-validator"; +import { DEFAULT_BLUR_AMPLITUDE_THRESHOLD } from "../../constants"; +import { BlurAmplitudeConfig } from "../../types/imageComparison"; +import { Tooltip } from "../Tooltip"; +import { useConfigHook } from "./useConfigHook"; + +// Rendered by the config form of every library that supports the filter. +export const BlurAmplitudeFields: React.FunctionComponent = () => { + const [config, updateConfig] = useConfigHook(); + + // absent: the backend default applies; null: the field was cleared, which + // the required validator then refuses + const threshold = + config.blurAmplitudeThreshold === undefined + ? DEFAULT_BLUR_AMPLITUDE_THRESHOLD + : (config.blurAmplitudeThreshold ?? ""); + + return ( + + + + updateConfig("blurAmplitudeFilter", checked) + } + color="primary" + name="blurAmplitudeFilter" + /> + } + /> + + {config.blurAmplitudeFilter === true && ( + ) => { + const value = parseFloat(event.target.value); + updateConfig( + "blurAmplitudeThreshold", + Number.isNaN(value) ? null : value, + ); + }} + /> + )} + + ); +}; diff --git a/src/components/ProjectForm/LooksSameConfigForm.tsx b/src/components/ProjectForm/LooksSameConfigForm.tsx index be8fba9b..d64a72b2 100644 --- a/src/components/ProjectForm/LooksSameConfigForm.tsx +++ b/src/components/ProjectForm/LooksSameConfigForm.tsx @@ -3,6 +3,7 @@ import React from "react"; import { TextValidator } from "react-material-ui-form-validator"; import { LooksSameConfig } from "../../types/imageComparison"; import { useConfigHook } from "./useConfigHook"; +import { BlurAmplitudeFields } from "./BlurAmplitudeFields"; export const LooksSameConfigForm: React.FunctionComponent = () => { const [config, updateConfig] = useConfigHook(); @@ -101,6 +102,7 @@ export const LooksSameConfigForm: React.FunctionComponent = () => { /> } /> + ); }; diff --git a/src/components/ProjectForm/OdiffConfigForm.tsx b/src/components/ProjectForm/OdiffConfigForm.tsx index 886d6aec..6e4b9b29 100644 --- a/src/components/ProjectForm/OdiffConfigForm.tsx +++ b/src/components/ProjectForm/OdiffConfigForm.tsx @@ -3,6 +3,7 @@ import React from "react"; import { TextValidator } from "react-material-ui-form-validator"; import { OdiffConfig } from "../../types/imageComparison"; import { useConfigHook } from "./useConfigHook"; +import { BlurAmplitudeFields } from "./BlurAmplitudeFields"; export const OdiffConfigForm: React.FunctionComponent = () => { const [config, updateConfig] = useConfigHook(); @@ -69,6 +70,7 @@ export const OdiffConfigForm: React.FunctionComponent = () => { updateConfig("threshold", parseFloat(value)); }} /> + ); }; diff --git a/src/components/ProjectForm/PixelmatchConfigForm.tsx b/src/components/ProjectForm/PixelmatchConfigForm.tsx index 5ce6ffbb..f64949ce 100644 --- a/src/components/ProjectForm/PixelmatchConfigForm.tsx +++ b/src/components/ProjectForm/PixelmatchConfigForm.tsx @@ -4,6 +4,7 @@ import { TextValidator } from "react-material-ui-form-validator"; import { PixelmatchConfig } from "../../types/imageComparison"; import { Tooltip } from "../Tooltip"; import { useConfigHook } from "./useConfigHook"; +import { BlurAmplitudeFields } from "./BlurAmplitudeFields"; export const PixelmatchConfigForm: React.FunctionComponent = () => { const [config, updateConfig] = useConfigHook(); @@ -64,6 +65,7 @@ export const PixelmatchConfigForm: React.FunctionComponent = () => { updateConfig("threshold", parseFloat(value)); }} /> + ); }; diff --git a/src/components/ProjectForm/ProjectForm.test.tsx b/src/components/ProjectForm/ProjectForm.test.tsx index d0b5df4b..609dc36f 100644 --- a/src/components/ProjectForm/ProjectForm.test.tsx +++ b/src/components/ProjectForm/ProjectForm.test.tsx @@ -551,6 +551,103 @@ describe("ProjectForm", () => { ); }); + // switched on in each library's config; the backend applies it to all four + describe("rendering-noise filter", () => { + const noiseFilter = () => screen.getByLabelText("Ignore rendering noise"); + const noiseThreshold = () => + screen.queryByLabelText(/Rendering noise threshold/); + + it("is off by default and offers no threshold", async () => { + await renderForm(); + + expect(noiseFilter()).not.toBeChecked(); + expect(noiseThreshold()).not.toBeInTheDocument(); + }); + + it.each(ALL_ENGINES)("is offered for %s", async (engine) => { + const { user } = await renderForm(); + + await switchEngine(user, engine); + + expect(noiseFilter()).toBeInTheDocument(); + }); + + it("writes the switch and the threshold into the config json", async () => { + const { user } = await renderForm(); + + await user.click(noiseFilter()); + // the backend's default, until changed + expect(noiseThreshold()).toHaveValue(80); + await replaceNumber(user, noiseThreshold() as HTMLElement, "60.5"); + + expect(editConfig()).toEqual({ + ...JSON.parse(PIXELMATCH_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 60.5, + }); + }); + + it("shows the filter of a project saved with it on", async () => { + await renderForm({ + ...PROJECT, + imageComparisonConfig: JSON.stringify({ + ...JSON.parse(PIXELMATCH_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 42, + }), + }); + + expect(noiseFilter()).toBeChecked(); + expect(noiseThreshold()).toHaveValue(42); + }); + + it("does not submit a threshold above 128", async () => { + const onSubmit = vi.fn(); + const { user } = await renderForm(PROJECT, onSubmit); + + await user.click(noiseFilter()); + await replaceNumber(user, noiseThreshold() as HTMLElement, "129"); + await user.click(screen.getByTestId("submitButton")); + + expect(await screen.findByText("Enter at most 128")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + // the filter is not one of a library's own options: swapping in another + // library's defaults must not switch it off + it("keeps the filter when the library changes", async () => { + const { user } = await renderForm(); + + await user.click(noiseFilter()); + await replaceNumber(user, noiseThreshold() as HTMLElement, "50"); + await switchEngine(user, ImageComparison.odiff); + + expect(editConfig()).toEqual({ + ...JSON.parse(ODIFF_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + blurAmplitudeThreshold: 50, + }); + }); + + it("keeps the filter when the VLM provider changes", async () => { + const { user } = await renderForm({ + ...PROJECT, + imageComparison: ImageComparison.vlm, + imageComparisonConfig: JSON.stringify({ + ...JSON.parse(VLM_DEFAULT_CONFIG), + blurAmplitudeFilter: true, + }), + }); + + await choose(user, /VLM Provider/, "Google Gemini"); + + expect(editConfig()).toMatchObject({ + provider: "gemini", + blurAmplitudeFilter: true, + }); + }); + }); + describe("vlm config", () => { const vlm: ProjectDto = { ...PROJECT, diff --git a/src/components/ProjectForm/ProjectForm.tsx b/src/components/ProjectForm/ProjectForm.tsx index eff9d3f4..28d3db50 100644 --- a/src/components/ProjectForm/ProjectForm.tsx +++ b/src/components/ProjectForm/ProjectForm.tsx @@ -20,7 +20,7 @@ import { LooksSameConfigForm } from "./LooksSameConfigForm"; import { OdiffConfigForm } from "./OdiffConfigForm"; import { PixelmatchConfigForm } from "./PixelmatchConfigForm"; import { VlmConfigForm } from "./VlmConfigForm"; -import { getDefaultConfig } from "./utils"; +import { getDefaultConfig, withBlurAmplitudeOf } from "./utils"; export const ProjectForm: React.FunctionComponent = () => { const { projectEditState: project } = useProjectState(); @@ -214,7 +214,10 @@ export const ProjectForm: React.FunctionComponent = () => { imageComparison, imageComparisonConfig: project.imageComparison !== imageComparison - ? getDefaultConfig(imageComparison) + ? withBlurAmplitudeOf( + getDefaultConfig(imageComparison), + project.imageComparisonConfig, + ) : project.imageComparisonConfig, }); }} diff --git a/src/components/ProjectForm/VlmConfigForm.tsx b/src/components/ProjectForm/VlmConfigForm.tsx index 4a194fa5..e7918af5 100644 --- a/src/components/ProjectForm/VlmConfigForm.tsx +++ b/src/components/ProjectForm/VlmConfigForm.tsx @@ -15,6 +15,8 @@ import { } from "../../contexts"; import { GeminiConfigForm } from "./GeminiConfigForm"; import { OllamaConfigForm } from "./OllamaConfigForm"; +import { BlurAmplitudeFields } from "./BlurAmplitudeFields"; +import { withBlurAmplitudeOf } from "./utils"; export const VlmConfigForm: React.FunctionComponent = () => { const [config] = useConfigHook(); @@ -58,7 +60,10 @@ export const VlmConfigForm: React.FunctionComponent = () => { setProjectEditState(projectDispatch, { ...project, - imageComparisonConfig: JSON.stringify(newConfig), + imageComparisonConfig: withBlurAmplitudeOf( + JSON.stringify(newConfig), + project.imageComparisonConfig, + ), }); }; @@ -82,6 +87,7 @@ export const VlmConfigForm: React.FunctionComponent = () => { ) : ( )} + ); }; diff --git a/src/components/ProjectForm/utils.ts b/src/components/ProjectForm/utils.ts index 4b7c4dd2..68faf908 100644 --- a/src/components/ProjectForm/utils.ts +++ b/src/components/ProjectForm/utils.ts @@ -4,7 +4,10 @@ import { PIXELMATCH_DEFAULT_CONFIG, VLM_DEFAULT_CONFIG, } from "../../constants"; -import { ImageComparison } from "../../types/imageComparison"; +import { + BlurAmplitudeConfig, + ImageComparison, +} from "../../types/imageComparison"; export const getDefaultConfig = (imageComparison: ImageComparison): string => { switch (imageComparison) { @@ -20,3 +23,21 @@ export const getDefaultConfig = (imageComparison: ImageComparison): string => { return PIXELMATCH_DEFAULT_CONFIG; } }; + +/** + * Carries the rendering-noise filter's keys from one config into another. The + * filter is not one of a library's own options, so swapping in a library's (or + * a VLM provider's) defaults must not silently switch it off. + */ +export const withBlurAmplitudeOf = ( + config: string, + previousConfig: string, +): string => { + const { blurAmplitudeFilter, blurAmplitudeThreshold }: BlurAmplitudeConfig = + JSON.parse(previousConfig); + return JSON.stringify({ + ...JSON.parse(config), + blurAmplitudeFilter, + blurAmplitudeThreshold, + }); +}; diff --git a/src/constants/project.ts b/src/constants/project.ts index d9429a10..38afb2fa 100644 --- a/src/constants/project.ts +++ b/src/constants/project.ts @@ -3,6 +3,8 @@ import { ImageComparison } from "../types/imageComparison"; export const PIXELMATCH_DEFAULT_CONFIG = '{"threshold":0.1,"ignoreAntialiasing":true,"allowDiffDimensions":false}'; +// the backend's default when a config turns the filter on without a threshold +export const DEFAULT_BLUR_AMPLITUDE_THRESHOLD = 80; export const LOOKSSAME_DEFAULT_CONFIG = '{"strict":false,"tolerance":2.3,"ignoreAntialiasing":true,"antialiasingTolerance":0,"ignoreCaret":true,"allowDiffDimensions":false}'; export const ODIFF_DEFAULT_CONFIG = diff --git a/src/types/imageComparison.ts b/src/types/imageComparison.ts index b05e37e6..92adffe7 100644 --- a/src/types/imageComparison.ts +++ b/src/types/imageComparison.ts @@ -1,8 +1,5 @@ export type ImageComparisonConfig = - | PixelmatchConfig - | LooksSameConfig - | OdiffConfig - | VlmConfig; + PixelmatchConfig | LooksSameConfig | OdiffConfig | VlmConfig; export enum ImageComparison { pixelmatch = "pixelmatch", @@ -11,13 +8,25 @@ export enum ImageComparison { vlm = "vlm", } -export interface PixelmatchConfig { +/** + * Switches on the backend's rendering-noise filter: a failing comparison whose + * change, blurred by about a pixel, is no stronger than anti-aliasing passes. + * Kept in the config of each library that supports it; the backend reads and + * strips these keys before the library sees its options. + */ +export interface BlurAmplitudeConfig { + blurAmplitudeFilter?: boolean; + // 0-128; absent means the backend default + blurAmplitudeThreshold?: number | null; +} + +export interface PixelmatchConfig extends BlurAmplitudeConfig { allowDiffDimensions?: boolean; ignoreAntialiasing: boolean; threshold: number; } -export interface LooksSameConfig { +export interface LooksSameConfig extends BlurAmplitudeConfig { /** * strict comparsion */ @@ -41,7 +50,7 @@ export interface LooksSameConfig { allowDiffDimensions?: boolean; } -export interface OdiffConfig { +export interface OdiffConfig extends BlurAmplitudeConfig { /** Output full diff image. */ outputDiffMask: boolean; /** Do not compare images and produce output if images layout is different. */ @@ -55,7 +64,7 @@ export interface OdiffConfig { /** * Base configuration shared by all VLM providers */ -export interface BaseVlmConfig { +export interface BaseVlmConfig extends BlurAmplitudeConfig { /** * Custom prompt for image comparison. */ @@ -80,7 +89,7 @@ export interface BaseVlmConfig { * Configuration for Ollama provider */ export interface OllamaVlmConfig extends BaseVlmConfig { - provider?: 'ollama'; + provider?: "ollama"; /** * Ollama vision model name. @@ -94,7 +103,7 @@ export interface OllamaVlmConfig extends BaseVlmConfig { * Configuration for Gemini provider */ export interface GeminiVlmConfig extends BaseVlmConfig { - provider: 'gemini'; + provider: "gemini"; /** * Gemini model name.