From b99c142bd5bbd3f6809ffe08d7c64ffa5b60b442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 9 Sep 2024 16:31:04 +0100 Subject: [PATCH] lyrics: isolate test configuration Create 'helpers.ConfigMixin' which sets up testing configuration. This is helpful for tests (e.g. test_lyrics.py) that only need the configuration and do not require temp dir. (#5102) Refactor lyrics tests to fix the issue global beets config issue. Additionally, add 'integration_test' mark that can be used to mark tests that should only run once a week. --- beets/test/helper.py | 44 ++--- docs/changelog.rst | 3 + test/plugins/test_lyrics.py | 334 ++++++++++++++++-------------------- 3 files changed, 171 insertions(+), 210 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 19f7299ed8..994ef71175 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -51,7 +51,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 @@ -156,12 +156,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 @@ -187,8 +202,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``. @@ -205,15 +218,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) @@ -232,8 +236,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 @@ -462,7 +464,7 @@ def setUp(self): self.i = _common.item(self.lib) -class PluginMixin: +class PluginMixin(ConfigMixin): plugin: ClassVar[str] preload_plugin: ClassVar[bool] = True @@ -483,7 +485,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() @@ -504,7 +506,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", {}) @@ -515,10 +517,10 @@ def unload_plugins(self) -> None: @contextmanager def configure_plugin(self, config: list[Any] | dict[str, Any]): if isinstance(config, list): - beets.config[self.plugin] = config + self.config[self.plugin] = config else: for key, value in config.items(): - beets.config[self.plugin][key] = value + self.config[self.plugin][key] = value self.load_plugins(self.plugin) yield @@ -638,7 +640,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/docs/changelog.rst b/docs/changelog.rst index a9ed03e422..cc002504b7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -84,6 +84,9 @@ Other changes: calculate the bpm. Previously this import was being done immediately, so every ``beet`` invocation was being delayed by a couple of seconds. :bug:`5185` +* :doc:`plugins/lyrics`: Rewrite lyrics tests using pytest to provide isolated + configuration for each test case. + :bug:`5133` 2.0.0 (May 30, 2024) -------------------- diff --git a/test/plugins/test_lyrics.py b/test/plugins/test_lyrics.py index 1adb614638..f9eacaf240 100644 --- a/test/plugins/test_lyrics.py +++ b/test/plugins/test_lyrics.py @@ -18,14 +18,15 @@ import os import re import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest import requests +from bs4 import BeautifulSoup, SoupStrainer -from beets import logging from beets.library import Item from beets.test import _common +from beets.test.helper import PluginMixin from beets.util import bytestring_path from beetsplug import lyrics @@ -35,12 +36,7 @@ "Beets song": "via plugins, beets becomes a panacea", } -log = logging.getLogger("beets.test_lyrics") -raw_backend = lyrics.Backend({}, log) -google = lyrics.Google(MagicMock(), log) -genius = lyrics.Genius(MagicMock(), log) -tekstowo = lyrics.Tekstowo(MagicMock(), log) -lrclib = lyrics.LRCLib(MagicMock(), log) +_p = pytest.param class LyricsPluginTest(unittest.TestCase): @@ -151,19 +147,6 @@ def test_remove_credits(self): if the beat aint crackin""" assert lyrics.remove_credits(text) == text - def test_is_lyrics(self): - texts = ["LyricsMania.com - Copyright (c) 2013 - All Rights Reserved"] - texts += [ - """All material found on this site is property\n - of mywickedsongtext brand""" - ] - for t in texts: - assert not google.is_lyrics(t) - - def test_slugify(self): - text = "http://site.com/\xe7afe-au_lait(boisson)" - assert google.slugify(text) == "http://site.com/cafe_au_lait" - def test_scrape_strip_cruft(self): text = """  one @@ -185,15 +168,6 @@ def test_scrape_merge_paragraphs(self): text = "one

two

three" assert lyrics._scrape_merge_paragraphs(text) == "one\ntwo\nthree" - def test_missing_lyrics(self): - lyrics = """ -Lyricsmania staff is working hard for you to add $TITLE lyrics as soon -as they'll be released by $ARTIST, check back soon! -In case you have the lyrics to $TITLE and want to send them to us, fill out -the following form. -""" - assert not google.is_lyrics(lyrics) - def url_to_filename(url): url = re.sub(r"https?://|www.", "", url) @@ -224,17 +198,54 @@ def __call__(self, url, filename=None): LYRICS_ROOT_DIR = os.path.join(_common.RSRC, b"lyrics") -class LyricsGoogleBaseTest(unittest.TestCase): - def setUp(self): - """Set up configuration.""" - try: - __import__("bs4") - except ImportError: - self.skipTest("Beautiful Soup 4 not available") +class LyricsBackend(PluginMixin): + plugin = "lyrics" + + @pytest.fixture + def plugin_config(self): + """Return lyrics configuration to test.""" + return {} + + @pytest.fixture(autouse=True) + def _setup_config(self, backend_name, plugin_config): + """Add plugin configuration to beets configuration.""" + plugin_config["sources"] = [backend_name] + self.config[self.plugin].set(plugin_config) + + @pytest.fixture + def backend(self): + """Return a lyrics backend instance.""" + return lyrics.LyricsPlugin().backends[0] + @pytest.mark.integration_test + def test_backend_source(self, backend): + """Test default backends with a song known to exist in respective + databases. + """ + title = "Lady Madonna" + res = backend.fetch("The Beatles", title) + assert PHRASE_BY_TITLE[title] in res.lower() -@pytest.mark.integration_test -class TestSources: + +class TestGoogleLyrics(LyricsBackend): + """Test scraping heuristics on a fake html page.""" + + source = dict( + url="http://www.example.com", + artist="John Doe", + title="Beets song", + path="/lyrics/beetssong", + ) + + @pytest.fixture(scope="class") + def backend_name(self): + return "google" + + @pytest.fixture(scope="class") + def plugin_config(self): + return {"google_API_key": "test"} + + @pytest.mark.integration_test @pytest.mark.parametrize( "title, url", [ @@ -260,77 +271,40 @@ class TestSources: ), ], ) - def test_google_source(self, title, url): + def test_backend_source(self, backend, title, url): """Test if lyrics present on websites registered in beets google custom search engine are correctly scraped. """ - response = raw_backend.fetch_url(url) + response = backend.fetch_url(url) result = lyrics.scrape_lyrics_from_html(response).lower() - assert google.is_lyrics(result) + assert backend.is_lyrics(result) assert PHRASE_BY_TITLE[title] in result - @pytest.mark.parametrize( - "backend", - [ - pytest.param(lyrics.Genius, marks=skip_ci), - lyrics.Tekstowo, - lyrics.LRCLib, - # lyrics.MusiXmatch, - ], - ) - def test_backend_source(self, backend): - """Test default backends with a song known to exist in respective - databases. - """ - plugin = lyrics.LyricsPlugin() - backend = backend(plugin.config, plugin._log) - title = "Lady Madonna" - res = backend.fetch("The Beatles", title) - assert PHRASE_BY_TITLE[title] in res.lower() - - -class LyricsGooglePluginMachineryTest(LyricsGoogleBaseTest): - """Test scraping heuristics on a fake html page.""" - - source = dict( - url="http://www.example.com", - artist="John Doe", - title="Beets song", - path="/lyrics/beetssong", - ) - - def setUp(self): - """Set up configuration""" - LyricsGoogleBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - @patch.object(lyrics.Backend, "fetch_url", MockFetchUrl()) - def test_mocked_source_ok(self): + def test_mocked_source_ok(self, backend): """Test that lyrics of the mocked page are correctly scraped""" url = self.source["url"] + self.source["path"] - res = lyrics.scrape_lyrics_from_html(raw_backend.fetch_url(url)) - assert google.is_lyrics(res), url + res = lyrics.scrape_lyrics_from_html(backend.fetch_url(url)) + assert backend.is_lyrics(res), url assert PHRASE_BY_TITLE[self.source["title"]] in res.lower() @patch.object(lyrics.Backend, "fetch_url", MockFetchUrl()) - def test_is_page_candidate_exact_match(self): + def test_is_page_candidate_exact_match(self, backend): """Test matching html page title with song infos -- when song infos are present in the title. """ - from bs4 import BeautifulSoup, SoupStrainer - s = self.source url = str(s["url"] + s["path"]) - html = raw_backend.fetch_url(url) + html = backend.fetch_url(url) soup = BeautifulSoup( html, "html.parser", parse_only=SoupStrainer("title") ) - assert google.is_page_candidate( + assert backend.is_page_candidate( url, soup.title.string, s["title"], s["artist"] ), url - def test_is_page_candidate_fuzzy_match(self): + def test_is_page_candidate_fuzzy_match(self, backend): """Test matching html page title with song infos -- when song infos are not present in the title. """ @@ -339,16 +313,16 @@ def test_is_page_candidate_fuzzy_match(self): url_title = "example.com | Beats song by John doe" # very small diffs (typo) are ok eg 'beats' vs 'beets' with same artist - assert google.is_page_candidate( + assert backend.is_page_candidate( url, url_title, s["title"], s["artist"] ), url # reject different title url_title = "example.com | seets bong lyrics by John doe" - assert not google.is_page_candidate( + assert not backend.is_page_candidate( url, url_title, s["title"], s["artist"] ), url - def test_is_page_candidate_special_chars(self): + def test_is_page_candidate_special_chars(self, backend): """Ensure that `is_page_candidate` doesn't crash when the artist and such contain special regular expression characters. """ @@ -357,30 +331,45 @@ def test_is_page_candidate_special_chars(self): url = s["url"] + s["path"] url_title = "foo" - google.is_page_candidate(url, url_title, s["title"], "Sunn O)))") - + backend.is_page_candidate(url, url_title, s["title"], "Sunn O)))") -# test Genius backend + def test_is_lyrics(self, backend): + texts = ["LyricsMania.com - Copyright (c) 2013 - All Rights Reserved"] + texts += [ + """All material found on this site is property\n + of mywickedsongtext brand""" + ] + for t in texts: + assert not backend.is_lyrics(t) + def test_slugify(self, backend): + text = "http://site.com/\xe7afe-au_lait(boisson)" + assert backend.slugify(text) == "http://site.com/cafe_au_lait" -class GeniusBaseTest(unittest.TestCase): - def setUp(self): - """Set up configuration.""" - try: - __import__("bs4") - except ImportError: - self.skipTest("Beautiful Soup 4 not available") + def test_missing_lyrics(self, backend): + lyrics = """ +Lyricsmania staff is working hard for you to add $TITLE lyrics as soon +as they'll be released by $ARTIST, check back soon! +In case you have the lyrics to $TITLE and want to send them to us, fill out +the following form. +""" + assert not backend.is_lyrics(lyrics) -class GeniusScrapeLyricsFromHtmlTest(GeniusBaseTest): - """tests Genius._scrape_lyrics_from_html()""" +class TestGeniusLyrics(LyricsBackend): + @pytest.fixture(scope="class") + def backend_name(self): + return "genius" - def setUp(self): - """Set up configuration""" - GeniusBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() + @pytest.mark.xfail( + bool(os.environ.get("GITHUB_ACTIONS")), + reason="Genius returns 403 FORBIDDEN in CI", + raises=requests.HTTPError, + ) + def test_backend_source(self, backend): + super().test_backend_source(backend) - def test_no_lyrics_div(self): + def test_no_lyrics_div(self, backend): """Ensure we don't crash when the scraping the html for a genius page doesn't contain

""" @@ -388,38 +377,27 @@ def test_no_lyrics_div(self): # expected return value None url = "https://genius.com/sample" mock = MockFetchUrl() - assert genius._scrape_lyrics_from_html(mock(url)) is None + assert backend._scrape_lyrics_from_html(mock(url)) is None - def test_good_lyrics(self): + def test_good_lyrics(self, backend): """Ensure we are able to scrape a page with lyrics""" url = "https://genius.com/Ttng-chinchilla-lyrics" mock = MockFetchUrl() - lyrics = genius._scrape_lyrics_from_html(mock(url)) + lyrics = backend._scrape_lyrics_from_html(mock(url)) assert lyrics is not None assert lyrics.count("\n") == 28 - def test_good_lyrics_multiple_divs(self): + def test_good_lyrics_multiple_divs(self, backend): """Ensure we are able to scrape a page with lyrics""" url = "https://genius.com/2pac-all-eyez-on-me-lyrics" mock = MockFetchUrl() - lyrics = genius._scrape_lyrics_from_html(mock(url)) + lyrics = backend._scrape_lyrics_from_html(mock(url)) assert lyrics is not None assert lyrics.count("\n") == 133 - # TODO: find an example of a lyrics page with multiple divs and test it - - -class GeniusFetchTest(GeniusBaseTest): - """tests Genius.fetch()""" - - def setUp(self): - """Set up configuration""" - GeniusBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - @patch.object(lyrics.Genius, "_scrape_lyrics_from_html") @patch.object(lyrics.Backend, "fetch_url", return_value=True) - def test_json(self, mock_fetch_url, mock_scrape): + def test_json(self, mock_fetch_url, mock_scrape, backend): """Ensure we're finding artist matches""" with patch.object( lyrics.Genius, @@ -447,54 +425,38 @@ def test_json(self, mock_fetch_url, mock_scrape): ) as mock_json: # genius uses zero-width-spaces (\u200B) for lowercase # artists so we make sure we can match those - assert genius.fetch("blackbear", "Idfc") is not None + assert backend.fetch("blackbear", "Idfc") is not None mock_fetch_url.assert_called_once_with("blackbear_url") mock_scrape.assert_called_once_with(True) # genius uses the hyphen minus (\u002D) as their dash - assert genius.fetch("El-p", "Idfc") is not None + assert backend.fetch("El-p", "Idfc") is not None mock_fetch_url.assert_called_with("El-p_url") mock_scrape.assert_called_with(True) # test no matching artist - assert genius.fetch("doesntexist", "none") is None + assert backend.fetch("doesntexist", "none") is None # test invalid json mock_json.return_value = None - assert genius.fetch("blackbear", "Idfc") is None + assert backend.fetch("blackbear", "Idfc") is None -# test Tekstowo +class TestTekstowoLyrics(LyricsBackend): + @pytest.fixture(scope="class") + def backend_name(self): + return "tekstowo" - -class TekstowoBaseTest(unittest.TestCase): - def setUp(self): - """Set up configuration.""" - try: - __import__("bs4") - except ImportError: - self.skipTest("Beautiful Soup 4 not available") - - -class TekstowoExtractLyricsTest(TekstowoBaseTest): - """tests Tekstowo.extract_lyrics()""" - - def setUp(self): - """Set up configuration""" - TekstowoBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - tekstowo.config = self.plugin.config - - def test_good_lyrics(self): + def test_good_lyrics(self, backend): """Ensure we are able to scrape a page with lyrics""" url = "https://www.tekstowo.pl/piosenka,24kgoldn,city_of_angels_1.html" mock = MockFetchUrl() assert ( - tekstowo.extract_lyrics(mock(url), "24kGoldn", "City of Angels") + backend.extract_lyrics(mock(url), "24kGoldn", "City of Angels") is not None ) - def test_no_lyrics(self): + def test_no_lyrics(self, backend): """Ensure we don't crash when the scraping the html for a Tekstowo page doesn't contain lyrics """ @@ -504,7 +466,7 @@ def test_no_lyrics(self): ) mock = MockFetchUrl() assert ( - tekstowo.extract_lyrics( + backend.extract_lyrics( mock(url), "Beethoven", "Beethoven Piano Sonata 17" "Tempest The 3rd Movement", @@ -512,7 +474,7 @@ def test_no_lyrics(self): is None ) - def test_song_no_match(self): + def test_song_no_match(self, backend): """Ensure we return None when a song does not match the search query""" # https://github.com/beetbox/beets/issues/4406 # expected return value None @@ -522,22 +484,13 @@ def test_song_no_match(self): ) mock = MockFetchUrl() assert ( - tekstowo.extract_lyrics( + backend.extract_lyrics( mock(url), "Kelly Bailey", "Black Mesa Inbound" ) is None ) - -class TekstowoParseSearchResultsTest(TekstowoBaseTest): - """tests Tekstowo.parse_search_results()""" - - def setUp(self): - """Set up configuration""" - TekstowoBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - - def test_multiple_results(self): + def test_multiple_results(self, backend): """Ensure we are able to scrape a page with multiple search results""" url = ( "https://www.tekstowo.pl/szukaj,wykonawca,juice+wrld" @@ -545,31 +498,41 @@ def test_multiple_results(self): ) mock = MockFetchUrl() assert ( - tekstowo.parse_search_results(mock(url)) + backend.parse_search_results(mock(url)) == "http://www.tekstowo.pl/piosenka,juice_wrld," "lucid_dreams__remix__ft__lil_uzi_vert.html" ) - def test_no_results(self): + def test_no_results(self, backend): """Ensure we are able to scrape a page with no search results""" url = ( "https://www.tekstowo.pl/szukaj,wykonawca," "agfdgja,tytul,agfdgafg.html" ) mock = MockFetchUrl() - assert tekstowo.parse_search_results(mock(url)) is None - + assert backend.parse_search_results(mock(url)) is None -# test LRCLib backend +class TestLRCLibLyrics(LyricsBackend): + @pytest.fixture(scope="class") + def backend_name(self): + return "lrclib" -class LRCLibLyricsTest(unittest.TestCase): - def setUp(self): - self.plugin = lyrics.LyricsPlugin() - lrclib.config = self.plugin.config + @pytest.fixture + def mock_get(self): + with patch("beetsplug.lyrics.requests.get") as mock: + yield mock - @patch("beetsplug.lyrics.requests.get") - def test_fetch_synced_lyrics(self, mock_get): + @pytest.mark.parametrize( + "plugin_config, expected_lyrics_type", + [ + ({"synced": True}, "syncedLyrics"), + ({"synced": False}, "plainLyrics"), + ], + ) + def test_synced_config_option( + self, backend, mock_get, expected_lyrics_type + ): mock_response = { "syncedLyrics": "[00:00.00] la la la", "plainLyrics": "la la la", @@ -577,15 +540,10 @@ def test_fetch_synced_lyrics(self, mock_get): mock_get.return_value.json.return_value = mock_response mock_get.return_value.status_code = 200 - lyrics = lrclib.fetch("la", "la", "la", 999) - assert lyrics == mock_response["plainLyrics"] - - self.plugin.config["synced"] = True - lyrics = lrclib.fetch("la", "la", "la", 999) - assert lyrics == mock_response["syncedLyrics"] + lyrics = backend.fetch("la", "la", "la", 999) + assert lyrics == mock_response[expected_lyrics_type] - @patch("beetsplug.lyrics.requests.get") - def test_fetch_plain_lyrics(self, mock_get): + def test_fetch_plain_lyrics(self, backend, mock_get): mock_response = { "syncedLyrics": "", "plainLyrics": "la la la", @@ -593,12 +551,11 @@ def test_fetch_plain_lyrics(self, mock_get): mock_get.return_value.json.return_value = mock_response mock_get.return_value.status_code = 200 - lyrics = lrclib.fetch("la", "la", "la", 999) + lyrics = backend.fetch("la", "la", "la", 999) assert lyrics == mock_response["plainLyrics"] - @patch("beetsplug.lyrics.requests.get") - def test_fetch_not_found(self, mock_get): + def test_fetch_not_found(self, backend, mock_get): mock_response = { "statusCode": 404, "error": "Not Found", @@ -607,15 +564,14 @@ def test_fetch_not_found(self, mock_get): mock_get.return_value.json.return_value = mock_response mock_get.return_value.status_code = 404 - lyrics = lrclib.fetch("la", "la", "la", 999) + lyrics = backend.fetch("la", "la", "la", 999) assert lyrics is None - @patch("beetsplug.lyrics.requests.get") - def test_fetch_exception(self, mock_get): + def test_fetch_exception(self, backend, mock_get): mock_get.side_effect = requests.RequestException - lyrics = lrclib.fetch("la", "la", "la", 999) + lyrics = backend.fetch("la", "la", "la", 999) assert lyrics is None