Fix image diff corner case when misMatchPercentage equals misMatchTolerance #110

Merged
merged 1 commit into from Sep 6, 2015

Projects

None yet

2 participants

@adrianolek
Contributor

The issue is that when misMatchPercentage is equal to misMatchTolerance the isWithinMisMatchTolerance property of the result is false even though the message says the tolerance isn't exceeded.

After the fix the isWithinMisMatchTolerance will be true in this scenario.

@christian-bromann christian-bromann commented on the diff Sep 6, 2015
lib/saveImageDiff.js
@@ -94,7 +94,7 @@ module.exports = function(imageDiff,done) {
misMatchPercentage: misMatchTolerance,
isExactSameImage: misMatchTolerance === 0,
isSameDimensions: imageDiff.isSameDimensions,
- isWithinMisMatchTolerance: that.misMatchTolerance > misMatchTolerance
+ isWithinMisMatchTolerance: true
@christian-bromann
christian-bromann Sep 6, 2015 Member

wouldn't be the actual fix be that.misMatchTolerance >= misMatchTolerance?

@adrianolek
adrianolek Sep 6, 2015 Contributor

It would be redundant. Take a look at the outer condition, which looks like this:

if(this.misMatchTolerance < misMatchTolerance) {
    //...
    this.self.resultObject //...
        isWithinMisMatchTolerance: false
    //...
} else {
    //...
    this.self.resultObject //...
        isWithinMisMatchTolerance: true
    //...
}

if you'd fix it with that.misMatchTolerance >= misMatchTolerance then it would be:

if(this.misMatchTolerance < misMatchTolerance) {
    //...
    this.self.resultObject //...
        isWithinMisMatchTolerance: false
    //...
} else {
    //...
    this.self.resultObject //...
        isWithinMisMatchTolerance: that.misMatchTolerance >= misMatchTolerance
    //...
}

So actually the else from the outer condition covers that.misMatchTolerance >= misMatchTolerance

@christian-bromann
Member

Ahh now I see it. Yeah the comparison seems to be obsolete then. Thanks!

@christian-bromann christian-bromann merged commit 2f0793a into webdriverio:master Sep 6, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment