Lint, complete coverage

beetsplug/fromfilename.py

@@ -138,7 +138,6 @@ class FilenameMatch(MutableMapping[str, str | None]):
         return self._matches.values()
 
 
-
 class FromFilenamePlugin(BeetsPlugin):
     def __init__(self) -> None:
         super().__init__()
@@ -165,16 +164,17 @@ class FromFilenamePlugin(BeetsPlugin):
     @cached_property
     def file_patterns(self) -> list[re.Pattern[str]]:
         return self._user_pattern_to_regex(
-            self.config["patterns"]["file"].as_str_seq())
+            self.config["patterns"]["file"].as_str_seq()
+        )
 
     @cached_property
     def folder_patterns(self) -> list[re.Pattern[str]]:
         return self._user_pattern_to_regex(
             self.config["patterns"]["folder"].as_str_seq()
-                                           )
+        )
 
     def filename_task(self, task: ImportTask, session: ImportSession) -> None:
-        """ Examines all files in the given import task for any missing
+        """Examines all files in the given import task for any missing
         information it can gather from the file and folder names.
 
         Once the information has been obtained and checked, it
@@ -196,19 +196,17 @@ class FromFilenamePlugin(BeetsPlugin):
         # Apply the information
         self._apply_matches(album_matches, track_matches)
 
-    def _user_pattern_to_regex(self, patterns: list[str]) -> list[re.Pattern[str]]:
+    def _user_pattern_to_regex(
+        self, patterns: list[str]
+    ) -> list[re.Pattern[str]]:
         """Compile user patterns into a list of usable regex
         patterns. Catches errors are continues without bad regex patterns.
         """
         return [
-            re.compile(regexp) for p in patterns if (
-            regexp := self._parse_user_pattern_strings(p))
-                ]
-
-    @staticmethod
-    def _escape(text: str) -> str:
-        # escape brackets for fstring logs
-        return re.sub("}", "}}", re.sub("{", "{{", text))
+            re.compile(regexp)
+            for p in patterns
+            if (regexp := self._parse_user_pattern_strings(p))
+        ]
 
     @staticmethod
     def _get_path_strings(items: list[Item]) -> tuple[str, dict[Item, str]]:
@@ -222,18 +220,20 @@ class FromFilenamePlugin(BeetsPlugin):
                 parent_folder = path.parent.stem
         return parent_folder, filenames
 
-    def _check_user_matches(self, text: str,
-        patterns: list[re.Pattern[str]]) -> FilenameMatch:
+    def _check_user_matches(
+        self, text: str, patterns: list[re.Pattern[str]]
+    ) -> FilenameMatch:
         for p in patterns:
-            if (usermatch := p.fullmatch(text)):
+            if usermatch := p.fullmatch(text):
                 return FilenameMatch(usermatch.groupdict())
-        return None
+        return FilenameMatch()
 
-    def _build_track_matches(self,
-        item_filenames: dict[Item, str]) -> dict[Item, FilenameMatch]:
+    def _build_track_matches(
+        self, item_filenames: dict[Item, str]
+    ) -> dict[Item, FilenameMatch]:
         track_matches: dict[Item, FilenameMatch] = {}
         for item, filename in item_filenames.items():
-            if (m := self._check_user_matches(filename, self.file_patterns)):
+            if m := self._check_user_matches(filename, self.file_patterns):
                 track_matches[item] = m
             else:
                 match = self._parse_track_info(filename)
@@ -264,7 +264,7 @@ class FromFilenamePlugin(BeetsPlugin):
 
     def _parse_album_info(self, text: str) -> FilenameMatch:
         # Check if a user pattern matches
-        if (m := self._check_user_matches(text, self.folder_patterns)):
+        if m := self._check_user_matches(text, self.folder_patterns):
             return m
         matches = FilenameMatch()
         # Start with the extra fields to make parsing
@@ -301,7 +301,9 @@ class FromFilenamePlugin(BeetsPlugin):
         return matches
 
     def _apply_matches(
-        self, album_match: FilenameMatch, track_matches: dict[Item, FilenameMatch]
+        self,
+        album_match: FilenameMatch,
+        track_matches: dict[Item, FilenameMatch],
     ) -> None:
         """Apply all valid matched fields to all items in the match dictionary."""
         match = album_match
@@ -404,7 +406,9 @@ class FromFilenamePlugin(BeetsPlugin):
 
     def _parse_user_pattern_strings(self, text: str) -> str | None:
         # escape any special characters
-        fields: list[str] = [s.lower() for s in re.findall(r"\$([a-zA-Z\_]+)", text)]
+        fields: list[str] = [
+            s.lower() for s in re.findall(r"\$([a-zA-Z\_]+)", text)
+        ]
         if not fields:
             # if there are no usable fields
             return None
@@ -422,7 +426,9 @@ class FromFilenamePlugin(BeetsPlugin):
         return text
 
     def _sanity_check_matches(
-        self, album_match: FilenameMatch, track_matches: dict[Item, FilenameMatch]
+        self,
+        album_match: FilenameMatch,
+        track_matches: dict[Item, FilenameMatch],
     ) -> None:
         """Check to make sure data is coherent between
         track and album matches. Largely looking to see

test/plugins/test_fromfilename.py

@@ -22,10 +22,13 @@ from beetsplug.fromfilename import FilenameMatch, FromFilenamePlugin
 
 
 class Session:
+    """Mock session, not used by the plugin."""
+
     pass
 
 
 def mock_item(**kwargs):
+    """Mock item with blank defaults."""
     defaults = dict(
         title="",
         artist="",
@@ -41,6 +44,7 @@ def mock_item(**kwargs):
 
 
 def mock_task(items):
+    """Mock task for ease of testing."""
     return ImportTask(toppath=None, paths=None, items=items)
 
 
@@ -117,6 +121,7 @@ def mock_task(items):
     ],
 )
 def test_parse_track_info(text, matchgroup):
+    """Test parsing track information from a filename."""
     f = FromFilenamePlugin()
     m = f._parse_track_info(text)
     assert dict(matchgroup.items()) == dict(m.items())
@@ -128,15 +133,7 @@ def test_parse_track_info(text, matchgroup):
         (
             # highly unlikely
             "",
-            FilenameMatch(
-                {
-                    "albumartist": None,
-                    "album": None,
-                    "year": None,
-                    "catalognum": None,
-                    "media": None,
-                }
-            ),
+            FilenameMatch(),
         ),
         (
             "1970",
@@ -263,6 +260,7 @@ def test_parse_track_info(text, matchgroup):
     ],
 )
 def test_parse_album_info(text, matchgroup):
+    """Test parsing album information from a folder name."""
     f = FromFilenamePlugin()
     m = f._parse_album_info(text)
     assert matchgroup == m
@@ -273,12 +271,16 @@ def test_parse_album_info(text, matchgroup):
     [
         (
             "$albumartist - $album ($year)  {$comments}",
-            r"(?P<albumartist>.+)\ \-\ (?P<album>.+)\ \((?P<year>.+)\)\ \ \{(?P<comments>.+)\}",
+            (
+                r"(?P<albumartist>.+)\ \-\ (?P<album>.+)\ "
+                r"\((?P<year>.+)\)\ \ \{(?P<comments>.+)\}"
+            ),
         ),
         ("$", None),
     ],
 )
 def test_parse_user_pattern_strings(string, pattern):
+    """Test converting a user's format string to regexp"""
     f = FromFilenamePlugin()
     assert f._parse_user_pattern_strings(string) == pattern
 
@@ -514,8 +516,6 @@ class TestFromFilename(PluginMixin):
                 ),
             ],
             [
-                # Even though it might be clear to human eyes,
-                # we can't guess since the various flag is thrown
                 mock_item(
                     path=(
                         "/303 Alliance 012/"
@@ -540,13 +540,9 @@ class TestFromFilename(PluginMixin):
         ],
     )
     def test_sanity_check(self, expected_items):
-        """
-        Take a list of expected items, create a task with just the paths.
-
-        Goal is to ensure that sanity check
-        correctly adjusts the parsed artists and albums
-
-        After parsing, compare to the expected items.
+        """Take a list of expected items, create a task with just the paths.
+        Assert the conditions that cause sanity check to change artist and title
+        fields.
         """
         task = mock_task([mock_item(path=item.path) for item in expected_items])
         f = FromFilenamePlugin()
@@ -569,6 +565,7 @@ class TestFromFilename(PluginMixin):
         assert res[1].title == exp[1].title
 
     def test_singleton_import(self):
+        """Ensure that singletons behave correctly."""
         task = SingletonImportTask(
             toppath=None, item=mock_item(path="/01 Track.wav")
         )
@@ -577,9 +574,25 @@ class TestFromFilename(PluginMixin):
         assert task.item.track == 1
         assert task.item.title == "Track"
 
-    # TODO: Test with items that already have data, or other types of bad data.
-
-    # TODO: Test with items that have perfectly fine data for the most part
+    def test_item_with_existing_data(self):
+        """Ensure that existing metadata is not overwritten, no matter
+        how incorrect it may be."""
+        path = "/Album Artist - Album (1999)/01 - Track Title.wav"
+        albumartist = "Other Artist"
+        title = "Existing Title"
+        given = mock_item(
+            path=path,
+            albumartist=albumartist,
+            album=" ",
+            title=title,
+            year=2024,
+        )
+        f = FromFilenamePlugin()
+        f.filename_task(mock_task([given]), Session())
+        assert given.title == title
+        assert given.albumartist == albumartist
+        assert given.album == "Album"
+        assert given.year == 2024
 
     @pytest.mark.parametrize(
         "fields,expected",
@@ -621,11 +634,7 @@ class TestFromFilename(PluginMixin):
         ],
     )
     def test_fields(self, fields, expected):
-        """
-        With a set item and changing list of fields
-
-        After parsing, compare to the original with the expected attributes defined.
-        """
+        """Test that the applied fields can be adjusted by the user."""
         path = (
             "/Album Artist - Album (2025) [FLAC CD] {CATALOGNUM}/"
             "1-2 Artist - Track.wav"
@@ -653,7 +662,10 @@ class TestFromFilename(PluginMixin):
                     "file": ["$artist - $track - $title"],
                 },
                 mock_item(
-                    path="/(Comment) - {Album Artist} - {Album}/Artist - 02 - Title.flac",
+                    path=(
+                        "/(Comment) - {Album Artist} - {Album}"
+                        "/Artist - 02 - Title.flac"
+                    ),
                     comments="Comment",
                     albumartist="Album Artist",
                     album="Album",
@@ -668,7 +680,10 @@ class TestFromFilename(PluginMixin):
                     "file": ["$artist - $track - $title"],
                 },
                 mock_item(
-                    path="/(Comment) - {Album Artist} - {Album}/Artist - 02 - Title.flac",
+                    path=(
+                        "/(Comment) - {Album Artist} - {Album}"
+                        "/Artist - 02 - Title.flac"
+                    ),
                     artist="Artist",
                     track=2,
                     title="Title",
@@ -678,6 +693,7 @@ class TestFromFilename(PluginMixin):
         ],
     )
     def test_user_patterns(self, patterns, expected):
+        """Test recognizing data from a given user pattern."""
         task = mock_task([mock_item(path=expected.path)])
         with self.configure_plugin({"patterns": patterns}):
             f = FromFilenamePlugin()
@@ -691,6 +707,3 @@ class TestFromFilename(PluginMixin):
             assert res.catalognum == expected.catalognum
             assert res.year == expected.year
             assert res.title == expected.title
-
-    def test_escape(self):
-        assert FromFilenamePlugin._escape("{text}") == "{{text}}"