mirror of
https://github.com/beetbox/beets.git
synced 2026-01-05 07:23:33 +01:00
Refactor lyrics backend tests to use pytest fixtures
- Replaced unittest.mock with pytest fixtures for better test isolation and readability. - Simplified test cases by using parameterized tests. - Added `requests-mock` dependency to `pyproject.toml` and `poetry.lock`. - Removed redundant helper functions and classes.
This commit is contained in:
parent
b9bc2cbc04
commit
fc49902f3a
5 changed files with 139 additions and 199 deletions
4
.github/workflows/ci.yaml
vendored
4
.github/workflows/ci.yaml
vendored
|
|
@ -44,13 +44,13 @@ jobs:
|
|||
- if: ${{ env.IS_MAIN_PYTHON != 'true' }}
|
||||
name: Test without coverage
|
||||
run: |
|
||||
poetry install --extras=autobpm
|
||||
poetry install --extras=autobpm --extras=lyrics
|
||||
poe test
|
||||
|
||||
- if: ${{ env.IS_MAIN_PYTHON == 'true' }}
|
||||
name: Test with coverage
|
||||
run: |
|
||||
poetry install --extras=autobpm --extras=docs --extras=replaygain --extras=reflink
|
||||
poetry install --extras=autobpm --extras=lyrics --extras=docs --extras=replaygain --extras=reflink
|
||||
poe docs
|
||||
poe test-with-coverage
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
21
poetry.lock
generated
21
poetry.lock
generated
|
|
@ -1,4 +1,4 @@
|
|||
# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand.
|
||||
# This file is automatically @generated by Poetry 1.8.5 and should not be changed by hand.
|
||||
|
||||
[[package]]
|
||||
name = "accessible-pygments"
|
||||
|
|
@ -2577,6 +2577,23 @@ urllib3 = ">=1.21.1,<3"
|
|||
socks = ["PySocks (>=1.5.6,!=1.5.7)"]
|
||||
use-chardet-on-py3 = ["chardet (>=3.0.2,<6)"]
|
||||
|
||||
[[package]]
|
||||
name = "requests-mock"
|
||||
version = "1.12.1"
|
||||
description = "Mock out responses from the requests package"
|
||||
optional = false
|
||||
python-versions = ">=3.5"
|
||||
files = [
|
||||
{file = "requests-mock-1.12.1.tar.gz", hash = "sha256:e9e12e333b525156e82a3c852f22016b9158220d2f47454de9cae8a77d371401"},
|
||||
{file = "requests_mock-1.12.1-py2.py3-none-any.whl", hash = "sha256:b1e37054004cdd5e56c84454cc7df12b25f90f382159087f4b6915aaeef39563"},
|
||||
]
|
||||
|
||||
[package.dependencies]
|
||||
requests = ">=2.22,<3"
|
||||
|
||||
[package.extras]
|
||||
fixture = ["fixtures"]
|
||||
|
||||
[[package]]
|
||||
name = "requests-oauthlib"
|
||||
version = "2.0.0"
|
||||
|
|
@ -3285,4 +3302,4 @@ web = ["flask", "flask-cors"]
|
|||
[metadata]
|
||||
lock-version = "2.0"
|
||||
python-versions = ">=3.9,<4"
|
||||
content-hash = "2edbbe1f3488fb9d3a05e2d60c23d3fd1afaa8154d2a71ffad83b34476ceac78"
|
||||
content-hash = "d609e83f7ffeefc12e28d627e5646aa5c1a6f5a56d7013bb649a468069550dba"
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ python3-discogs-client = ">=2.3.15"
|
|||
py7zr = "*"
|
||||
pyxdg = "*"
|
||||
rarfile = "*"
|
||||
requests-mock = ">=1.12.1"
|
||||
requests_oauthlib = "*"
|
||||
responses = ">=0.3.0"
|
||||
|
||||
|
|
|
|||
|
|
@ -16,18 +16,14 @@
|
|||
|
||||
import itertools
|
||||
import os
|
||||
import re
|
||||
import unittest
|
||||
from functools import partial
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
import requests
|
||||
from bs4 import BeautifulSoup, SoupStrainer
|
||||
|
||||
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
|
||||
|
||||
PHRASE_BY_TITLE = {
|
||||
|
|
@ -175,33 +171,9 @@ class LyricsPluginTest(unittest.TestCase):
|
|||
assert lyrics._scrape_merge_paragraphs(text) == "one\ntwo\nthree"
|
||||
|
||||
|
||||
def url_to_filename(url):
|
||||
url = re.sub(r"https?://|www.", "", url)
|
||||
url = re.sub(r".html", "", url)
|
||||
fn = "".join(x for x in url if (x.isalnum() or x == "/"))
|
||||
fn = fn.split("/")
|
||||
fn = os.path.join(
|
||||
LYRICS_ROOT_DIR,
|
||||
bytestring_path(fn[0]),
|
||||
bytestring_path(fn[-1] + ".txt"),
|
||||
)
|
||||
return fn
|
||||
|
||||
|
||||
class MockFetchUrl:
|
||||
def __init__(self, pathval="fetched_path"):
|
||||
self.pathval = pathval
|
||||
self.fetched = None
|
||||
|
||||
def __call__(self, url, filename=None):
|
||||
self.fetched = url
|
||||
fn = url_to_filename(url)
|
||||
with open(fn, encoding="utf8") as f:
|
||||
content = f.read()
|
||||
return content
|
||||
|
||||
|
||||
LYRICS_ROOT_DIR = os.path.join(_common.RSRC, b"lyrics")
|
||||
@pytest.fixture(scope="module")
|
||||
def lyrics_root_dir(pytestconfig: pytest.Config):
|
||||
return pytestconfig.rootpath / "test" / "rsrc" / "lyrics"
|
||||
|
||||
|
||||
class LyricsBackendTest(PluginMixin):
|
||||
|
|
@ -221,6 +193,12 @@ class LyricsBackendTest(PluginMixin):
|
|||
lyrics_plugin = lyrics.LyricsPlugin()
|
||||
return lyrics_plugin.backends[0]
|
||||
|
||||
@pytest.fixture
|
||||
def lyrics_html(self, lyrics_root_dir, file_name):
|
||||
return (lyrics_root_dir / f"{file_name}.txt").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
|
||||
@pytest.mark.integration_test
|
||||
def test_backend_source(self, backend):
|
||||
"""Test default backends with a song known to exist in respective
|
||||
|
|
@ -228,21 +206,16 @@ class LyricsBackendTest(PluginMixin):
|
|||
"""
|
||||
title = "Lady Madonna"
|
||||
|
||||
res = backend.fetch("The Beatles", title)
|
||||
lyrics = backend.fetch("The Beatles", title)
|
||||
|
||||
assert res
|
||||
assert PHRASE_BY_TITLE[title] in res.lower()
|
||||
assert lyrics
|
||||
assert PHRASE_BY_TITLE[title] in lyrics.lower()
|
||||
|
||||
|
||||
class TestGoogleLyrics(LyricsBackendTest):
|
||||
"""Test scraping heuristics on a fake html page."""
|
||||
|
||||
source = dict(
|
||||
url="http://www.example.com",
|
||||
artist="John Doe",
|
||||
title="Beets song",
|
||||
path="/lyrics/beetssong",
|
||||
)
|
||||
TITLE = "Beets song"
|
||||
|
||||
@pytest.fixture(scope="class")
|
||||
def backend_name(self):
|
||||
|
|
@ -252,6 +225,10 @@ class TestGoogleLyrics(LyricsBackendTest):
|
|||
def plugin_config(self):
|
||||
return {"google_API_key": "test"}
|
||||
|
||||
@pytest.fixture(scope="class")
|
||||
def file_name(self):
|
||||
return "examplecom/beetssong"
|
||||
|
||||
@pytest.mark.integration_test
|
||||
@pytest.mark.parametrize(
|
||||
"title, url",
|
||||
|
|
@ -292,80 +269,55 @@ class TestGoogleLyrics(LyricsBackendTest):
|
|||
assert backend.is_lyrics(result)
|
||||
assert PHRASE_BY_TITLE[title] in result
|
||||
|
||||
@patch.object(lyrics.Backend, "fetch_url", MockFetchUrl())
|
||||
def test_mocked_source_ok(self, backend):
|
||||
def test_mocked_source_ok(self, backend, lyrics_html):
|
||||
"""Test that lyrics of the mocked page are correctly scraped"""
|
||||
url = self.source["url"] + self.source["path"]
|
||||
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()
|
||||
result = lyrics.scrape_lyrics_from_html(lyrics_html).lower()
|
||||
|
||||
@patch.object(lyrics.Backend, "fetch_url", MockFetchUrl())
|
||||
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.
|
||||
"""
|
||||
s = self.source
|
||||
url = str(s["url"] + s["path"])
|
||||
html = backend.fetch_url(url)
|
||||
soup = BeautifulSoup(
|
||||
html, "html.parser", parse_only=SoupStrainer("title")
|
||||
assert result
|
||||
assert backend.is_lyrics(result)
|
||||
assert PHRASE_BY_TITLE[self.TITLE] in result
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url_title, artist, should_be_candidate",
|
||||
[
|
||||
("John Doe - beets song Lyrics", "John Doe", True),
|
||||
("example.com | Beats song by John doe", "John Doe", True),
|
||||
("example.com | seets bong lyrics by John doe", "John Doe", False),
|
||||
("foo", "Sun O)))", False),
|
||||
],
|
||||
)
|
||||
def test_is_page_candidate(
|
||||
self, backend, lyrics_html, url_title, artist, should_be_candidate
|
||||
):
|
||||
result = backend.is_page_candidate(
|
||||
"http://www.example.com/lyrics/beetssong",
|
||||
url_title,
|
||||
self.TITLE,
|
||||
artist,
|
||||
)
|
||||
assert backend.is_page_candidate(
|
||||
url, soup.title.string, s["title"], s["artist"]
|
||||
), url
|
||||
assert bool(result) == should_be_candidate
|
||||
|
||||
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.
|
||||
"""
|
||||
s = self.source
|
||||
url = s["url"] + s["path"]
|
||||
url_title = "example.com | Beats song by John doe"
|
||||
|
||||
# very small diffs (typo) are ok eg 'beats' vs 'beets' with same artist
|
||||
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 backend.is_page_candidate(
|
||||
url, url_title, s["title"], s["artist"]
|
||||
), url
|
||||
|
||||
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.
|
||||
"""
|
||||
# https://github.com/beetbox/beets/issues/1673
|
||||
s = self.source
|
||||
url = s["url"] + s["path"]
|
||||
url_title = "foo"
|
||||
|
||||
backend.is_page_candidate(url, url_title, s["title"], "Sunn O)))")
|
||||
|
||||
def test_is_lyrics(self, backend):
|
||||
texts = ["LyricsMania.com - Copyright (c) 2013 - All Rights Reserved"]
|
||||
texts += [
|
||||
@pytest.mark.parametrize(
|
||||
"lyrics",
|
||||
[
|
||||
"LyricsMania.com - Copyright (c) 2013 - All Rights Reserved",
|
||||
"""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"
|
||||
|
||||
def test_missing_lyrics(self, backend):
|
||||
lyrics = """
|
||||
of mywickedsongtext brand""",
|
||||
"""
|
||||
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.
|
||||
"""
|
||||
""",
|
||||
],
|
||||
)
|
||||
def test_bad_lyrics(self, backend, lyrics):
|
||||
assert not backend.is_lyrics(lyrics)
|
||||
|
||||
def test_slugify(self, backend):
|
||||
text = "http://site.com/\xe7afe-au_lait(boisson)"
|
||||
assert backend.slugify(text) == "http://site.com/cafe_au_lait"
|
||||
|
||||
|
||||
class TestGeniusLyrics(LyricsBackendTest):
|
||||
@pytest.fixture(scope="class")
|
||||
|
|
@ -377,31 +329,18 @@ class TestGeniusLyrics(LyricsBackendTest):
|
|||
def test_backend_source(self, backend):
|
||||
super().test_backend_source(backend)
|
||||
|
||||
def test_no_lyrics_div(self, backend):
|
||||
"""Ensure we don't crash when the scraping the html for a genius page
|
||||
doesn't contain <div class="lyrics"></div>
|
||||
"""
|
||||
# https://github.com/beetbox/beets/issues/3535
|
||||
# expected return value None
|
||||
url = "https://genius.com/sample"
|
||||
mock = MockFetchUrl()
|
||||
assert backend._scrape_lyrics_from_html(mock(url)) is None
|
||||
@pytest.mark.parametrize(
|
||||
"file_name, expected_line_count",
|
||||
[
|
||||
("geniuscom/2pacalleyezonmelyrics", 134),
|
||||
("geniuscom/Ttngchinchillalyrics", 29),
|
||||
("geniuscom/sample", 0), # see https://github.com/beetbox/beets/issues/3535
|
||||
],
|
||||
) # fmt: skip
|
||||
def test_scrape(self, backend, lyrics_html, expected_line_count):
|
||||
result = backend._scrape_lyrics_from_html(lyrics_html) or ""
|
||||
|
||||
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 = backend._scrape_lyrics_from_html(mock(url))
|
||||
assert lyrics is not None
|
||||
assert lyrics.count("\n") == 28
|
||||
|
||||
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 = backend._scrape_lyrics_from_html(mock(url))
|
||||
assert lyrics is not None
|
||||
assert lyrics.count("\n") == 133
|
||||
assert len(result.splitlines()) == expected_line_count
|
||||
|
||||
@patch.object(lyrics.Genius, "_scrape_lyrics_from_html")
|
||||
@patch.object(lyrics.Backend, "fetch_url", return_value=True)
|
||||
|
|
@ -455,22 +394,18 @@ class TestTekstowoLyrics(LyricsBackendTest):
|
|||
def backend_name(self):
|
||||
return "tekstowo"
|
||||
|
||||
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 backend.extract_lyrics(mock(url))
|
||||
|
||||
def test_no_lyrics(self, backend):
|
||||
"""Ensure we don't crash when the scraping the html for a Tekstowo page
|
||||
doesn't contain lyrics
|
||||
"""
|
||||
url = (
|
||||
"https://www.tekstowo.pl/piosenka,beethoven,"
|
||||
"beethoven_piano_sonata_17_tempest_the_3rd_movement.html"
|
||||
)
|
||||
mock = MockFetchUrl()
|
||||
assert not backend.extract_lyrics(mock(url))
|
||||
@pytest.mark.parametrize(
|
||||
"file_name, expecting_lyrics",
|
||||
[
|
||||
("tekstowopl/piosenka24kgoldncityofangels1", True),
|
||||
(
|
||||
"tekstowopl/piosenkabeethovenbeethovenpianosonata17tempestthe3rdmovement", # noqa: E501
|
||||
False,
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_scrape(self, backend, lyrics_html, expecting_lyrics):
|
||||
assert bool(backend.extract_lyrics(lyrics_html)) == expecting_lyrics
|
||||
|
||||
|
||||
class TestLRCLibLyrics(LyricsBackendTest):
|
||||
|
|
@ -479,64 +414,51 @@ class TestLRCLibLyrics(LyricsBackendTest):
|
|||
return "lrclib"
|
||||
|
||||
@pytest.fixture
|
||||
def mock_get(self):
|
||||
with patch("beetsplug.lyrics.requests.get") as mock:
|
||||
yield mock
|
||||
def fetch_lyrics(self, backend, requests_mock, response_data):
|
||||
requests_mock.get(lyrics.LRCLib.base_url, json=response_data)
|
||||
|
||||
return partial(backend.fetch, "la", "la", "la")
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"plugin_config, expected_lyrics_type",
|
||||
"response_data",
|
||||
[
|
||||
({"synced": True}, "syncedLyrics"),
|
||||
({"synced": False}, "plainLyrics"),
|
||||
{
|
||||
"syncedLyrics": "[00:00.00] la la la",
|
||||
"plainLyrics": "la la la",
|
||||
}
|
||||
],
|
||||
)
|
||||
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",
|
||||
}
|
||||
mock_get.return_value.json.return_value = mock_response
|
||||
mock_get.return_value.status_code = 200
|
||||
|
||||
lyrics = backend.fetch("la", "la", "la", 999)
|
||||
assert lyrics == mock_response[expected_lyrics_type]
|
||||
@pytest.mark.parametrize(
|
||||
"plugin_config, expected_lyrics",
|
||||
[
|
||||
({"synced": True}, "[00:00.00] la la la"),
|
||||
({"synced": False}, "la la la"),
|
||||
],
|
||||
)
|
||||
def test_synced_config_option(self, fetch_lyrics, expected_lyrics):
|
||||
assert fetch_lyrics() == expected_lyrics
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"plugin_config", [({"synced": True}), ({"synced": False})]
|
||||
"response_data, expected_lyrics",
|
||||
[
|
||||
pytest.param(
|
||||
{"syncedLyrics": "", "plainLyrics": "la la la"},
|
||||
"la la la",
|
||||
id="pick plain lyrics",
|
||||
),
|
||||
pytest.param(
|
||||
{
|
||||
"statusCode": 404,
|
||||
"error": "Not Found",
|
||||
"message": "Failed to find specified track",
|
||||
},
|
||||
None,
|
||||
id="not found",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_fetch_plain_lyrics(self, backend, mock_get):
|
||||
mock_response = {
|
||||
"syncedLyrics": "",
|
||||
"plainLyrics": "la la la",
|
||||
}
|
||||
mock_get.return_value.json.return_value = mock_response
|
||||
mock_get.return_value.status_code = 200
|
||||
|
||||
lyrics = backend.fetch("la", "la", "la", 999)
|
||||
|
||||
assert lyrics == mock_response["plainLyrics"]
|
||||
|
||||
def test_fetch_not_found(self, backend, mock_get):
|
||||
mock_response = {
|
||||
"statusCode": 404,
|
||||
"error": "Not Found",
|
||||
"message": "Failed to find specified track",
|
||||
}
|
||||
mock_get.return_value.json.return_value = mock_response
|
||||
mock_get.return_value.status_code = 404
|
||||
|
||||
lyrics = backend.fetch("la", "la", "la", 999)
|
||||
|
||||
assert lyrics is None
|
||||
|
||||
def test_fetch_exception(self, backend, mock_get):
|
||||
mock_get.side_effect = requests.RequestException
|
||||
|
||||
lyrics = backend.fetch("la", "la", "la", 999)
|
||||
|
||||
assert lyrics is None
|
||||
def test_fetch_lyrics(self, fetch_lyrics, expected_lyrics):
|
||||
assert fetch_lyrics() == expected_lyrics
|
||||
|
||||
|
||||
# test utilities
|
||||
|
|
|
|||
Loading…
Reference in a new issue