From 69cdd58240b7e4c3d20f62b011b74e282c478792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 16 May 2026 07:41:49 +0100 Subject: [PATCH 1/9] discogs: remove redundant check for required fields --- beetsplug/discogs/__init__.py | 14 -------------- test/plugins/test_discogs.py | 11 ----------- 2 files changed, 25 deletions(-) diff --git a/beetsplug/discogs/__init__.py b/beetsplug/discogs/__init__.py index 0a5584a27..4a4f4a85c 100644 --- a/beetsplug/discogs/__init__.py +++ b/beetsplug/discogs/__init__.py @@ -345,20 +345,6 @@ class DiscogsPlugin(SearchApiMetadataSourcePlugin[IDResponse]): ) return None - # Sanity check for required fields. The list of required fields is - # defined at Guideline 1.3.1.a, but in practice some releases might be - # lacking some of these fields. This function expects at least: - # `artists` (>0), `title`, `id`, `tracklist` (>0) - # https://www.discogs.com/help/doc/submission-guidelines-general-rules - if not all( - [ - result.data.get(k) - for k in ["artists", "title", "id", "tracklist"] - ] - ): - self._log.warning("Release does not contain the required fields") - return None - artist_data = [a.data for a in result.artists] # Information for the album artist albumartist = ArtistState.from_config( diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 25c0763e6..a8df02c7e 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -329,17 +329,6 @@ class TestDGAlbumInfo(TestHelper): assert d.album == "TITLE" assert len(d.tracks) == 1 - def test_parse_release_without_required_fields(self, caplog): - """Test parsing of a release that does not have the required fields.""" - release = Bag(data={}, refresh=lambda *args: None) - with caplog.at_level("DEBUG"): - d = DiscogsPlugin().get_album_info(release) - - assert d is None - assert ( - "Release does not contain the required fields" in caplog.messages[0] - ) - def test_default_genre_style_settings(self): """Test genre default settings, genres to genre, styles to style""" release = self._make_release_from_positions(["1", "2"]) From a126a3aaa3d14b913f58ed4946d94e7dbe35173b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 1 Jul 2026 18:48:45 +0100 Subject: [PATCH 2/9] discogs: Use Release instead of Bag in tests --- beets/test/_common.py | 13 ------- test/plugins/test_discogs.py | 70 ++++++++++++------------------------ 2 files changed, 22 insertions(+), 61 deletions(-) diff --git a/beets/test/_common.py b/beets/test/_common.py index e01bf20c3..8afe3b8cf 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -167,19 +167,6 @@ def touch(path): open(syspath(path), "a").close() -class Bag: - """An object that exposes a set of fields given as keyword - arguments. Any field not found in the dictionary appears to be None. - Used for mocking Album objects and the like. - """ - - def __init__(self, **fields): - self.fields = fields - - def __getattr__(self, key): - return self.fields.get(key) - - # Platform mocking. diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index a8df02c7e..3dfaa8fd3 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -1,12 +1,13 @@ """Tests for discogs plugin.""" +from typing import Any from unittest.mock import Mock, patch import pytest +from discogs_client import Client, Release from beets import config from beets.library import Item -from beets.test._common import Bag from beets.test.helper import TestHelper from beetsplug.discogs import ArtistState, DiscogsPlugin @@ -23,17 +24,23 @@ def _artist(name: str, **kwargs): } | kwargs +def get_release(data: dict[str, Any]) -> Release: + return Release(Client("doesn't matter"), data) + + @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) class TestDGAlbumInfo(TestHelper): def _make_release(self, tracks=None): - """Returns a Bag that mimics a discogs_client.Release. The list - of elements on the returned Bag is incomplete, including just - those required for the tests on this class.""" + """Return discogs_client.Release. + + The returned object is incomplete, including just the fields required + for tests in this module. + """ data = { - "id": "ALBUM ID", - "uri": "https://www.discogs.com/release/release/13633721", + "id": 11111111, + "uri": "https://www.discogs.com/release/111111111", "title": "ALBUM TITLE", - "year": "3001", + "year": 3001, "artists": [_artist("ARTIST NAME", join=",")], "formats": [ { @@ -46,20 +53,10 @@ class TestDGAlbumInfo(TestHelper): "genres": ["STYLE1", "STYLE2"], "styles": ["GENRE1", "GENRE2"], "labels": [{"name": "LABEL NAME", "catno": "CATALOG NUMBER"}], - "tracklist": [], + "tracklist": tracks or [], } - if tracks: - for recording in tracks: - data["tracklist"].append(recording) - - return Bag( - data=data, - # Make some fields available as properties, as they are - # accessed by DiscogsPlugin methods. - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) + return get_release(data) def _make_track(self, title, position="", duration="", type_=None): return { @@ -70,8 +67,7 @@ class TestDGAlbumInfo(TestHelper): } def _make_release_from_positions(self, positions): - """Return a Bag that mimics a discogs_client.Release with a - tracklist where tracks have the specified `positions`.""" + """Return discogs_client.Release with tracks at the given positions.""" tracks = [ self._make_track(f"TITLE{i}", position) for (i, position) in enumerate(positions, start=1) @@ -319,11 +315,7 @@ class TestDGAlbumInfo(TestHelper): "artists": [_artist("ARTIST NAME", id=321)], "title": "TITLE", } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) + release = get_release(data) d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME" assert d.album == "TITLE" @@ -377,12 +369,7 @@ class TestDGAlbumInfo(TestHelper): "title": "title", "labels": [{"name": "LABEL NAME (5)", "catno": "catalog number"}], } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) - d = DiscogsPlugin().get_album_info(release) + d = DiscogsPlugin().get_album_info(get_release(data)) assert d.artist == "ARTIST NAME & OTHER ARTIST" assert d.artists == ["ARTIST NAME", "OTHER ARTIST"] assert d.artists_ids == ["321", "322"] @@ -414,12 +401,7 @@ class TestDGAlbumInfo(TestHelper): "title": "title", "labels": [{"name": "LABEL NAME (5)", "catno": "catalog number"}], } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) - d = DiscogsPlugin().get_album_info(release) + d = DiscogsPlugin().get_album_info(get_release(data)) assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" assert d.artists == ["ARTIST NAME (2)", "OTHER ARTIST (5)"] assert d.tracks[0].artist == "TEST ARTIST (5)" @@ -495,11 +477,7 @@ class TestAnv: ], "title": "title", } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) + release = get_release(data) plugin = DiscogsPlugin() plugin.config["anv"].set( {"artist": False, "album_artist": False, "artist_credit": False} @@ -609,11 +587,7 @@ def test_anv_album_artist(): "artists": [_artist("ARTIST (4)", id=321, anv="VARIATION")], "title": "title", } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) + release = get_release(data) config["discogs"]["anv"]["album_artist"] = False config["discogs"]["anv"]["artist"] = True config["discogs"]["anv"]["artist_credit"] = False From 951af6d3e8478e47eca7093edb144c5577c82edb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 1 Jul 2026 20:26:22 +0100 Subject: [PATCH 3/9] discogs: Use a single global fixture to kill DiscogsPlugin.setup --- test/plugins/test_discogs.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 3dfaa8fd3..15b316f55 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -1,7 +1,6 @@ """Tests for discogs plugin.""" from typing import Any -from unittest.mock import Mock, patch import pytest from discogs_client import Client, Release @@ -28,7 +27,14 @@ def get_release(data: dict[str, Any]) -> Release: return Release(Client("doesn't matter"), data) -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +@pytest.fixture(autouse=True) +def _patch_discogs_setup(monkeypatch): + """Autouse fixture to patch DiscogsPlugin.setup for each test.""" + monkeypatch.setattr( + "beetsplug.discogs.DiscogsPlugin.setup", lambda *_: None + ) + + class TestDGAlbumInfo(TestHelper): def _make_release(self, tracks=None): """Return discogs_client.Release. @@ -410,7 +416,6 @@ class TestDGAlbumInfo(TestHelper): config["discogs"]["strip_disambiguation"] = True -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) class TestDGSearchQuery(TestHelper): def test_default_search_filters_without_extra_tags(self): """Discogs search uses only the type filter when no extra_tags are set.""" @@ -568,7 +573,6 @@ class TestAnv: self._assert_fields(album_info, expected_album_fields) -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_anv_album_artist(): """Test using artist name variations when the album artist is the same as the track artist, but only the track artist @@ -603,7 +607,6 @@ def test_anv_album_artist(): assert r.tracks[0].artists_credit == ["ARTIST"] -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_parse_featured_artists(): """Tests the plugins ability to parse a featured artist. Ignores artists that are not listed as featured.""" @@ -641,7 +644,6 @@ def test_parse_featured_artists(): assert t.composers == ["RANDOM"] -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_parse_extraartist_roles(): plugin = DiscogsPlugin() artistinfo = ArtistState.from_config(plugin.config, [_artist("ARTIST")]) @@ -692,7 +694,6 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): assert result == (expected_media, expected_albumtype) -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_va_buildartistinfo(): config["va_name"] = "VARIOUS ARTISTS" expected_info = { From 1343d613d3435dc0a4ab1b0e2f651f953028a96e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 1 Jul 2026 20:45:56 +0100 Subject: [PATCH 4/9] discogs: Move test_parse_tracklist_positions... under TestTracklist --- test/plugins/test_discogs.py | 118 +++++++++++------------------------ 1 file changed, 35 insertions(+), 83 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 15b316f55..d7a4180d5 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -10,6 +10,8 @@ from beets.library import Item from beets.test.helper import TestHelper from beetsplug.discogs import ArtistState, DiscogsPlugin +_p = pytest.param + def _artist(name: str, **kwargs): return { @@ -35,7 +37,7 @@ def _patch_discogs_setup(monkeypatch): ) -class TestDGAlbumInfo(TestHelper): +class DiscogsTestMixin: def _make_release(self, tracks=None): """Return discogs_client.Release. @@ -80,6 +82,8 @@ class TestDGAlbumInfo(TestHelper): ] return self._make_release(tracks) + +class TestDGAlbumInfo(DiscogsTestMixin, TestHelper): def test_parse_media_for_tracks(self): tracks = [ self._make_track("TITLE ONE", "1", "01:01"), @@ -164,88 +168,6 @@ class TestDGAlbumInfo(TestHelper): assert t[3].index == 4 assert t[3].medium_total == 1 - def test_parse_tracklist_without_sides(self): - """Test standard Discogs position 12.2.9#1: "without sides".""" - release = self._make_release_from_positions(["1", "2", "3"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 - assert len(d.tracks) == 3 - - def test_parse_tracklist_with_sides(self): - """Test standard Discogs position 12.2.9#2: "with sides".""" - release = self._make_release_from_positions(["A1", "A2", "B1", "B2"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 # 2 sides = 1 LP - assert len(d.tracks) == 4 - - def test_parse_tracklist_multiple_lp(self): - """Test standard Discogs position 12.2.9#3: "multiple LP".""" - release = self._make_release_from_positions(["A1", "A2", "B1", "C1"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 2 # 3 sides = 1 LP + 1 LP - assert len(d.tracks) == 4 - - def test_parse_tracklist_multiple_cd(self): - """Test standard Discogs position 12.2.9#4: "multiple CDs".""" - release = self._make_release_from_positions( - ["1-1", "1-2", "2-1", "3-1"] - ) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 3 - assert len(d.tracks) == 4 - - def test_parse_tracklist_non_standard(self): - """Test non standard Discogs position.""" - release = self._make_release_from_positions(["I", "II", "III", "IV"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 - assert len(d.tracks) == 4 - - def test_parse_tracklist_subtracks_dot(self): - """Test standard Discogs position 12.2.9#5: "sub tracks, dots".""" - release = self._make_release_from_positions(["1", "2.1", "2.2", "3"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 - assert len(d.tracks) == 3 - - release = self._make_release_from_positions( - ["A1", "A2.1", "A2.2", "A3"] - ) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 - assert len(d.tracks) == 3 - - def test_parse_tracklist_subtracks_letter(self): - """Test standard Discogs position 12.2.9#5: "sub tracks, letter".""" - release = self._make_release_from_positions(["A1", "A2a", "A2b", "A3"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 - assert len(d.tracks) == 3 - - release = self._make_release_from_positions( - ["A1", "A2.a", "A2.b", "A3"] - ) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 1 - assert len(d.tracks) == 3 - - def test_parse_tracklist_subtracks_extra_material(self): - """Test standard Discogs position 12.2.9#6: "extra material".""" - release = self._make_release_from_positions(["1", "2", "Video 1"]) - d = DiscogsPlugin().get_album_info(release) - - assert d.mediums == 2 - assert len(d.tracks) == 3 - def test_parse_tracklist_subtracks_indices(self): """Test parsing of subtracks that include index tracks.""" release = self._make_release_from_positions(["", "", "1.1", "1.2"]) @@ -416,6 +338,36 @@ class TestDGAlbumInfo(TestHelper): config["discogs"]["strip_disambiguation"] = True +class TestTracklist(DiscogsTestMixin): + @pytest.fixture + def plugin(self): + return DiscogsPlugin() + + @pytest.mark.parametrize( + "positions,expected_mediums,expected_tracks", + [ + _p(["1", "2", "3"], 1, 3, id="without-sides"), + _p(["A1", "A2", "B1", "B2"], 1, 4, id="with-sides"), + _p(["A1", "A2", "B1", "C1"], 2, 4, id="multiple-lp"), + _p(["1-1", "1-2", "2-1", "3-1"], 3, 4, id="multiple-cd"), + _p(["I", "II", "III", "IV"], 1, 4, id="non-standard"), + _p(["1", "2.1", "2.2", "3"], 1, 3, id="subtracks-dot"), + _p(["A1", "A2.1", "A2.2", "A3"], 1, 3, id="subtracks-dot-with-sides"), + _p(["A1", "A2a", "A2b", "A3"], 1, 3, id="subtracks-letter"), + _p(["A1", "A2.a", "A2.b", "A3"], 1, 3, id="subtracks-letter-with-dot"), + _p(["1", "2", "Video 1"], 2, 3, id="subtracks-extra-material"), + ], + ) # fmt: skip + def test_parse_tracklist_positions( + self, plugin, positions, expected_mediums, expected_tracks + ): + release = self._make_release_from_positions(positions) + d = plugin.get_album_info(release) + + assert d.mediums == expected_mediums + assert len(d.tracks) == expected_tracks + + class TestDGSearchQuery(TestHelper): def test_default_search_filters_without_extra_tags(self): """Discogs search uses only the type filter when no extra_tags are set.""" From bc0591fdb61a930c8f2d1e8b3e4f73c409bfa005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 2 Jul 2026 00:01:21 +0100 Subject: [PATCH 5/9] discogs: Consolidate test_parse_tracklist_subtracks... tests --- test/plugins/test_discogs.py | 173 +++++++++++++++++++---------------- 1 file changed, 95 insertions(+), 78 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index d7a4180d5..56169ba41 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -25,6 +25,15 @@ def _artist(name: str, **kwargs): } | kwargs +def _track(title: str, position: str = "", duration: str = "", **kwargs): + return { + "title": title, + "position": position, + "duration": duration, + "type_": "track", + } | kwargs + + def get_release(data: dict[str, Any]) -> Release: return Release(Client("doesn't matter"), data) @@ -66,18 +75,10 @@ class DiscogsTestMixin: return get_release(data) - def _make_track(self, title, position="", duration="", type_=None): - return { - "title": title, - "position": position, - "duration": duration, - "type_": type_, - } - def _make_release_from_positions(self, positions): """Return discogs_client.Release with tracks at the given positions.""" tracks = [ - self._make_track(f"TITLE{i}", position) + _track(f"TITLE{i}", position) for (i, position) in enumerate(positions, start=1) ] return self._make_release(tracks) @@ -86,8 +87,8 @@ class DiscogsTestMixin: class TestDGAlbumInfo(DiscogsTestMixin, TestHelper): def test_parse_media_for_tracks(self): tracks = [ - self._make_track("TITLE ONE", "1", "01:01"), - self._make_track("TITLE TWO", "2", "02:02"), + _track("TITLE ONE", "1", "01:01"), + _track("TITLE TWO", "2", "02:02"), ] release = self._make_release(tracks=tracks) @@ -168,78 +169,12 @@ class TestDGAlbumInfo(DiscogsTestMixin, TestHelper): assert t[3].index == 4 assert t[3].medium_total == 1 - def test_parse_tracklist_subtracks_indices(self): - """Test parsing of subtracks that include index tracks.""" - release = self._make_release_from_positions(["", "", "1.1", "1.2"]) - # Track 1: Index track with medium title - release.data["tracklist"][0]["title"] = "MEDIUM TITLE" - # Track 2: Index track with track group title - release.data["tracklist"][1]["title"] = "TRACK GROUP TITLE" - - d = DiscogsPlugin().get_album_info(release) - assert d.mediums == 1 - assert d.tracks[0].disctitle == "MEDIUM TITLE" - assert len(d.tracks) == 1 - assert d.tracks[0].title == "TRACK GROUP TITLE" - - def test_parse_tracklist_subtracks_nested_logical(self): - """Test parsing of subtracks defined inside a index track that are - logical subtracks (ie. should be grouped together into a single track). - """ - release = self._make_release_from_positions(["1", "", "3"]) - # Track 2: Index track with track group title, and sub_tracks - release.data["tracklist"][1]["title"] = "TRACK GROUP TITLE" - release.data["tracklist"][1]["sub_tracks"] = [ - self._make_track("TITLE ONE", "2.1", "01:01"), - self._make_track("TITLE TWO", "2.2", "02:02"), - ] - - d = DiscogsPlugin().get_album_info(release) - assert d.mediums == 1 - assert len(d.tracks) == 3 - assert d.tracks[1].title == "TRACK GROUP TITLE" - - def test_parse_tracklist_subtracks_nested_physical(self): - """Test parsing of subtracks defined inside a index track that are - physical subtracks (ie. should not be grouped together). - """ - release = self._make_release_from_positions(["1", "", "4"]) - # Track 2: Index track with track group title, and sub_tracks - release.data["tracklist"][1]["title"] = "TRACK GROUP TITLE" - release.data["tracklist"][1]["sub_tracks"] = [ - self._make_track("TITLE ONE", "2", "01:01"), - self._make_track("TITLE TWO", "3", "02:02"), - ] - - d = DiscogsPlugin().get_album_info(release) - assert d.mediums == 1 - assert len(d.tracks) == 4 - assert d.tracks[1].title == "TITLE ONE" - assert d.tracks[2].title == "TITLE TWO" - - def test_parse_tracklist_disctitles(self): - """Test parsing of index tracks that act as disc titles.""" - release = self._make_release_from_positions( - ["", "1-1", "1-2", "", "2-1"] - ) - # Track 1: Index track with medium title (Cd1) - release.data["tracklist"][0]["title"] = "MEDIUM TITLE CD1" - # Track 4: Index track with medium title (Cd2) - release.data["tracklist"][3]["title"] = "MEDIUM TITLE CD2" - - d = DiscogsPlugin().get_album_info(release) - assert d.mediums == 2 - assert d.tracks[0].disctitle == "MEDIUM TITLE CD1" - assert d.tracks[1].disctitle == "MEDIUM TITLE CD1" - assert d.tracks[2].disctitle == "MEDIUM TITLE CD2" - assert len(d.tracks) == 3 - def test_parse_minimal_release(self): """Test parsing of a release with the minimal amount of information.""" data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [self._make_track("A", "1", "01:01")], + "tracklist": [_track("A", "1", "01:01")], "artists": [_artist("ARTIST NAME", id=321)], "title": "TITLE", } @@ -367,6 +302,88 @@ class TestTracklist(DiscogsTestMixin): assert d.mediums == expected_mediums assert len(d.tracks) == expected_tracks + @pytest.mark.parametrize( + "tracks,expected_mediums,expected_tracks", + [ + _p( + [ + _track("MEDIUM TITLE"), + _track("TRACK GROUP TITLE"), + _track("TITLE ONE", "1.1"), + _track("TITLE TWO", "1.2"), + ], + 1, + [("TRACK GROUP TITLE", "MEDIUM TITLE")], + id="flat-logical-subtracks", + ), + _p( + [ + _track("TITLE ONE", "1"), + _track( + "TRACK GROUP TITLE", + sub_tracks=[ + _track("SUBTITLE ONE", "2.1", "01:01"), + _track("SUBTITLE TWO", "2.2", "02:02"), + ], + ), + _track("TITLE THREE", "3"), + ], + 1, + [ + ("TITLE ONE", None), + ("TRACK GROUP TITLE", None), + ("TITLE THREE", None), + ], + id="nested-logical-subtracks", + ), + _p( + [ + _track("TITLE ONE", "1"), + _track( + "TRACK GROUP TITLE", + sub_tracks=[ + _track("SUBTITLE ONE", "2", "01:01"), + _track("SUBTITLE TWO", "3", "02:02"), + ], + ), + _track("TITLE FOUR", "4"), + ], + 1, + [ + ("TITLE ONE", None), + ("SUBTITLE ONE", None), + ("SUBTITLE TWO", None), + ("TITLE FOUR", None), + ], + id="nested-physical-subtracks", + ), + _p( + [ + _track("MEDIUM TITLE CD1"), + _track("TITLE ONE", "1-1"), + _track("TITLE TWO", "1-2"), + _track("MEDIUM TITLE CD2"), + _track("TITLE THREE", "2-1"), + ], + 2, + [ + ("TITLE ONE", "MEDIUM TITLE CD1"), + ("TITLE TWO", "MEDIUM TITLE CD1"), + ("TITLE THREE", "MEDIUM TITLE CD2"), + ], + id="medium-titles", + ), + ], + ) # fmt: skip + def test_parse_tracklist_index_tracks( + self, plugin, tracks, expected_mediums, expected_tracks + ): + release = self._make_release(tracks) + album = plugin.get_album_info(release) + + assert album.mediums == expected_mediums + assert [(t.title, t.disctitle) for t in album.tracks] == expected_tracks + class TestDGSearchQuery(TestHelper): def test_default_search_filters_without_extra_tags(self): From b8f43e994a55cd73a6e1c9a092401a3c155363c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 2 Jul 2026 01:02:02 +0100 Subject: [PATCH 6/9] discogs: Test inherited track group artist --- .git-blame-ignore-revs | 2 ++ test/plugins/test_discogs.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 3cbe1703a..0e75c53da 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -237,3 +237,5 @@ ca8e9f69a8a0254aa767dc86f6d808d02f6af05d b1701c2e12b2de8e5462affe46ecc6816a5fc508 # typing: type beets.dbcore.types ea3d6af6169d33be009b86bf5607baae7cf9652a +# Fix #6177, remove derived types, refactor coalesce tracks +9efe87101ce03706660129633e03147a222765cf diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 56169ba41..d212c7388 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -384,6 +384,33 @@ class TestTracklist(DiscogsTestMixin): assert album.mediums == expected_mediums assert [(t.title, t.disctitle) for t in album.tracks] == expected_tracks + def test_parse_tracklist_inherited_artists(self, plugin): + """Verify grouped tracks combine explicit and inherited artist credits. + + This covers releases where a track group provides the default artist for + sub-tracks that do not declare one themselves. + + Note: this is based on the following release: + https://www.discogs.com/release/3647530 + """ + track_artist = "TRACK ARTIST" + group_artist = "GROUP ARTIST" + tracks = [ + _track( + "TRACK GROUP TITLE", + artists=[_artist(group_artist)], + sub_tracks=[ + _track("TRACK ONE", "2", artists=[_artist(track_artist)]), + _track("SUBTITLE TWO", "3"), + ], + ) + ] + release = self._make_release(tracks) + album = plugin.get_album_info(release) + + assert album.mediums == 1 + assert {t.artist for t in album.tracks} == {track_artist, group_artist} + class TestDGSearchQuery(TestHelper): def test_default_search_filters_without_extra_tags(self): From bd9c753b998c75d5a3daaf10863d7c6e35c83aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 2 Jul 2026 01:09:15 +0100 Subject: [PATCH 7/9] discogs: Test index_tracks behaviour --- test/plugins/test_discogs.py | 38 +++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index d212c7388..47fe15723 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -275,8 +275,14 @@ class TestDGAlbumInfo(DiscogsTestMixin, TestHelper): class TestTracklist(DiscogsTestMixin): @pytest.fixture - def plugin(self): - return DiscogsPlugin() + def plugin_config(self): + return {} + + @pytest.fixture + def plugin(self, plugin_config): + plugin = DiscogsPlugin() + plugin.config.set(plugin_config) + return plugin @pytest.mark.parametrize( "positions,expected_mediums,expected_tracks", @@ -303,9 +309,10 @@ class TestTracklist(DiscogsTestMixin): assert len(d.tracks) == expected_tracks @pytest.mark.parametrize( - "tracks,expected_mediums,expected_tracks", + "plugin_config,tracks,expected_mediums,expected_tracks", [ _p( + {"index_tracks": False}, [ _track("MEDIUM TITLE"), _track("TRACK GROUP TITLE"), @@ -317,6 +324,7 @@ class TestTracklist(DiscogsTestMixin): id="flat-logical-subtracks", ), _p( + {"index_tracks": False}, [ _track("TITLE ONE", "1"), _track( @@ -337,6 +345,7 @@ class TestTracklist(DiscogsTestMixin): id="nested-logical-subtracks", ), _p( + {"index_tracks": False}, [ _track("TITLE ONE", "1"), _track( @@ -358,6 +367,29 @@ class TestTracklist(DiscogsTestMixin): id="nested-physical-subtracks", ), _p( + {"index_tracks": True}, + [ + _track("TITLE ONE", "1"), + _track( + "TRACK GROUP TITLE", + sub_tracks=[ + _track("SUBTITLE ONE", "2", "01:01"), + _track("SUBTITLE TWO", "3", "02:02"), + ], + ), + _track("TITLE FOUR", "4"), + ], + 1, + [ + ("TITLE ONE", None), + ("TRACK GROUP TITLE: SUBTITLE ONE", None), + ("TRACK GROUP TITLE: SUBTITLE TWO", None), + ("TITLE FOUR", None), + ], + id="nested-physical-subtracks-with-index-tracks", + ), + _p( + {"index_tracks": False}, [ _track("MEDIUM TITLE CD1"), _track("TITLE ONE", "1-1"), From bbb1b5c1efdfff41e20e7422fd408e454f7d71a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 2 Jul 2026 01:10:26 +0100 Subject: [PATCH 8/9] discogs: And now add full _coalesce_tracks coverage --- test/plugins/test_discogs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 47fe15723..d8c8ce592 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -296,6 +296,7 @@ class TestTracklist(DiscogsTestMixin): _p(["A1", "A2.1", "A2.2", "A3"], 1, 3, id="subtracks-dot-with-sides"), _p(["A1", "A2a", "A2b", "A3"], 1, 3, id="subtracks-letter"), _p(["A1", "A2.a", "A2.b", "A3"], 1, 3, id="subtracks-letter-with-dot"), + _p(["1.1", "1.2", "2.1", "2.2"], 1, 2, id="multiple-subtrack-groups"), _p(["1", "2", "Video 1"], 2, 3, id="subtracks-extra-material"), ], ) # fmt: skip From c46f6a549befb152aa3f0ede8d21d014d93bbeb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 2 Jul 2026 02:50:09 +0100 Subject: [PATCH 9/9] discogs: Dedupe test_strip_disambiguation --- test/plugins/test_discogs.py | 85 +++++++++++++++--------------------- 1 file changed, 35 insertions(+), 50 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index d8c8ce592..aa7533885 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -47,6 +47,16 @@ def _patch_discogs_setup(monkeypatch): class DiscogsTestMixin: + @pytest.fixture + def plugin_config(self): + return {} + + @pytest.fixture + def plugin(self, plugin_config): + plugin = DiscogsPlugin() + plugin.config.set(plugin_config) + return plugin + def _make_release(self, tracks=None): """Return discogs_client.Release. @@ -211,40 +221,10 @@ class TestDGAlbumInfo(DiscogsTestMixin, TestHelper): assert d.style is None assert d.genres == ["GENRE1", "GENRE2"] - def test_strip_disambiguation(self): - """Test removing disambiguation from all disambiguated fields.""" - data = { - "id": 123, - "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [ - { - "title": "track", - "position": "A", - "type_": "track", - "duration": "5:44", - "artists": [_artist("TEST ARTIST (5)", id=11146)], - } - ], - "artists": [ - _artist("ARTIST NAME (2)", id=321, join="&"), - _artist("OTHER ARTIST (5)", id=322), - ], - "title": "title", - "labels": [{"name": "LABEL NAME (5)", "catno": "catalog number"}], - } - d = DiscogsPlugin().get_album_info(get_release(data)) - assert d.artist == "ARTIST NAME & OTHER ARTIST" - assert d.artists == ["ARTIST NAME", "OTHER ARTIST"] - assert d.artists_ids == ["321", "322"] - assert d.tracks[0].artist == "TEST ARTIST" - assert d.tracks[0].artists == ["TEST ARTIST"] - assert d.tracks[0].artist_id == "11146" - assert d.tracks[0].artists_ids == ["11146"] - assert d.label == "LABEL NAME" - def test_strip_disambiguation_false(self): - """Test disabling disambiguation removal from all disambiguated fields.""" - config["discogs"]["strip_disambiguation"] = False +class TestStripDisambiguation(DiscogsTestMixin): + @pytest.fixture + def album_info(self, plugin): data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", @@ -264,26 +244,31 @@ class TestDGAlbumInfo(DiscogsTestMixin, TestHelper): "title": "title", "labels": [{"name": "LABEL NAME (5)", "catno": "catalog number"}], } - d = DiscogsPlugin().get_album_info(get_release(data)) - assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" - assert d.artists == ["ARTIST NAME (2)", "OTHER ARTIST (5)"] - assert d.tracks[0].artist == "TEST ARTIST (5)" - assert d.tracks[0].artists == ["TEST ARTIST (5)"] - assert d.label == "LABEL NAME (5)" - config["discogs"]["strip_disambiguation"] = True + return plugin.get_album_info(get_release(data)) + + @pytest.mark.parametrize("plugin_config", [{"strip_disambiguation": True}]) + def test_strip_disambiguation(self, album_info): + """Test removing disambiguation from all disambiguated fields.""" + assert album_info.artist == "ARTIST NAME & OTHER ARTIST" + assert album_info.artists == ["ARTIST NAME", "OTHER ARTIST"] + assert album_info.artists_ids == ["321", "322"] + assert album_info.tracks[0].artist == "TEST ARTIST" + assert album_info.tracks[0].artists == ["TEST ARTIST"] + assert album_info.tracks[0].artist_id == "11146" + assert album_info.tracks[0].artists_ids == ["11146"] + assert album_info.label == "LABEL NAME" + + @pytest.mark.parametrize("plugin_config", [{"strip_disambiguation": False}]) + def test_dont_strip_disambiguation(self, album_info): + """Test disabling disambiguation removal from all disambiguated fields.""" + assert album_info.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" + assert album_info.artists == ["ARTIST NAME (2)", "OTHER ARTIST (5)"] + assert album_info.tracks[0].artist == "TEST ARTIST (5)" + assert album_info.tracks[0].artists == ["TEST ARTIST (5)"] + assert album_info.label == "LABEL NAME (5)" class TestTracklist(DiscogsTestMixin): - @pytest.fixture - def plugin_config(self): - return {} - - @pytest.fixture - def plugin(self, plugin_config): - plugin = DiscogsPlugin() - plugin.config.set(plugin_config) - return plugin - @pytest.mark.parametrize( "positions,expected_mediums,expected_tracks", [