e4b5cbbe60
The auto and manual wishlist flows each built the same ~20-field
download_batches row in separate places (auto album, auto residual, manual
placeholder, manual sub-batches) — four near-identical literals that could (and
did) drift apart, producing subtly different batch shapes between the flows.
Extract make_wishlist_batch_row() as the single source of truth: it emits the
consistent core field set, with the genuinely per-flow differences as explicit
arguments — initial phase ('queued' for auto / 'analysis' for manual), the
auto-only auto_initiated/auto_processing_timestamp/current_cycle via
extra_fields, and album-vs-residual contexts. All four sites now go through it,
so every wishlist batch has an IDENTICAL shape (this also removes the field
drift that confused the modal-hydration code).
Deliberately NOT unified — and left explicit in each caller, per the
'don't cargo-cult genuinely-different code' principle: the grouping decision
(auto groups only on the albums cycle), batch-id allocation (manual reuses the
caller's placeholder id for the first sub-batch), and dispatch (auto
parallel-submits album batches to the dedicated pool + residual to the shared
pool; manual runs them serially on one thread). Those are real behavioral
differences, not duplication.
Behavior-preserving: verified safe to normalize the row shape (grep confirmed
every reader uses .get() with defaults, no key-presence checks). The existing
auto (test_automation.py) and manual (test_manual_download.py) characterization
suites stay green = differential proof of identical behavior. Adds
test_batch_factory.py (core fields, album/residual, extra_fields, no shared
mutable state, consistent key shape). 131 wishlist tests pass.
102 lines
3.8 KiB
Python
102 lines
3.8 KiB
Python
"""Tests for make_wishlist_batch_row — the single source of truth for a wishlist
|
|
download_batches row, shared by the auto and manual flows so their batch shapes
|
|
can't drift apart.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from core.wishlist.processing import make_wishlist_batch_row
|
|
|
|
|
|
_CORE_KEYS = {
|
|
'phase', 'playlist_id', 'playlist_name', 'queue', 'active_count',
|
|
'max_concurrent', 'queue_index', 'analysis_total', 'analysis_processed',
|
|
'analysis_results', 'permanently_failed_tracks', 'cancelled_tracks',
|
|
'force_download_all', 'profile_id', 'is_album_download', 'album_context',
|
|
'artist_context', 'wishlist_run_id',
|
|
}
|
|
|
|
|
|
def _row(**overrides):
|
|
base = dict(
|
|
playlist_id='wishlist', playlist_name='Wishlist', track_count=3,
|
|
max_concurrent=4, profile_id=1, phase='analysis',
|
|
)
|
|
base.update(overrides)
|
|
return make_wishlist_batch_row(**base)
|
|
|
|
|
|
def test_core_fields_always_present_and_consistent():
|
|
row = _row()
|
|
assert _CORE_KEYS <= set(row.keys())
|
|
# Fresh-batch invariants.
|
|
assert row['queue'] == [] and row['active_count'] == 0 and row['queue_index'] == 0
|
|
assert row['analysis_processed'] == 0
|
|
assert row['analysis_results'] == [] and row['permanently_failed_tracks'] == []
|
|
assert row['cancelled_tracks'] == set()
|
|
assert row['force_download_all'] is True
|
|
assert row['analysis_total'] == 3
|
|
assert row['max_concurrent'] == 4
|
|
assert row['profile_id'] == 1
|
|
|
|
|
|
def test_residual_defaults_are_per_track():
|
|
row = _row()
|
|
assert row['is_album_download'] is False
|
|
assert row['album_context'] is None and row['artist_context'] is None
|
|
assert row['wishlist_run_id'] is None
|
|
|
|
|
|
def test_album_batch_carries_context():
|
|
row = _row(
|
|
phase='queued', run_id='run-1', is_album=True,
|
|
album_context={'name': 'Album One'}, artist_context={'name': 'Artist 1'},
|
|
)
|
|
assert row['phase'] == 'queued'
|
|
assert row['is_album_download'] is True
|
|
assert row['album_context'] == {'name': 'Album One'}
|
|
assert row['artist_context'] == {'name': 'Artist 1'}
|
|
assert row['wishlist_run_id'] == 'run-1'
|
|
|
|
|
|
def test_extra_fields_merged_for_auto():
|
|
row = _row(extra_fields={
|
|
'auto_initiated': True, 'auto_processing_timestamp': 123.0,
|
|
'current_cycle': 'albums',
|
|
})
|
|
assert row['auto_initiated'] is True
|
|
assert row['auto_processing_timestamp'] == 123.0
|
|
assert row['current_cycle'] == 'albums'
|
|
|
|
|
|
def test_manual_row_has_no_auto_fields():
|
|
"""Manual rows must not carry the auto-only fields (no extra_fields)."""
|
|
row = _row(phase='analysis')
|
|
assert 'auto_initiated' not in row
|
|
assert 'current_cycle' not in row
|
|
|
|
|
|
def test_fresh_rows_do_not_share_mutable_state():
|
|
"""Each row must get its OWN queue/list/set — not a shared reference that
|
|
one batch's tasks could leak into another's."""
|
|
a = _row()
|
|
b = _row()
|
|
a['queue'].append('task-1')
|
|
a['cancelled_tracks'].add('x')
|
|
assert b['queue'] == []
|
|
assert b['cancelled_tracks'] == set()
|
|
assert b['analysis_results'] == []
|
|
|
|
|
|
def test_auto_and_manual_rows_share_identical_key_shape():
|
|
"""The drift-prevention guarantee: an auto album row and a manual album row
|
|
expose the same set of keys (modulo the auto-only extras), so the modal /
|
|
status code sees a consistent shape from both flows."""
|
|
manual = _row(phase='analysis', run_id='r', is_album=True,
|
|
album_context={'name': 'A'}, artist_context={'name': 'B'})
|
|
auto = _row(phase='queued', run_id='r', is_album=True,
|
|
album_context={'name': 'A'}, artist_context={'name': 'B'},
|
|
extra_fields={'auto_initiated': True, 'current_cycle': 'albums'})
|
|
# Auto is a strict superset (the auto-only extras); the shared core is identical.
|
|
assert set(manual.keys()) <= set(auto.keys())
|
|
assert set(auto.keys()) - set(manual.keys()) == {'auto_initiated', 'current_cycle'}
|