Improve flags structure and add tests

Author: snejus

beetsplug/lyrics.py

@@ -42,7 +42,7 @@
 
 if TYPE_CHECKING:
     from beets.importer import ImportTask
-    from beets.library import Item
+    from beets.library import Item, Library
 
     from ._typing import (
         GeniusAPI,
@@ -936,7 +936,6 @@ def translator(self) -> Translator | None:
 
     def __init__(self):
         super().__init__()
-        self.import_stages = [self.imported]
         self.config.add(
             {
                 "auto": True,
@@ -955,6 +954,7 @@ def __init__(self):
                 "fallback": None,
                 "force": False,
                 "local": False,
+                "print": False,
                 "synced": False,
                 # Musixmatch is disabled by default as they are currently blocking
                 # requests with the beets user agent.
@@ -968,14 +968,16 @@ def __init__(self):
         self.config["google_engine_ID"].redact = True
         self.config["genius_api_key"].redact = True
 
+        if self.config["auto"]:
+            self.import_stages = [self.imported]
+
     def commands(self):
         cmd = ui.Subcommand("lyrics", help="fetch song lyrics")
         cmd.parser.add_option(
             "-p",
             "--print",
-            dest="printlyr",
             action="store_true",
-            default=False,
+            default=self.config["print"].get(),
             help="print lyrics to console",
         )
         cmd.parser.add_option(
@@ -990,34 +992,27 @@ def commands(self):
         cmd.parser.add_option(
             "-f",
             "--force",
-            dest="force_refetch",
             action="store_true",
-            default=False,
+            default=self.config["force"].get(),
             help="always re-download lyrics",
         )
         cmd.parser.add_option(
             "-l",
             "--local",
-            dest="local_only",
             action="store_true",
-            default=False,
+            default=self.config["local"].get(),
             help="do not fetch missing lyrics",
         )
 
-        def func(lib, opts, args):
+        def func(lib: Library, opts, args) -> None:
             # The "write to files" option corresponds to the
             # import_write config value.
-            items = list(lib.items(ui.decargs(args)))
+            self.config.set(vars(opts))
+            items = list(lib.items(args))
             for item in items:
-                if not opts.local_only and not self.config["local"]:
-                    self.fetch_item_lyrics(
-                        item,
-                        ui.should_write(),
-                        opts.force_refetch or self.config["force"],
-                    )
-                if item.lyrics:
-                    if opts.printlyr:
-                        ui.print_(item.lyrics)
+                self.add_item_lyrics(item, ui.should_write())
+                if item.lyrics and opts.print:
+                    ui.print_(item.lyrics)
 
             if opts.rest_directory and (
                 items := [i for i in items if i.lyrics]
@@ -1029,32 +1024,34 @@ def func(lib, opts, args):
 
     def imported(self, _, task: ImportTask) -> None:
         """Import hook for fetching lyrics automatically."""
-        if self.config["auto"]:
-            for item in task.imported_items():
-                self.fetch_item_lyrics(item, False, self.config["force"])
+        for item in task.imported_items():
+            self.add_item_lyrics(item, False)
+
+    def find_lyrics(self, item: Item) -> str:
+        album, length = item.album, round(item.length)
+        matches = (
+            [
+                lyrics
+                for t in titles
+                if (lyrics := self.get_lyrics(a, t, album, length))
+            ]
+            for a, titles in search_pairs(item)
+        )
 
-    def fetch_item_lyrics(self, item: Item, write: bool, force: bool) -> None:
+        return "\n\n---\n\n".join(next(filter(None, matches), []))
+
+    def add_item_lyrics(self, item: Item, write: bool) -> None:
         """Fetch and store lyrics for a single item. If ``write``, then the
         lyrics will also be written to the file itself.
         """
-        # Skip if the item already has lyrics.
-        if not force and item.lyrics:
-            self.info("🔵 Lyrics already present: {}", item)
+        if self.config["local"]:
             return
 
-        lyrics_matches = []
-        album, length = item.album, round(item.length)
-        for artist, titles in search_pairs(item):
-            lyrics_matches = [
-                self.get_lyrics(artist, title, album, length)
-                for title in titles
-            ]
-            if any(lyrics_matches):
-                break
-
-        lyrics = "\n\n---\n\n".join(filter(None, lyrics_matches))
+        if not self.config["force"] and item.lyrics:
+            self.info("🔵 Lyrics already present: {}", item)
+            return
 
-        if lyrics:
+        if lyrics := self.find_lyrics(item):
             self.info("🟢 Found lyrics: {0}", item)
             if translator := self.translator:
                 initial_lyrics = lyrics

docs/plugins/lyrics.rst

@@ -47,6 +47,7 @@ Default configuration:
         force: no
         google_API_key: null
         google_engine_ID: 009217259823014548361:lndtuqkycfu
+        print: no
         sources: [lrclib, google, genius, tekstowo]
         synced: no
 
@@ -74,6 +75,7 @@ The available options are:
 - **google_engine_ID**: The custom search engine to use.
   Default: The `beets custom search engine`_, which gathers an updated list of
   sources known to be scrapeable.
+- **print**: Print lyrics to the console.
 - **sources**: List of sources to search for lyrics. An asterisk ``*`` expands
   to all available sources. The ``google`` source will be automatically
   deactivated if no ``google_API_key`` is setup.

test/plugins/test_lyrics.py

@@ -23,7 +23,7 @@
 import pytest
 
 from beets.library import Item
-from beets.test.helper import PluginMixin
+from beets.test.helper import PluginMixin, TestHelper
 from beetsplug import lyrics
 
 from .lyrics_pages import LyricsPage, lyrics_pages
@@ -35,6 +35,14 @@
 }
 
 
+@pytest.fixture(scope="module")
+def helper():
+    helper = TestHelper()
+    helper.setup_beets()
+    yield helper
+    helper.teardown_beets()
+
+
 class TestLyricsUtils:
     @pytest.mark.parametrize(
         "artist, title",
@@ -232,6 +240,27 @@ def test_error_handling(
         assert last_log
         assert re.search(expected_log_match, last_log, re.I)
 
+    @pytest.mark.parametrize(
+        "plugin_config, found, expected",
+        [
+            ({}, "new", "old"),
+            ({"force": True}, "new", "new"),
+            ({"force": True, "local": True}, "new", "old"),
+            ({"force": True, "fallback": None}, "", "old"),
+            ({"force": True, "fallback": ""}, "", ""),
+            ({"force": True, "fallback": "default"}, "", "default"),
+        ],
+    )
+    def test_overwrite_config(
+        self, monkeypatch, helper, lyrics_plugin, found, expected
+    ):
+        monkeypatch.setattr(lyrics_plugin, "find_lyrics", lambda _: found)
+        item = helper.create_item(id=1, lyrics="old")
+
+        lyrics_plugin.add_item_lyrics(item, False)
+
+        assert item.lyrics == expected
+
 
 class LyricsBackendTest(LyricsPluginMixin):
     @pytest.fixture
@@ -281,8 +310,13 @@ def _patch_google_search(self, requests_mock, lyrics_page):
 
     def test_backend_source(self, lyrics_plugin, lyrics_page: LyricsPage):
         """Test parsed lyrics from each of the configured lyrics pages."""
-        lyrics_info = lyrics_plugin.get_lyrics(
-            lyrics_page.artist, lyrics_page.track_title, "", 186
+        lyrics_info = lyrics_plugin.find_lyrics(
+            Item(
+                artist=lyrics_page.artist,
+                title=lyrics_page.track_title,
+                album="",
+                length=186.0,
+            )
         )
 
         assert lyrics_info