From 1880c5b69b396602b1bab12030024381ab2fde69 Mon Sep 17 00:00:00 2001 From: Roman Kuznetsov Date: Thu, 22 Feb 2024 00:00:28 +0300 Subject: [PATCH] feat: add ability to specify maxDiffPixels in assertView --- README.md | 6 +- .../capture-processors/assert-refs.js | 30 +++++- .../assert-view/errors/image-diff-error.ts | 92 ++++++++++++------- src/browser/commands/assert-view/index.js | 9 +- src/browser/commands/types.ts | 18 ++++ src/config/defaults.js | 2 + .../capture-processors/assert-refs.js | 48 ++-------- .../assert-view/errors/image-diff-error.js | 2 +- .../src/browser/commands/assert-view/index.js | 55 ++++++++++- 9 files changed, 174 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index a32fdc452..9ddf6f47d 100644 --- a/README.md +++ b/README.md @@ -709,6 +709,8 @@ Parameters: - ignoreElements (optional) `String|String[]` – elements, matching specified selectors will be ignored when comparing images - tolerance (optional) `Number` – overrides config [browsers](#browsers).[tolerance](#tolerance) value - antialiasingTolerance (optional) `Number` – overrides config [browsers](#browsers).[antialiasingTolerance](#antialiasingTolerance) value + - maxDiffPixels (optional) `Number` - maximum amount of different pixels to still consider screenshots "same". Could be usefull, when you have 2-5 different pixels you can't get rid of using `tolerance` and `antialiasingTolerance`. Note: this should be considered a last resort and only used in small number of cases where necessary. + - maxDiffPixelRatio (optional) `Number` (between 0 and 1) - maximum ratio of different pixels to still consider screenshots "same". Note: this should be considered a last resort and only used in small number of cases where necessary. - allowViewportOverflow (optional) `Boolean` – by default Hermione throws an error if element is outside the viewport bounds. This option disables check that element is outside of the viewport left, top, right or bottom bounds. And in this case if browser option [compositeImage](#compositeimage) set to `false`, then only visible part of the element will be captured. But if [compositeImage](#compositeimage) set to `true` (default), then in the resulting screenshot will appear the whole element with not visible parts outside of the bottom bounds of viewport. - captureElementFromTop (optional) `Boolean` - ability to set capture element from the top area or from current position. In the first case viewport will be scrolled to the top of the element. Default value is `true` - compositeImage (optional) `Boolean` - overrides config [browsers](#browsers).[compositeImage](#compositeImage) value @@ -1124,7 +1126,9 @@ Default options used when calling [assertView](https://github.com/gemini-testing ignoreElements: [], captureElementFromTop: true, allowViewportOverflow: false, - disableAnimation: true + disableAnimation: true, + maxDiffPixels: 0, + maxDiffPixelRatio: 0 ``` #### openAndWaitOpts diff --git a/src/browser/commands/assert-view/capture-processors/assert-refs.js b/src/browser/commands/assert-view/capture-processors/assert-refs.js index 89c619f85..3541dd984 100644 --- a/src/browser/commands/assert-view/capture-processors/assert-refs.js +++ b/src/browser/commands/assert-view/capture-processors/assert-refs.js @@ -4,12 +4,21 @@ const Promise = require("bluebird"); const { ImageDiffError } = require("../errors/image-diff-error"); const { NoRefImageError } = require("../errors/no-ref-image-error"); -exports.handleNoRefImage = (currImg, refImg, state) => { - return Promise.reject(NoRefImageError.create(state, currImg, refImg)); +exports.handleNoRefImage = (currImg, refImg, stateName) => { + return Promise.reject(NoRefImageError.create(stateName, currImg, refImg)); }; -exports.handleImageDiff = (currImg, refImg, state, opts) => { - const { tolerance, antialiasingTolerance, canHaveCaret, diffAreas, config, diffBuffer } = opts; +exports.handleImageDiff = (currImg, refImg, stateName, opts) => { + const { + tolerance, + antialiasingTolerance, + canHaveCaret, + diffAreas, + config, + diffBuffer, + differentPixels, + diffRatio, + } = opts; const { buildDiffOpts, system: { diffColor }, @@ -25,5 +34,16 @@ exports.handleImageDiff = (currImg, refImg, state, opts) => { ...buildDiffOpts, }; - return Promise.reject(ImageDiffError.create(state, currImg, refImg, diffOpts, diffAreas, diffBuffer)); + return Promise.reject( + ImageDiffError.create({ + stateName, + currImg, + refImg, + diffOpts, + diffAreas, + diffBuffer, + differentPixels, + diffRatio, + }), + ); }; diff --git a/src/browser/commands/assert-view/errors/image-diff-error.ts b/src/browser/commands/assert-view/errors/image-diff-error.ts index 4750f0166..0a8cd555a 100644 --- a/src/browser/commands/assert-view/errors/image-diff-error.ts +++ b/src/browser/commands/assert-view/errors/image-diff-error.ts @@ -14,14 +14,16 @@ interface DiffOptions extends LooksSameOptions { type DiffAreas = Pick; -type ImageDiffErrorConstructor = new ( - stateName: string, - currImg: ImageInfo, - refImg: ImageInfo, - diffOpts: DiffOptions, - diffAreas: DiffAreas, - diffBuffer: Buffer, -) => T; +type ImageDiffErrorConstructor = new (params: { + stateName: string; + currImg: ImageInfo; + refImg: ImageInfo; + diffOpts: DiffOptions; + diffAreas: DiffAreas; + diffBuffer: Buffer; + differentPixels: number; + diffRatio: number; +}) => T; interface ImageDiffErrorData { stateName: string; @@ -31,6 +33,8 @@ interface ImageDiffErrorData { diffBounds: LooksSameResult["diffBounds"]; diffClusters: LooksSameResult["diffClusters"]; diffBuffer: Buffer; + differentPixels: number; + diffRatio: number; } export class ImageDiffError extends BaseStateError { @@ -39,42 +43,58 @@ export class ImageDiffError extends BaseStateError { diffBounds?: DiffAreas["diffBounds"]; diffClusters?: DiffAreas["diffClusters"]; diffBuffer: Buffer; + differentPixels: number; + diffRatio: number; static create( this: ImageDiffErrorConstructor, - stateName: string, - currImg: ImageInfo, - refImg: ImageInfo, - diffOpts: DiffOptions, - diffAreas = {} as DiffAreas, - diffBuffer: Buffer, + { + stateName, + currImg, + refImg, + diffOpts, + diffAreas = {} as DiffAreas, + diffBuffer, + differentPixels, + diffRatio, + }: { + stateName: string; + currImg: ImageInfo; + refImg: ImageInfo; + diffOpts: DiffOptions; + diffAreas?: DiffAreas; + diffBuffer: Buffer; + differentPixels: number; + diffRatio: number; + }, ): T { - return new this(stateName, currImg, refImg, diffOpts, diffAreas, diffBuffer); + return new this({ stateName, currImg, refImg, diffOpts, diffAreas, diffBuffer, differentPixels, diffRatio }); } static fromObject(this: ImageDiffErrorConstructor, data: ImageDiffErrorData): T { - const { diffBounds, diffClusters } = data; - return new this( - data.stateName, - data.currImg, - data.refImg, - data.diffOpts, - { - diffBounds, - diffClusters, - }, - data.diffBuffer, - ); + const { diffBounds, diffClusters, ...rest } = data; + return new this({ ...rest, diffAreas: { diffBounds, diffClusters } }); } - constructor( - stateName: string, - currImg: ImageInfo, - refImg: ImageInfo, - diffOpts: DiffOptions, - { diffBounds, diffClusters } = {} as DiffAreas, - diffBuffer: Buffer, - ) { + constructor({ + stateName, + currImg, + refImg, + diffOpts, + diffAreas: { diffBounds, diffClusters } = {} as DiffAreas, + diffBuffer, + differentPixels, + diffRatio, + }: { + stateName: string; + currImg: ImageInfo; + refImg: ImageInfo; + diffOpts: DiffOptions; + diffAreas?: DiffAreas; + diffBuffer: Buffer; + differentPixels: number; + diffRatio: number; + }) { super(stateName, currImg, refImg); this.message = `images are different for "${stateName}" state`; @@ -82,6 +102,8 @@ export class ImageDiffError extends BaseStateError { this.diffBounds = diffBounds; this.diffClusters = diffClusters; this.diffBuffer = diffBuffer; + this.differentPixels = differentPixels; + this.diffRatio = diffRatio; } saveDiffTo(diffPath: string): Promise { diff --git a/src/browser/commands/assert-view/index.js b/src/browser/commands/assert-view/index.js index d5475b42f..35118568e 100644 --- a/src/browser/commands/assert-view/index.js +++ b/src/browser/commands/assert-view/index.js @@ -104,10 +104,15 @@ module.exports = browser => { diffClusters, diffImage, metaInfo = {}, + differentPixels, + totalPixels, } = await Image.compare(refBuffer, currBuffer, imageCompareOpts); Object.assign(refImg, metaInfo.refImg); - if (!equal) { + const diffRatio = differentPixels / totalPixels; + const isMinorDiff = opts.maxDiffPixels >= differentPixels || opts.maxDiffPixelRatio >= diffRatio; + + if (!equal && !isMinorDiff) { const diffBuffer = await diffImage.createBuffer("png"); const diffAreas = { diffBounds, diffClusters }; const { tolerance, antialiasingTolerance } = opts; @@ -119,6 +124,8 @@ module.exports = browser => { config, emitter, diffBuffer, + differentPixels, + diffRatio, }; await fs.outputFile(currImg.path, currBuffer); diff --git a/src/browser/commands/types.ts b/src/browser/commands/types.ts index 3b3b6a658..44bb79d1d 100644 --- a/src/browser/commands/types.ts +++ b/src/browser/commands/types.ts @@ -64,6 +64,24 @@ export interface AssertViewOpts extends Partial { * @defaultValue `true` */ disableAnimation?: boolean; + /** + * Ability to ignore small amount of different pixels (2-5) while screenshots are still being considered as "same" + * + * @remarks + * Usefull, when you have 2-5 different pixels you can't get rid of using `tolerance` and `antialiasingTolerance`. + * + * @defaultValue `0` + */ + maxDiffPixels?: number; + /** + * Ability to ignore small ratio of different pixels (between 0 and 1) while screenshots are still being considered as "same" + * + * @remarks + * Usefull, when you have a small fraction of different pixels you can't get rid of using `tolerance` and `antialiasingTolerance`. + * + * @defaultValue `0` + */ + maxDiffPixelsRatio?: number; } export type AssertViewCommand = (state: string, selectors: string | string[], opts?: AssertViewOpts) => Promise; diff --git a/src/config/defaults.js b/src/config/defaults.js index 325719277..896ee3607 100644 --- a/src/config/defaults.js +++ b/src/config/defaults.js @@ -27,6 +27,8 @@ module.exports = { ignoreElements: [], captureElementFromTop: true, allowViewportOverflow: false, + maxDiffPixels: 0, + maxDiffPixelRatio: 0, }, openAndWaitOpts: { waitNetworkIdle: true, diff --git a/test/src/browser/commands/assert-view/capture-processors/assert-refs.js b/test/src/browser/commands/assert-view/capture-processors/assert-refs.js index c55fa45e3..1bcb25d8c 100644 --- a/test/src/browser/commands/assert-view/capture-processors/assert-refs.js +++ b/test/src/browser/commands/assert-view/capture-processors/assert-refs.js @@ -45,26 +45,14 @@ describe("browser/commands/assert-view/capture-processors/assert-refs", () => { }; await handleImageDiff_({ config }).catch(() => { - assert.calledOnceWith( - ImageDiffError.create, - sinon.match.any, - sinon.match.any, - sinon.match.any, - sinon.match({ foo: "bar", baz: "qux" }), - ); + assert.calledOnceWith(ImageDiffError.create, sinon.match({ diffOpts: { foo: "bar", baz: "qux" } })); }); }); ["tolerance", "antialiasingTolerance"].forEach(option => { it(`"${option}" option`, async () => { await handleImageDiff_({ [option]: 1 }).catch(() => { - assert.calledOnceWith( - ImageDiffError.create, - sinon.match.any, - sinon.match.any, - sinon.match.any, - sinon.match({ [option]: 1 }), - ); + assert.calledOnceWith(ImageDiffError.create, sinon.match({ diffOpts: { [option]: 1 } })); }); }); @@ -74,13 +62,7 @@ describe("browser/commands/assert-view/capture-processors/assert-refs", () => { }; await handleImageDiff_({ [option]: 2, config }).catch(() => { - assert.calledOnceWith( - ImageDiffError.create, - sinon.match.any, - sinon.match.any, - sinon.match.any, - sinon.match({ [option]: 1 }), - ); + assert.calledOnceWith(ImageDiffError.create, sinon.match({ diffOpts: { [option]: 1 } })); }); }); }); @@ -92,13 +74,7 @@ describe("browser/commands/assert-view/capture-processors/assert-refs", () => { }; await handleImageDiff_({ config, canHaveCaret: false }).catch(() => { - assert.calledOnceWith( - ImageDiffError.create, - sinon.match.any, - sinon.match.any, - sinon.match.any, - sinon.match({ ignoreCaret: false }), - ); + assert.calledOnceWith(ImageDiffError.create, sinon.match({ diffOpts: { ignoreCaret: false } })); }); }); @@ -108,13 +84,7 @@ describe("browser/commands/assert-view/capture-processors/assert-refs", () => { }; await handleImageDiff_({ config, canHaveCaret: true }).catch(() => { - assert.calledOnceWith( - ImageDiffError.create, - sinon.match.any, - sinon.match.any, - sinon.match.any, - sinon.match({ ignoreCaret: false }), - ); + assert.calledOnceWith(ImageDiffError.create, sinon.match({ diffOpts: { ignoreCaret: false } })); }); }); }); @@ -125,13 +95,7 @@ describe("browser/commands/assert-view/capture-processors/assert-refs", () => { }; await handleImageDiff_({ config, canHaveCaret: true }).catch(() => { - assert.calledOnceWith( - ImageDiffError.create, - sinon.match.any, - sinon.match.any, - sinon.match.any, - sinon.match({ ignoreCaret: true }), - ); + assert.calledOnceWith(ImageDiffError.create, sinon.match({ diffOpts: { ignoreCaret: true } })); }); }); }); diff --git a/test/src/browser/commands/assert-view/errors/image-diff-error.js b/test/src/browser/commands/assert-view/errors/image-diff-error.js index abb19ec8e..beb70a1aa 100644 --- a/test/src/browser/commands/assert-view/errors/image-diff-error.js +++ b/test/src/browser/commands/assert-view/errors/image-diff-error.js @@ -13,7 +13,7 @@ const mkImageDiffError = (opts = {}) => { diffOpts: { foo: "bar" }, }); - return new ImageDiffError(stateName, currImg, refImg, diffOpts); + return new ImageDiffError({ stateName, currImg, refImg, diffOpts }); }; describe("ImageDiffError", () => { diff --git a/test/src/browser/commands/assert-view/index.js b/test/src/browser/commands/assert-view/index.js index 9b4881a85..826a8b78e 100644 --- a/test/src/browser/commands/assert-view/index.js +++ b/test/src/browser/commands/assert-view/index.js @@ -622,9 +622,11 @@ describe("assertView command", () => { assert.calledOnceWith( ImageDiffError.create, - "state", - { path: "/curr/path", size: { width: 100, height: 200 } }, - { path: "/ref/path", size: { width: 300, height: 400 } }, + sinon.match({ + stateName: "state", + currImg: { path: "/curr/path", size: { width: 100, height: 200 } }, + refImg: { path: "/ref/path", size: { width: 300, height: 400 } }, + }), ); }); @@ -779,6 +781,53 @@ describe("assertView command", () => { }); }); }); + + describe("if maxDiffPixels or maxDiffPixelRatio is specified", () => { + beforeEach(() => { + sandbox.stub(ImageDiffError, "create").returns(Object.create(ImageDiffError.prototype)); + }); + + it("and images are still considered not same", async () => { + const browser = await initBrowser_(); + browser.prepareScreenshot.resolves({ canHaveCaret: true }); + Image.compare.resolves({ + equal: false, + diffImage: { createBuffer: sandbox.stub() }, + differentPixels: 10, + totalPixels: 50, + }); + + await fn(browser, "state", "selector", { maxDiffPixels: 5, maxDiffPixelRatio: 0.1 }); + + assert.calledOnceWith( + ImageDiffError.create, + sinon.match({ + differentPixels: 10, + diffRatio: 0.2, + }), + ); + }); + + [ + { caseName: "absolute maxDiffPixels", maxDiffPixels: 5, maxDiffPixelRatio: 0.4 }, + { caseName: "relative maxDiffPixelRatio", maxDiffPixels: 20, maxDiffPixelRatio: 0.1 }, + ].forEach(({ caseName, maxDiffPixelRatio, maxDiffPixels }) => { + it(`and images are same ${caseName}`, async () => { + const browser = await initBrowser_(); + browser.prepareScreenshot.resolves({ canHaveCaret: true }); + Image.compare.resolves({ + equal: false, + diffImage: { createBuffer: sandbox.stub() }, + differentPixels: 10, + totalPixels: 50, + }); + + await fn(browser, "state", "selector", { maxDiffPixels, maxDiffPixelRatio }); + + assert.notCalled(ImageDiffError.create); + }); + }); + }); }); it("should remember several success assert view calls", async () => {