The #681 commit (eba7f61e) collapsed the inline duplicate card-renderer
inside importPageSearchAlbum into a single _renderSuggestionCard call.
The structural test for the issue #524 lookup-cache pattern was still
counting inline cache writes and expecting >=2, which started failing
in CI now that the search-results renderer routes through the shared
helper.
Rewritten to assert the actual invariant of the consolidated design:
* _renderSuggestionCard contains exactly one _albumLookup write
* No other inline write exists (a second write means a caller is
re-implementing the renderer instead of calling the helper — the
exact duplication the #524 fix consolidated away)
Same regression guard, matches the new architecture.
132 lines
5.6 KiB
Python
132 lines
5.6 KiB
Python
"""Pin the import-page album-lookup cache pattern in
|
|
``webui/static/stats-automations.js`` — github issue #524 regression
|
|
guard at the source-text level.
|
|
|
|
Why a structural test instead of a behavioral JS test:
|
|
|
|
``stats-automations.js`` is a ~7k-line file with a lot of global state
|
|
+ inline DOM rendering. Loading it into a sandboxed Node `vm` context
|
|
(the pattern used in `tests/static/test_discover_section_controller.mjs`)
|
|
would require stubbing dozens of unrelated dependencies. The file
|
|
needs to be modularized before behavioral tests are practical for
|
|
arbitrary functions in it.
|
|
|
|
Until then, this test fails the suite if the critical pattern from
|
|
the #524 fix gets removed:
|
|
|
|
1. The album cache (``_albumLookup`` field on ``importPageState``)
|
|
2. Card renderers populating the cache before emitting the onclick
|
|
3. The match-POST builder reading source/name/artist from the cache
|
|
|
|
If anyone deletes the cache, the click handler, or the cache writes,
|
|
this test catches it before the regression ships.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
|
|
_REPO_ROOT = Path(__file__).resolve().parents[1]
|
|
_SOURCE = _REPO_ROOT / "webui" / "static" / "stats-automations.js"
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def js_source() -> str:
|
|
return _SOURCE.read_text(encoding="utf-8")
|
|
|
|
|
|
def test_album_lookup_cache_field_exists_on_state(js_source: str):
|
|
"""importPageState must have an `_albumLookup` field. Without it,
|
|
card renderers have nowhere to stash source/name/artist for the
|
|
click handler to read."""
|
|
assert "_albumLookup:" in js_source, (
|
|
"importPageState._albumLookup field missing — the album cache "
|
|
"that backs the source-routing fix for issue #524 has been "
|
|
"removed. The click handler will fall back to passing only "
|
|
"album_id and the backend will silently misroute lookups again."
|
|
)
|
|
|
|
|
|
def test_select_album_handler_reads_cache(js_source: str):
|
|
"""importPageSelectAlbum must read source / name / artist from
|
|
the cache and include them in the match POST body. The whole
|
|
point of the fix."""
|
|
# Find the function body
|
|
match = re.search(
|
|
r"async function importPageSelectAlbum\([^)]*\) \{(.*?)^\}",
|
|
js_source, re.DOTALL | re.MULTILINE,
|
|
)
|
|
assert match, "importPageSelectAlbum function not found"
|
|
body = match.group(1)
|
|
|
|
# Must read from the lookup cache
|
|
assert "_albumLookup[" in body, (
|
|
"importPageSelectAlbum no longer reads from "
|
|
"importPageState._albumLookup — match POST will drop source "
|
|
"again, see issue #524."
|
|
)
|
|
|
|
# Must build a matchBody that includes source + album_name + album_artist
|
|
for required_field in ("source:", "album_name:", "album_artist:"):
|
|
assert required_field in body, (
|
|
f"matchBody missing required field {required_field!r}. "
|
|
"Backend's get_artist_album_tracks needs source to route "
|
|
"the lookup to the correct metadata client. Without it, "
|
|
"cross-source album_ids fall through to the failure-fallback "
|
|
"dict (Unknown Artist / album_id-as-title / 0 tracks). "
|
|
"See issue #524 for the original symptom."
|
|
)
|
|
|
|
|
|
def test_card_renderer_populates_cache_before_onclick(js_source: str):
|
|
"""The shared card-renderer ``_renderSuggestionCard`` must write to
|
|
``_albumLookup`` before emitting the onclick — otherwise the click
|
|
handler reads an empty cache for newly-displayed albums.
|
|
|
|
Originally this test required >=2 cache writes (one per inline
|
|
renderer), but the search-results inline render was consolidated
|
|
into a single ``_renderSuggestionCard`` call as part of the #681
|
|
fix. The invariant now is: the shared renderer populates the cache,
|
|
and every render call site goes through it (no inline duplicates)."""
|
|
# 1. The shared renderer must contain the cache write.
|
|
match = re.search(
|
|
r"function _renderSuggestionCard\([^)]*\) \{(.*?)^\}",
|
|
js_source, re.DOTALL | re.MULTILINE,
|
|
)
|
|
assert match, "_renderSuggestionCard function not found"
|
|
body = match.group(1)
|
|
assert re.search(r"_albumLookup\[a\.id\]\s*=\s*\{", body), (
|
|
"_renderSuggestionCard no longer writes to _albumLookup before "
|
|
"emitting the onclick — every card rendered through this helper "
|
|
"would have an empty cache on click, regressing issue #524."
|
|
)
|
|
|
|
# 2. No inline card render allowed outside the shared helper.
|
|
# A second `_albumLookup[a.id] = {` write means a caller is
|
|
# re-implementing the renderer instead of calling the helper —
|
|
# that's exactly the duplication the #524 fix consolidated away.
|
|
cache_writes = re.findall(r"_albumLookup\[a\.id\]\s*=\s*\{", js_source)
|
|
assert len(cache_writes) == 1, (
|
|
f"Expected exactly 1 _albumLookup write (inside _renderSuggestionCard), "
|
|
f"found {len(cache_writes)}. A new inline card-render site has "
|
|
"duplicated the cache-write logic — route the new caller through "
|
|
"_renderSuggestionCard(a, primarySource) instead."
|
|
)
|
|
|
|
|
|
def test_cache_entry_carries_source_field(js_source: str):
|
|
"""The cache must store `source:` per entry — not just id/name/artist."""
|
|
write_blocks = re.findall(
|
|
r"_albumLookup\[a\.id\]\s*=\s*\{[^}]*\}",
|
|
js_source,
|
|
)
|
|
assert write_blocks, "no _albumLookup writes found"
|
|
assert any("source:" in block for block in write_blocks), (
|
|
"_albumLookup cache entries must include `source` — that's the "
|
|
"field the click handler forwards to /api/import/album/match "
|
|
"to route the lookup to the correct provider."
|
|
)
|