diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 18c77b2d27..7efb1220e3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -37,7 +37,15 @@ jobs: poetry install --extras=replaygain --extras=reflink - name: Install Python dependencies - run: poetry install --only=main,test --extras=autobpm + run: poetry install --only=main,test --extras=autobpm --extras=lyrics + + - name: Get changed lyrics files + id: lyrics-update + uses: tj-actions/changed-files@v45 + with: + files: | + beetsplug/lyrics.py + test/plugins/test_lyrics.py - if: ${{ env.IS_MAIN_PYTHON != 'true' }} name: Test without coverage @@ -45,6 +53,8 @@ jobs: - if: ${{ env.IS_MAIN_PYTHON == 'true' }} name: Test with coverage + env: + LYRICS_UPDATED: ${{ steps.lyrics-update.outputs.any_changed }} uses: liskin/gh-problem-matcher-wrap@v3 with: linters: pytest diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index ee4edbdcb7..e52d57d47e 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -382,7 +382,7 @@ to get a basic view on how tests are written. Since we are currently migrating the tests from `unittest`_ to `pytest`_, new tests should be written using `pytest`_. Contributions migrating existing tests are welcome! -External API requests under test should be mocked with `requests_mock`_, +External API requests under test should be mocked with `requests-mock`_, However, we still want to know whether external APIs are up and that they return expected responses, therefore we test them weekly with our `integration test`_ suite. diff --git a/beets/test/helper.py b/beets/test/helper.py index 124063d766..4effa47f82 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -48,7 +48,7 @@ import beets import beets.plugins -from beets import autotag, config, importer, logging, util +from beets import autotag, importer, logging, util from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.importer import ImportSession from beets.library import Album, Item, Library @@ -153,12 +153,27 @@ def check_reflink_support(path: str) -> bool: return reflink.supported_at(path) +class ConfigMixin: + @cached_property + def config(self) -> beets.IncludeLazyConfig: + """Base beets configuration for tests.""" + config = beets.config + config.sources = [] + config.read(user=False, defaults=True) + + config["plugins"] = [] + config["verbose"] = 1 + config["ui"]["color"] = False + config["threaded"] = False + return config + + NEEDS_REFLINK = unittest.skipUnless( check_reflink_support(gettempdir()), "no reflink support for libdir" ) -class TestHelper(_common.Assertions): +class TestHelper(_common.Assertions, ConfigMixin): """Helper mixin for high-level cli and plugin tests. This mixin provides methods to isolate beets' global state provide @@ -184,8 +199,6 @@ def setup_beets(self): - ``libdir`` Path to a subfolder of ``temp_dir``, containing the library's media files. Same as ``config['directory']``. - - ``config`` The global configuration used by beets. - - ``lib`` Library instance created with the settings from ``config``. @@ -202,15 +215,6 @@ def setup_beets(self): ) self.env_patcher.start() - self.config = beets.config - self.config.sources = [] - self.config.read(user=False, defaults=True) - - self.config["plugins"] = [] - self.config["verbose"] = 1 - self.config["ui"]["color"] = False - self.config["threaded"] = False - self.libdir = os.path.join(self.temp_dir, b"libdir") os.mkdir(syspath(self.libdir)) self.config["directory"] = os.fsdecode(self.libdir) @@ -229,8 +233,6 @@ def teardown_beets(self): self.io.restore() self.lib._close() self.remove_temp_dir() - beets.config.clear() - beets.config._materialized = False # Library fixtures methods @@ -452,7 +454,7 @@ def setUp(self): self.i = _common.item(self.lib) -class PluginMixin: +class PluginMixin(ConfigMixin): plugin: ClassVar[str] preload_plugin: ClassVar[bool] = True @@ -473,7 +475,7 @@ def load_plugins(self, *plugins: str) -> None: """ # FIXME this should eventually be handled by a plugin manager plugins = (self.plugin,) if hasattr(self, "plugin") else plugins - beets.config["plugins"] = plugins + self.config["plugins"] = plugins beets.plugins.load_plugins(plugins) beets.plugins.find_plugins() @@ -494,7 +496,7 @@ def unload_plugins(self) -> None: # FIXME this should eventually be handled by a plugin manager for plugin_class in beets.plugins._instances: plugin_class.listeners = None - beets.config["plugins"] = [] + self.config["plugins"] = [] beets.plugins._classes = set() beets.plugins._instances = {} Item._types = getattr(Item, "_original_types", {}) @@ -504,7 +506,7 @@ def unload_plugins(self) -> None: @contextmanager def configure_plugin(self, config: Any): - beets.config[self.plugin].set(config) + self.config[self.plugin].set(config) self.load_plugins(self.plugin) yield @@ -624,7 +626,7 @@ def _get_import_session(self, import_dir: bytes) -> ImportSession: def setup_importer( self, import_dir: bytes | None = None, **kwargs ) -> ImportSession: - config["import"].set_args({**self.default_import_config, **kwargs}) + self.config["import"].set_args({**self.default_import_config, **kwargs}) self.importer = self._get_import_session(import_dir or self.import_dir) return self.importer diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 899bf39921..3c99bfb8a4 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -26,12 +26,19 @@ import unicodedata import warnings from functools import partial -from typing import ClassVar +from typing import TYPE_CHECKING, ClassVar from urllib.parse import quote, urlencode import requests from unidecode import unidecode +import beets +from beets import plugins, ui + +if TYPE_CHECKING: + from beets.importer import ImportTask + from beets.library import Item + try: import bs4 from bs4 import SoupStrainer @@ -47,10 +54,6 @@ except ImportError: HAS_LANGDETECT = False - -import beets -from beets import plugins, ui - DIV_RE = re.compile(r"<(/?)div>?", re.I) COMMENT_RE = re.compile(r"", re.S) TAG_RE = re.compile(r"<[^>]*>") @@ -152,7 +155,13 @@ def generate_alternatives(string, patterns): alternatives.append(match.group(1)) return alternatives - title, artist, artist_sort = item.title, item.artist, item.artist_sort + title, artist, artist_sort = ( + item.title.strip(), + item.artist.strip(), + item.artist_sort.strip(), + ) + if not title or not artist: + return () patterns = [ # Remove any featuring artists from the artists name @@ -161,7 +170,7 @@ def generate_alternatives(string, patterns): artists = generate_alternatives(artist, patterns) # Use the artist_sort as fallback only if it differs from artist to avoid # repeated remote requests with the same search terms - if artist != artist_sort: + if artist_sort and artist.lower() != artist_sort.lower(): artists.append(artist_sort) patterns = [ @@ -222,7 +231,7 @@ def __init__(self, config, log): self._log = log self.config = config - def fetch_url(self, url): + def fetch_url(self, url, **kwargs): """Retrieve the content at a given URL, or return None if the source is unreachable. """ @@ -240,6 +249,7 @@ def fetch_url(self, url): "User-Agent": USER_AGENT, }, timeout=10, + **kwargs, ) except requests.RequestException as exc: self._log.debug("lyrics request failed: {0}", exc) @@ -250,20 +260,27 @@ def fetch_url(self, url): self._log.debug("failed to fetch: {0} ({1})", url, r.status_code) return None - def fetch(self, artist, title, album=None, length=None): - raise NotImplementedError() + def fetch( + self, artist: str, title: str, album: str, length: int + ) -> str | None: + raise NotImplementedError class LRCLib(Backend): base_url = "https://lrclib.net/api/get" - def fetch(self, artist, title, album=None, length=None): - params = { + def fetch( + self, artist: str, title: str, album: str, length: int + ) -> str | None: + params: dict[str, str | int] = { "artist_name": artist, "track_name": title, - "album_name": album, - "duration": length, } + if album: + params["album_name"] = album + + if length: + params["duration"] = length try: response = requests.get( @@ -316,7 +333,7 @@ def encode(cls, text: str) -> str: return quote(unidecode(text)) - def fetch(self, artist, title, album=None, length=None): + def fetch(self, artist: str, title: str, *_) -> str | None: url = self.build_url(artist, title) html = self.fetch_url(url) @@ -364,7 +381,7 @@ def __init__(self, config, log): "User-Agent": USER_AGENT, } - def fetch(self, artist, title, album=None, length=None): + def fetch(self, artist: str, title: str, *_) -> str | None: """Fetch lyrics from genius.com Because genius doesn't allow accessing lyrics via the api, @@ -495,7 +512,7 @@ class Tekstowo(DirectBackend): def encode(cls, text: str) -> str: return cls.non_alpha_to_underscore(unidecode(text.lower())) - def fetch(self, artist, title, album=None, length=None): + def fetch(self, artist: str, title: str, *_) -> str | None: if html := self.fetch_url(self.build_url(artist, title)): return self.extract_lyrics(html) @@ -536,6 +553,8 @@ def _scrape_strip_cruft(html, plain_text_out=False): html = BREAK_RE.sub("\n", html) #
eats up surrounding '\n'. html = re.sub(r"(?s)<(script).*?", "", html) # Strip script tags. html = re.sub("\u2005", " ", html) # replace unicode with regular space + html = re.sub("