c4c922c40f
Two findings from JohnBaumb on the engine refactor.
(1) Every download client returned None when self._engine was None,
just logging an error. The orchestrator's download_with_fallback
treated None as "source declined", so the user got no feedback —
download silently disappeared. Now each client raises a RuntimeError
on the engine-not-wired path. download_with_fallback already catches
plugin exceptions, logs a warning, and tries the next source — so
the visible behavior is "real error in logs + fallback to next
source" instead of "silent drop". Six clients touched (deezer, hifi,
qobuz, soundcloud, tidal, youtube). Pinning tests updated to expect
raise.
(2) Monitor's engine.get_all_downloads() walked every plugin
including soulseek, but the same monitor loop already pulled slskd
transfers via the transfers/downloads endpoint a few lines earlier —
soulseek's records were being fetched twice per tick. Same issue in
web_server.py's get_cached_transfer_data path. Added an exclude
parameter to engine.get_all_downloads(); both call sites now pass
('soulseek',). New test pins the exclude semantic.
Also fixed a stray 8-space over-indent on the for-loop body in
get_cached_transfer_data (cosmetic, JohnBaumb flagged the same
pattern in monitor.py earlier).
131 lines
4.5 KiB
Python
131 lines
4.5 KiB
Python
"""Phase A pinning tests for SoundcloudClient — UPDATED for Phase C7.
|
|
|
|
SoundCloud's quirk: 3-part filename `track_id||permalink_url||display_name`
|
|
because yt-dlp consumes the URL, not the track_id, to actually download.
|
|
The engine record holds both fields so the worker can call
|
|
`_download_sync(download_id, permalink_url, display_name)` correctly.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import threading
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
|
|
from core.download_engine import DownloadEngine
|
|
from core.soundcloud_client import SoundcloudClient
|
|
|
|
|
|
def _run_async(coro):
|
|
loop = asyncio.new_event_loop()
|
|
try:
|
|
return loop.run_until_complete(coro)
|
|
finally:
|
|
loop.close()
|
|
|
|
|
|
@pytest.fixture
|
|
def sc_client_with_engine():
|
|
client = SoundcloudClient.__new__(SoundcloudClient)
|
|
client.download_path = Path('./test_sc_downloads')
|
|
client.shutdown_check = None
|
|
client._engine = None
|
|
engine = DownloadEngine()
|
|
client.set_engine(engine)
|
|
return client, engine
|
|
|
|
|
|
def test_download_returns_none_for_filename_with_too_few_parts(sc_client_with_engine):
|
|
client, _ = sc_client_with_engine
|
|
result = _run_async(client.download('soundcloud', 'just-id-no-url', 0))
|
|
assert result is None
|
|
|
|
|
|
def test_download_returns_none_for_empty_track_id_or_url(sc_client_with_engine):
|
|
client, _ = sc_client_with_engine
|
|
assert _run_async(client.download('soundcloud', '||https://x.com/y', 0)) is None
|
|
assert _run_async(client.download('soundcloud', 'track123||', 0)) is None
|
|
|
|
|
|
def test_download_raises_when_engine_not_wired():
|
|
"""Defensive: client without engine reference must raise so the
|
|
orchestrator's download_with_fallback surfaces the error and
|
|
moves on to the next source. Returning None silently would drop
|
|
the download with no user feedback (per JohnBaumb)."""
|
|
import pytest
|
|
client = SoundcloudClient.__new__(SoundcloudClient)
|
|
client._engine = None
|
|
with pytest.raises(RuntimeError, match="engine reference"):
|
|
_run_async(client.download('soundcloud', 'v||t', 0))
|
|
|
|
|
|
def test_download_accepts_three_part_filename_with_display(sc_client_with_engine):
|
|
"""Pinning: 3-part filename `track_id||permalink_url||display`
|
|
is the canonical form. All three fields go into the engine record."""
|
|
client, engine = sc_client_with_engine
|
|
started = threading.Event()
|
|
release = threading.Event()
|
|
|
|
def slow_impl(*args, **kwargs):
|
|
started.set()
|
|
release.wait(timeout=1.0)
|
|
return '/tmp/done.mp3'
|
|
|
|
with patch.object(client, '_download_sync', side_effect=slow_impl):
|
|
download_id = _run_async(client.download(
|
|
'soundcloud',
|
|
'sc-12345||https://soundcloud.com/artist/song||Some Display Title',
|
|
0,
|
|
))
|
|
started.wait(timeout=1.0)
|
|
record = engine.get_record('soundcloud', download_id)
|
|
|
|
assert record['track_id'] == 'sc-12345'
|
|
assert record['permalink_url'] == 'https://soundcloud.com/artist/song'
|
|
assert record['display_name'] == 'Some Display Title'
|
|
release.set()
|
|
|
|
|
|
def test_download_falls_back_display_name_to_track_id_when_two_part(sc_client_with_engine):
|
|
"""Pinning: 2-part filename → display name = track_id."""
|
|
client, engine = sc_client_with_engine
|
|
started = threading.Event()
|
|
release = threading.Event()
|
|
|
|
def slow_impl(*args, **kwargs):
|
|
started.set()
|
|
release.wait(timeout=1.0)
|
|
return '/tmp/x.mp3'
|
|
|
|
with patch.object(client, '_download_sync', side_effect=slow_impl):
|
|
download_id = _run_async(client.download(
|
|
'soundcloud', 'sc-99||https://soundcloud.com/x/y', 0,
|
|
))
|
|
started.wait(timeout=1.0)
|
|
assert engine.get_record('soundcloud', download_id)['display_name'] == 'sc-99'
|
|
release.set()
|
|
|
|
|
|
def test_download_populates_engine_record_with_initial_state(sc_client_with_engine):
|
|
client, engine = sc_client_with_engine
|
|
started = threading.Event()
|
|
release = threading.Event()
|
|
|
|
def slow_impl(*args, **kwargs):
|
|
started.set()
|
|
release.wait(timeout=1.0)
|
|
return '/tmp/x.mp3'
|
|
|
|
with patch.object(client, '_download_sync', side_effect=slow_impl):
|
|
download_id = _run_async(client.download(
|
|
'soundcloud', 'sc-1||https://soundcloud.com/x||Title', 0,
|
|
))
|
|
started.wait(timeout=1.0)
|
|
record = engine.get_record('soundcloud', download_id)
|
|
assert record['username'] == 'soundcloud'
|
|
assert record['state'] in ('Initializing', 'InProgress, Downloading')
|
|
assert 'permalink_url' in record
|
|
release.set()
|