feat: allow folders on snapshot identifier

d3c0d3dpt commented 3 years ago

Allow users to add paths to the Snapshot Identifier option.

Description

Improved creation of folders (via mkdirp) to use the path.dirname of the screenshot, instead of only using the screenshots/diff folders.

Motivation and Context

Allow better organization of screenshots, allowing to use folders. This allows to improve the snapshot organization when using visual tests for different pages, making use of the customSnapshotIdentifier option to dynamically organize the snapshots according to the test set they belong to.

How Has This Been Tested?

Unit tests added;

Tested on another application by using the following:

function customSnapshotIdentifier (parameters: { currentTestName: string }): string {
    const matches = /^(.*) Screen (\d+ x \d+) (.*)$/.exec(parameters.currentTestName);

    if (matches === null || matches.length < 4) {
        throw new Error('Invalid test name!');
    }

    return path.join(matches[1], matches[2], matches[3]);
}

A name like "Home Screen 1280 x 720 Full Page Render" is converted to "Home/1280 x 720/Full Page Render" where screenshots are organized by page, screen size and test name.

Types of Changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation (adding or updating documentation)
[ ] Dependency update

Checklist:

[ ] My change requires a change to the documentation and I have updated the documentation accordingly.
[x] My changes are in sync with the code style of this project.
[x] There aren't any other open Pull Requests for the same issue/update.
[ ] These changes should be applied to a maintenance branch.
[ ] I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using Jest-Image-Snapshot?

Allowing better organization of snapshots.

d3c0d3dpt commented 3 years ago

Hi guys, I'm having some doubts regarding the checklist item "My change requires a change to the documentation and I have updated the documentation accordingly.". How should I add this to the documentation?

CLAassistant commented 3 years ago

All committers have signed the CLA.

JAdshead commented 3 years ago

In regards to updating the readme, I would add that the customSnapshotIdentifier can also be used to add custom directory path.

This feature is currently available through the customSnapshotsDir parameter. I can see the advantage of this change but my initial impression is that this feature could reduce the clarity of the API.

d3c0d3dpt commented 3 years ago

The customSnapshotsDir options does not allows a function to be passed. When no value it's given, the screenshots will be created alongside the test files, which provides a different screenshot organisation than keeping all of them on a separate folder (achieved when the value is given).

oneamexbot commented 3 years ago

:tada: This PR is included in version 4.5.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

americanexpress / jest-image-snapshot