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

Exposed fetchMisMatchPercentage #83

Open
wants to merge 2 commits into
base: 1.9.4
Choose a base branch
from

Conversation

mlincoln4
Copy link

My boss sent you the following email:

We are currently working on a project using the resemble helper and have been trying to find a good way to get the mismatch percentage of a pair of images without triggering a failure. When looking in the codeceptjs-resemblehelper/index.js file I found that there is a function that does exactly that, _fetchMisMatchPercentage(). However, in order to be able to use this function, I have had to remove the underscore from the beginning of the name. Would it be possible to update this so that function can be exposed?
Also a side-note, it seems that in the _compareImages() function, the options only handle boundingBox/ignoredBox and not boundingBoxes/ignoredBoxes:

resemble.outputSettings({
boundingBox: options.boundingBox,
ignoredBox: options.ignoredBox,
…options.outputSettings,
});

Could this also possibly be updated?


I have made the small changes in this pull request. Let me know if you have any problems with it. Thanks a lot!

@puneet0191 puneet0191 changed the base branch from master to 1.9.4 October 31, 2020 04:45
@puneet0191
Copy link
Member

Thanks a lot @mlincoln4 I will prepare for 1.9.4 release.

@@ -66,6 +66,8 @@ class ResembleHelper extends Helper {
resemble.outputSettings({
boundingBox: options.boundingBox,
ignoredBox: options.ignoredBox,
boundingBoxes: options.boundingBoxes,
Copy link
Member

Choose a reason for hiding this comment

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

@mlincoln4 this change is probably going to impact https://github.com/codeceptjs/codeceptjs-resemblehelper/blob/master/index.js#L379 could you please make sure it works as expected

Copy link
Member

Choose a reason for hiding this comment

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

see where we set the boundingBox option,

options.boundingBox = await this._getBoundingBox(selector);

Copy link
Author

Choose a reason for hiding this comment

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

Well that is checking the visual difference for an element, in which case I don't think bounding boxes need to be supported since you are already specifying an element for the bounds. I am not sure though. That part isn't as important to me, I just happened to notice the plural ones weren't currently supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants