feat: remove unused snapshots in snapshot file #11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
noahnu
reviewed
Dec 4, 2019
| @@ -25,24 +25,34 @@ def __init__( | |||
| self._serializer_class = serializer_class | |||
| self._test_location = test_location | |||
| self._executions = 0 | |||
| self._session = session | |||
|
|
|||
| from .session import SnapshotSession | |||
There was a problem hiding this comment.
Is there a way to refactor so we don't have to import within the method?
There was a problem hiding this comment.
I think so but the refactor is quite extensive and not valuable imo since the only usecase here was for the type hinting.
|
|
||
| def _in_snapshot_dir(self, path: str) -> bool: | ||
| parts = path.split(os.path.sep) | ||
| return SNAPSHOT_DIRNAME in parts | ||
|
|
||
| @lru_cache(maxsize=32) |
There was a problem hiding this comment.
did you notice a performance issue?
There was a problem hiding this comment.
Nope, but I didn't see any reason why it should be executed every single time an assertion was registered. Might be a bit premature...
noahnu
reviewed
Dec 4, 2019
noahnu
reviewed
Dec 4, 2019
|
When testing this locally I noticed unused snapshots -- so it works but we should still remove these snapshots before merging this. I also have .githooks.ini modified locally for some reason after running the linter. |
for some reason it keeps adding it
noahnu
approved these changes
Dec 4, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DevQA
inv testshould show unused snapshots in single fileassert actual == snapshot_pngline in the image plugin testhttps://github.com/tophat/syrupy/blob/1ad07661f3842557d1553c74f6b7976a7f8d81ef/tests/test_image_plugin.py#L22
inv testshould show that the image file is also unusedinv test -ushould update the snapshot files and remove the image snapshot fileinv testshould show no snapshot file is unusedtest_snapshot.pyfileinv test -ushould delete thetest_snapshot.yamlfileComments