gabriel / musehub public
test_musehub_htmx_migration_final.py python
337 lines 13.1 KB
Raw
c0f0b481 release: merge dev → main (#5) Gabriel Cardona <cgcardona@gmail.com> 5d ago
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 ("in_repo_search", "/{O}/{S}/search"),
124 ("activity", "/{O}/{S}/activity"),
125 ("credits", "/{O}/{S}/credits"),
126 ("branches", "/{O}/{S}/branches"),
127 ("tags", "/{O}/{S}/tags"),
128 ]
129
130 # Subset of _MIGRATED_ROUTES whose handlers call htmx_fragment_or_full() —
131 # these must return a bare fragment (no <html>) when HX-Request: true.
132 _HTMX_FRAGMENT_ROUTES: list[tuple[str, str]] = [
133 ("explore", "/explore"),
134 ("trending", "/trending"),
135 ("global_search", "/search"),
136 ("repo_home", "/{O}/{S}"),
137 ("commits_list", "/{O}/{S}/commits"),
138 ("pr_list", "/{O}/{S}/pulls"),
139 ("issue_list", "/{O}/{S}/issues"),
140 ("releases", "/{O}/{S}/releases"),
141 ("sessions", "/{O}/{S}/sessions"),
142 ("in_repo_search", "/{O}/{S}/search"),
143 ("activity", "/{O}/{S}/activity"),
144 ("branches", "/{O}/{S}/branches"),
145 ]
146
147
148 # ── Seed helper ──────────────────────────────────────────────────────────────
149
150
151 async def _seed_repo(db: AsyncSession) -> str:
152 """Seed a minimal public repo for route-level tests; return its repo_id."""
153 repo = MusehubRepo(
154 name=_SLUG,
155 owner=_OWNER,
156 slug=_SLUG,
157 visibility="public",
158 owner_user_id="uid-htmx-audit-owner",
159 )
160 db.add(repo)
161 await db.commit()
162 await db.refresh(repo)
163 return str(repo.repo_id)
164
165
166 # ── Static / code-analysis tests (no HTTP calls needed) ─────────────────────
167
168
169 def test_no_apifetch_in_page_templates() -> None:
170 """No SSR-migrated page template may contain a live ``apiFetch`` call.
171
172 Canvas/audio/visualization pages are explicitly exempted (see
173 ``_APIFETCH_EXEMPT_PAGES``). All other templates in ``pages/`` represent
174 listing/CRUD pages that must serve data server-side via Jinja2, not via
175 client-side API calls.
176
177 A failure here means a page template was migrated to SSR but still has a
178 leftover ``apiFetch`` call that is now dead code.
179 """
180 violations: list[str] = []
181 for html_file in sorted(_PAGES_DIR.glob("*.html")):
182 if html_file.name in _APIFETCH_EXEMPT_PAGES:
183 continue
184 content = html_file.read_text()
185 if "apiFetch(" in content:
186 # Count occurrences for a clear error message.
187 count = content.count("apiFetch(")
188 violations.append(f"{html_file.name}: {count} apiFetch call(s)")
189
190 assert not violations, (
191 "Dead apiFetch calls found in SSR-migrated page templates "
192 "(add to _APIFETCH_EXEMPT_PAGES if the page is legitimately canvas/audio):\n"
193 + "\n".join(f" • {v}" for v in violations)
194 )
195
196
197 def test_no_inline_html_builders_in_ui_py() -> None:
198 """No ``_render_*_html`` function may exist in ui.py or any ui_*.py.
199
200 The old pattern was to build HTML strings in Python (``_render_row_html``,
201 ``_render_header_html``, etc.) and return them as ``HTMLResponse``. The
202 SSR migration replaced all of these with Jinja2 template rendering. Any
203 remaining ``_render_*_html`` definition is a regression.
204
205 Known exception: ``ui_user_profile.py::_render_profile_html`` — the user
206 profile page is a complex JS-hydrated shell (heatmap, badges, activity tabs)
207 that was intentionally kept outside the listing-page SSR migration scope.
208 """
209 # Files exempt from this check: they intentionally keep JS-shell patterns
210 # because they render complex interactive widgets (heatmaps, canvases, etc.)
211 # that cannot be expressed as static Jinja2 HTML.
212 _EXEMPT_UI_FILES: frozenset[str] = frozenset({"ui_user_profile.py"})
213
214 pattern = re.compile(r"\bdef\s+_render_\w+_html\b")
215 violations: list[str] = []
216 for py_file in [_UI_PY] + sorted(_UI_EXTRA):
217 if py_file.name in _EXEMPT_UI_FILES:
218 continue
219 content = py_file.read_text()
220 matches = pattern.findall(content)
221 if matches:
222 violations.append(f"{py_file.name}: {matches}")
223
224 assert not violations, (
225 "Legacy _render_*_html functions found — remove and replace with Jinja2:\n"
226 + "\n".join(f" • {v}" for v in violations)
227 )
228
229
230 def test_musehub_js_has_htmx_config_request_bridge() -> None:
231 """``musehub.js`` injects the Bearer token on every HTMX request.
232
233 The ``htmx:configRequest`` listener must be registered so HTMX partial
234 requests carry the same ``Authorization`` header as the initial page load.
235 Without this bridge, HTMX-driven tab switches and filter reloads are
236 rejected by the auth middleware.
237
238 Complementary to ``test_musehub_ui_htmx_infra.py::test_musehub_js_has_htmx_config_request_bridge``.
239 This test additionally verifies the Bearer token is actually set in the
240 request headers (not just that the listener is registered).
241 """
242 content = _MUSEHUB_JS.read_text()
243 assert "htmx:configRequest" in content, (
244 "htmx:configRequest listener missing from musehub.js — "
245 "HTMX requests will lack Authorization headers"
246 )
247 assert "Authorization" in content, (
248 "Authorization header injection missing from musehub.js configRequest handler"
249 )
250 assert "Bearer" in content, (
251 "Bearer token pattern missing from musehub.js configRequest handler"
252 )
253
254
255 def test_dead_templates_removed() -> None:
256 """Orphan templates that were superseded by the SSR migration must not exist.
257
258 ``release_list.html`` was replaced by ``releases.html`` (issue #572).
259 ``repo.html`` was replaced by ``repo_home.html`` (issue #560 era).
260 Keeping orphan templates with ``apiFetch`` calls creates confusion about
261 which template is active and inflates the apiFetch audit surface.
262 """
263 assert not (_PAGES_DIR / "release_list.html").exists(), (
264 "release_list.html still present — delete it (replaced by releases.html)"
265 )
266 assert not (_PAGES_DIR / "repo.html").exists(), (
267 "repo.html still present — delete it (replaced by repo_home.html)"
268 )
269
270
271 # ── HTTP route tests — require seeded repo ───────────────────────────────────
272
273
274 @pytest.mark.anyio
275 @pytest.mark.parametrize("route_id,url_tpl", _MIGRATED_ROUTES, ids=[r[0] for r in _MIGRATED_ROUTES])
276 async def test_all_page_routes_return_200(
277 client: AsyncClient,
278 db_session: AsyncSession,
279 route_id: str,
280 url_tpl: str,
281 ) -> None:
282 """Every SSR-migrated UI route returns HTTP 200 with text/html content.
283
284 Data is fetched server-side; the page must render without client-side JS
285 execution. Routes that need a repo are tested against a seeded fixture
286 repo (``{O}/{S}``). Empty-state rendering (no commits, no releases, etc.)
287 is acceptable — the test only verifies the route does not 404/500.
288 """
289 await _seed_repo(db_session)
290 url = _url(url_tpl)
291 response = await client.get(url)
292 assert response.status_code == 200, (
293 f"Route '{route_id}' ({url}) returned {response.status_code}, expected 200"
294 )
295 assert "text/html" in response.headers.get("content-type", ""), (
296 f"Route '{route_id}' ({url}) did not return text/html"
297 )
298
299
300 @pytest.mark.anyio
301 @pytest.mark.parametrize(
302 "route_id,url_tpl",
303 _HTMX_FRAGMENT_ROUTES,
304 ids=[r[0] for r in _HTMX_FRAGMENT_ROUTES],
305 )
306 async def test_all_page_routes_return_fragment_for_htmx_request(
307 client: AsyncClient,
308 db_session: AsyncSession,
309 route_id: str,
310 url_tpl: str,
311 ) -> None:
312 """Routes using ``htmx_fragment_or_full`` return a bare fragment on ``HX-Request: true``.
313
314 The fragment must:
315 - Return HTTP 200.
316 - NOT include ``<html``, ``<head``, or ``<body`` (those belong to the shell).
317 - Include some non-empty HTML content (not a blank response).
318
319 This ensures HTMX tab-switching and filter-reloading swaps only the target
320 container, not the full page, preventing double-navigation flash.
321 """
322 await _seed_repo(db_session)
323 url = _url(url_tpl)
324 response = await client.get(url, headers={"HX-Request": "true"})
325 assert response.status_code == 200, (
326 f"Route '{route_id}' ({url}) returned {response.status_code} on HX-Request"
327 )
328 body = response.text
329 assert "<html" not in body, (
330 f"Route '{route_id}' returned full page shell on HX-Request (found <html)"
331 )
332 assert "<head" not in body, (
333 f"Route '{route_id}' returned full page shell on HX-Request (found <head)"
334 )
335 assert len(body.strip()) > 0, (
336 f"Route '{route_id}' returned empty fragment on HX-Request"
337 )