-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add filename to result #78
Changes from 6 commits
bb56e5a
59ccb77
d2af165
dfc4bab
0a9e560
c39a965
9150006
2ac00b6
a4fa2d3
e11ada3
992cca3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,8 +126,9 @@ export default class BaseCompare { | |
return path.join(runtimeConfigPath, `${name}-${suffix}.json`); | ||
} | ||
|
||
createResultReport(misMatchPercentage, isWithinMisMatchTolerance, isSameDimensions) { | ||
createResultReport(filePaths, misMatchPercentage, isWithinMisMatchTolerance, isSameDimensions) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Chaning the order of arguments is a breaking change for all guys who uses custom compare methods. I would like to avoid this. Moreover this is to inflexibel and only useful for file comparison. I suggest to replace filePaths with an object that wil be assigned via Object.assign to the returned object. Then custom or other compare methods can enhance the report with additional information. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zinserjan Would you rather I made it the last argument, then? That way, if I am remembering my JS correctly, the value would just be null for those who aren't using the functionality? |
||
return { | ||
filePaths, | ||
misMatchPercentage, | ||
isWithinMisMatchTolerance, | ||
isSameDimensions, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,13 @@ export default class LocalCompare extends BaseCompare { | |
|
||
const referenceExists = await fs.exists(referencePath); | ||
|
||
var fileNames; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use const and assign value directly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: extra line |
||
fileNames = { | ||
reference: referencePath, | ||
actual: screenshotPath | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}; | ||
|
||
if (referenceExists) { | ||
log('reference exists, compare it with the taken now'); | ||
const captured = new Buffer(base64Screenshot, 'base64'); | ||
|
@@ -38,23 +45,25 @@ export default class LocalCompare extends BaseCompare { | |
|
||
const diffPath = this.getDiffFile(context); | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: extra line |
||
if (misMatchPercentage > misMatchTolerance) { | ||
log(`Image is different! ${misMatchPercentage}%`); | ||
const png = compareData.getDiffImage().pack(); | ||
await this.writeDiff(png, diffPath); | ||
fileNames.diff = diffPath; | ||
|
||
return this.createResultReport(misMatchPercentage, false, isSameDimensions); | ||
return this.createResultReport(fileNames, misMatchPercentage, false, isSameDimensions); | ||
} else { | ||
log(`Image is within tolerance or the same`); | ||
await fs.remove(diffPath); | ||
|
||
return this.createResultReport(misMatchPercentage, true, isSameDimensions); | ||
return this.createResultReport(fileNames, misMatchPercentage, true, isSameDimensions); | ||
} | ||
|
||
} else { | ||
log('first run - create reference file'); | ||
await fs.outputFile(referencePath, base64Screenshot, 'base64'); | ||
return this.createResultReport(0, true, true); | ||
return this.createResultReport(fileNames, 0, true, true); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this move to a dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I think this happened when I used yarn to add it. I will switch.