c557d9196e
Three changes tightening the controller before opening the PR. DROP MAGIC `extractItems` DEFAULTS Controller used to auto-pull `data.items` / `data.albums` / `data.artists` / `data.tracks` / `data.results` when no extractor was supplied. Removed the fallback chain — every section now MUST provide an explicit `extractItems(data) => array`. Validated at register-time so misuse fails immediately, not silently on first load against an endpoint that happened to return two arrays. Cin standard: explicit > implicit. Magic key-grabbing could pick the wrong one in edge cases (e.g. an endpoint returning both `data.albums` and `data.results` would have grabbed albums when the section actually wanted results). All 10 existing controller call sites already passed explicit extractors, so no migration churn — this is purely tightening the contract for future sections. REPLACE `renderItems` NULL-RETURN CONVENTION WITH `manualDom: true` Your Albums and similar sections that delegate to existing renderers that target a CHILD element of `contentEl` used to signal "leave the container alone" by returning null/undefined from `renderItems`. That convention is easy to confuse with an accidental missing-return error. Replaced with an explicit `manualDom: true` config flag. Renderer is still called for its side-effects, controller just skips the innerHTML swap. Clearer intent at the call site. Updated `loadYourAlbums` to use the new flag. PIN THE CONTROLLER CONTRACT WITH JS TESTS Added `tests/static/test_discover_section_controller.mjs` — 32 tests covering the controller's lifecycle contract: - Config validation (every required field, mutual exclusivity of fetchUrl/data, type checks on contentEl) - Happy-path fetch → parse → render - Empty state (default empty render, hideWhenEmpty + sectionEl, success=false treated as empty, custom isSuccess override) - Stale state (fires when isStale returns true, wins over empty, custom renderStale override) - Error state (HTTP non-ok, fetch throws, showErrorToast fires window.showToast, default off doesn't fire) - No-fetch `data:` mode (value + function form, doesn't call fetch) - manualDom mode (skips innerHTML swap, still calls renderer) - Callable `fetchUrl` (resolved at load time, refresh re-resolves) - Load coalescing (concurrent loads share one fetch) - Refresh bypasses coalescing (re-fires fetch every call) - Hook error containment (throwing renderer/onSuccess hooks don't crash the controller) Runs via Node's stable built-in `--test` runner — no package.json, no jest/vitest dependency, no compile step. Just `node --test`. Pytest wrapper at `tests/test_discover_section_controller_js.py` shells out to node and asserts clean exit, so the JS tests fail the regular pytest sweep if the controller contract drifts. Skipped gracefully when node isn't available or is < 22. Closes the "controller is a contract, pin it at the test boundary" gap that Cin would have flagged on review. VERIFICATION - 2205/2205 full pytest suite green (was 2204 + 1 new wrapper) - 32/32 `node --test` pass on the controller test file directly - ruff clean - node --check clean on all touched JS files
76 lines
2.3 KiB
Python
76 lines
2.3 KiB
Python
"""Run the JS tests for `webui/static/discover-section-controller.js`
|
|
under the regular pytest sweep.
|
|
|
|
The actual contract tests live in
|
|
`tests/static/test_discover_section_controller.mjs` and run via
|
|
Node.js's stable built-in test runner (`node --test`). This shim
|
|
shells out to that runner and asserts a clean exit so the JS tests
|
|
fail the suite if the controller contract drifts.
|
|
|
|
Skipped when:
|
|
- `node` isn't on PATH (e.g. Python-only dev container).
|
|
- Node version < 22 (the built-in `--test` runner went stable in 18
|
|
but the assert-flavor we use is 22+).
|
|
|
|
Run directly:
|
|
node --test tests/static/test_discover_section_controller.mjs
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import shutil
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
|
|
_REPO_ROOT = Path(__file__).resolve().parents[1]
|
|
_TEST_FILE = _REPO_ROOT / "tests" / "static" / "test_discover_section_controller.mjs"
|
|
|
|
|
|
def _node_available() -> bool:
|
|
if not shutil.which("node"):
|
|
return False
|
|
try:
|
|
result = subprocess.run(
|
|
["node", "--version"],
|
|
capture_output=True, text=True, timeout=10,
|
|
)
|
|
except (subprocess.SubprocessError, FileNotFoundError):
|
|
return False
|
|
if result.returncode != 0:
|
|
return False
|
|
# Output looks like "v22.21.0"
|
|
raw = (result.stdout or "").strip().lstrip("v")
|
|
try:
|
|
major = int(raw.split(".")[0])
|
|
except (ValueError, IndexError):
|
|
return False
|
|
return major >= 22
|
|
|
|
|
|
def test_discover_section_controller_js():
|
|
"""Pin the JS controller's lifecycle contract via `node --test`."""
|
|
if not _node_available():
|
|
pytest.skip("Node.js >= 22 required to run the JS controller tests")
|
|
|
|
if not _TEST_FILE.exists():
|
|
pytest.skip(f"JS test file missing: {_TEST_FILE}")
|
|
|
|
result = subprocess.run(
|
|
["node", "--test", str(_TEST_FILE)],
|
|
capture_output=True, text=True,
|
|
cwd=str(_REPO_ROOT),
|
|
timeout=60,
|
|
)
|
|
|
|
if result.returncode != 0:
|
|
# Surface the node test runner output so failures are
|
|
# debuggable from the pytest log without re-running by hand.
|
|
pytest.fail(
|
|
"JS controller tests failed:\n\n"
|
|
f"--- stdout ---\n{result.stdout}\n"
|
|
f"--- stderr ---\n{result.stderr}",
|
|
pytrace=False,
|
|
)
|