Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor lyrics tests, do not search for empty metadata #5452

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Commits on Oct 12, 2024

  1. Configuration menu
    Copy the full SHA
    75cf42c View commit details
    Browse the repository at this point in the history
  2. Address failing google sources tests

    Two google sources failed to return the expected output. I looked into
    each case why parsing failed:
    
    - lyrics on musica.com contain <aside> Google Ads
    - each lyrics line on lacoccinelle.net is wrapped within alternating
      <em> and <strong> tags
    
    Thus remove these tags as part of the HTML cleanup logic.
    snejus committed Oct 12, 2024
    Configuration menu
    Copy the full SHA
    480012a View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    5000c59 View commit details
    Browse the repository at this point in the history

Commits on Oct 18, 2024

  1. 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.
    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    a339556 View commit details
    Browse the repository at this point in the history
  2. 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.
    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    7e6bc32 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    2af9a8c View commit details
    Browse the repository at this point in the history
  4. Remove outdated GeniusLyrics test

    The test for GeniusLyrics was heavily patched and no longer provided
    useful coverage. It has been removed to clean up the test suite.
    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    62348d6 View commit details
    Browse the repository at this point in the history
  5. Refactor test_slug to pytest

    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    f124440 View commit details
    Browse the repository at this point in the history
  6. Refactor search_pairs tests to use pytest parametrize

    - Consolidated multiple test cases into parameterized tests for better
      readability and maintainability.
    - Simplified assertions by comparing lists of actual and expected
      artists/titles.
    - Added `unexpected_empty_artist` marker to handle cases which
      unexpectedly return an empty artist. This seems to be happen when
      `artist_sort` field is empty.
    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    ffcb62f View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    954c369 View commit details
    Browse the repository at this point in the history
  8. Do not attempt to fetch lyrics with empty data

    Modified `search_pairs` function in `lyrics.py` to:
    
    * Firstly strip each of `artist`, `artist_sort` and `title` fields
    * Only generate alternatives if both `artist` and `title` are not empty
    * Ensure that `artist_sort` is not empty and not equal to artist (ignoring
      case) before appending it to the artists
    
    Extended tests to cover the changes.
    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    41d6cec View commit details
    Browse the repository at this point in the history
  9. Remove pytest.param alias _p

    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    a81024e View commit details
    Browse the repository at this point in the history
  10. Make album, duration required for LyricsPlugin.fetch

    Since at least one Backend requires album` and `duration` arguments
    (`LRCLib`), the caller (`LyricsPlugin.fetch_item_lyrics`) must always
    provide them.
    
    Since they need to provided, we need to enforce this by defining them as
    positional arguments.
    
    Why is this important? I found that integrated `LRCLib` tests have been
    passing, but they called `LRCLib.fetch` with values for `artist` and
    `title` fields only, while the actual functionality *always* provides
    values for `album` and `duration` fields too.
    
    When I adjusted the test to provide values for the missing fields,
    I found that it failed. This makes sense: Lib `album` and `duration`
    filters are strict on LRCLib, so I was not surprised the lyrics could
    not be found.
    
    Thus I adjusted `LRCLib` backend implementation to only filter by each
    of these fields when their values are truthy.
    snejus committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    9f7c1ef View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    3026a35 View commit details
    Browse the repository at this point in the history

Commits on Oct 19, 2024

  1. Test lyrics texts explicitly

    Add explicit checks for lyrics texts fetched from the tested sources.
    
    - Introduced `LyricsPage` class to represent lyrics pages for integrated
      tests.
    - Configured expected lyrics for each of the URLs that are being
      fetched.
    - Consolidated integrated tests in a new `TestLyricsSources` class.
    - Mocked Google Search API to return the lyrics page under test.
    snejus committed Oct 19, 2024
    Configuration menu
    Copy the full SHA
    e70f61b View commit details
    Browse the repository at this point in the history