wip

Author: 15r10nk

src/inline_snapshot/_change.py

@@ -16,15 +16,29 @@
 
 from inline_snapshot._external._external_location import Location
 from inline_snapshot._external._format import Format
+from inline_snapshot._external._format import get_format_handler_from_suffix
 from inline_snapshot._source_file import SourceFile
 
 from ._rewrite_code import ChangeRecorder
 from ._rewrite_code import end_of
 from ._rewrite_code import start_of
 
 
+class ChangeBase:
+    flag: str
+
+    def rich_diff(self):
+        raise NotImplementedError
+
+    def apply_external_changes(self):
+        raise NotImplementedError
+
+    def apply(self, recorder: ChangeRecorder):
+        raise NotImplementedError
+
+
 @dataclass()
-class ExternalChange:
+class ExternalChange(ChangeBase):
     flag: str
 
     new_file: Path
@@ -35,7 +49,6 @@ class ExternalChange:
     format: Format
 
     def rich_diff(self):
-        pass
 
         title = str(self.new_location)
         if title != (old_name := str(self.old_location)):
@@ -58,10 +71,35 @@ def apply(self, recorder: ChangeRecorder):
 
 
 @dataclass()
-class Change:
+class ExternalRemove(ChangeBase):
+    flag: str
+
+    old_location: Location
+
+    def rich_diff(self):
+
+        title = f"delete {self.old_location}"
+
+        with self.old_location.load() as old_file:
+            assert self.old_location.suffix
+            format = get_format_handler_from_suffix(self.old_location.suffix)
+            return title, format.rich_show(old_file)
+
+    def apply_external_changes(self):
+        self.old_location.delete()
+
+    def apply(self, recorder: ChangeRecorder):
+        pass
+
+
+@dataclass()
+class Change(ChangeBase):
     flag: str
     file: SourceFile
 
+    def rich_diff(self):
+        return None
+
     @property
     def filename(self):
         return self.file.filename

src/inline_snapshot/_external/_external.py

@@ -32,7 +32,6 @@ def __init__(self, name: str, expr, context: AdapterContext):
         The external data is by default stored inside `<pytest_config_dir>/.inline-snapshot/external`,
         where `<pytest_config_dir>` is replaced by the directory containing the Pytest configuration file, if any.
         To store data in a different location, set the `storage-dir` option in pyproject.toml.
-        Data which is outsourced but not referenced in the source code jet has a '-new' suffix in the filename.
 
         Parameters:
             name: the name of the external stored object.

src/inline_snapshot/_external/_external_location.py

@@ -24,6 +24,9 @@ def load(self) -> Generator[Path]:
     def store(self, new_file: Path):
         raise NotImplementedError
 
+    def delete(self):
+        raise NotImplementedError
+
     def exists(self):
         raise NotImplementedError
 
@@ -57,7 +60,7 @@ def from_name(
             storage, path = name.split(":", 1)
         else:
             raise ValueError(
-                "path has to be of the form <hash>.<suffix> or <partial_hash>*.<suffix>"
+                f"path '{name}' has to be of the form <hash>.<suffix> or <partial_hash>*.<suffix>"
             )
 
         if "." in path:
@@ -108,6 +111,14 @@ def store(self, new_file: Path):
         storage = state().all_storages[self.storage]
         storage.store(self, new_file)
 
+    def delete(self):
+        from inline_snapshot._global_state import state
+
+        assert self.storage
+
+        storage = state().all_storages[self.storage]
+        storage.delete(self)
+
     def exists(self):
         return self.stem
 

src/inline_snapshot/_external/_storage.py

@@ -6,7 +6,12 @@
 import uuid
 from contextlib import contextmanager
 from pathlib import Path
+from typing import TYPE_CHECKING
 from typing import Generator
+from typing import Iterator
+
+if TYPE_CHECKING:
+    from inline_snapshot._change import ChangeBase
 
 from ._external_location import ExternalLocation
 
@@ -35,6 +40,9 @@ def store(self, location: ExternalLocation, file_path: Path):
         """
         raise NotImplementedError
 
+    def delete(self, location: ExternalLocation):
+        raise NotImplementedError
+
     def new_location(
         self, location: ExternalLocation, file_path: Path
     ) -> ExternalLocation:
@@ -46,7 +54,9 @@ def new_location(
     def cleanup(self):
         raise NotImplementedError
 
-    def sync_used_externals(self, used_externals: list[ExternalLocation]) -> int:
+    def sync_used_externals(
+        self, used_externals: list[ExternalLocation]
+    ) -> Iterator[ChangeBase]:
         raise NotImplementedError
 
 
@@ -70,6 +80,10 @@ def store(self, location: ExternalLocation, file_path: Path):
 
         shutil.copy(str(file_path), str(snapshot_path))
 
+    def delete(self, location: ExternalLocation):
+        snapshot_path = self._get_path(location)
+        snapshot_path.unlink()
+
     def new_location(
         self, location: ExternalLocation, file_path: Path
     ) -> ExternalLocation:
@@ -101,8 +115,10 @@ def _get_path(self, location: ExternalLocation) -> Path:
     def cleanup(self):
         pass
 
-    def sync_used_externals(self, used_externals: list[ExternalLocation]) -> int:
-        return 0
+    def sync_used_externals(
+        self, used_externals: list[ExternalLocation]
+    ) -> Iterator[ChangeBase]:
+        return iter(())
 
 
 class HashStorage(StorageProtocol):
@@ -129,13 +145,18 @@ def store(self, location: ExternalLocation, file_path: Path):
         with file_path.open("rb") as f:
             hash_name = file_digest(f, "sha256").hexdigest()
 
+        assert location.suffix
+
         if not (self.directory / (hash_name + location.suffix)).exists():
-            # TODO: remove -new
             shutil.copy(
                 str(file_path),
-                str(self.directory / (hash_name + "-new" + location.suffix)),
+                str(self.directory / (hash_name + location.suffix)),
             )
 
+    def delete(self, location: ExternalLocation):
+        path = self._lookup_path(location.path)
+        path.unlink()
+
     def new_location(
         self, location: ExternalLocation, file_path: Path
     ) -> ExternalLocation:
@@ -154,24 +175,20 @@ def prune_new_files(self):
         for file in self.directory.glob("*-new.*"):
             file.unlink()
 
-    def sync_used_externals(self, used_externals: list[ExternalLocation]) -> int:
+    def sync_used_externals(
+        self, used_externals: list[ExternalLocation]
+    ) -> Iterator[ChangeBase]:
         unused_externals = self.list()
         for location in used_externals:
             used = self.lookup_all(location.path)
-            for u in used:
-                self.persist(u)
             unused_externals -= used
 
-        n = 0
-
+        from inline_snapshot._change import ExternalRemove
         from inline_snapshot._global_state import state
 
         if state().update_flags.trim:
             for name in unused_externals:
-                self.remove(name)
-                n += 1
-
-        return n
+                yield ExternalRemove("trim", ExternalLocation.from_name(name))
 
     def cleanup(self):
         self.prune_new_files()
@@ -183,15 +200,6 @@ def list(self) -> set[str]:
         else:
             return set()
 
-    def persist(self, name):
-        try:
-            file = self._lookup_path(name)
-        except StorageLookupError:
-            return
-        if file.stem.endswith("-new"):
-            stem = file.stem[:-4]
-            file.rename(file.with_name(stem + file.suffix))
-
     def _lookup_path(self, name) -> Path:
         if "*" not in name:
             p = Path(name)
@@ -211,6 +219,9 @@ def _lookup_path(self, name) -> Path:
     def lookup_all(self, name: str) -> set[str]:
         return {file.name for file in self.directory.glob(name)}
 
+    def lookup_all_path(self, name: str) -> set[path]:
+        return {file for file in self.directory.glob(name)}
+
     def remove(self, name):
         self._lookup_path(name).unlink()
 

src/inline_snapshot/pytest_plugin.py

@@ -384,6 +384,14 @@ def apply_changes(flag):
             for category in all_categories:
                 snapshot_changes[category] += 1
 
+        used_externals2 = _find_external.used_externals()
+
+        for name, storage in state().all_storages.items():
+            for external_change in storage.sync_used_externals(
+                [e for e in used_externals2 if e.storage == name]
+            ):
+                changes[external_change.flag].append(external_change)
+
         capture.suspend_global_capture(in_=True)
         try:
 
@@ -482,8 +490,9 @@ def header():
                         any_changes = True
 
                 for change in changes[flag]:
-                    if hasattr(change, "rich_diff"):
-                        title, content = change.rich_diff()
+                    diff = change.rich_diff()
+                    if diff is not None:
+                        title, content = diff
                         console().print(
                             Panel(
                                 content,
@@ -530,14 +539,6 @@ def header():
 
                 cr.fix_all()
 
-            used_externals2 = _find_external.used_externals()
-
-            for name, storage in state().all_storages.items():
-                num = storage.sync_used_externals(
-                    [e for e in used_externals2 if e.storage == name]
-                )
-                if num:
-                    console().print(f"removed {num} unused externals\n")
         finally:
             capture.resume_global_capture()
 

tests/test_external.py

@@ -43,7 +43,7 @@ def test_a():
         ["--inline-snapshot=create"],
         changed_files=snapshot(
             {
-                ".inline-snapshot/external/ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb-new.txt": "a",
+                ".inline-snapshot/external/ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb.txt": "a",
                 "test_something.py": """\
 from inline_snapshot import external
 def test_a():
@@ -105,7 +105,7 @@ def test_diskstorage():
 
         with raises(
             snapshot(
-                "StorageLookupError: hash collision files=['a140c0c1eda2def2b830363ba362aa4d7d255c262960544821f556e16661b6ff-new.txt', 'a4e624d686e03ed2767c0abd85c14426b0b1157d2ce81d27bb4fe4f6f01d688a-new.txt']"
+                "StorageLookupError: hash collision files=['a140c0c1eda2def2b830363ba362aa4d7d255c262960544821f556e16661b6ff.txt', 'a4e624d686e03ed2767c0abd85c14426b0b1157d2ce81d27bb4fe4f6f01d688a.txt']"
             )
         ):
             assert outsource("test4") == external("hash:a*.txt")
@@ -310,7 +310,7 @@ def test_a():
     project.run()
 
     assert project.storage() == snapshot(
-        ["9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08-new.txt"]
+        ["9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08.txt"]
     )
 
     project.run("--inline-snapshot=create")
@@ -368,7 +368,7 @@ def test_errors():
 
         with raises(
             snapshot(
-                "ValueError: path has to be of the form <hash>.<suffix> or <partial_hash>*.<suffix>"
+                "ValueError: path 'invalid' has to be of the form <hash>.<suffix> or <partial_hash>*.<suffix>"
             )
         ):
             external("invalid")
@@ -474,7 +474,7 @@ def test_something():
     assert project.storage() == snapshot(
         [
             "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae.txt",
-            "8dc140e6fe831481a2005ae152ffe32a9974aa92a260dfbac780d6a87154bb0b-new.txt",
+            "8dc140e6fe831481a2005ae152ffe32a9974aa92a260dfbac780d6a87154bb0b.txt",
         ]
     )
 
@@ -499,7 +499,7 @@ def test_something():
     assert project.storage() == snapshot(
         [
             "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae.txt",
-            "8dc140e6fe831481a2005ae152ffe32a9974aa92a260dfbac780d6a87154bb0b-new.txt",
+            "8dc140e6fe831481a2005ae152ffe32a9974aa92a260dfbac780d6a87154bb0b.txt",
         ]
     )
 

tests/test_external_formats.py

@@ -15,7 +15,7 @@ def test_a():
         ["--inline-snapshot=create"],
         changed_files=snapshot(
             {
-                ".inline-snapshot/external/17f5ce5ea0f8711b6b20414da84373fb56176c3a3112c86c94529d3e29dacac3-new.json": """\
+                ".inline-snapshot/external/17f5ce5ea0f8711b6b20414da84373fb56176c3a3112c86c94529d3e29dacac3.json": """\
 [
   1,
   2