Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ability to specify ignoreDiffPixelCount in assertView #849

Merged
merged 1 commit into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ 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
- ignoreDiffPixelCount (optional) `Number | string` - the maximum amount of different pixels to still consider screenshots "the same". For example, when set to 5, it means that if there are 5 or fewer different pixels between two screenshots, they will still be considered the same. Alternatively, you can also define the maximum difference as a percentage (for example, 3%) of the image size. This option is useful when you encounter a few pixels difference that cannot be eliminated using the tolerance and antialiasingTolerance settings. The default value is 0.
- 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
Expand Down Expand Up @@ -1124,7 +1125,8 @@ Default options used when calling [assertView](https://github.com/gemini-testing
ignoreElements: [],
captureElementFromTop: true,
allowViewportOverflow: false,
disableAnimation: true
disableAnimation: true,
ignoreDiffPixelCount: 0,
```

#### openAndWaitOpts
Expand Down
30 changes: 25 additions & 5 deletions src/browser/commands/assert-view/capture-processors/assert-refs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -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({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this method's params to be a single object. This would be passed into html-reporter

stateName,
currImg,
refImg,
diffOpts,
diffAreas,
diffBuffer,
differentPixels,
diffRatio,
}),
);
};
92 changes: 57 additions & 35 deletions src/browser/commands/assert-view/errors/image-diff-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ interface DiffOptions extends LooksSameOptions {

type DiffAreas = Pick<LooksSameResult, "diffClusters" | "diffBounds">;

type ImageDiffErrorConstructor<T> = new (
stateName: string,
currImg: ImageInfo,
refImg: ImageInfo,
diffOpts: DiffOptions,
diffAreas: DiffAreas,
diffBuffer: Buffer,
) => T;
type ImageDiffErrorConstructor<T> = new (params: {
stateName: string;
currImg: ImageInfo;
refImg: ImageInfo;
diffOpts: DiffOptions;
diffAreas: DiffAreas;
diffBuffer: Buffer;
differentPixels: number;
diffRatio: number;
}) => T;

interface ImageDiffErrorData {
stateName: string;
Expand All @@ -31,6 +33,8 @@ interface ImageDiffErrorData {
diffBounds: LooksSameResult["diffBounds"];
diffClusters: LooksSameResult["diffClusters"];
diffBuffer: Buffer;
differentPixels: number;
diffRatio: number;
}

export class ImageDiffError extends BaseStateError {
Expand All @@ -39,49 +43,67 @@ export class ImageDiffError extends BaseStateError {
diffBounds?: DiffAreas["diffBounds"];
diffClusters?: DiffAreas["diffClusters"];
diffBuffer: Buffer;
differentPixels: number;
diffRatio: number;

static create<T extends ImageDiffError>(
this: ImageDiffErrorConstructor<T>,
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<T>(this: ImageDiffErrorConstructor<T>, 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`;
this.diffOpts = diffOpts;
this.diffBounds = diffBounds;
this.diffClusters = diffClusters;
this.diffBuffer = diffBuffer;
this.differentPixels = differentPixels;
this.diffRatio = diffRatio;
}

saveDiffTo(diffPath: string): Promise<null> {
Expand Down
25 changes: 24 additions & 1 deletion src/browser/commands/assert-view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ const { BaseStateError } = require("./errors/base-state-error");
const { AssertViewError } = require("./errors/assert-view-error");
const InvalidPngError = require("./errors/invalid-png-error");

const getIgnoreDiffPixelCountRatio = value => {
const percent = _.isString(value) && value.endsWith("%") ? parseFloat(value.slice(0, -1)) : false;

if (percent === false || _.isNaN(percent)) {
throw new Error(`Invalid ignoreDiffPixelCount value: got ${value}, but expected number or '\${number}%'`);
}

if (percent > 100 || percent < 0) {
throw new Error(`Invalid ignoreDiffPixelCount value: percent should be in range between 0 and 100`);
}

return percent / 100;
};

module.exports = browser => {
const screenShooter = ScreenShooter.create(browser);
const { publicAPI: session, config } = browser;
Expand Down Expand Up @@ -104,10 +118,17 @@ 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 = _.isString(opts.ignoreDiffPixelCount)
? diffRatio <= getIgnoreDiffPixelCountRatio(opts.ignoreDiffPixelCount)
: differentPixels <= opts.ignoreDiffPixelCount;

if (!equal && !isMinorDiff) {
const diffBuffer = await diffImage.createBuffer("png");
const diffAreas = { diffBounds, diffClusters };
const { tolerance, antialiasingTolerance } = opts;
Expand All @@ -119,6 +140,8 @@ module.exports = browser => {
config,
emitter,
diffBuffer,
differentPixels,
diffRatio,
};

await fs.outputFile(currImg.path, currBuffer);
Expand Down
15 changes: 15 additions & 0 deletions src/browser/commands/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,21 @@ export interface AssertViewOpts extends Partial<AssertViewOptsConfig> {
* @defaultValue `true`
*/
disableAnimation?: boolean;
/**
* Ability to ignore a small amount of different pixels to classify screenshots as being "identical"
*
* @example 5
* @example '1.5%'
*
* @remarks
* Useful when you encounter a few pixels difference that cannot be eliminated using the tolerance and antialiasingTolerance settings.
*
* @note
* This should be considered a last resort and only used in small number of cases where necessary.
*
* @defaultValue `0`
*/
ignoreDiffPixelCount?: `${number}%` | number;
}

export type AssertViewCommand = (state: string, selectors: string | string[], opts?: AssertViewOpts) => Promise<void>;
Expand Down
1 change: 1 addition & 0 deletions src/config/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ module.exports = {
ignoreElements: [],
captureElementFromTop: true,
allowViewportOverflow: false,
ignoreDiffPixelCount: 0,
},
openAndWaitOpts: {
waitNetworkIdle: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }));
});
});

Expand All @@ -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 } }));
});
});
});
Expand All @@ -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 } }));
});
});

Expand All @@ -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 } }));
});
});
});
Expand All @@ -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 } }));
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Loading
Loading