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.

Sign up for GitHub

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

Jump to bottom

Add isNewScreenshot to results #103

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add isNewScreenshot to results #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85be033
Add isNewScreenshot to results
Oct 14, 2019
6973907
Fixes typo and some tests
Oct 14, 2019
88ae2ec
Fixes rest of the tests
Oct 14, 2019
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
5 changes: 3 additions & 2 deletions src/methods/BaseCompare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,13 @@ export default class BaseCompare {
return path.join(runtimeConfigPath, `${name}-${suffix}.json`);
}

createResultReport(misMatchPercentage, isWithinMisMatchTolerance, isSameDimensions) {
createResultReport(misMatchPercentage, isWithinMisMatchTolerance, isSameDimensions, isNewScreenshot) {
return {
misMatchPercentage,
isWithinMisMatchTolerance,
isSameDimensions,
isExactSameImage: misMatchPercentage === 0
isExactSameImage: misMatchPercentage === 0,
isNewScreenshot
};
}

Expand Down
6 changes: 3 additions & 3 deletions src/methods/LocalCompare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ export default class LocalCompare extends BaseCompare {
const png = compareData.getDiffImage().pack();
await this.writeDiff(png, diffPath);

return this.createResultReport(misMatchPercentage, false, isSameDimensions);
return this.createResultReport(misMatchPercentage, false, isSameDimensions, false);
} else {
log(`Image is within tolerance or the same`);
await fs.remove(diffPath);

return this.createResultReport(misMatchPercentage, true, isSameDimensions);
return this.createResultReport(misMatchPercentage, true, isSameDimensions, false);
}

} else {
log('first run - create reference file');
await fs.outputFile(referencePath, base64Screenshot, 'base64');
return this.createResultReport(0, true, true);
return this.createResultReport(0, true, true, true);
Copy link

@emilyrohrbough emilyrohrbough Oct 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

although it makes sense, it feels weird to return the results isWithinMisMatchTolerance = true, isSameDimensions = true and isExactSameImage = true.

Copy link
Author

Choose a reason for hiding this comment

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

Can't argue with that. I think, though, changing the semantics of this return could break existing tests. If there was going to be a major (breaking) release, this could be changed. On the other hand, since this doesn't actually do any comparisons, I'm not sure the values could be relied on for anything. Perhaps they should be undefined?

Copy link

Choose a reason for hiding this comment

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

Yeah, I think we would want to keep it as is otherwise I'd probably break people's current compare checks/results when a new ref screenshot is generated.

}
}

Expand Down
3 changes: 2 additions & 1 deletion src/methods/SaveScreenshot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ export default class SaveScreenshot extends BaseCompare {

async processScreenshot(context, base64Screenshot) {
const screenshotPath = this.getScreenshotFile(context);
const screenshotExists = await fs.exists(screenshotPath);

log(`create screenshot file at ${screenshotPath}`);
await fs.outputFile(screenshotPath, base64Screenshot, 'base64');
return this.createResultReport(0, true, true);
return this.createResultReport(0, true, true, !screenshotExists);
}

}
28 changes: 23 additions & 5 deletions test/unit/methods/LocalCompare.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,20 @@ describe('LocalCompare', function () {
misMatchPercentage: 0,
isWithinMisMatchTolerance: true,
isSameDimensions: true,
isExactSameImage: true
isExactSameImage: true,
isNewScreenshot: false
};

this.resultNewFile = {
misMatchPercentage: 0,
isWithinMisMatchTolerance: true,
isSameDimensions: true,
isExactSameImage: true,
isNewScreenshot: true
};
});

it('creates the captured screenshot', async function () {
it('creates the captured screenshot', async function () {
const context = {};
const base64Screenshot = await readAsBase64(path.join(dirFixture, 'image/100x100.png'));

Expand All @@ -85,7 +94,7 @@ describe('LocalCompare', function () {
assert.isTrue(this.getReferenceFile.calledWithExactly(context), 'Reference getter should receive context as arg');

// check image results
assert.deepEqual(results, this.resultIdentical, 'Result should be reported');
assert.deepEqual(results, this.resultNewFile, 'Result should be reported');

// check if reference image was created
const existsReference = await fs.exists(this.referencFile);
Expand All @@ -105,7 +114,7 @@ describe('LocalCompare', function () {
assert.isTrue(this.getReferenceFile.calledWithExactly(context), 'Reference getter should receive context as arg');

// check image results
assert.deepEqual(resultFirst, this.resultIdentical, 'Result should be reported');
assert.deepEqual(resultFirst, this.resultNewFile, 'Result should be reported');

// check if reference was created
const existsReference = await fs.exists(this.screenshotFile);
Expand Down Expand Up @@ -143,7 +152,7 @@ describe('LocalCompare', function () {
assert.isTrue(this.getReferenceFile.calledWithExactly(context), 'Reference getter should receive context as arg');

// check image results
assert.deepEqual(resultFirst, this.resultIdentical, 'Result should be reported');
assert.deepEqual(resultFirst, this.resultNewFile, 'Result should be reported');

// check if reference was created
const existsReference = await fs.exists(this.screenshotFile);
Expand All @@ -165,6 +174,7 @@ describe('LocalCompare', function () {
assert.isFalse(resultSecond.isExactSameImage, 'Images should diff');
assert.isFalse(resultSecond.isWithinMisMatchTolerance, 'Images should be marked as diff');
assert.isTrue(resultSecond.isSameDimensions, 'Image dimensioms should be the same');
assert.isFalse(resultSecond.isNewScreenshot, 'Image should not be new screenshot')

// check if reference is still the same
const statsSecond = await fs.stat(this.referencFile);
Expand Down Expand Up @@ -272,6 +282,7 @@ describe('LocalCompare', function () {
assert.isAbove(result.misMatchPercentage, this.misMatchTolerance, 'Images should diff');
assert.isFalse(result.isExactSameImage, 'Images should diff');
assert.isFalse(result.isWithinMisMatchTolerance, 'Images should be marked as diff');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was created
const existsDiff = await fs.exists(this.diffFile);
Expand Down Expand Up @@ -307,6 +318,7 @@ describe('LocalCompare', function () {
assert.isAtMost(result.misMatchPercentage, this.misMatchTolerance, 'Images should diff');
assert.isFalse(result.isExactSameImage, 'Images should diff');
assert.isTrue(result.isWithinMisMatchTolerance, 'Diff should be in tolerance');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was not created
const existsDiff = await fs.exists(this.diffFile);
Expand All @@ -321,6 +333,7 @@ describe('LocalCompare', function () {
assert.isAbove(result.misMatchPercentage, this.misMatchTolerance, 'Images should diff');
assert.isFalse(result.isExactSameImage, 'Images should diff');
assert.isFalse(result.isWithinMisMatchTolerance, 'Images should be marked as diff');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was created
const existsDiff = await fs.exists(this.diffFile);
Expand Down Expand Up @@ -359,6 +372,7 @@ describe('LocalCompare', function () {
assert.isAtMost(result.misMatchPercentage, this.misMatchTolerance, 'Images should diff');
assert.isFalse(result.isExactSameImage, 'Images should diff');
assert.isTrue(result.isWithinMisMatchTolerance, 'Diff should be in tolerance');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was not created
const existsDiff = await fs.exists(this.diffFile);
Expand All @@ -373,6 +387,7 @@ describe('LocalCompare', function () {
assert.isAbove(result.misMatchPercentage, this.misMatchTolerance, 'Images should diff');
assert.isFalse(result.isExactSameImage, 'Images should diff');
assert.isFalse(result.isWithinMisMatchTolerance, 'Images should be marked as diff');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was created
const existsDiff = await fs.exists(this.diffFile);
Expand Down Expand Up @@ -424,6 +439,7 @@ describe('LocalCompare', function () {
assert.isAbove(result.misMatchPercentage, 0, 'Images should diff');
assert.isFalse(result.isExactSameImage, 'Images should diff');
assert.isFalse(result.isWithinMisMatchTolerance, 'Diff should not be in tolerance');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was created
const existsDiff = await fs.exists(this.diffFile);
Expand Down Expand Up @@ -457,6 +473,7 @@ describe('LocalCompare', function () {
// check diff results
assert.isTrue(result.isExactSameImage, 'Images should not diff');
assert.isTrue(result.isWithinMisMatchTolerance, 'Diff should be in tolerance');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was not created
const existsDiff = await fs.exists(this.diffFile);
Expand Down Expand Up @@ -494,6 +511,7 @@ describe('LocalCompare', function () {
// check diff results
assert.isTrue(result.isExactSameImage, 'Images should not diff');
assert.isTrue(result.isWithinMisMatchTolerance, 'Diff should be in tolerance');
assert.isFalse(result.isNewScreenshot, 'Image should not be new screenshot');

// check if diff image was not created
const existsDiff = await fs.exists(this.diffFile);
Expand Down
17 changes: 13 additions & 4 deletions test/unit/methods/SaveScreenshot.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,16 @@ describe('SaveScreenshot', function () {
misMatchPercentage: 0,
isWithinMisMatchTolerance: true,
isSameDimensions: true,
isExactSameImage: true
isExactSameImage: true,
isNewScreenshot: false
};

this.resultNewFile = {
misMatchPercentage: 0,
isWithinMisMatchTolerance: true,
isSameDimensions: true,
isExactSameImage: true,
isNewScreenshot: true
};
});

Expand All @@ -66,7 +75,7 @@ describe('SaveScreenshot', function () {
assert.isTrue(this.getReferenceFile.calledWithExactly(context), 'Reference getter should receive context as arg');

// check image results
assert.deepEqual(results, this.resultIdentical, 'Result should be reported');
assert.deepEqual(results, this.resultNewFile, 'Result should be reported');

// check if reference image was created
const existsReference = await fs.exists(this.referencFile);
Expand All @@ -86,7 +95,7 @@ describe('SaveScreenshot', function () {
assert.isTrue(this.getReferenceFile.calledWithExactly(context), 'Reference getter should receive context as arg');

// check image results
assert.deepEqual(resultFirst, this.resultIdentical, 'Result should be reported');
assert.deepEqual(resultFirst, this.resultNewFile, 'Result should be reported');

// check if reference was created
const existsReference = await fs.exists(this.referencFile);
Expand Down Expand Up @@ -127,7 +136,7 @@ describe('SaveScreenshot', function () {
assert.isTrue(this.getReferenceFile.calledWithExactly(context), 'Reference getter should receive context as arg');

// check image results
assert.deepEqual(resultFirst, this.resultIdentical, 'Result should be reported');
assert.deepEqual(resultFirst, this.resultNewFile, 'Result should be reported');

// check if reference was created
const existsReference = await fs.exists(this.referencFile);
Expand Down
4 changes: 3 additions & 1 deletion test/unit/methods/Spectre.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ describe('Spectre', function () {
misMatchPercentage: 0,
isWithinMisMatchTolerance: true,
isSameDimensions: true,
isExactSameImage: true
isExactSameImage: true,
isNewScreenshot: undefined,
}, 'Result should be reported');

nock(this.url)
Expand Down Expand Up @@ -167,6 +168,7 @@ describe('Spectre', function () {
isWithinMisMatchTolerance: false,
isSameDimensions: true,
isExactSameImage: false,
isNewScreenshot: undefined,
}, 'Result should be reported');

await instance.onComplete();
Expand Down