test_musehub_htmx_migration_final.py
python
| 1 | """Final audit tests for the HTMX migration — issue #587. |
| 2 | |
| 3 | Verifies that the full MuseHub SSR + HTMX + Alpine.js migration is complete: |
| 4 | - No dead ``apiFetch`` calls remain in SSR-migrated page templates. |
| 5 | - All SSR-migrated routes return 200 with HTML content. |
| 6 | - Routes using ``htmx_fragment_or_full`` return bare fragment (no ``<html>``) on |
| 7 | ``HX-Request: true``. |
| 8 | - No legacy inline HTML builders (``_render_*_html`` functions) remain in ui modules. |
| 9 | - The HTMX JWT auth bridge (``htmx:configRequest`` Bearer token injection) is |
| 10 | present in ``musehub.js``. |
| 11 | |
| 12 | Test naming: ``test_<what>_<scenario>``. |
| 13 | |
| 14 | Canvas/audio/visualization pages (arrange, listen, piano_roll, graph, timeline, |
| 15 | analysis sub-pages, etc.) legitimately use ``apiFetch`` for client-side data |
| 16 | rendering and are explicitly exempted from the apiFetch audit. |
| 17 | |
| 18 | Covers: |
| 19 | - test_no_apifetch_in_page_templates |
| 20 | - test_all_page_routes_return_200 (parametrized) |
| 21 | - test_all_page_routes_return_fragment_for_htmx_request (parametrized) |
| 22 | - test_no_inline_html_builders_in_ui_py |
| 23 | - test_musehub_js_has_htmx_config_request_bridge |
| 24 | """ |
| 25 | from __future__ import annotations |
| 26 | |
| 27 | import re |
| 28 | from pathlib import Path |
| 29 | |
| 30 | import pytest |
| 31 | from httpx import AsyncClient |
| 32 | from sqlalchemy.ext.asyncio import AsyncSession |
| 33 | |
| 34 | from musehub.db.musehub_models import MusehubRepo |
| 35 | |
| 36 | # ── Filesystem roots ───────────────────────────────────────────────────────── |
| 37 | |
| 38 | _REPO_ROOT = Path(__file__).parent.parent |
| 39 | _PAGES_DIR = _REPO_ROOT / "musehub" / "templates" / "musehub" / "pages" |
| 40 | _MUSEHUB_JS = _REPO_ROOT / "musehub" / "templates" / "musehub" / "static" / "musehub.js" |
| 41 | _UI_PY = _REPO_ROOT / "musehub" / "api" / "routes" / "musehub" / "ui.py" |
| 42 | _UI_EXTRA = list((_REPO_ROOT / "musehub" / "api" / "routes" / "musehub").glob("ui_*.py")) |
| 43 | |
| 44 | # ── Exempted pages: visualization/canvas/audio — apiFetch is intentional ──── |
| 45 | # |
| 46 | # These pages render charts, piano-roll canvases, or audio waveforms that |
| 47 | # require client-side JS to fetch and render binary/JSON data streams. They |
| 48 | # were NOT part of the SSR listing-page migration (issues #555–#586) and |
| 49 | # their ``apiFetch`` calls are the live data-fetching mechanism, not dead code. |
| 50 | _APIFETCH_EXEMPT_PAGES: frozenset[str] = frozenset( |
| 51 | { |
| 52 | # Canvas / MIDI / audio players |
| 53 | "arrange.html", |
| 54 | "listen.html", |
| 55 | "piano_roll.html", |
| 56 | "embed.html", |
| 57 | "score.html", |
| 58 | # Analysis dashboards — JS fetches JSON for chart rendering |
| 59 | "analysis.html", |
| 60 | "contour.html", |
| 61 | "tempo.html", |
| 62 | "dynamics.html", |
| 63 | "key.html", |
| 64 | "meter.html", |
| 65 | "chord_map.html", |
| 66 | "groove.html", |
| 67 | "groove_check.html", |
| 68 | "emotion.html", |
| 69 | "form.html", |
| 70 | "form_structure.html", |
| 71 | "motifs.html", |
| 72 | # Visualization / interactive graph pages |
| 73 | "graph.html", |
| 74 | "timeline.html", |
| 75 | "compare.html", |
| 76 | "divergence.html", |
| 77 | # Complex pages with audio analysis and inline commenting |
| 78 | "commit.html", |
| 79 | # Feed & discovery pages that remain JS-driven |
| 80 | "feed.html", |
| 81 | # Topics — uses raw fetch for a non-apiFetch endpoint |
| 82 | "topics.html", |
| 83 | # Repo/insights pages with mixed SSR+JS widgets |
| 84 | "insights.html", |
| 85 | "repo_home.html", |
| 86 | # Context viewer — AI JSON fetch |
| 87 | "context.html", |
| 88 | "diff.html", |
| 89 | } |
| 90 | ) |
| 91 | |
| 92 | # ── Test routes ────────────────────────────────────────────────────────────── |
| 93 | # |
| 94 | # Parametrized route table. Each entry is (test_id, url_template). |
| 95 | # ``{O}`` and ``{S}`` are replaced with the seeded owner/slug at runtime. |
| 96 | # |
| 97 | # «no-repo» routes work without any seeded repo. |
| 98 | # «repo» routes require the fixture repo to be seeded first. |
| 99 | |
| 100 | _OWNER = "htmx-test-user" |
| 101 | _SLUG = "migration-audit" |
| 102 | |
| 103 | |
| 104 | def _url(path: str) -> str: |
| 105 | """Expand {O}/{S} placeholders into the fixture owner/slug.""" |
| 106 | return path.replace("{O}", _OWNER).replace("{S}", _SLUG) |
| 107 | |
| 108 | |
| 109 | # All SSR-migrated routes: those that use htmx_fragment_or_full() or |
| 110 | # negotiate_response() and return pre-rendered Jinja2 HTML. |
| 111 | _MIGRATED_ROUTES: list[tuple[str, str]] = [ |
| 112 | # Fixed routes — no repo required |
| 113 | ("explore", "/explore"), |
| 114 | ("trending", "/trending"), |
| 115 | ("global_search", "/search"), |
| 116 | # Repo-scoped listing routes |
| 117 | ("repo_home", "/{O}/{S}"), |
| 118 | ("commits_list", "/{O}/{S}/commits"), |
| 119 | ("pr_list", "/{O}/{S}/pulls"), |
| 120 | ("issue_list", "/{O}/{S}/issues"), |
| 121 | ("releases", "/{O}/{S}/releases"), |
| 122 | ("sessions", "/{O}/{S}/sessions"), |
| 123 | ("activity", "/{O}/{S}/activity"), |
| 124 | ("credits", "/{O}/{S}/credits"), |
| 125 | ("branches", "/{O}/{S}/branches"), |
| 126 | ("tags", "/{O}/{S}/tags"), |
| 127 | ] |
| 128 | |
| 129 | # Subset of _MIGRATED_ROUTES whose handlers call htmx_fragment_or_full() — |
| 130 | # these must return a bare fragment (no <html>) when HX-Request: true. |
| 131 | _HTMX_FRAGMENT_ROUTES: list[tuple[str, str]] = [ |
| 132 | ("explore", "/explore"), |
| 133 | ("trending", "/trending"), |
| 134 | ("global_search", "/search"), |
| 135 | ("repo_home", "/{O}/{S}"), |
| 136 | ("commits_list", "/{O}/{S}/commits"), |
| 137 | ("pr_list", "/{O}/{S}/pulls"), |
| 138 | ("issue_list", "/{O}/{S}/issues"), |
| 139 | ("releases", "/{O}/{S}/releases"), |
| 140 | ("sessions", "/{O}/{S}/sessions"), |
| 141 | ("activity", "/{O}/{S}/activity"), |
| 142 | ("branches", "/{O}/{S}/branches"), |
| 143 | ] |
| 144 | |
| 145 | |
| 146 | # ── Seed helper ────────────────────────────────────────────────────────────── |
| 147 | |
| 148 | |
| 149 | async def _seed_repo(db: AsyncSession) -> str: |
| 150 | """Seed a minimal public repo for route-level tests; return its repo_id.""" |
| 151 | repo = MusehubRepo( |
| 152 | name=_SLUG, |
| 153 | owner=_OWNER, |
| 154 | slug=_SLUG, |
| 155 | visibility="public", |
| 156 | owner_user_id="uid-htmx-audit-owner", |
| 157 | ) |
| 158 | db.add(repo) |
| 159 | await db.commit() |
| 160 | await db.refresh(repo) |
| 161 | return str(repo.repo_id) |
| 162 | |
| 163 | |
| 164 | # ── Static / code-analysis tests (no HTTP calls needed) ───────────────────── |
| 165 | |
| 166 | |
| 167 | def test_no_apifetch_in_page_templates() -> None: |
| 168 | """No SSR-migrated page template may contain a live ``apiFetch`` call. |
| 169 | |
| 170 | Canvas/audio/visualization pages are explicitly exempted (see |
| 171 | ``_APIFETCH_EXEMPT_PAGES``). All other templates in ``pages/`` represent |
| 172 | listing/CRUD pages that must serve data server-side via Jinja2, not via |
| 173 | client-side API calls. |
| 174 | |
| 175 | A failure here means a page template was migrated to SSR but still has a |
| 176 | leftover ``apiFetch`` call that is now dead code. |
| 177 | """ |
| 178 | violations: list[str] = [] |
| 179 | for html_file in sorted(_PAGES_DIR.glob("*.html")): |
| 180 | if html_file.name in _APIFETCH_EXEMPT_PAGES: |
| 181 | continue |
| 182 | content = html_file.read_text() |
| 183 | if "apiFetch(" in content: |
| 184 | # Count occurrences for a clear error message. |
| 185 | count = content.count("apiFetch(") |
| 186 | violations.append(f"{html_file.name}: {count} apiFetch call(s)") |
| 187 | |
| 188 | assert not violations, ( |
| 189 | "Dead apiFetch calls found in SSR-migrated page templates " |
| 190 | "(add to _APIFETCH_EXEMPT_PAGES if the page is legitimately canvas/audio):\n" |
| 191 | + "\n".join(f" • {v}" for v in violations) |
| 192 | ) |
| 193 | |
| 194 | |
| 195 | def test_no_inline_html_builders_in_ui_py() -> None: |
| 196 | """No ``_render_*_html`` function may exist in ui.py or any ui_*.py. |
| 197 | |
| 198 | The old pattern was to build HTML strings in Python (``_render_row_html``, |
| 199 | ``_render_header_html``, etc.) and return them as ``HTMLResponse``. The |
| 200 | SSR migration replaced all of these with Jinja2 template rendering. Any |
| 201 | remaining ``_render_*_html`` definition is a regression. |
| 202 | |
| 203 | Known exception: ``ui_user_profile.py::_render_profile_html`` — the user |
| 204 | profile page is a complex JS-hydrated shell (heatmap, badges, activity tabs) |
| 205 | that was intentionally kept outside the listing-page SSR migration scope. |
| 206 | """ |
| 207 | # Files exempt from this check: they intentionally keep JS-shell patterns |
| 208 | # because they render complex interactive widgets (heatmaps, canvases, etc.) |
| 209 | # that cannot be expressed as static Jinja2 HTML. |
| 210 | _EXEMPT_UI_FILES: frozenset[str] = frozenset({"ui_user_profile.py"}) |
| 211 | |
| 212 | pattern = re.compile(r"\bdef\s+_render_\w+_html\b") |
| 213 | violations: list[str] = [] |
| 214 | for py_file in [_UI_PY] + sorted(_UI_EXTRA): |
| 215 | if py_file.name in _EXEMPT_UI_FILES: |
| 216 | continue |
| 217 | content = py_file.read_text() |
| 218 | matches = pattern.findall(content) |
| 219 | if matches: |
| 220 | violations.append(f"{py_file.name}: {matches}") |
| 221 | |
| 222 | assert not violations, ( |
| 223 | "Legacy _render_*_html functions found — remove and replace with Jinja2:\n" |
| 224 | + "\n".join(f" • {v}" for v in violations) |
| 225 | ) |
| 226 | |
| 227 | |
| 228 | def test_musehub_js_has_htmx_config_request_bridge() -> None: |
| 229 | """``musehub.js`` injects the Bearer token on every HTMX request. |
| 230 | |
| 231 | The ``htmx:configRequest`` listener must be registered so HTMX partial |
| 232 | requests carry the same ``Authorization`` header as the initial page load. |
| 233 | Without this bridge, HTMX-driven tab switches and filter reloads are |
| 234 | rejected by the auth middleware. |
| 235 | |
| 236 | Complementary to ``test_musehub_ui_htmx_infra.py::test_musehub_js_has_htmx_config_request_bridge``. |
| 237 | This test additionally verifies the Bearer token is actually set in the |
| 238 | request headers (not just that the listener is registered). |
| 239 | """ |
| 240 | content = _MUSEHUB_JS.read_text() |
| 241 | assert "htmx:configRequest" in content, ( |
| 242 | "htmx:configRequest listener missing from musehub.js — " |
| 243 | "HTMX requests will lack Authorization headers" |
| 244 | ) |
| 245 | assert "Authorization" in content, ( |
| 246 | "Authorization header injection missing from musehub.js configRequest handler" |
| 247 | ) |
| 248 | assert "Bearer" in content, ( |
| 249 | "Bearer token pattern missing from musehub.js configRequest handler" |
| 250 | ) |
| 251 | |
| 252 | |
| 253 | def test_dead_templates_removed() -> None: |
| 254 | """Orphan templates that were superseded by the SSR migration must not exist. |
| 255 | |
| 256 | ``release_list.html`` was replaced by ``releases.html`` (issue #572). |
| 257 | ``repo.html`` was replaced by ``repo_home.html`` (issue #560 era). |
| 258 | Keeping orphan templates with ``apiFetch`` calls creates confusion about |
| 259 | which template is active and inflates the apiFetch audit surface. |
| 260 | """ |
| 261 | assert not (_PAGES_DIR / "release_list.html").exists(), ( |
| 262 | "release_list.html still present — delete it (replaced by releases.html)" |
| 263 | ) |
| 264 | assert not (_PAGES_DIR / "repo.html").exists(), ( |
| 265 | "repo.html still present — delete it (replaced by repo_home.html)" |
| 266 | ) |
| 267 | |
| 268 | |
| 269 | # ── HTTP route tests — require seeded repo ─────────────────────────────────── |
| 270 | |
| 271 | |
| 272 | @pytest.mark.anyio |
| 273 | @pytest.mark.parametrize("route_id,url_tpl", _MIGRATED_ROUTES, ids=[r[0] for r in _MIGRATED_ROUTES]) |
| 274 | async def test_all_page_routes_return_200( |
| 275 | client: AsyncClient, |
| 276 | db_session: AsyncSession, |
| 277 | route_id: str, |
| 278 | url_tpl: str, |
| 279 | ) -> None: |
| 280 | """Every SSR-migrated UI route returns HTTP 200 with text/html content. |
| 281 | |
| 282 | Data is fetched server-side; the page must render without client-side JS |
| 283 | execution. Routes that need a repo are tested against a seeded fixture |
| 284 | repo (``{O}/{S}``). Empty-state rendering (no commits, no releases, etc.) |
| 285 | is acceptable — the test only verifies the route does not 404/500. |
| 286 | """ |
| 287 | await _seed_repo(db_session) |
| 288 | url = _url(url_tpl) |
| 289 | response = await client.get(url) |
| 290 | assert response.status_code == 200, ( |
| 291 | f"Route '{route_id}' ({url}) returned {response.status_code}, expected 200" |
| 292 | ) |
| 293 | assert "text/html" in response.headers.get("content-type", ""), ( |
| 294 | f"Route '{route_id}' ({url}) did not return text/html" |
| 295 | ) |
| 296 | |
| 297 | |
| 298 | @pytest.mark.anyio |
| 299 | @pytest.mark.parametrize( |
| 300 | "route_id,url_tpl", |
| 301 | _HTMX_FRAGMENT_ROUTES, |
| 302 | ids=[r[0] for r in _HTMX_FRAGMENT_ROUTES], |
| 303 | ) |
| 304 | async def test_all_page_routes_return_fragment_for_htmx_request( |
| 305 | client: AsyncClient, |
| 306 | db_session: AsyncSession, |
| 307 | route_id: str, |
| 308 | url_tpl: str, |
| 309 | ) -> None: |
| 310 | """Routes using ``htmx_fragment_or_full`` return a bare fragment on ``HX-Request: true``. |
| 311 | |
| 312 | The fragment must: |
| 313 | - Return HTTP 200. |
| 314 | - NOT include ``<html``, ``<head``, or ``<body`` (those belong to the shell). |
| 315 | - Include some non-empty HTML content (not a blank response). |
| 316 | |
| 317 | This ensures HTMX tab-switching and filter-reloading swaps only the target |
| 318 | container, not the full page, preventing double-navigation flash. |
| 319 | """ |
| 320 | await _seed_repo(db_session) |
| 321 | url = _url(url_tpl) |
| 322 | response = await client.get(url, headers={"HX-Request": "true"}) |
| 323 | assert response.status_code == 200, ( |
| 324 | f"Route '{route_id}' ({url}) returned {response.status_code} on HX-Request" |
| 325 | ) |
| 326 | body = response.text |
| 327 | assert "<html" not in body, ( |
| 328 | f"Route '{route_id}' returned full page shell on HX-Request (found <html)" |
| 329 | ) |
| 330 | assert "<head" not in body, ( |
| 331 | f"Route '{route_id}' returned full page shell on HX-Request (found <head)" |
| 332 | ) |
| 333 | assert len(body.strip()) > 0, ( |
| 334 | f"Route '{route_id}' returned empty fragment on HX-Request" |
| 335 | ) |