-
Notifications
You must be signed in to change notification settings - Fork 36
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
Tests with many snapshots are sorted 1, 10, 11, ..., 2, 20, ..., 3, 4, 5, ..., not numeric order #657
Comments
I like the idea of implementing #569 first, removing the index from the name field and then sorting by (name, index). |
I fixed this in the v4 PR here (not released yet): 4ca0716 however my solution is causing performance issues. I suspect because of the nested tuples in my solution. Will need to improve the performance or come up with another approach before releasing syrupy v4. I suppose we could try sort only on the text after the last |
So as not to hold up the v4 release, I'm not making this the default behaviour, however it can be accomplished via a custom extension with a couple lines of code: https://github.com/tophat/syrupy/blob/f416e9a408bc252fdeab1888cb8b7f0cd8fdef92/tests/syrupy/extensions/amber_sorted/test_amber_sort.py |
# [4.0.0](v3.0.6...v4.0.0) (2023-02-02) ### Bug Fixes * defer snapshot writes until end of session ([#606](#606)) ([68f1d5f](68f1d5f)) * ensure all pytest options are serializable ([#667](#667)) ([e8ed9f2](e8ed9f2)) * improve pytest-xdist compatibility ([9b9090f](9b9090f)) * lru_cache on snapshot reads ([#629](#629)) ([c1a675f](c1a675f)) * remove legacy path usage to support no:legacypath, closes [#677](#677) ([#684](#684)) ([6385979](6385979)) ### Code Refactoring * simplify data serializer for ambr ([#676](#676)) ([3d296e1](3d296e1)) * write performance improvements, api clarity ([#645](#645)) ([2c31c39](2c31c39)) ### Features * **json:** serialize None as null, close [#622](#622) ([c330680](c330680)) * numerically sort snapshots if possible, close [#657](#657) ([4ca0716](4ca0716)) * **serializer:** preserve key ordering of OrderedDict ([0a2289a](0a2289a)) * support overriding the amber serializer class ([#683](#683)) ([662c93f](662c93f)) * update python version, pytest version ([#658](#658)) ([c360b95](c360b95)) ### BREAKING CHANGES * Serializers may now throw a TaintedSnapshotError which will tell the user to regenerate the snapshot even if the underlying data has not changed. This is to support rolling out more subtle changes to the serializers, such as the introduction of serializer metadata. * Renamed DataSerializer to AmberDataSerializer. * **serializer:** Key order is now preserved if using OrderedDict in both the Amber serializer and JSON serializer. * **json:** The JSONSnapshotExtension now serializes Python's None as "null" rather than "None". * Raise minimum python version to 3.8.1 and min. pytest version to v7. * PyTestLocation.filename has been renamed to .basename * refactor: add test_location kwarg to get_snapshot_name * refactor: get_snapshot_name is now static as a classmethod * refactor: remove pre and post read/write hooks * Pre and post read/write hooks have been removed without replacement to make internal refactor simpler. Please open a GitHub issue if you have a use case for these hooks. * refactor: rename Fossil to Collection * The term 'fossil' has been replaced by the clearer term 'collection'. * refactor: pass test_location to read_snapshot * refactor: remove singular write_snapshot method * refactor: dirname property to method * refactor: pass test_location to discover_snapshots * refactor: remove usage of self.test_location * refactor: make write_snapshot a classmethod * refactor: do not instantiate extension with test_location * Numerous instance methods have been refactored as classmethods.
🎉 This issue has been resolved in version 4.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
A neat solution 👍 Thank you! |
Describe the bug
Snapshots are stored sorted in the
.ambr
, but this seems to be doing a string comparison on the index, meaning10
sorts before2
, etc.To reproduce
Run
pytest --snapshot-update test_many.py
:Resulting
__snapshots__/test_many.ambr
:Expected behavior
Snapshots should be sorted in numeric order 1, 2, ..., 9, 10, ..., 20, ..., 24.
Some options:
Screenshots
N/A
Environment (please complete the following information):
Additional context
Thank you for syrupy!
The text was updated successfully, but these errors were encountered: