diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-02 17:31:50 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-02 17:31:50 +0200 |
| commit | 7b25f1c09ba1b8692988d9616f3c33c97af9f3ca (patch) | |
| tree | 1a7d346ea6a3847269df4aeff3c6b41d515b5d7b /packages/meshbay-hub/tests | |
| parent | 10f8266e7152d7dc38dbfe2449327829bf020ad1 (diff) | |
| download | meshbay-7b25f1c09ba1b8692988d9616f3c33c97af9f3ca.tar.gz | |
fix(hub): stop the maintenance loop racing the tests, and pin _ASSETS
Two defects found while closing out the Search merge, neither of them in
that feature.
The maintenance loop. create_app's lifespan starts cleanup_loop as an
asyncio task, so every test — each entering that lifespan — ran a purge
pass concurrently with its own requests. On SQLite :memory: that is not
merely noisy: the engine uses a StaticPool, one connection for the whole
process, so the request's session and the cleanup task's session
interleave transactions on the same connection. A registration could
commit and then be invisible to the login three lines later, surfacing
as 401 Invalid credentials for an account created moments before, in
roughly one run of test_node_ws_auth.py in four.
The purge itself is not at fault and this is not a production condition.
A passive SQL listener caught the DELETE removing 0 rows, and the INSERT
carrying status='active' — so neither the pending-account mechanism nor
the purge filter is involved, and PostgreSQL gives every session its own
connection. What the fixture removes is the second user of the shared
one. 60 runs of the previously flaky file, 0 failures; reproductions
before the fix landed on attempts 4, 6, 13 and 29 of separate loops, so
a clean run of 60 has about a 1% chance of being luck.
_ASSETS. source-merge.js shipped missing from webapp._ASSETS, the
cache-busting hash's input list — exactly the silent failure docs/apps.md
§4 step 5 warns about: the file changes, the asset URL does not, and a
browser holding the old page keeps the old copy. Harmless this time only
because search-page.js changed in the same commit and is listed, which is
the worst way for it to go unnoticed. Found by re-reading that checklist
for the doc pass, not by any test — so there is a test now, holding
_ASSETS to every .js in static/ (sw.js excepted, unversioned on purpose).
It was the only one missing.
Phase 9 of docs/refactoring-search.md also lands here: mediacenter.md
§10.6, musicbay.md §9b, photos.md §10b, apps.md §2b and step 5.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AbwJDbNTkiRUh7HTWEoyss
Diffstat (limited to 'packages/meshbay-hub/tests')
| -rw-r--r-- | packages/meshbay-hub/tests/conftest.py | 27 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_asset_versioning.py | 27 |
2 files changed, 53 insertions, 1 deletions
diff --git a/packages/meshbay-hub/tests/conftest.py b/packages/meshbay-hub/tests/conftest.py index 2769f7e..cb595c5 100644 --- a/packages/meshbay-hub/tests/conftest.py +++ b/packages/meshbay-hub/tests/conftest.py @@ -96,3 +96,30 @@ def _skip_email_verification(monkeypatch): monkeypatch.setattr( "meshbay_hub.api.users._create_and_send_verification", _noop) monkeypatch.setattr("meshbay_hub.mail._send", lambda msg: True) + + +@pytest.fixture(autouse=True) +def _no_cleanup_task(monkeypatch): + """ + Do not run the maintenance loop under test. + + `create_app`'s lifespan starts `cleanup_loop` as an asyncio task, so every + test — each of which enters that lifespan — ran a purge pass concurrently + with its own requests. On SQLite `:memory:` that is not merely noisy: the + engine uses a **StaticPool**, one connection for the whole process, so the + request's session and the cleanup task's session interleave their + transactions on the *same* connection. A registration could commit and then + not be visible to the login three lines later, which surfaced as + `401 Invalid credentials` for an account created moments before, in about + one run of `test_node_ws_auth.py` in four. + + The purge itself is not at fault and this is not a production condition: + the DELETE was measured removing 0 rows, and PostgreSQL gives every session + its own connection. What is removed here is the second user of the shared + one. Tests that want the maintenance behaviour call the `purge_*` functions + directly, which is how they are covered. + """ + async def _noop(get_session): + return + + monkeypatch.setattr("meshbay_hub.tasks.cleanup.cleanup_loop", _noop) diff --git a/packages/meshbay-hub/tests/test_asset_versioning.py b/packages/meshbay-hub/tests/test_asset_versioning.py index e5889f0..3d84d99 100644 --- a/packages/meshbay-hub/tests/test_asset_versioning.py +++ b/packages/meshbay-hub/tests/test_asset_versioning.py @@ -23,7 +23,7 @@ import re import pytest from fastapi.testclient import TestClient -from meshbay_hub.api.webapp import ASSET_V, STATIC_DIR, _asset_version +from meshbay_hub.api.webapp import _ASSETS, ASSET_V, STATIC_DIR, _asset_version from meshbay_hub.app import create_app @@ -101,3 +101,28 @@ def test_the_fingerprint_follows_the_content(tmp_path, monkeypatch): finally: target.write_bytes(original) assert _asset_version() == before, "the fingerprint is not reproducible" + + +def test_every_static_script_participates_in_the_fingerprint(): + """ + `_ASSETS` is hand-maintained, and forgetting an entry fails silently: the + file is imported by the page, so the browser fetches it, but it does not + feed the content hash — so a change confined to that one file ships at the + URL a cache already holds. Nothing errors, and the symptom is a fix that + "doesn't work" on exactly the machines that visited before. + + Found by `source-merge.js`, added to the Search view and left out of the + list. It happened to be harmless that day because `search-page.js` changed + in the same commit and *is* listed — which is the worst way for this to go + unnoticed. The checklist in docs/apps.md §4 step 5 names this trap; this + enforces it instead of relying on remembering. + + `sw.js` is the one deliberate exclusion — the service worker is served + unversioned on purpose (`test_the_service_worker_is_not_versioned`). + """ + on_disk = {p.name for p in STATIC_DIR.glob("*.js")} - {"sw.js"} + missing = sorted(on_disk - set(_ASSETS)) + assert not missing, ( + f"static scripts missing from webapp._ASSETS: {missing}. A change to " + "one of these will not move the asset URL, so a browser that cached " + "the page keeps running the old copy") |