From 2681c83c5b23f9f4f2bb4941013b07a88dc33e6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 16 Oct 2024 06:37:53 +0100 Subject: [PATCH 1/4] Remove redundant generate_album_info and generate_track_info functions These functions were used to generate mock data for tests but have been replaced with direct instantiation of AlbumInfo and TrackInfo objects. This change simplifies the test code and removes unnecessary helper functions. --- beets/test/helper.py | 79 --------------- test/plugins/test_mbsync.py | 190 +++++++++--------------------------- 2 files changed, 45 insertions(+), 224 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 19f7299ed..322921fb5 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -20,9 +20,6 @@ information or mock the environment. - `has_program` checks the presence of a command on the system. -- The `generate_album_info` and `generate_track_info` functions return - fixtures to be used when mocking the autotagger. - - The `ImportSessionFixture` allows one to run importer code while controlling the interactions through code. @@ -803,82 +800,6 @@ class TerminalImportMixin(ImportHelper): ) -def generate_album_info(album_id, track_values): - """Return `AlbumInfo` populated with mock data. - - Sets the album info's `album_id` field is set to the corresponding - argument. For each pair (`id`, `values`) in `track_values` the `TrackInfo` - from `generate_track_info` is added to the album info's `tracks` field. - Most other fields of the album and track info are set to "album - info" and "track info", respectively. - """ - tracks = [generate_track_info(id, values) for id, values in track_values] - album = AlbumInfo( - album_id="album info", - album="album info", - artist="album info", - artist_id="album info", - tracks=tracks, - ) - for field in ALBUM_INFO_FIELDS: - setattr(album, field, "album info") - - return album - - -ALBUM_INFO_FIELDS = [ - "album", - "album_id", - "artist", - "artist_id", - "asin", - "albumtype", - "va", - "label", - "barcode", - "artist_sort", - "releasegroup_id", - "catalognum", - "language", - "country", - "albumstatus", - "media", - "albumdisambig", - "releasegroupdisambig", - "artist_credit", - "data_source", - "data_url", -] - - -def generate_track_info(track_id="track info", values={}): - """Return `TrackInfo` populated with mock data. - - The `track_id` field is set to the corresponding argument. All other - string fields are set to "track info". - """ - track = TrackInfo( - title="track info", - track_id=track_id, - ) - for field in TRACK_INFO_FIELDS: - setattr(track, field, "track info") - for field, value in values.items(): - setattr(track, field, value) - return track - - -TRACK_INFO_FIELDS = [ - "artist", - "artist_id", - "artist_sort", - "disctitle", - "artist_credit", - "data_source", - "data_url", -] - - class AutotagStub: """Stub out MusicBrainz album and track matcher and control what the autotagger returns. diff --git a/test/plugins/test_mbsync.py b/test/plugins/test_mbsync.py index 6cfa6704e..f65df4256 100644 --- a/test/plugins/test_mbsync.py +++ b/test/plugins/test_mbsync.py @@ -12,17 +12,11 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. - from unittest.mock import patch -from beets import config +from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.library import Item -from beets.test.helper import ( - PluginTestCase, - capture_log, - generate_album_info, - generate_track_info, -) +from beets.test.helper import PluginTestCase, capture_log class MbsyncCliTest(PluginTestCase): @@ -31,28 +25,28 @@ class MbsyncCliTest(PluginTestCase): @patch("beets.autotag.mb.album_for_id") @patch("beets.autotag.mb.track_for_id") def test_update_library(self, track_for_id, album_for_id): - album_for_id.return_value = generate_album_info( - "album id", [("track id", {"release_track_id": "release track id"})] - ) - track_for_id.return_value = generate_track_info( - "singleton track id", {"title": "singleton info"} - ) - album_item = Item( - album="old title", + album="old album", mb_albumid="81ae60d4-5b75-38df-903a-db2cfa51c2c6", - mb_trackid="old track id", - mb_releasetrackid="release track id", - path="", + mb_trackid="track id", ) - album = self.lib.add_album([album_item]) + self.lib.add_album([album_item]) - item = Item( - title="old title", - mb_trackid="b8c2cf90-83f9-3b5f-8ccd-31fb866fcf37", - path="", + singleton = Item( + title="old title", mb_trackid="b8c2cf90-83f9-3b5f-8ccd-31fb866fcf37" + ) + self.lib.add(singleton) + + album_for_id.return_value = AlbumInfo( + album_id="album id", + album="new album", + tracks=[ + TrackInfo(track_id=album_item.mb_trackid, title="new title") + ], + ) + track_for_id.return_value = TrackInfo( + track_id=singleton.mb_trackid, title="new title" ) - self.lib.add(item) with capture_log() as logs: self.run_command("mbsync") @@ -60,130 +54,36 @@ class MbsyncCliTest(PluginTestCase): assert "Sending event: albuminfo_received" in logs assert "Sending event: trackinfo_received" in logs - item.load() - assert item.title == "singleton info" + singleton.load() + assert singleton.title == "new title" album_item.load() - assert album_item.title == "track info" + assert album_item.title == "new title" assert album_item.mb_trackid == "track id" + assert album_item.get_album().album == "new album" - album.load() - assert album.album == "album info" + def test_custom_format(self): + for item in [ + Item(artist="albumartist", album="no id"), + Item( + artist="albumartist", + album="invalid id", + mb_albumid="a1b2c3d4", + ), + ]: + self.lib.add_album([item]) - def test_message_when_skipping(self): - config["format_item"] = "$artist - $album - $title" - config["format_album"] = "$albumartist - $album" + for item in [ + Item(artist="artist", title="no id"), + Item(artist="artist", title="invalid id", mb_trackid="a1b2c3d4"), + ]: + self.lib.add(item) - # Test album with no mb_albumid. - # The default format for an album include $albumartist so - # set that here, too. - album_invalid = Item( - albumartist="album info", album="album info", path="" - ) - self.lib.add_album([album_invalid]) - - # default format with capture_log("beets.mbsync") as logs: - self.run_command("mbsync") - e = ( - "mbsync: Skipping album with no mb_albumid: " - + "album info - album info" - ) - assert e == logs[0] - - # custom format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync", "-f", "'$album'") - e = "mbsync: Skipping album with no mb_albumid: 'album info'" - assert e == logs[0] - - # restore the config - config["format_item"] = "$artist - $album - $title" - config["format_album"] = "$albumartist - $album" - - # Test singleton with no mb_trackid. - # The default singleton format includes $artist and $album - # so we need to stub them here - item_invalid = Item( - artist="album info", - album="album info", - title="old title", - path="", - ) - self.lib.add(item_invalid) - - # default format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync") - e = ( - "mbsync: Skipping singleton with no mb_trackid: " - + "album info - album info - old title" - ) - assert e == logs[0] - - # custom format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync", "-f", "'$title'") - e = "mbsync: Skipping singleton with no mb_trackid: 'old title'" - assert e == logs[0] - - def test_message_when_invalid(self): - config["format_item"] = "$artist - $album - $title" - config["format_album"] = "$albumartist - $album" - - # Test album with invalid mb_albumid. - # The default format for an album include $albumartist so - # set that here, too. - album_invalid = Item( - albumartist="album info", - album="album info", - mb_albumid="a1b2c3d4", - path="", - ) - self.lib.add_album([album_invalid]) - - # default format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync") - e = ( - "mbsync: Skipping album with invalid mb_albumid: " - + "album info - album info" - ) - assert e == logs[0] - - # custom format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync", "-f", "'$album'") - e = "mbsync: Skipping album with invalid mb_albumid: 'album info'" - assert e == logs[0] - - # restore the config - config["format_item"] = "$artist - $album - $title" - config["format_album"] = "$albumartist - $album" - - # Test singleton with invalid mb_trackid. - # The default singleton format includes $artist and $album - # so we need to stub them here - item_invalid = Item( - artist="album info", - album="album info", - title="old title", - mb_trackid="a1b2c3d4", - path="", - ) - self.lib.add(item_invalid) - - # default format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync") - e = ( - "mbsync: Skipping singleton with invalid mb_trackid: " - + "album info - album info - old title" - ) - assert e == logs[0] - - # custom format - with capture_log("beets.mbsync") as logs: - self.run_command("mbsync", "-f", "'$title'") - e = "mbsync: Skipping singleton with invalid mb_trackid: 'old title'" - assert e == logs[0] + self.run_command("mbsync", "-f", "'%if{$album,$album,$title}'") + assert set(logs) == { + "mbsync: Skipping album with no mb_albumid: 'no id'", + "mbsync: Skipping album with invalid mb_albumid: 'invalid id'", + "mbsync: Skipping singleton with no mb_trackid: 'no id'", + "mbsync: Skipping singleton with invalid mb_trackid: 'invalid id'", + } From d1611113424737a976dcbfafe401b193f0140032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 16 Oct 2024 12:33:17 +0100 Subject: [PATCH 2/4] test helpers: remove redundant _get_item_count It always returns 1. --- beets/test/helper.py | 11 ++--------- test/plugins/test_aura.py | 1 + 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 322921fb5..b919e1c2c 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -246,16 +246,15 @@ class TestHelper(_common.Assertions): The item is attached to the database from `self.lib`. """ - item_count = self._get_item_count() values_ = { "title": "t\u00eftle {0}", "artist": "the \u00e4rtist", "album": "the \u00e4lbum", - "track": item_count, + "track": 1, "format": "MP3", } values_.update(values) - values_["title"] = values_["title"].format(item_count) + values_["title"] = values_["title"].format(1) values_["db"] = self.lib item = Item(**values_) if "path" not in values: @@ -372,12 +371,6 @@ class TestHelper(_common.Assertions): return path - def _get_item_count(self): - if not hasattr(self, "__item_count"): - count = 0 - self.__item_count = count + 1 - return count - # Running beets commands def run_command(self, *args, **kwargs): diff --git a/test/plugins/test_aura.py b/test/plugins/test_aura.py index c0a76b1c5..25a730cab 100644 --- a/test/plugins/test_aura.py +++ b/test/plugins/test_aura.py @@ -91,6 +91,7 @@ class TestAuraResponse: "artist": item.artist, "size": Path(os.fsdecode(item.path)).stat().st_size, "title": item.title, + "track": 1, }, "relationships": { "albums": {"data": [{"id": str(album.id), "type": "album"}]}, From 41907a96a6ef5e9fb78f88857761222482a41a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 16 Oct 2024 12:37:22 +0100 Subject: [PATCH 3/4] Remove _common.album and use Album instead --- beets/test/_common.py | 24 ---------------- test/test_library.py | 4 +-- test/test_sort.py | 65 +++++++++++++++++++++++-------------------- 3 files changed, 37 insertions(+), 56 deletions(-) diff --git a/beets/test/_common.py b/beets/test/_common.py index b6e7c5dcc..7fa843b63 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -110,30 +110,6 @@ def item(lib=None): return i -def album(lib=None): - global _item_ident - _item_ident += 1 - i = beets.library.Album( - artpath=None, - albumartist="some album artist", - albumartist_sort="some sort album artist", - albumartist_credit="some album artist credit", - album="the album", - genre="the genre", - year=2014, - month=2, - day=5, - tracktotal=0, - disctotal=1, - comp=False, - mb_albumid="someID-1", - mb_albumartistid="someID-1", - ) - if lib: - lib.add(i) - return i - - # Dummy import session. def import_session(lib=None, loghandler=None, paths=[], query=[], cli=False): cls = commands.TerminalImportSession if cli else importer.ImportSession diff --git a/test/test_library.py b/test/test_library.py index c48044210..b5e6d4eeb 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -30,6 +30,7 @@ from mediafile import MediaFile, UnreadableFileError import beets.dbcore.query import beets.library from beets import config, plugins, util +from beets.library import Album from beets.test import _common from beets.test._common import item from beets.test.helper import BeetsTestCase, ItemInDBTestCase @@ -81,8 +82,7 @@ class StoreTest(ItemInDBTestCase): assert "composer" not in self.i._dirty def test_store_album_cascades_flex_deletes(self): - album = _common.album() - album.flex1 = "Flex-1" + album = Album(flex1="Flex-1") self.lib.add(album) item = _common.item() item.album_id = album.id diff --git a/test/test_sort.py b/test/test_sort.py index f212bd654..1fc8a0463 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -16,6 +16,7 @@ import beets.library from beets import config, dbcore +from beets.library import Album from beets.test import _common from beets.test.helper import BeetsTestCase @@ -26,28 +27,32 @@ class DummyDataTestCase(BeetsTestCase): def setUp(self): super().setUp() - albums = [_common.album() for _ in range(3)] - albums[0].album = "Album A" - albums[0].genre = "Rock" - albums[0].year = 2001 - albums[0].flex1 = "Flex1-1" - albums[0].flex2 = "Flex2-A" - albums[0].albumartist = "Foo" - albums[0].albumartist_sort = None - albums[1].album = "Album B" - albums[1].genre = "Rock" - albums[1].year = 2001 - albums[1].flex1 = "Flex1-2" - albums[1].flex2 = "Flex2-A" - albums[1].albumartist = "Bar" - albums[1].albumartist_sort = None - albums[2].album = "Album C" - albums[2].genre = "Jazz" - albums[2].year = 2005 - albums[2].flex1 = "Flex1-1" - albums[2].flex2 = "Flex2-B" - albums[2].albumartist = "Baz" - albums[2].albumartist_sort = None + albums = [ + Album( + album="Album A", + genre="Rock", + year=2001, + flex1="Flex1-1", + flex2="Flex2-A", + albumartist="Foo", + ), + Album( + album="Album B", + genre="Rock", + year=2001, + flex1="Flex1-2", + flex2="Flex2-A", + albumartist="Bar", + ), + Album( + album="Album C", + genre="Jazz", + year=2005, + flex1="Flex1-1", + flex2="Flex2-B", + albumartist="Baz", + ), + ] for album in albums: self.lib.add(album) @@ -378,14 +383,14 @@ class CaseSensitivityTest(DummyDataTestCase, BeetsTestCase): def setUp(self): super().setUp() - album = _common.album() - album.album = "album" - album.genre = "alternative" - album.year = "2001" - album.flex1 = "flex1" - album.flex2 = "flex2-A" - album.albumartist = "bar" - album.albumartist_sort = None + album = Album( + album="album", + genre="alternative", + year="2001", + flex1="flex1", + flex2="flex2-A", + albumartist="bar", + ) self.lib.add(album) item = _common.item() From 6f41872a26c70431d10dd050d16a390aab302fb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 16 Oct 2024 12:38:21 +0100 Subject: [PATCH 4/4] Remove unused logic from beets.test and exclude it from coverage Fun fact: it was the coverage data that revealed that this logic is not used. --- beets/test/_common.py | 6 ------ beets/test/helper.py | 8 +------- setup.cfg | 1 + 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/beets/test/_common.py b/beets/test/_common.py index 7fa843b63..757d461bd 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -58,17 +58,12 @@ log = logging.getLogger("beets") log.propagate = True log.setLevel(logging.DEBUG) -# Dummy item creation. -_item_ident = 0 - # OS feature test. HAVE_SYMLINK = sys.platform != "win32" HAVE_HARDLINK = sys.platform != "win32" def item(lib=None): - global _item_ident - _item_ident += 1 i = beets.library.Item( title="the title", artist="the artist", @@ -93,7 +88,6 @@ def item(lib=None): comments="the comments", bpm=8, comp=True, - path=f"somepath{_item_ident}", length=60.0, bitrate=128000, format="FLAC", diff --git a/beets/test/helper.py b/beets/test/helper.py index b919e1c2c..bba99dde6 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -713,10 +713,6 @@ class ImportSessionFixture(ImportSession): default_resolution = "REMOVE" - def add_resolution(self, resolution): - assert isinstance(resolution, self.Resolution) - self._resolutions.append(resolution) - def resolve_duplicate(self, task, found_duplicates): try: res = self._resolutions.pop(0) @@ -769,12 +765,10 @@ class TerminalImportSessionFixture(TerminalImportSession): self.io.addinput("T") elif choice == importer.action.SKIP: self.io.addinput("S") - elif isinstance(choice, int): + else: self.io.addinput("M") self.io.addinput(str(choice)) self._add_choice_input() - else: - raise Exception("Unknown choice %s" % choice) class TerminalImportMixin(ImportHelper): diff --git a/setup.cfg b/setup.cfg index f69490630..8cf0dc3d0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -14,6 +14,7 @@ markers = data_file = .reports/coverage/data branch = true relative_files = true +omit = beets/test/* [coverage:report] precision = 2