feat: add onlyDiff in options #317
Conversation
|
|
src/diff-snapshot.js
Outdated
| function composeDiff(options) { | ||
| const { | ||
| diffDirection, baselineImage, diffImage, receivedImage, imageWidth, imageHeight, onlyDiff, | ||
| } = options; | ||
| const composer = new ImageComposer({ | ||
| direction: diffDirection, | ||
| }); | ||
|
|
||
| if (!onlyDiff) { | ||
| composer.addImage(baselineImage, imageWidth, imageHeight); | ||
| } | ||
| composer.addImage(diffImage, imageWidth, imageHeight); | ||
| if (!onlyDiff) { | ||
| composer.addImage(receivedImage, imageWidth, imageHeight); | ||
| } | ||
| return composer; |
There was a problem hiding this comment.
I created a new function, otherwise the complexity of diffImageToSnapshot was too high
|
@code-forger I pushed an update |
|
@code-forger can you have another look? 🙏 |
|
@10xLaCroixDrinker can you have a look? 🙏 |
__tests__/diff-snapshot.spec.js
Outdated
| expect(diffResult).toHaveProperty('pass', false); | ||
| // White image without the baseline, nor the received (white because we mock pixelmatch) | ||
| expect(diffResult).toHaveProperty('imgSrcString', 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAABkCAYAAABw4pVUAAAA30lEQVR4Ae3BIQEAAACDMAT9M78G4ptcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcilyKXIpcygA9UAGR3m7UvwAAAABJRU5ErkJggg=='); | ||
| expect(diffResult).toHaveProperty('diffOutputPath', path.join(mockSnapshotsDir, '__diff_output__', `${mockSnapshotIdentifier}-diff.png`)); |
There was a problem hiding this comment.
These assertions don't give me any confidence. Can we add something that clearly checks that only the diff is included?
There was a problem hiding this comment.
What if I add to the stubs folder the expected result of test instead of doing a manual data:image/png;base64,…?
There was a problem hiding this comment.
Edit: I removed this test and I moved it to an integration test, so that we can have a real check and a real diff image
Ayc0
requested review from
10xLaCroixDrinker and
code-forger
and removed request for
code-forger and
10xLaCroixDrinker
|
@10xLaCroixDrinker do you know if we also need @code-forger's approval (as a "request changes" was applied)? |
# [6.1.0](v6.0.0...v6.1.0) (2022-12-02) ### Features * add onlyDiff in options ([#317](#317)) ([4bad752](4bad752))
|
🎉 This PR is included in version 6.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Thanks! 🎉 |
|
FYI I'm also changing the types for this new option in DefinitelyTyped/DefinitelyTyped#63501 |
…n version 6.1.0 by @Ayc0 * [jest-image-snapshot] add `onlyDiff` option Add changes introduced in americanexpress/jest-image-snapshot#317 to the types * [jest-image-snapshot] update version reference * [jest-image-snapshot] add reference to author Ayc0 * [jest-image-snapshot] add tests for onlyDiff
# [6.1.0](americanexpress/jest-image-snapshot@v6.0.0...v6.1.0) (2022-12-02) ### Features * add onlyDiff in options ([americanexpress#317](americanexpress#317)) ([4bad752](americanexpress@4bad752))
Description
Add a new option
onlyDiffintoMatchImageSnapshotMotivation and Context
When working with complex rules, we may want to handle the 3 images independently: the received image, the diff image (with only the diff), and the expected image.
How Has This Been Tested?
onlyDiff(or withonlyDiff = false)onlyDiff = trueTypes of Changes
Checklist:
What is the Impact to Developers Using Jest-Image-Snapshot?