AcoustID: report errors honestly instead of masking them as 'Skipped'
An invalid API key (and rate limits / missing chromaprint / fingerprint failures) all collapsed into the same None as a genuine no-match, so: - every download showed a benign 'AcoustID: Skipped', and - the 'Test API key' button reported a dead key as VALID because test_api_key trusted 'no exception = valid' but fingerprint_and_lookup swallows the error and returns None. A broken AcoustID setup looked completely normal — which cost a real debugging session to untangle. - New AcoustIDClient.lookup_with_status() returns a structured result that distinguishes ok / no_match / error / no_backend / fingerprint_error / unavailable. fingerprint_and_lookup() stays dict-or-None (library scanner / auto-import / their tests unchanged) as a thin wrapper over it. - verify_audio_file() uses it: a real error -> new VerificationResult.ERROR (-> _acoustid_result='error' -> the existing red 'Error' history badge), a genuine no-match -> SKIP 'No match in AcoustID database'. ERROR never quarantines (an outage/bad key must not punish good files). - test_api_key() now validates via the authoritative direct API call (error code 4 = invalid key) instead of the swallowed-exception path. Tests: structured-status distinction, legacy-wrapper contract, verify ERROR vs SKIP, and test_api_key invalid-vs-accepted. Existing verify tests updated to stub lookup_with_status (a stub returning just recordings is inferred as ok).
This commit is contained in:
BoulderBadgeDad
parent
a703c5fdc2
commit
ba6c39bae3
6 changed files with 281 additions and 89 deletions
|
|
@ -282,8 +282,9 @@ class AcoustIDClient:
|
|||
|
||||
def test_api_key(self) -> Tuple[bool, str]:
|
||||
"""
|
||||
Validate the API key by fingerprinting a real audio file and looking it up.
|
||||
Falls back to a direct API call if no audio files are available.
|
||||
Validate the API key with a direct AcoustID lookup call. An invalid key
|
||||
is reported as invalid (error code 4); any other error means the key was
|
||||
accepted.
|
||||
|
||||
Returns:
|
||||
Tuple of (success, message)
|
||||
|
|
@ -294,24 +295,12 @@ class AcoustIDClient:
|
|||
import requests
|
||||
|
||||
try:
|
||||
# Try to find a real audio file to fingerprint for an end-to-end test
|
||||
test_file = self._find_test_audio_file()
|
||||
|
||||
if test_file and CHROMAPRINT_AVAILABLE:
|
||||
logger.info(f"Testing API key with real audio file: {test_file}")
|
||||
try:
|
||||
result = self.fingerprint_and_lookup(test_file)
|
||||
# If we get here without exception, the API key is valid
|
||||
# (invalid keys raise or return error before results)
|
||||
return True, "AcoustID API key is valid"
|
||||
except Exception as e:
|
||||
error_str = str(e).lower()
|
||||
if 'invalid' in error_str and 'api' in error_str:
|
||||
return False, "Invalid AcoustID API key - get one from https://acoustid.org/new-application"
|
||||
# Fingerprint/lookup failed for non-key reasons, fall through to direct test
|
||||
logger.warning(f"Real file test failed ({e}), trying direct API call")
|
||||
|
||||
# Fallback: direct API call with minimal fingerprint
|
||||
# Authoritative key check: a direct API lookup with a dummy
|
||||
# fingerprint. AcoustID validates the client key first, so an
|
||||
# invalid key returns error code 4 regardless of the fingerprint.
|
||||
# (The previous real-file path trusted "no exception = valid", but
|
||||
# fingerprint_and_lookup swallows the invalid-key error and returns
|
||||
# None — so it reported broken keys as valid. #756-adjacent.)
|
||||
url = 'https://api.acoustid.org/v2/lookup'
|
||||
params = {
|
||||
'client': self.api_key,
|
||||
|
|
@ -326,7 +315,6 @@ class AcoustIDClient:
|
|||
if data.get('status') == 'error':
|
||||
error = data.get('error', {})
|
||||
error_code = error.get('code', 0)
|
||||
error_msg = error.get('message', 'Unknown error')
|
||||
|
||||
# Error code 4 is specifically "invalid API key"
|
||||
if error_code == 4:
|
||||
|
|
@ -346,33 +334,33 @@ class AcoustIDClient:
|
|||
logger.error(f"Error testing AcoustID API key: {e}")
|
||||
return False, f"Error: {str(e)}"
|
||||
|
||||
def fingerprint_and_lookup(self, audio_file: str) -> Optional[Dict[str, Any]]:
|
||||
"""
|
||||
Generate fingerprint and look up recording in AcoustID.
|
||||
def lookup_with_status(self, audio_file: str) -> Dict[str, Any]:
|
||||
"""Fingerprint + AcoustID lookup returning a STRUCTURED result.
|
||||
|
||||
This is the main method - combines fingerprinting and lookup in one call.
|
||||
Unlike fingerprint_and_lookup() (which collapses every outcome into
|
||||
dict-or-None), this distinguishes a genuine no-match from an actual
|
||||
error — an invalid API key, rate limit, missing chromaprint, or a
|
||||
fingerprint failure. That distinction is what lets the UI show "AcoustID
|
||||
Error" (something is broken — fix it) instead of a benign-looking
|
||||
"Skipped" that silently hides a dead key.
|
||||
|
||||
Args:
|
||||
audio_file: Path to the audio file
|
||||
|
||||
Returns:
|
||||
Dict with:
|
||||
'recordings': list of dicts with 'mbid', 'title', 'artist', 'score'
|
||||
'best_score': float (highest score across all results)
|
||||
'recording_mbids': list of unique MBIDs (for backward compat)
|
||||
Or None on error.
|
||||
Returns dict with:
|
||||
'status': 'ok' | 'no_match' | 'error' | 'no_backend'
|
||||
| 'fingerprint_error' | 'unsupported' | 'unavailable'
|
||||
| 'not_found'
|
||||
'recordings': list (meaningful only for 'ok')
|
||||
'best_score': float
|
||||
'recording_mbids': list
|
||||
'error': human-readable detail for any non-'ok' status
|
||||
'invalid_key': bool (True when the API specifically rejected the key)
|
||||
"""
|
||||
if not ACOUSTID_AVAILABLE:
|
||||
logger.debug("Cannot lookup: pyacoustid not available")
|
||||
return None
|
||||
|
||||
return {'status': 'unavailable', 'recordings': [], 'error': 'pyacoustid library not installed'}
|
||||
if not self.api_key:
|
||||
logger.debug("Cannot lookup: no API key")
|
||||
return None
|
||||
|
||||
return {'status': 'unavailable', 'recordings': [], 'error': 'No AcoustID API key configured'}
|
||||
if not os.path.isfile(audio_file):
|
||||
logger.warning(f"Cannot lookup: file not found: {audio_file}")
|
||||
return None
|
||||
return {'status': 'not_found', 'recordings': [], 'error': f'File not found: {audio_file}'}
|
||||
|
||||
# Check channel count — chromaprint crashes (SIGABRT) on >2 channel files (e.g. 5.1 surround)
|
||||
try:
|
||||
|
|
@ -382,7 +370,8 @@ class AcoustIDClient:
|
|||
channels = getattr(mf.info, 'channels', 2)
|
||||
if channels and channels > 2:
|
||||
logger.warning(f"Skipping AcoustID: file has {channels} channels (surround audio): {audio_file}")
|
||||
return None
|
||||
return {'status': 'unsupported', 'recordings': [],
|
||||
'error': f'{channels}-channel (surround) audio not supported by chromaprint'}
|
||||
except Exception as e:
|
||||
logger.debug(f"Could not check channel count, proceeding anyway: {e}")
|
||||
|
||||
|
|
@ -392,17 +381,12 @@ class AcoustIDClient:
|
|||
api_key_preview = f"{self.api_key[:8]}..." if self.api_key and len(self.api_key) > 8 else "NOT SET"
|
||||
logger.info(f"Fingerprinting and looking up: {audio_file} (API key: {api_key_preview})")
|
||||
|
||||
# Use match() which handles fingerprinting + lookup + parsing
|
||||
logger.debug("Running acoustid.match()...")
|
||||
recordings = []
|
||||
seen_mbids = set()
|
||||
best_score = 0.0
|
||||
|
||||
for result in acoustid.match(
|
||||
self.api_key,
|
||||
audio_file,
|
||||
parse=True
|
||||
):
|
||||
for result in acoustid.match(self.api_key, audio_file, parse=True):
|
||||
# match() with parse=True returns (score, recording_id, title, artist)
|
||||
if not isinstance(result, tuple) or len(result) < 2:
|
||||
logger.warning(f"Unexpected result format: {result}")
|
||||
|
|
@ -420,45 +404,57 @@ class AcoustIDClient:
|
|||
|
||||
if recording_id and recording_id not in seen_mbids:
|
||||
seen_mbids.add(recording_id)
|
||||
recordings.append({
|
||||
'mbid': recording_id,
|
||||
'title': title,
|
||||
'artist': artist,
|
||||
'score': score,
|
||||
})
|
||||
recordings.append({'mbid': recording_id, 'title': title, 'artist': artist, 'score': score})
|
||||
logger.debug(f"Found match: {title} by {artist} (MBID: {recording_id}, score: {score})")
|
||||
|
||||
if not recordings:
|
||||
logger.info(f"No AcoustID matches found for: {audio_file}")
|
||||
return None
|
||||
return {'status': 'no_match', 'recordings': [], 'best_score': best_score,
|
||||
'recording_mbids': [], 'error': 'Track not found in AcoustID database'}
|
||||
|
||||
logger.info(f"AcoustID found {len(recordings)} recording(s) (best score: {best_score:.2f})")
|
||||
return {
|
||||
'recordings': recordings,
|
||||
'best_score': best_score,
|
||||
'recording_mbids': list(seen_mbids),
|
||||
}
|
||||
return {'status': 'ok', 'recordings': recordings, 'best_score': best_score,
|
||||
'recording_mbids': list(seen_mbids)}
|
||||
|
||||
except acoustid.NoBackendError:
|
||||
logger.error("Chromaprint library not found and fpcalc not available")
|
||||
return None
|
||||
return {'status': 'no_backend', 'recordings': [],
|
||||
'error': 'Chromaprint/fpcalc not installed (install libchromaprint1)'}
|
||||
except acoustid.FingerprintGenerationError as e:
|
||||
logger.warning(f"Failed to fingerprint {audio_file}: {e}")
|
||||
return None
|
||||
return {'status': 'fingerprint_error', 'recordings': [], 'error': f'Could not fingerprint file: {e}'}
|
||||
except acoustid.WebServiceError as e:
|
||||
# Log more details about the API error
|
||||
api_key_preview = f"{self.api_key[:8]}..." if self.api_key and len(self.api_key) > 8 else "???"
|
||||
logger.warning(f"AcoustID API error (key: {api_key_preview}): {e}")
|
||||
# Check for common errors
|
||||
error_str = str(e).lower()
|
||||
if 'invalid' in error_str or 'unknown' in error_str:
|
||||
logger.error("API key appears to be invalid - check your AcoustID settings")
|
||||
# Old pyacoustid reports an invalid key as the bare "status: error"
|
||||
# (it drops the detail), so treat that as an invalid-key signal too.
|
||||
invalid = ('invalid' in error_str or 'unknown' in error_str or 'status: error' in error_str)
|
||||
if invalid:
|
||||
logger.error("AcoustID API key appears to be invalid — check your AcoustID settings")
|
||||
elif 'rate' in error_str or 'limit' in error_str:
|
||||
logger.warning("Rate limited by AcoustID - will retry later")
|
||||
return None
|
||||
logger.warning("Rate limited by AcoustID — will retry later")
|
||||
return {'status': 'error', 'recordings': [], 'invalid_key': invalid,
|
||||
'error': f'AcoustID API error: {e}'}
|
||||
except Exception as e:
|
||||
logger.error(f"Unexpected error in AcoustID lookup: {e}", exc_info=True)
|
||||
return None
|
||||
return {'status': 'error', 'recordings': [], 'error': f'Unexpected error: {e}'}
|
||||
|
||||
def fingerprint_and_lookup(self, audio_file: str) -> Optional[Dict[str, Any]]:
|
||||
"""Legacy dict-or-None lookup. Returns the recordings dict on a confirmed
|
||||
match, else None. Kept for callers that only need "did we identify it"
|
||||
(library scanner, auto-import). Callers that must report WHY a lookup
|
||||
didn't match (verification badge, key test) should use
|
||||
``lookup_with_status`` so an error isn't mistaken for a no-match.
|
||||
"""
|
||||
res = self.lookup_with_status(audio_file)
|
||||
if res.get('status') == 'ok':
|
||||
return {
|
||||
'recordings': res['recordings'],
|
||||
'best_score': res.get('best_score', 0.0),
|
||||
'recording_mbids': res.get('recording_mbids', []),
|
||||
}
|
||||
return None
|
||||
|
||||
def refresh_config(self):
|
||||
"""Refresh cached config values (call after settings change)."""
|
||||
|
|
|
|||
|
|
@ -50,8 +50,9 @@ class VerificationResult(Enum):
|
|||
"""Possible outcomes of audio verification."""
|
||||
PASS = "pass" # Title/artist match - file is correct
|
||||
FAIL = "fail" # Title/artist mismatch - wrong file downloaded
|
||||
SKIP = "skip" # Could not verify (error or unavailable) - continue normally
|
||||
SKIP = "skip" # Genuinely couldn't verify (no match in DB) - continue normally
|
||||
DISABLED = "disabled" # Verification not enabled
|
||||
ERROR = "error" # Lookup errored (invalid key / rate limit / no backend) - continue, but flag it
|
||||
|
||||
|
||||
def _normalize(text: str) -> str:
|
||||
|
|
@ -399,18 +400,33 @@ class AcoustIDVerification:
|
|||
logger.debug(f"AcoustID verification skipped: {reason}")
|
||||
return VerificationResult.SKIP, reason
|
||||
|
||||
# Step 2: Fingerprint and lookup in AcoustID
|
||||
# Step 2: Fingerprint and lookup in AcoustID (structured so an
|
||||
# actual error — invalid key / rate limit / no chromaprint — is
|
||||
# reported distinctly from a genuine no-match, instead of both
|
||||
# silently surfacing as "Skipped").
|
||||
logger.info(f"Fingerprinting and looking up: {audio_file_path}")
|
||||
acoustid_result = self.acoustid_client.fingerprint_and_lookup(audio_file_path)
|
||||
lookup = self.acoustid_client.lookup_with_status(audio_file_path) or {}
|
||||
status = lookup.get('status')
|
||||
# Infer status by content when absent (a caller/stub that returned
|
||||
# just recordings): recordings => matched, none => no match.
|
||||
if status is None:
|
||||
status = 'ok' if lookup.get('recordings') else 'no_match'
|
||||
|
||||
if not acoustid_result:
|
||||
return VerificationResult.SKIP, "Track not found in AcoustID database"
|
||||
if status in ('error', 'no_backend', 'fingerprint_error', 'unavailable'):
|
||||
# Something is broken (not the track's fault) — never quarantine
|
||||
# on this; surface it so the user can fix it.
|
||||
return VerificationResult.ERROR, lookup.get('error', 'AcoustID lookup failed')
|
||||
|
||||
if status != 'ok':
|
||||
# no_match / unsupported / not_found — genuinely could not verify.
|
||||
return VerificationResult.SKIP, lookup.get('error', 'No match in AcoustID database')
|
||||
|
||||
acoustid_result = lookup
|
||||
recordings = acoustid_result.get('recordings', [])
|
||||
best_score = acoustid_result.get('best_score', 0)
|
||||
|
||||
if not recordings:
|
||||
return VerificationResult.SKIP, "AcoustID returned no recordings"
|
||||
return VerificationResult.SKIP, "No match in AcoustID database"
|
||||
|
||||
logger.debug(
|
||||
f"AcoustID returned {len(recordings)} recording(s) "
|
||||
|
|
|
|||
|
|
@ -140,7 +140,7 @@ class TestIssue442Regression:
|
|||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
# AcoustID returns the recording with kanji artist
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'YAMANAIAME', 'artist': '澤野弘之', 'mbid': 'rec-x'},
|
||||
|
|
@ -165,7 +165,7 @@ class TestIssue442Regression:
|
|||
"""Reporter's case 2 — Sergey Lazarev / Сергей Лазарев."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'On the Other Side', 'artist': 'Sergey Lazarev', 'mbid': 'rec-y'},
|
||||
|
|
@ -196,7 +196,7 @@ class TestBackwardCompat:
|
|||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
# Wrong artist entirely — Latin script both sides, sim ~0
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Some Track', 'artist': 'Khalil Turk & Friends'},
|
||||
|
|
@ -214,7 +214,7 @@ class TestBackwardCompat:
|
|||
"""Exact title + artist match → PASS regardless of aliases."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Dirty White Boy', 'artist': 'Foreigner'},
|
||||
|
|
@ -233,7 +233,7 @@ class TestBackwardCompat:
|
|||
verifier, fake_service = stubbed_verifier
|
||||
fake_service.lookup_artist_aliases.side_effect = Exception("MB down")
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Dirty White Boy', 'artist': 'Foreigner'},
|
||||
|
|
@ -261,7 +261,7 @@ class TestAliasLookupCalledOncePerVerify:
|
|||
fire 60+ MB lookups (cached or not, that's wasteful)."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'X', 'artist': '澤野弘之'},
|
||||
|
|
@ -295,7 +295,7 @@ class TestLazyAliasResolution:
|
|||
this PR."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Dirty White Boy', 'artist': 'Foreigner'},
|
||||
|
|
@ -317,7 +317,7 @@ class TestLazyAliasResolution:
|
|||
as expected."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'YAMANAIAME', 'artist': '澤野弘之'},
|
||||
|
|
@ -343,7 +343,7 @@ class TestLazyAliasResolution:
|
|||
# Force a code path that hits multiple sites: title matches
|
||||
# several recordings but the best-match's artist sim is below
|
||||
# threshold (forces secondary scan path).
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
verifier.acoustid_client.lookup_with_status.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'X', 'artist': 'Different Latin Artist'}, # 0 alias hit
|
||||
|
|
|
|||
180
tests/test_acoustid_error_reporting.py
Normal file
180
tests/test_acoustid_error_reporting.py
Normal file
|
|
@ -0,0 +1,180 @@
|
|||
"""AcoustID error-vs-no-match reporting.
|
||||
|
||||
Regression for the masking bug: an invalid API key (and other lookup errors)
|
||||
used to collapse into the same `None` as a genuine no-match, so the UI showed a
|
||||
benign "Skipped" and the "Test API key" button reported a dead key as valid.
|
||||
These tests pin the distinction end to end:
|
||||
- lookup_with_status separates ok / no_match / error / no_backend / unavailable
|
||||
- fingerprint_and_lookup (legacy) stays dict-or-None
|
||||
- verify_audio_file -> ERROR for a real error, SKIP for a genuine no-match
|
||||
- test_api_key reports an invalid key (API error code 4) as invalid
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
import core.acoustid_client as acc
|
||||
from core.acoustid_client import AcoustIDClient
|
||||
from core.acoustid_verification import AcoustIDVerification, VerificationResult
|
||||
|
||||
|
||||
# ── lookup_with_status: structured status distinction ──────────────────────
|
||||
|
||||
def _client_with_fake_acoustid(monkeypatch, *, match=None, raises=None):
|
||||
"""An AcoustIDClient wired to a fake `acoustid` module so we can drive
|
||||
match() without network or chromaprint."""
|
||||
fake = types.ModuleType("acoustid")
|
||||
|
||||
class WebServiceError(Exception):
|
||||
pass
|
||||
|
||||
class NoBackendError(Exception):
|
||||
pass
|
||||
|
||||
class FingerprintGenerationError(Exception):
|
||||
pass
|
||||
|
||||
fake.WebServiceError = WebServiceError
|
||||
fake.NoBackendError = NoBackendError
|
||||
fake.FingerprintGenerationError = FingerprintGenerationError
|
||||
|
||||
def _match(api_key, audio_file, parse=True):
|
||||
if raises is not None:
|
||||
raise raises
|
||||
return match or []
|
||||
|
||||
fake.match = _match
|
||||
monkeypatch.setitem(sys.modules, "acoustid", fake)
|
||||
monkeypatch.setattr(acc, "ACOUSTID_AVAILABLE", True)
|
||||
|
||||
c = AcoustIDClient()
|
||||
c._api_key = "testkey123" # bypass config
|
||||
return c, fake
|
||||
|
||||
|
||||
def test_lookup_status_ok(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"not audio") # mutagen -> None, channel check skipped
|
||||
c, _ = _client_with_fake_acoustid(monkeypatch, match=[(0.95, "mbid-1", "Title", "Artist")])
|
||||
res = c.lookup_with_status(str(f))
|
||||
assert res["status"] == "ok"
|
||||
assert res["recordings"] and res["recordings"][0]["mbid"] == "mbid-1"
|
||||
|
||||
|
||||
def test_lookup_status_no_match(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"not audio")
|
||||
c, _ = _client_with_fake_acoustid(monkeypatch, match=[])
|
||||
res = c.lookup_with_status(str(f))
|
||||
assert res["status"] == "no_match"
|
||||
assert res["recordings"] == []
|
||||
|
||||
|
||||
def test_lookup_status_error_on_webservice(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"not audio")
|
||||
c, fake = _client_with_fake_acoustid(monkeypatch)
|
||||
# invalid key surfaces (old pyacoustid) as the bare "status: error"
|
||||
monkeypatch.setattr(c, "_api_key", "testkey123")
|
||||
|
||||
def _raise(*a, **k):
|
||||
raise fake.WebServiceError("status: error")
|
||||
fake.match = _raise
|
||||
|
||||
res = c.lookup_with_status(str(f))
|
||||
assert res["status"] == "error"
|
||||
assert res["invalid_key"] is True
|
||||
|
||||
|
||||
def test_lookup_status_no_backend(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"not audio")
|
||||
c, fake = _client_with_fake_acoustid(monkeypatch)
|
||||
|
||||
def _raise(*a, **k):
|
||||
raise fake.NoBackendError()
|
||||
fake.match = _raise
|
||||
|
||||
assert c.lookup_with_status(str(f))["status"] == "no_backend"
|
||||
|
||||
|
||||
def test_lookup_status_unavailable_without_key(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"x")
|
||||
monkeypatch.setattr(acc, "ACOUSTID_AVAILABLE", True)
|
||||
c = AcoustIDClient()
|
||||
c._api_key = "" # no key
|
||||
assert c.lookup_with_status(str(f))["status"] == "unavailable"
|
||||
|
||||
|
||||
# ── fingerprint_and_lookup keeps its dict-or-None contract ─────────────────
|
||||
|
||||
def test_legacy_wrapper_returns_dict_on_match(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"not audio")
|
||||
c, _ = _client_with_fake_acoustid(monkeypatch, match=[(0.9, "mbid-1", "T", "A")])
|
||||
out = c.fingerprint_and_lookup(str(f))
|
||||
assert out is not None and out["recordings"][0]["mbid"] == "mbid-1"
|
||||
|
||||
|
||||
def test_legacy_wrapper_returns_none_on_error(tmp_path, monkeypatch):
|
||||
f = tmp_path / "a.bin"; f.write_bytes(b"not audio")
|
||||
c, fake = _client_with_fake_acoustid(monkeypatch)
|
||||
|
||||
def _raise(*a, **k):
|
||||
raise fake.WebServiceError("status: error")
|
||||
fake.match = _raise
|
||||
assert c.fingerprint_and_lookup(str(f)) is None
|
||||
|
||||
|
||||
# ── verify_audio_file: ERROR vs SKIP ───────────────────────────────────────
|
||||
|
||||
def _verifier_with_lookup(result):
|
||||
v = AcoustIDVerification()
|
||||
client = MagicMock()
|
||||
client.is_available.return_value = (True, "ready")
|
||||
client.lookup_with_status.return_value = result
|
||||
v.acoustid_client = client
|
||||
return v
|
||||
|
||||
|
||||
def test_verify_reports_error_for_api_error():
|
||||
v = _verifier_with_lookup({"status": "error", "recordings": [], "error": "AcoustID API error: invalid"})
|
||||
result, msg = v.verify_audio_file("/x.flac", "Song", "Artist")
|
||||
assert result == VerificationResult.ERROR
|
||||
assert "error" in msg.lower() or "invalid" in msg.lower()
|
||||
|
||||
|
||||
def test_verify_reports_skip_for_no_match():
|
||||
v = _verifier_with_lookup({"status": "no_match", "recordings": [], "error": "Track not found in AcoustID database"})
|
||||
result, msg = v.verify_audio_file("/x.flac", "Song", "Artist")
|
||||
assert result == VerificationResult.SKIP
|
||||
assert "no match" in msg.lower() or "not found" in msg.lower()
|
||||
|
||||
|
||||
# ── test_api_key: invalid key reported as invalid ──────────────────────────
|
||||
|
||||
def _api_response(payload):
|
||||
r = MagicMock()
|
||||
r.json.return_value = payload
|
||||
return r
|
||||
|
||||
|
||||
def test_test_api_key_invalid_when_code_4(monkeypatch):
|
||||
c = AcoustIDClient()
|
||||
c._api_key = "badkey"
|
||||
with patch("requests.get",
|
||||
return_value=_api_response({"status": "error", "error": {"code": 4, "message": "invalid API key"}})):
|
||||
ok, msg = c.test_api_key()
|
||||
assert ok is False
|
||||
assert "invalid" in msg.lower()
|
||||
|
||||
|
||||
def test_test_api_key_valid_when_accepted(monkeypatch):
|
||||
c = AcoustIDClient()
|
||||
c._api_key = "goodkey"
|
||||
# A non-key error (e.g. bad dummy fingerprint) means the key was accepted.
|
||||
with patch("requests.get",
|
||||
return_value=_api_response({"status": "error", "error": {"code": 3, "message": "invalid fingerprint"}})):
|
||||
ok, msg = c.test_api_key()
|
||||
assert ok is True
|
||||
assert "valid" in msg.lower()
|
||||
|
|
@ -52,7 +52,7 @@ def verifier(monkeypatch):
|
|||
def is_available(self):
|
||||
return True, 'available'
|
||||
|
||||
def fingerprint_and_lookup(self, path):
|
||||
def lookup_with_status(self, path):
|
||||
# Each test injects its own desired return value via
|
||||
# monkeypatch on this method; default is empty.
|
||||
return None
|
||||
|
|
@ -62,8 +62,8 @@ def verifier(monkeypatch):
|
|||
|
||||
|
||||
def _stub_lookup(verifier, *, recordings, best_score):
|
||||
"""Make `fingerprint_and_lookup` return a fabricated AcoustID result."""
|
||||
verifier.acoustid_client.fingerprint_and_lookup = lambda path: {
|
||||
"""Make `lookup_with_status` return a fabricated AcoustID result."""
|
||||
verifier.acoustid_client.lookup_with_status = lambda path: {
|
||||
'recordings': recordings,
|
||||
'best_score': best_score,
|
||||
'recording_mbids': [r.get('id') for r in recordings if r.get('id')],
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ def verifier():
|
|||
def is_available(self):
|
||||
return True, "available"
|
||||
|
||||
def fingerprint_and_lookup(self, path):
|
||||
def lookup_with_status(self, path):
|
||||
return None # tests inject via _stub_lookup below
|
||||
|
||||
v.acoustid_client = _StubClient()
|
||||
|
|
@ -46,7 +46,7 @@ def verifier():
|
|||
|
||||
|
||||
def _stub_lookup(verifier, *, recordings, best_score=0.95):
|
||||
verifier.acoustid_client.fingerprint_and_lookup = lambda path: {
|
||||
verifier.acoustid_client.lookup_with_status = lambda path: {
|
||||
"recordings": recordings,
|
||||
"best_score": best_score,
|
||||
"recording_mbids": [r.get("id") for r in recordings if r.get("id")],
|
||||
|
|
|
|||
Loading…
Reference in a new issue