|
14 | 14 |
|
15 | 15 | """Tests for the 'lastgenre' plugin.""" |
16 | 16 |
|
17 | | -from unittest.mock import Mock, patch |
| 17 | +import re |
| 18 | +from collections import defaultdict |
| 19 | +from unittest.mock import MagicMock, Mock, patch |
18 | 20 |
|
| 21 | +import confuse |
19 | 22 | import pytest |
20 | 23 |
|
| 24 | +from beets.library import Album |
21 | 25 | from beets.test import _common |
22 | 26 | from beets.test.helper import IOMixin, PluginTestCase |
23 | 27 | from beetsplug import lastgenre |
| 28 | +from beetsplug.lastgenre.utils import is_ignored |
24 | 29 |
|
25 | 30 |
|
26 | 31 | class LastGenrePluginTest(IOMixin, PluginTestCase): |
@@ -202,6 +207,80 @@ def test_sort_by_depth(self): |
202 | 207 | res = lastgenre.sort_by_depth(tags, self.plugin.c14n_branches) |
203 | 208 | assert res == ["ambient", "electronic"] |
204 | 209 |
|
| 210 | + # Ignorelist tests in resolve_genres and _is_ignored |
| 211 | + |
| 212 | + def test_ignorelist_filters_genres_in_resolve(self): |
| 213 | + """Ignored genres are stripped by _resolve_genres (no c14n). |
| 214 | +
|
| 215 | + Artist-specific and global patterns are both applied. |
| 216 | + """ |
| 217 | + self._setup_config(whitelist=False, canonical=False) |
| 218 | + self.plugin.ignore_patterns = defaultdict( |
| 219 | + list, |
| 220 | + { |
| 221 | + "the artist": [re.compile(r"^metal$", re.IGNORECASE)], |
| 222 | + "*": [re.compile(r"^rock$", re.IGNORECASE)], |
| 223 | + }, |
| 224 | + ) |
| 225 | + result = self.plugin._resolve_genres( |
| 226 | + ["metal", "rock", "jazz"], artist="the artist" |
| 227 | + ) |
| 228 | + assert "metal" not in result, ( |
| 229 | + "artist-specific ignored genre must be removed" |
| 230 | + ) |
| 231 | + assert "rock" not in result, "globally ignored genre must be removed" |
| 232 | + assert "jazz" in result, "non-ignored genre must survive" |
| 233 | + |
| 234 | + def test_ignorelist_stops_c14n_ancestry_walk(self): |
| 235 | + """An ignored tag's c14n parents don't bleed into the result. |
| 236 | +
|
| 237 | + Without ignorelist, 'delta blues' canonicalizes to 'blues'. |
| 238 | + With 'delta blues' ignored the tag is skipped entirely in the |
| 239 | + c14n loop, so 'blues' must not appear either. |
| 240 | + """ |
| 241 | + self._setup_config(whitelist=False, canonical=True, count=99) |
| 242 | + self.plugin.ignore_patterns = defaultdict( |
| 243 | + list, |
| 244 | + { |
| 245 | + "the artist": [re.compile(r"^delta blues$", re.IGNORECASE)], |
| 246 | + }, |
| 247 | + ) |
| 248 | + result = self.plugin._resolve_genres( |
| 249 | + ["delta blues"], artist="the artist" |
| 250 | + ) |
| 251 | + assert result == [], ( |
| 252 | + "ignored tag must not contribute c14n parents to the result" |
| 253 | + ) |
| 254 | + |
| 255 | + def test_ignorelist_c14n_no_whitelist_keeps_oldest_ancestor(self): |
| 256 | + """With c14n on and whitelist off, ignorelist must not change the |
| 257 | + parent-selection rule: only the oldest ancestor is returned. |
| 258 | + """ |
| 259 | + self._setup_config(whitelist=False, canonical=True, count=99) |
| 260 | + # ignorelist targets an unrelated genre — must not affect parent walking |
| 261 | + self.plugin.ignore_patterns = defaultdict( |
| 262 | + list, |
| 263 | + {"*": [re.compile(r"^jazz$", re.IGNORECASE)]}, |
| 264 | + ) |
| 265 | + result = self.plugin._resolve_genres(["delta blues"]) |
| 266 | + assert result == ["blues"], ( |
| 267 | + "oldest ancestor only must be returned, not the full parent chain" |
| 268 | + ) |
| 269 | + |
| 270 | + def test_ignorelist_c14n_no_whitelist_drops_ignored_ancestor(self): |
| 271 | + """With c14n on and whitelist off, if the oldest ancestor itself is |
| 272 | + ignored it must be dropped and the tag contributes nothing. |
| 273 | + """ |
| 274 | + self._setup_config(whitelist=False, canonical=True, count=99) |
| 275 | + self.plugin.ignore_patterns = defaultdict( |
| 276 | + list, |
| 277 | + {"*": [re.compile(r"^blues$", re.IGNORECASE)]}, |
| 278 | + ) |
| 279 | + result = self.plugin._resolve_genres(["delta blues"]) |
| 280 | + assert result == [], ( |
| 281 | + "ignored oldest ancestor must not appear in the result" |
| 282 | + ) |
| 283 | + |
205 | 284 |
|
206 | 285 | @pytest.fixture |
207 | 286 | def config(config): |
@@ -636,3 +715,224 @@ def mock_fetch_artist_genre(self, artist): |
636 | 715 |
|
637 | 716 | # Run |
638 | 717 | assert plugin._get_genre(item) == expected_result |
| 718 | + |
| 719 | + |
| 720 | +class TestIgnorelist: |
| 721 | + """Ignorelist pattern matching tests independent of resolve_genres.""" |
| 722 | + |
| 723 | + @pytest.mark.parametrize( |
| 724 | + "ignorelist_dict, artist, genre, expected_forbidden", |
| 725 | + [ |
| 726 | + # Global ignorelist - simple word |
| 727 | + ({"*": ["spoken word"]}, "Any Artist", "spoken word", True), |
| 728 | + ({"*": ["spoken word"]}, "Any Artist", "jazz", False), |
| 729 | + # Global ignorelist - regex pattern |
| 730 | + ( |
| 731 | + {"*": [".*electronic.*"]}, |
| 732 | + "Any Artist", |
| 733 | + "ambient electronic", |
| 734 | + True, |
| 735 | + ), |
| 736 | + ({"*": [".*electronic.*"]}, "Any Artist", "jazz", False), |
| 737 | + # Artist-specific ignorelist |
| 738 | + ({"metallica": ["metal"]}, "Metallica", "metal", True), |
| 739 | + ({"metallica": ["metal"]}, "Iron Maiden", "metal", False), |
| 740 | + # Case insensitive matching |
| 741 | + ({"metallica": ["metal"]}, "METALLICA", "METAL", True), |
| 742 | + # Full-match behavior: plain "metal" must not match "heavy metal" |
| 743 | + ({"metallica": ["metal"]}, "Metallica", "heavy metal", False), |
| 744 | + # Regex behavior: explicit pattern ".*metal.*" may match "heavy metal" |
| 745 | + ({"metallica": [".*metal.*"]}, "Metallica", "heavy metal", True), |
| 746 | + # Artist-specific ignorelist - exact match |
| 747 | + ( |
| 748 | + {"metallica": ["^Heavy Metal$"]}, |
| 749 | + "Metallica", |
| 750 | + "classic metal", |
| 751 | + False, |
| 752 | + ), |
| 753 | + # Combined global and artist-specific |
| 754 | + ( |
| 755 | + {"*": ["spoken word"], "metallica": ["metal"]}, |
| 756 | + "Metallica", |
| 757 | + "spoken word", |
| 758 | + True, |
| 759 | + ), |
| 760 | + ( |
| 761 | + {"*": ["spoken word"], "metallica": ["metal"]}, |
| 762 | + "Metallica", |
| 763 | + "metal", |
| 764 | + True, |
| 765 | + ), |
| 766 | + # Complex regex pattern with multiple features (raw string) |
| 767 | + ( |
| 768 | + { |
| 769 | + "fracture": [ |
| 770 | + r"^(heavy|black|power|death)?\s?(metal|rock)$|\w+-metal\d*$" |
| 771 | + ] |
| 772 | + }, |
| 773 | + "Fracture", |
| 774 | + "power metal", |
| 775 | + True, |
| 776 | + ), |
| 777 | + # Complex regex pattern with multiple features (regular string) |
| 778 | + ( |
| 779 | + {"amon tobin": ["d(rum)?[ n/]*b(ass)?"]}, |
| 780 | + "Amon Tobin", |
| 781 | + "dnb", |
| 782 | + True, |
| 783 | + ), |
| 784 | + # Empty ignorelist |
| 785 | + ({}, "Any Artist", "any genre", False), |
| 786 | + ], |
| 787 | + ) |
| 788 | + def test_ignorelist_patterns( |
| 789 | + self, ignorelist_dict, artist, genre, expected_forbidden |
| 790 | + ): |
| 791 | + """Test ignorelist pattern matching logic directly.""" |
| 792 | + |
| 793 | + logger = Mock() |
| 794 | + |
| 795 | + # Set up compiled ignorelist directly (skipping file parsing) |
| 796 | + compiled_ignorelist = defaultdict(list) |
| 797 | + for artist_name, patterns in ignorelist_dict.items(): |
| 798 | + compiled_ignorelist[artist_name.lower()] = [ |
| 799 | + re.compile(pattern, re.IGNORECASE) for pattern in patterns |
| 800 | + ] |
| 801 | + |
| 802 | + result = is_ignored(logger, compiled_ignorelist, genre, artist) |
| 803 | + assert result == expected_forbidden |
| 804 | + |
| 805 | + @pytest.mark.parametrize( |
| 806 | + "ignorelist_config, expected_ignorelist", |
| 807 | + [ |
| 808 | + # Basic artist with single pattern |
| 809 | + ({"metallica": ["metal"]}, {"metallica": ["metal"]}), |
| 810 | + # Global ignorelist with '*' key |
| 811 | + ({"*": ["spoken word"]}, {"*": ["spoken word"]}), |
| 812 | + # Multiple patterns per artist |
| 813 | + ( |
| 814 | + {"metallica": ["metal", ".*rock.*"]}, |
| 815 | + {"metallica": ["metal", ".*rock.*"]}, |
| 816 | + ), |
| 817 | + # Combined global and artist-specific |
| 818 | + ( |
| 819 | + {"*": ["spoken word"], "metallica": ["metal"]}, |
| 820 | + {"*": ["spoken word"], "metallica": ["metal"]}, |
| 821 | + ), |
| 822 | + # Artist names are preserved by the current loader implementation. |
| 823 | + ({"METALLICA": ["METAL"]}, {"METALLICA": ["METAL"]}), |
| 824 | + # Invalid regex pattern that gets escaped (full-match literal fallback) |
| 825 | + ( |
| 826 | + {"artist": ["[invalid(regex"]}, |
| 827 | + {"artist": ["\\[invalid\\(regex"]}, |
| 828 | + ), |
| 829 | + # Disabled via False / empty dict — both produce empty dict |
| 830 | + (False, {}), |
| 831 | + ({}, {}), |
| 832 | + ], |
| 833 | + ) |
| 834 | + def test_ignorelist_config_format( |
| 835 | + self, ignorelist_config, expected_ignorelist |
| 836 | + ): |
| 837 | + """Test ignorelist parsing/compilation with isolated config state.""" |
| 838 | + cfg = confuse.Configuration("test", read=False) |
| 839 | + cfg.set({"lastgenre": {"ignorelist": ignorelist_config}}) |
| 840 | + |
| 841 | + # Mimic the plugin loader behavior in isolation to avoid global config bleed. |
| 842 | + if not cfg["lastgenre"]["ignorelist"].get(): |
| 843 | + string_ignorelist = {} |
| 844 | + else: |
| 845 | + raw_strs = cfg["lastgenre"]["ignorelist"].get( |
| 846 | + confuse.MappingValues(confuse.Sequence(str)) |
| 847 | + ) |
| 848 | + string_ignorelist = {} |
| 849 | + for artist, patterns in raw_strs.items(): |
| 850 | + compiled_patterns = [] |
| 851 | + for pattern in patterns: |
| 852 | + try: |
| 853 | + compiled_patterns.append( |
| 854 | + re.compile(pattern, re.IGNORECASE).pattern |
| 855 | + ) |
| 856 | + except re.error: |
| 857 | + compiled_patterns.append( |
| 858 | + re.compile( |
| 859 | + re.escape(pattern), re.IGNORECASE |
| 860 | + ).pattern |
| 861 | + ) |
| 862 | + string_ignorelist[artist] = compiled_patterns |
| 863 | + |
| 864 | + assert string_ignorelist == expected_ignorelist |
| 865 | + |
| 866 | + @pytest.mark.parametrize( |
| 867 | + "invalid_config, expected_error_message", |
| 868 | + [ |
| 869 | + # A plain string (e.g. accidental file path) instead of a mapping |
| 870 | + ( |
| 871 | + "/path/to/ignorelist.txt", |
| 872 | + "must be a dict", |
| 873 | + ), |
| 874 | + # An integer instead of a mapping |
| 875 | + ( |
| 876 | + 42, |
| 877 | + "must be a dict", |
| 878 | + ), |
| 879 | + # A list of strings instead of a mapping |
| 880 | + ( |
| 881 | + ["spoken word", "comedy"], |
| 882 | + "must be a dict", |
| 883 | + ), |
| 884 | + # A mapping with a non-list value |
| 885 | + ( |
| 886 | + {"metallica": "metal"}, |
| 887 | + "must be a list", |
| 888 | + ), |
| 889 | + ], |
| 890 | + ) |
| 891 | + def test_ignorelist_config_format_errors( |
| 892 | + self, invalid_config, expected_error_message |
| 893 | + ): |
| 894 | + """Test ignorelist config validation errors in isolated config.""" |
| 895 | + cfg = confuse.Configuration("test", read=False) |
| 896 | + cfg.set({"lastgenre": {"ignorelist": invalid_config}}) |
| 897 | + |
| 898 | + with pytest.raises(confuse.ConfigTypeError) as exc_info: |
| 899 | + _ = cfg["lastgenre"]["ignorelist"].get( |
| 900 | + confuse.MappingValues(confuse.Sequence(str)) |
| 901 | + ) |
| 902 | + |
| 903 | + assert expected_error_message in str(exc_info.value) |
| 904 | + |
| 905 | + def test_ignorelist_multivalued_album_artist_fallback(self, config): |
| 906 | + """`stage_artist=None` fallback must not re-drop per-artist results.""" |
| 907 | + config["lastgenre"]["ignorelist"] = { |
| 908 | + "Artist A": ["Metal"], |
| 909 | + "Artist Group": ["Metal"], |
| 910 | + } |
| 911 | + config["lastgenre"]["whitelist"] = False |
| 912 | + config["lastgenre"]["count"] = 10 |
| 913 | + |
| 914 | + plugin = lastgenre.LastGenrePlugin() |
| 915 | + plugin.setup() |
| 916 | + |
| 917 | + obj = MagicMock(spec=Album) |
| 918 | + obj.albumartist = "Artist Group" |
| 919 | + obj.album = "Album Title" |
| 920 | + obj.albumartists = ["Artist A", "Artist B"] |
| 921 | + obj.get.return_value = [] |
| 922 | + |
| 923 | + plugin.client = MagicMock() |
| 924 | + plugin.client.fetch_track_genre.return_value = [] |
| 925 | + plugin.client.fetch_album_genre.return_value = [] |
| 926 | + |
| 927 | + artist_genres = { |
| 928 | + "Artist A": ["Rock"], |
| 929 | + "Artist B": ["Metal", "Jazz"], |
| 930 | + } |
| 931 | + plugin.client.fetch_artist_genre.side_effect = lambda artist: ( |
| 932 | + artist_genres.get(artist, []) |
| 933 | + ) |
| 934 | + |
| 935 | + genres, label = plugin._get_genre(obj) |
| 936 | + |
| 937 | + assert "multi-valued album artist" in label |
| 938 | + assert "Metal" in genres |
0 commit comments