From 3676c39d651b8206ff0541122edc4037a7e8f4e5 Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Wed, 7 Jan 2026 13:02:34 -0800 Subject: [PATCH] Lint, complete coverage --- beetsplug/fromfilename.py | 54 ++++++++++++---------- test/plugins/test_fromfilename.py | 77 ++++++++++++++++++------------- 2 files changed, 75 insertions(+), 56 deletions(-) diff --git a/beetsplug/fromfilename.py b/beetsplug/fromfilename.py index c4bfd538a..1ecb19798 100644 --- a/beetsplug/fromfilename.py +++ b/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 diff --git a/test/plugins/test_fromfilename.py b/test/plugins/test_fromfilename.py index c00b56721..371a92b0d 100644 --- a/test/plugins/test_fromfilename.py +++ b/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.+)\ \-\ (?P.+)\ \((?P.+)\)\ \ \{(?P.+)\}", + ( + r"(?P.+)\ \-\ (?P.+)\ " + r"\((?P.+)\)\ \ \{(?P.+)\}" + ), ), ("$", 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}}"