Fix cleanup unused snapshots when targeting files (#127)

* test: clean up unused snapshots when targeting file

* test: targetting by node ids

* test: targeting by marker expresions

* test: refactor use testdir fixture

* fix: support node id test selection

* test: add test for snapshot location warning

* refactor: filter before checking if ran all items

* chore: update changelog

* test: compress collection testcases

* fix: support --pyargs syntax

* cr: wrap message in gettext for i88n

* cr: remove warning message whitespace

* cr: warning message formatting

CHANGELOG.md

@@ -9,6 +9,9 @@ From v1.0.0 onwards, this project adheres to [Semantic Versioning](https://semve
 ## Master (Unreleased)
 
 - Fix bug where untargeted snapshots would be deleted when using pytest in targeted mode (#123)
+- Fix bug where snapshot files were not cleaned up when running specific test files (#127)
+- Fix bug where targeting specific test nodes in a test file was not supported (#127)
+- Fix bug where targeting specific test modules using pyargs was not supported (#127)
 
 ## [v0.3.1](https://github.com/tophat/syrupy/compare/v0.3.0...v0.3.1)
 

src/syrupy/__init__.py

@@ -54,6 +54,14 @@ def pytest_assertrepr_compare(op: str, left: Any, right: Any) -> Optional[List[s
     return None
 
 
+def __is_testpath(arg: str) -> bool:
+    return not arg.startswith("-") and bool(glob.glob(arg.split("::")[0]))
+
+
+def __is_testmodule(arg: str) -> bool:
+    return arg == "--pyargs"
+
+
 def pytest_sessionstart(session: Any) -> None:
     """
     Initialize snapshot session before tests are collected and ran.
@@ -65,7 +73,7 @@ def pytest_sessionstart(session: Any) -> None:
         update_snapshots=config.option.update_snapshots,
         base_dir=config.rootdir,
         is_providing_paths=any(
-            not arg.startswith("-") and glob.glob(arg)
+            __is_testpath(arg) or __is_testmodule(arg)
             for arg in config.invocation_params.args
         ),
     )

src/syrupy/data.py

@@ -50,6 +50,8 @@ def get(self, snapshot_name: str) -> Optional["Snapshot"]:
 
     def add(self, snapshot: "Snapshot") -> None:
         self._snapshots[snapshot.name] = snapshot
+        if snapshot.name != SNAPSHOT_EMPTY_FOSSIL_KEY:
+            self.remove(SNAPSHOT_EMPTY_FOSSIL_KEY)
 
     def merge(self, snapshot_fossil: "SnapshotFossil") -> None:
         for snapshot in snapshot_fossil:
@@ -82,7 +84,7 @@ def has_snapshots(self) -> bool:
 
 @attr.s(frozen=True)
 class SnapshotUnknownFossil(SnapshotFossil):
-    """This is a saved fossil that is unclaimed by any extension currenly in use"""
+    """This is a saved fossil that is unclaimed by any extension currently in use"""
 
     _snapshots: Dict[str, "Snapshot"] = attr.ib(default=SNAPSHOTS_UNKNOWN, init=False)
 

src/syrupy/extensions/base.py

@@ -5,6 +5,7 @@
     abstractmethod,
 )
 from difflib import ndiff
+from gettext import gettext
 from itertools import zip_longest
 from typing import (
     TYPE_CHECKING,
@@ -123,12 +124,18 @@ def write_snapshot(self, *, data: "SerializedData", index: int) -> None:
         """
         self._pre_write(data=data, index=index)
         snapshot_location = self.get_location(index=index)
+        if not self.test_location.matches_snapshot_location(snapshot_location):
+            warning_msg = gettext(
+                "\nCan not relate snapshot location '{}' to the test location."
+                "\nConsider adding '{}' to the generated location."
+            ).format(snapshot_location, self.test_location.filename)
+            warnings.warn(warning_msg)
         snapshot_name = self.get_snapshot_name(index=index)
         if not self.test_location.matches_snapshot_name(snapshot_name):
-            warning_msg = f"""
-            Can not relate snapshot name '{snapshot_name}' to the test location.
-            Consider adding '{self.test_location.testname}' to the generated name.
-            """
+            warning_msg = gettext(
+                "\nCan not relate snapshot name '{}' to the test location."
+                "\nConsider adding '{}' to the generated name."
+            ).format(snapshot_name, self.test_location.testname)
             warnings.warn(warning_msg)
         snapshot_fossil = SnapshotFossil(location=snapshot_location)
         snapshot_fossil.add(Snapshot(name=snapshot_name, data=data))
@@ -182,7 +189,7 @@ def _write_snapshot_fossil(self, *, snapshot_fossil: "SnapshotFossil") -> None:
 
     @property
     def _dirname(self) -> str:
-        test_dirname = os.path.dirname(self.test_location.filename)
+        test_dirname = os.path.dirname(self.test_location.filepath)
         return os.path.join(test_dirname, SNAPSHOT_DIRNAME)
 
     @property
@@ -192,7 +199,7 @@ def _file_extension(self) -> str:
 
     def _get_file_basename(self, *, index: int) -> str:
         """Returns file basename without extension. Used to create full filepath."""
-        return f"{os.path.splitext(os.path.basename(self.test_location.filename))[0]}"
+        return self.test_location.filename
 
     def __ensure_snapshot_dir(self, *, index: int) -> None:
         """

src/syrupy/extensions/single_file.py

@@ -43,9 +43,7 @@ def _get_file_basename(self, *, index: int) -> str:
     @property
     def _dirname(self) -> str:
         original_dirname = super(SingleFileSnapshotExtension, self)._dirname
-        file_basename = os.path.basename(str(self.test_location.filename))
-        sub_dirname = os.path.splitext(file_basename)[0]
-        return os.path.join(original_dirname, sub_dirname)
+        return os.path.join(original_dirname, self.test_location.filename)
 
     def _read_snapshot_fossil(self, *, snapshot_location: str) -> "SnapshotFossil":
         snapshot_fossil = SnapshotFossil(location=snapshot_location)

src/syrupy/location.py

@@ -1,3 +1,4 @@
+import os
 from typing import (
     Any,
     Optional,
@@ -7,7 +8,7 @@
 class TestLocation(object):
     def __init__(self, node: Any):
         self._node = node
-        self.filename = self._node.fspath
+        self.filepath = self._node.fspath
         self.modulename = self._node.obj.__module__
         self.methodname = self._node.obj.__name__
         self.nodename = getattr(self._node, "name", None)
@@ -18,7 +19,14 @@ def classname(self) -> Optional[str]:
         classes = self._node.obj.__qualname__.split(".")[:-1]
         return ".".join(classes) if classes else None
 
+    @property
+    def filename(self) -> str:
+        return str(os.path.splitext(os.path.basename(self.filepath))[0])
+
     def matches_snapshot_name(self, snapshot_name: str) -> bool:
         matches_basemethod = str(self.methodname) in snapshot_name
         matches_testnode = snapshot_name in str(self.nodename)
         return matches_basemethod or matches_testnode
+
+    def matches_snapshot_location(self, snapshot_location: str) -> bool:
+        return self.filename in snapshot_location

src/syrupy/report.py

@@ -49,16 +49,9 @@ class SnapshotReport(object):
     updated: "SnapshotFossils" = attr.ib(factory=SnapshotFossils)
     used: "SnapshotFossils" = attr.ib(factory=SnapshotFossils)
 
-    def filter_fossils(self, item: "SnapshotFossil") -> bool:
-        if not self.ran_items or not self.is_providing_paths:
-            return True
-
-        target_snaps = {snap.location for snap in self.used}
-
-        return item.location in target_snaps
-
     def __attrs_post_init__(self) -> None:
         for assertion in self.assertions:
+            self.discovered.merge(assertion.extension.discover_snapshots())
             for result in assertion.executions.values():
                 snapshot_fossil = SnapshotFossil(location=result.snapshot_location)
                 snapshot_fossil.add(
@@ -74,14 +67,6 @@ def __attrs_post_init__(self) -> None:
                 else:
                     self.failed.update(snapshot_fossil)
 
-            discovered_snaps = assertion.extension.discover_snapshots()
-            filtered_snaps = {
-                snap.location: snap
-                for snap in discovered_snaps
-                if self.filter_fossils(snap)
-            }
-            self.discovered.merge(SnapshotFossils(filtered_snaps))
-
     @property
     def num_created(self) -> int:
         return self._count_snapshots(self.created)
@@ -113,6 +98,12 @@ def unused(self) -> "SnapshotFossils":
             self.discovered, self.used
         ):
             snapshot_location = unused_snapshot_fossil.location
+            if self.is_providing_paths and not any(
+                TestLocation(node).matches_snapshot_location(snapshot_location)
+                for node in self.ran_items
+            ):
+                continue
+
             if self.ran_all_collected_tests:
                 unused_snapshots = {*unused_snapshot_fossil}
                 mark_for_removal = snapshot_location not in self.used

tests/examples/test_custom_snapshot_directory.py

@@ -23,7 +23,7 @@
 class DifferentDirectoryExtension(AmberSnapshotExtension):
     @property
     def _dirname(self) -> str:
-        test_dirname = os.path.dirname(self.test_location.filename)
+        test_dirname = os.path.dirname(self.test_location.filepath)
         return os.path.join(test_dirname, DIFFERENT_DIRECTORY)
 
 

tests/test_integration_custom.py

@@ -34,6 +34,9 @@ def _write_snapshot_fossil(self, **kwargs):
             def delete_snapshots(self, **kwargs):
                 pass
 
+            def _get_file_basename(self, *, index = 0):
+                return self.test_location.filename[::-1]
+
 
         @pytest.fixture
         def snapshot_custom(snapshot):
@@ -57,5 +60,7 @@ def test_warns_on_snapshot_name(testdir, testcases):
     result_stdout = clean_output(result.stdout.str())
     assert "2 snapshots generated" in result_stdout
     assert "Warning:" in result_stdout
+    assert "Can not relate snapshot name" in result_stdout
+    assert "Can not relate snapshot location" in result_stdout
     assert "test_passed_custom" in result_stdout
     assert result.ret == 0