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: add support for custom snapshot names, close #555 #563

Merged
merged 13 commits into from
Nov 3, 2021

Conversation

noahnu
Copy link
Collaborator

@noahnu noahnu commented Oct 26, 2021

Description

Based on #561.

Related Issues

obendidi and others added 10 commits October 24, 2021 17:02
The snapshot_name_suffix will hold an optional suffix to be used instead
of index in the snapshot_name. teh suffix will be default be formatted to
be between brackets: "[<snapshot_name_suffix>]"
The snapshot_name_suffix will be used instead of index if it is provided
Update type of snapshot_name_suffix from str to Optional[str]

Co-authored-by: Noah <noahnu@gmail.com>
…_name_suffix is resolved when it is called as property of SnapshotAssertion and keeps teh same convention as before
@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #563 (f657109) into master (21cfed8) will decrease coverage by 0.26%.
The diff coverage is 100.00%.

❗ Current head f657109 differs from pull request most recent head b56c8c5. Consider uploading reports for the commit b56c8c5 to get more accurate results

@@            Coverage Diff             @@
##           master     #563      +/-   ##
==========================================
- Coverage   99.91%   99.64%   -0.27%     
==========================================
  Files          19       19              
  Lines        1130     1142      +12     
==========================================
+ Hits         1129     1138       +9     
- Misses          1        4       +3     

@noahnu noahnu mentioned this pull request Oct 26, 2021
3 tasks
@iamogbz
Copy link
Collaborator

iamogbz commented Oct 27, 2021

Nice! This looks similar to stuff I had to do here https://github.com/tophat/syrupy/pull/526/files, can rebase after this gets merged in

@noahnu
Copy link
Collaborator Author

noahnu commented Oct 27, 2021

What's left on this PR: we need to address #561 (comment)

@noahnu
Copy link
Collaborator Author

noahnu commented Nov 3, 2021

For context: had to wrap the custom name in square brackets to prevent the "cannot relate snapshot name" warning. Reason this works is because

for n in name.split("."):
stops reading the name at the first non-valid python id character, which would be [ in this case. Being able to relate snapshots is critical for discovering unused snapshots.

In the next major version of syrupy, I'd propose including the index in the snapshot itself, separate from the snapshot name so we can better differentiate the two.

@noahnu noahnu merged commit 81a8a45 into master Nov 3, 2021
@noahnu noahnu deleted the reduce_api_changes branch November 3, 2021 14:14
@noahnu
Copy link
Collaborator Author

noahnu commented Nov 3, 2021

@all-contributors add bendidi for code

@allcontributors
Copy link
Contributor

@noahnu

I've put up a pull request to add @Bendidi! 🎉

tophat-opensource-bot pushed a commit that referenced this pull request Nov 3, 2021
# [1.5.0](v1.4.7...v1.5.0) (2021-11-03)

### Features

* add support for custom snapshot names, close [#555](#555) ([#563](#563)) ([81a8a45](81a8a45))
@tophat-opensource-bot
Copy link
Collaborator

🎉 This PR is included in version 1.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

Give custom name to image snapshot file
4 participants