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.

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

feat: allow folders on snapshot identifier #267

Merged
merged 3 commits into from
Apr 29, 2021

Conversation

d3c0d3dpt
Copy link
Contributor

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)
  • 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.
  • My changes are in sync with the code style of this project.
  • 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 d3c0d3dpt requested a review from a team as a code owner April 9, 2021 16:05
@d3c0d3dpt
Copy link
Contributor Author

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
Copy link

CLAassistant commented Apr 28, 2021

CLA assistant check
All committers have signed the CLA.

@JAdshead
Copy link
Contributor

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
Copy link
Contributor Author

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).

@marcusrbrown marcusrbrown merged commit ad49a97 into americanexpress:master Apr 29, 2021
oneamexbot added a commit that referenced this pull request Apr 29, 2021
# [4.5.0](v4.4.1...v4.5.0) (2021-04-29)

### Features

* allow folders on snapshot identifier ([#267](#267)) ([ad49a97](ad49a97))
@oneamexbot
Copy link
Contributor

🎉 This PR is included in version 4.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

goverdhan07 pushed a commit to goverdhan07/jest-image-snapshot that referenced this pull request Jul 23, 2023
* feat: allow folders on snapshot identifier

* docs(readme): added path support to identifier option

Co-authored-by: Jonny Adshead <JAdshead@users.noreply.github.com>
goverdhan07 pushed a commit to goverdhan07/jest-image-snapshot that referenced this pull request Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants