MusicBrainz: Tests for new search behavior + WHATS_NEW entry

26 new unit tests in tests/test_musicbrainz_search.py covering:

- Cover Art URL construction (release + release-group scope, empty MBID,
  unknown scope fallback)
- Structured query splitting (hyphen, en-dash, em-dash, bare name, no
  false-positive splits on hyphens-inside-words)
- Artist search: score filtering, strict=False call contract, exception
  handling, genre extraction from MB tags, mbid/name validation
- Top-artist resolver: memoization by normalized query, sub-threshold
  returns None, negative-result caching, empty-query short-circuit
- Album search routing: bare query → browse path, structured query →
  text path, no-artist-match falls back to text, text path score filter
- Track search routing: browse path, dedupe-by-title across
  live/compilation variants, structured query → text path, text path
  score filter

All mock the underlying MusicBrainzClient — no network calls.

Also adds a WHATS_NEW entry under 2.40 explaining the three user-visible
changes: Artists section now populates, album/track results match the
searched artist instead of random title collisions, and search completes
in ~3 seconds instead of 30+.
pull/372/head
Broque Thomas 1 month ago
parent 73df2951e5
commit 394ac73877

@ -0,0 +1,351 @@
"""Tests for the MusicBrainz search adapter (core/musicbrainz_search.py).
Covers the behavior changes from the search-overhaul PR:
- Artist search is re-enabled and score-filtered
- Bare name queries route through artist-first browse
- Structured 'Artist - Title' queries stay on text search
- Top-artist resolution is memoized per instance
- Cover Art URLs are constructed, not probed
"""
from unittest.mock import MagicMock, patch
import pytest
from core.musicbrainz_search import (
MusicBrainzSearchClient,
_cover_art_url,
)
# ---------------------------------------------------------------------------
# Cover art URL construction
# ---------------------------------------------------------------------------
def test_cover_art_url_release_scope():
assert _cover_art_url('abc-123') == 'https://coverartarchive.org/release/abc-123/front-250'
def test_cover_art_url_release_group_scope():
assert _cover_art_url('abc-123', scope='release-group') == \
'https://coverartarchive.org/release-group/abc-123/front-250'
def test_cover_art_url_empty_mbid_returns_none():
assert _cover_art_url('') is None
assert _cover_art_url(None) is None
def test_cover_art_url_unknown_scope_falls_back_to_release():
assert _cover_art_url('abc', scope='garbage') == 'https://coverartarchive.org/release/abc/front-250'
# ---------------------------------------------------------------------------
# Structured query splitting
# ---------------------------------------------------------------------------
def test_split_structured_query_hyphen():
client = MusicBrainzSearchClient()
assert client._split_structured_query('Metallica - Master of Puppets') == ('Metallica', 'Master of Puppets')
def test_split_structured_query_en_dash():
client = MusicBrainzSearchClient()
assert client._split_structured_query('Metallica One') == ('Metallica', 'One')
def test_split_structured_query_em_dash():
client = MusicBrainzSearchClient()
assert client._split_structured_query('Metallica — Battery') == ('Metallica', 'Battery')
def test_split_structured_query_bare_name():
client = MusicBrainzSearchClient()
assert client._split_structured_query('metallica') == (None, 'metallica')
def test_split_structured_query_no_separator_with_hyphens_in_word():
# A hyphen inside a word (no surrounding spaces) should not split.
client = MusicBrainzSearchClient()
assert client._split_structured_query('t-pain') == (None, 't-pain')
# ---------------------------------------------------------------------------
# Artist search — score filtering and shape
# ---------------------------------------------------------------------------
def _mk_artist(name, mbid, score=100, tags=None):
return {
'id': mbid,
'name': name,
'score': score,
'tags': tags or [],
}
def test_search_artists_filters_by_score_threshold():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [
_mk_artist('Metallica', 'mb-real', score=100),
_mk_artist('Metallica Tribute', 'mb-tribute', score=60),
_mk_artist('Metallica Jam', 'mb-jam', score=58),
]
results = client.search_artists('metallica', limit=10)
assert len(results) == 1
assert results[0].name == 'Metallica'
assert results[0].id == 'mb-real'
def test_search_artists_uses_strict_false_for_fuzzy_match():
"""The adapter must use strict=False so MusicBrainz searches
alias+artist+sortname together strict mode would miss aliased names."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = []
client.search_artists('metallica')
client._client.search_artist.assert_called_once_with('metallica', limit=10, strict=False)
def test_search_artists_returns_empty_on_exception():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.side_effect = RuntimeError('network down')
assert client.search_artists('metallica') == []
def test_search_artists_extracts_tags_as_genres():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [
_mk_artist('Metallica', 'mb-real', score=100,
tags=[{'name': 'thrash metal', 'count': 20},
{'name': 'heavy metal', 'count': 15}]),
]
results = client.search_artists('metallica')
assert results[0].genres == ['thrash metal', 'heavy metal']
def test_search_artists_skips_entries_without_mbid_or_name():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [
{'id': 'mb-1', 'name': 'Good', 'score': 100},
{'id': '', 'name': 'Missing MBID', 'score': 100},
{'id': 'mb-2', 'name': '', 'score': 100},
]
results = client.search_artists('x')
assert [r.name for r in results] == ['Good']
# ---------------------------------------------------------------------------
# Top-artist resolution — memoization
# ---------------------------------------------------------------------------
def test_resolve_top_artist_memoizes_by_normalized_query():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)]
first = client._resolve_top_artist('metallica')
second = client._resolve_top_artist(' Metallica ') # Whitespace / case variant
assert first is not None
assert first['id'] == 'mb-1'
assert first is second
# HTTP call happens once despite two resolve calls.
assert client._client.search_artist.call_count == 1
def test_resolve_top_artist_returns_none_below_threshold():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [_mk_artist('Tribute', 'mb-trib', score=50)]
assert client._resolve_top_artist('obscure') is None
def test_resolve_top_artist_caches_negative_result():
"""After a lookup finds no good match, subsequent calls don't refetch."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = []
first = client._resolve_top_artist('nonexistent band')
second = client._resolve_top_artist('nonexistent band')
assert first is None
assert second is None
assert client._client.search_artist.call_count == 1
def test_resolve_top_artist_empty_query_returns_none_without_http():
client = MusicBrainzSearchClient()
client._client = MagicMock()
assert client._resolve_top_artist('') is None
client._client.search_artist.assert_not_called()
# ---------------------------------------------------------------------------
# Album search — routing
# ---------------------------------------------------------------------------
def test_search_albums_bare_query_uses_browse_path():
"""When a bare name resolves to an artist, we browse their release-groups
instead of text-searching release titles."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)]
client._client.browse_artist_release_groups.return_value = [
{'id': 'rg-1', 'title': 'Master of Puppets', 'primary-type': 'Album',
'first-release-date': '1986-03-03', 'secondary-types': []},
{'id': 'rg-2', 'title': 'Ride the Lightning', 'primary-type': 'Album',
'first-release-date': '1984-07-27', 'secondary-types': []},
]
albums = client.search_albums('metallica', limit=10)
client._client.browse_artist_release_groups.assert_called_once()
# Text-search path must NOT be taken.
client._client.search_release.assert_not_called()
# Browse results come back, newest first.
assert [a.name for a in albums] == ['Master of Puppets', 'Ride the Lightning']
assert all(a.artists == ['Metallica'] for a in albums)
def test_search_albums_structured_query_uses_text_path():
"""'Artist - Title' shape should text-search the title rather than
browsing all of the artist's discography."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_release.return_value = [
{'id': 'rel-1', 'title': 'Master of Puppets', 'score': 100,
'date': '1986', 'media': [{'track-count': 8}],
'release-group': {'id': 'rg-1', 'primary-type': 'Album'},
'artist-credit': [{'name': 'Metallica'}]},
]
albums = client.search_albums('Metallica - Master of Puppets', limit=10)
client._client.search_release.assert_called_once()
# Artist-first path must NOT be taken.
client._client.search_artist.assert_not_called()
client._client.browse_artist_release_groups.assert_not_called()
assert len(albums) == 1
assert albums[0].name == 'Master of Puppets'
def test_search_albums_falls_back_to_text_when_no_artist_match():
"""No artist above threshold → text-search the whole query."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
# Artist lookup returns nothing above threshold.
client._client.search_artist.return_value = [_mk_artist('X', 'mb-x', score=40)]
client._client.search_release.return_value = []
client.search_albums('very obscure band')
client._client.search_release.assert_called_once_with('very obscure band', artist_name=None, limit=10)
client._client.browse_artist_release_groups.assert_not_called()
def test_search_albums_text_path_filters_by_score():
client = MusicBrainzSearchClient()
client._client = MagicMock()
# Force text-search path by using a structured query.
client._client.search_release.return_value = [
{'id': 'rel-good', 'title': 'Good', 'score': 95,
'release-group': {'id': 'rg-1', 'primary-type': 'Album'},
'artist-credit': [{'name': 'Foo'}]},
{'id': 'rel-bad', 'title': 'Bad', 'score': 40,
'release-group': {'id': 'rg-2', 'primary-type': 'Album'},
'artist-credit': [{'name': 'Foo'}]},
]
albums = client.search_albums('Foo - Good', limit=10)
titles = [a.name for a in albums]
assert 'Good' in titles
assert 'Bad' not in titles
# ---------------------------------------------------------------------------
# Track search — routing
# ---------------------------------------------------------------------------
def test_search_tracks_bare_query_uses_browse_path():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)]
client._client.browse_artist_recordings.return_value = [
{'id': 'rec-1', 'title': 'One', 'length': 446000,
'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988',
'release-group': {'id': 'rg-1', 'primary-type': 'Album'}}],
'artist-credit': [{'name': 'Metallica'}]},
{'id': 'rec-2', 'title': 'Battery', 'length': 312000,
'releases': [{'id': 'rel-2', 'title': 'Master of Puppets', 'date': '1986',
'release-group': {'id': 'rg-2', 'primary-type': 'Album'}}],
'artist-credit': [{'name': 'Metallica'}]},
]
tracks = client.search_tracks('metallica', limit=10)
client._client.browse_artist_recordings.assert_called_once()
client._client.search_recording.assert_not_called()
assert len(tracks) == 2
assert {t.name for t in tracks} == {'One', 'Battery'}
def test_search_tracks_dedupes_by_title():
"""MusicBrainz has many live/compilation variants of the same song.
Browse results should be deduped by normalized title so we don't show
'One' three times."""
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)]
client._client.browse_artist_recordings.return_value = [
{'id': 'rec-1', 'title': 'One', 'length': 446000,
'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988'}],
'artist-credit': [{'name': 'Metallica'}]},
{'id': 'rec-1-live', 'title': 'One', 'length': 490000,
'releases': [{'id': 'rel-live', 'title': 'Live Shit', 'date': '1993'}],
'artist-credit': [{'name': 'Metallica'}]},
]
tracks = client.search_tracks('metallica', limit=10)
assert len(tracks) == 1
assert tracks[0].name == 'One'
def test_search_tracks_structured_query_uses_text_path():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.return_value = [
{'id': 'rec-1', 'title': 'One', 'score': 100,
'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988'}],
'artist-credit': [{'name': 'Metallica'}]},
]
tracks = client.search_tracks('Metallica - One', limit=10)
client._client.search_recording.assert_called_once()
client._client.search_artist.assert_not_called()
client._client.browse_artist_recordings.assert_not_called()
assert len(tracks) == 1
def test_search_tracks_text_path_filters_by_score():
client = MusicBrainzSearchClient()
client._client = MagicMock()
client._client.search_recording.return_value = [
{'id': 'rec-good', 'title': 'Good', 'score': 95,
'releases': [{'id': 'rel-1', 'title': 'X', 'date': '2020'}],
'artist-credit': [{'name': 'Foo'}]},
{'id': 'rec-bad', 'title': 'Bad', 'score': 40,
'releases': [{'id': 'rel-2', 'title': 'Y', 'date': '2021'}],
'artist-credit': [{'name': 'Foo'}]},
]
tracks = client.search_tracks('Foo - Good', limit=10)
titles = [t.name for t in tracks]
assert 'Good' in titles
assert 'Bad' not in titles

@ -3462,6 +3462,7 @@ const WHATS_NEW = {
{ title: 'Stale Search Requests No Longer Flash Empty Results on Fast Retype', desc: 'Cin flagged a race in createSearchController: when you typed a query then quickly re-typed before the first fetch returned, the first fetch\'s catch block (firing on AbortError after the second submitQuery aborted it) cleared loadingSources and notified the UI, causing a brief flash of empty/error state while the new query\'s fetch was still mid-flight. Added a monotonic _requestSeq token — each fetch captures the next value, and stale completions bail before mutating shared state. The controller still aborts in-flight fetches on supersession; this just keeps the abort-cleanup of the old request from clobbering the new one\'s spinner', page: 'search', unreleased: true },
{ title: 'Source Picker Dims Soulseek When slskd Isn\'t Configured', desc: 'Cin pointed out that the Soulseek icon was always rendered as configured, so users without slskd set up could click it and fire searches that would never succeed. Soulseek is now in the backend config-status registry as `required: [slskd_url]` and removed from the frontend\'s always-configured set. Without slskd, the icon dims and clicking it routes to Settings → Downloads tab (where the slskd URL field lives, gated behind the download-source dropdown) instead of Settings → Connections', page: 'search', unreleased: true },
{ title: 'Fix Discover Hero "View Discography" 404ing on Source Artists', desc: 'Clicking "View Discography" on the Discover page hero slideshow was calling navigateToArtistDetail without a source, so /api/artist-detail defaulted to a library lookup and returned 404 for artists that don\'t exist in your library (which is nearly every hero artist — they come from discover similar-artists, not the library). Regression from the unification PR that rewrote the click handler to route to /artist-detail but forgot to pass the source. Backend already sends artist.source on each hero entry; we now stash it as data-source on the discography button and thread it through to navigateToArtistDetail so the API call includes source=itunes/deezer/etc. and returns the synthesized discography', page: 'discover', unreleased: true },
{ title: 'MusicBrainz Search Actually Works Now', desc: 'kettui flagged during PR #371 review that the MusicBrainz source tab never returned artists and served garbage tracks/albums. Three things were wrong. First, the artist search was hardcoded to return an empty list — re-enabled with a proper fuzzy query (bare Lucene string against alias/artist/sortname indexes) and score-filtered at 80+ to drop tribute bands. Second, track and album searches used text-search on recording/release TITLES — so typing "metallica" matched random tracks literally named "Metallica" (all scoring 100 because they\'re exact title hits). Now a bare name query resolves to the top-scoring artist, then BROWSES that artist\'s release-groups and recordings directly — the same pattern Plex uses. Structured "Artist - Title" queries still take the text-search path since the user gave an explicit title to match. Third, the adapter was firing synchronous Cover Art Archive HEAD requests (up to 30s of blocking probes per search) — replaced with deterministic URL construction so the browser loads images lazily with <img onerror> fallback. Search completes in ~3 seconds instead of 30+ on cold cache. Also shipped: project URL in User-Agent per MB\'s rate-limit policy recommendations', page: 'search', unreleased: true },
],
'2.39': [
// --- April 22, 2026 ---

Loading…
Cancel
Save