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

feat: allow storing received screenshot on failure #298

Merged
merged 2 commits into from
May 30, 2022
Merged

feat: allow storing received screenshot on failure #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5374814
feat: allow storing received screenshot on failure
ablok May 11, 2022
27cabfe
chore: more test coverage because review comments
ablok May 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ See [the examples](./examples/README.md) for more detailed usage or read about a
* `comparisonMethod`: (default: `pixelmatch`) (options `pixelmatch` or `ssim`) The method by which images are compared. `pixelmatch` does a pixel by pixel comparison, whereas `ssim` does a structural similarity comparison. `ssim` is a new experimental feature for jest-image-snapshot, but may become the default comparison method in the future. For a better understanding of how to use SSIM, see [Recommendations when using SSIM Comparison](#recommendations-when-using-ssim-comparison).
* `customSnapshotsDir`: A custom absolute path of a directory to keep this snapshot in
* `customDiffDir`: A custom absolute path of a directory to keep this diff in
* `storeReceivedOnFailure`: (default: `false`) Store the received images seperately from the composed diff images on failure. This can be useful when updating baseline images from CI.
* `customReceivedDir`: A custom absolute path of a directory to keep this received image in
* `customSnapshotIdentifier`: A custom name to give this snapshot. If not provided one is computed automatically. When a function is provided it is called with an object containing `testPath`, `currentTestName`, `counter` and `defaultIdentifier` as its first argument. The function must return an identifier to use for the snapshot. If a path is given, the path will be created inside the snapshot/diff directories.
* `diffDirection`: (default: `horizontal`) (options `horizontal` or `vertical`) Changes diff image layout direction
* `noColors`: Removes coloring from console output, useful if storing the results in a file
Expand Down
2 changes: 2 additions & 0 deletions __tests__/__snapshots__/index.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ Object {
"diffDirection": "horizontal",
"failureThreshold": 0,
"failureThresholdType": "pixel",
"receivedDir": "path/to/__image_snapshots__/__received_output__",
"receivedImageBuffer": "pretendthisisanimagebuffer",
"snapshotIdentifier": "test-spec-js-test-1",
"snapshotsDir": "path/to/__image_snapshots__",
"storeReceivedOnFailure": false,
"updatePassedSnapshot": false,
"updateSnapshot": false,
}
Expand Down
82 changes: 82 additions & 0 deletions __tests__/diff-snapshot.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe('diff-snapshot', () => {

describe('diffImageToSnapshot', () => {
const mockSnapshotsDir = path.normalize('/path/to/snapshots');
const mockReceivedDir = path.normalize('/path/to/snapshots/__received_output__');
const mockDiffDir = path.normalize('/path/to/snapshots/__diff_output__');
const mockSnapshotIdentifier = 'id1';
const mockImagePath = './__tests__/stubs/TestImage.png';
Expand Down Expand Up @@ -147,6 +148,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -167,6 +169,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -190,6 +193,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand Down Expand Up @@ -219,12 +223,62 @@ describe('diff-snapshot', () => {
expect(mockWriteFileSync).toHaveBeenCalledTimes(1);
});

it('should write a received image if the test fails and storeReceivedOnFailure = true', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add another test for when storeReceivedOnFailure is false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an explicit test case for this situation. However, this was already tested implicitly since false is the default. I did change the implementation a little bit so that it also does not return the receivedSnapshotPath if storeReceivedOnFailure is set to false.

const diffImageToSnapshot = setupTest({ snapshotExists: true, pixelmatchResult: 5000 });
const result = diffImageToSnapshot({
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
storeReceivedOnFailure: true,
receivedDir: mockReceivedDir,
Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a case for when receivedDir is undefined

Copy link
Contributor Author

@ablok ablok May 13, 2022 •
edited

Choose a reason for hiding this comment

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

The check for undefined receivedDir and other custom paths is handled in the index.js file. There did not seem to be a specific test that checked for default values, so I added that in the index.spec.js file.

diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
failureThresholdType: 'pixel',
});

expect(result).toMatchObject({
diffOutputPath: path.join(mockSnapshotsDir, '__diff_output__', 'id1-diff.png'),
receivedSnapshotPath: path.join(mockSnapshotsDir, '__received_output__', 'id1-received.png'),
diffRatio: 0.5,
diffPixelCount: 5000,
pass: false,
});

expect(mockWriteFileSync).toHaveBeenCalledTimes(2);
});

it('should not write a received image if the test fails and storeReceivedOnFailure = false', () => {
const diffImageToSnapshot = setupTest({ snapshotExists: true, pixelmatchResult: 5000 });
const result = diffImageToSnapshot({
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
storeReceivedOnFailure: false,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
failureThresholdType: 'pixel',
});

expect(result).toMatchObject({
diffOutputPath: path.join(mockSnapshotsDir, '__diff_output__', 'id1-diff.png'),
diffRatio: 0.5,
diffPixelCount: 5000,
pass: false,
});

expect(mockWriteFileSync).toHaveBeenCalledTimes(1);
});

it('should fail if image passed is a different size', () => {
const diffImageToSnapshot = setupTest({ snapshotExists: true, pixelmatchResult: 5000 });
const result = diffImageToSnapshot({
receivedImageBuffer: mockBigImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand Down Expand Up @@ -254,6 +308,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 250,
Expand All @@ -271,6 +326,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockBigImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 250,
Expand All @@ -290,6 +346,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockBigImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -309,6 +366,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -326,6 +384,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 250,
Expand All @@ -344,6 +403,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 250,
Expand All @@ -361,6 +421,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0.025,
Expand All @@ -378,6 +439,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0.025,
Expand All @@ -396,6 +458,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -420,6 +483,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
customDiffConfig: {
Expand Down Expand Up @@ -450,6 +514,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
failureThreshold: 0,
failureThresholdType: 'pixel',
Expand All @@ -469,6 +534,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: path.join('parent', mockSnapshotIdentifier),
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
failureThreshold: 0,
failureThresholdType: 'pixel',
Expand All @@ -483,6 +549,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -498,6 +565,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -513,6 +581,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
updatePassedSnapshot: true,
Expand All @@ -529,6 +598,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: path.join('parent', mockSnapshotIdentifier),
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
updatePassedSnapshot: true,
Expand All @@ -545,6 +615,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
failureThreshold: 0,
Expand All @@ -560,6 +631,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -576,6 +648,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
updatePassedSnapshot: true,
Expand All @@ -592,6 +665,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
diffDirection: 'vertical',
updateSnapshot: false,
Expand All @@ -612,6 +686,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -629,6 +704,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -644,6 +720,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
updatePassedSnapshot: false,
Expand All @@ -662,6 +739,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
updatePassedSnapshot: true,
Expand All @@ -680,6 +758,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: true,
updatePassedSnapshot: false,
Expand All @@ -697,6 +776,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand All @@ -713,6 +793,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
updateSnapshot: false,
failureThreshold: 0,
Expand Down Expand Up @@ -740,6 +821,7 @@ describe('diff-snapshot', () => {
receivedImageBuffer: mockFailImageBuffer,
snapshotIdentifier: mockSnapshotIdentifier,
snapshotsDir: mockSnapshotsDir,
receivedDir: mockReceivedDir,
diffDir: mockDiffDir,
failureThreshold: 0,
failureThresholdType: 'pixel',
Expand Down