|
| 1 | +""" |
| 2 | +Real-browser regression test for DesignLedger's WASM/IndexedDB persistence |
| 3 | +(mlsysim/mlsysim/labs/state.py). |
| 4 | +
|
| 5 | +Why this exists |
| 6 | +---------------- |
| 7 | +#1985 was a fire-and-forget asyncio.create_task() bug in DesignLedger.save() |
| 8 | +that swallowed IndexedDB failures silently. PR #1988 fixed that -- but |
| 9 | +review of #1988 caught a *second*, more insidious bug that mocked unit |
| 10 | +tests (tests/test_state.py) could never catch: a Python name-mangling |
| 11 | +issue. `globalThis.__mlsys_temp_state`, written inside the DesignLedger |
| 12 | +class body, was silently rewritten by the Python compiler to |
| 13 | +`globalThis._DesignLedger__mlsys_temp_state`, desyncing it from the plain |
| 14 | +`__mlsys_temp_state` the embedded JS string read. The result: save_async() |
| 15 | +reported success on every call while actually persisting `undefined` -- |
| 16 | +deterministically, only when invoked as a bound DesignLedger method. |
| 17 | +
|
| 18 | +Mocked tests can't catch this class of bug because they replace |
| 19 | +save_async() entirely, so they only verify the bookkeeping logic (done |
| 20 | +callbacks, last_save_error, flush(), asave() exception propagation) built |
| 21 | +around whatever save_async() reports -- never whether save_async() itself |
| 22 | +tells the truth against real IndexedDB. |
| 23 | +
|
| 24 | +This test runs the REAL, unmodified save_async() against a REAL Pyodide |
| 25 | +runtime and REAL IndexedDB in headless Chromium, then reads the data back |
| 26 | +out through a completely separate connection to prove it was actually, |
| 27 | +durably persisted -- not just that no exception was raised. |
| 28 | +
|
| 29 | +Usage |
| 30 | +----- |
| 31 | + python3 -m pytest labs/tests/test_wasm_persistence.py -v |
| 32 | +
|
| 33 | +Requires Playwright with Chromium installed: |
| 34 | + pip install playwright |
| 35 | + python3 -m playwright install chromium |
| 36 | +""" |
| 37 | +from __future__ import annotations |
| 38 | + |
| 39 | +import functools |
| 40 | +import http.server |
| 41 | +import socketserver |
| 42 | +import shutil |
| 43 | +import threading |
| 44 | +from pathlib import Path |
| 45 | + |
| 46 | +import pytest |
| 47 | + |
| 48 | +STATE_PY = ( |
| 49 | + Path(__file__).resolve().parents[2] |
| 50 | + / "mlsysim" / "mlsysim" / "labs" / "state.py" |
| 51 | +) |
| 52 | +PORT = 8766 |
| 53 | +TRIALS = 5 |
| 54 | +PYODIDE_CDN = "https://cdn.jsdelivr.net/pyodide/v0.28.3/full/pyodide.js" |
| 55 | + |
| 56 | +PAGE_HTML = f"""<!doctype html> |
| 57 | +<html><head><meta charset="utf-8"></head> |
| 58 | +<body> |
| 59 | +<script src="{PYODIDE_CDN}"></script> |
| 60 | +<script type="module"> |
| 61 | + async function main() {{ |
| 62 | + try {{ |
| 63 | + const pyodide = await loadPyodide(); |
| 64 | + window.__pyodide = pyodide; |
| 65 | + const stateSrc = await (await fetch("state.py")).text(); |
| 66 | + pyodide.globals.set("__state_src", stateSrc); |
| 67 | + await pyodide.runPythonAsync(` |
| 68 | +import sys, types |
| 69 | +_mod = types.ModuleType("state_under_test") |
| 70 | +sys.modules[_mod.__name__] = _mod |
| 71 | +exec(__state_src, _mod.__dict__) |
| 72 | +DesignLedger = _mod.DesignLedger |
| 73 | +`); |
| 74 | + window.__ready = true; |
| 75 | + }} catch (e) {{ |
| 76 | + window.__initError = String(e && e.stack ? e.stack : e); |
| 77 | + }} |
| 78 | + }} |
| 79 | + main(); |
| 80 | +</script> |
| 81 | +</body></html> |
| 82 | +""" |
| 83 | + |
| 84 | + |
| 85 | +class _QuietHandler(http.server.SimpleHTTPRequestHandler): |
| 86 | + def log_message(self, fmt, *args): # noqa: A003 - stdlib override |
| 87 | + return |
| 88 | + |
| 89 | + |
| 90 | +def _start_server(directory: Path, port: int): |
| 91 | + handler = functools.partial(_QuietHandler, directory=str(directory)) |
| 92 | + server = socketserver.TCPServer(("127.0.0.1", port), handler) |
| 93 | + thread = threading.Thread(target=server.serve_forever, daemon=True) |
| 94 | + thread.start() |
| 95 | + return server |
| 96 | + |
| 97 | + |
| 98 | +@pytest.fixture(scope="module") |
| 99 | +def served_dir(tmp_path_factory): |
| 100 | + if not STATE_PY.is_file(): |
| 101 | + pytest.skip(f"state.py not found at {STATE_PY}") |
| 102 | + |
| 103 | + directory = tmp_path_factory.mktemp("wasm-persistence") |
| 104 | + shutil.copy(STATE_PY, directory / "state.py") |
| 105 | + (directory / "index.html").write_text(PAGE_HTML, encoding="utf-8") |
| 106 | + |
| 107 | + server = _start_server(directory, PORT) |
| 108 | + try: |
| 109 | + yield directory |
| 110 | + finally: |
| 111 | + server.shutdown() |
| 112 | + server.server_close() |
| 113 | + |
| 114 | + |
| 115 | +def test_design_ledger_save_async_persists_in_real_indexeddb(served_dir): |
| 116 | + """save_async() must actually persist to IndexedDB when called as a |
| 117 | + bound DesignLedger method -- not just report success. |
| 118 | +
|
| 119 | + Runs TRIALS independent attempts, each against a freshly-cleared |
| 120 | + IndexedDB, reading back through a *separate* connection each time to |
| 121 | + verify durability rather than trusting save_async()'s return value. |
| 122 | + """ |
| 123 | + from playwright.sync_api import sync_playwright |
| 124 | + |
| 125 | + failures: list[int] = [] |
| 126 | + |
| 127 | + with sync_playwright() as p: |
| 128 | + browser = p.chromium.launch() |
| 129 | + context = browser.new_context() |
| 130 | + try: |
| 131 | + for i in range(TRIALS): |
| 132 | + page = context.new_page() |
| 133 | + init_errors: list[str] = [] |
| 134 | + page.on("pageerror", lambda exc: init_errors.append(str(exc))) |
| 135 | + |
| 136 | + page.goto(f"http://127.0.0.1:{PORT}/index.html") |
| 137 | + page.wait_for_function( |
| 138 | + "window.__ready === true || window.__initError", |
| 139 | + timeout=30_000, |
| 140 | + ) |
| 141 | + init_error = page.evaluate("window.__initError || null") |
| 142 | + assert not init_error, f"Pyodide init failed: {init_error}" |
| 143 | + assert not init_errors, f"Uncaught page errors during init: {init_errors}" |
| 144 | + |
| 145 | + # Clear any prior IndexedDB state for a clean trial. |
| 146 | + page.evaluate( |
| 147 | + """ |
| 148 | + () => new Promise((resolve) => { |
| 149 | + const req = indexedDB.deleteDatabase("mlsys_ledger_db"); |
| 150 | + req.onsuccess = req.onerror = req.onblocked = () => resolve(); |
| 151 | + }) |
| 152 | + """ |
| 153 | + ) |
| 154 | + |
| 155 | + page.evaluate( |
| 156 | + f""" |
| 157 | + async () => {{ |
| 158 | + const pyodide = window.__pyodide; |
| 159 | + await pyodide.runPythonAsync(` |
| 160 | +ledger = DesignLedger() |
| 161 | +ledger._state.track = "trial-{i}" |
| 162 | +ledger._state.current_step = 1 |
| 163 | +ledger._state.history[1] = {{"trial": {i}}} |
| 164 | +await ledger.save_async() |
| 165 | +`); |
| 166 | + }} |
| 167 | + """ |
| 168 | + ) |
| 169 | + |
| 170 | + persisted = page.evaluate( |
| 171 | + """ |
| 172 | + () => new Promise((resolve, reject) => { |
| 173 | + const req = indexedDB.open("mlsys_ledger_db", 1); |
| 174 | + req.onsuccess = (e) => { |
| 175 | + const db = e.target.result; |
| 176 | + const tx = db.transaction("ledger", "readonly"); |
| 177 | + const getReq = tx.objectStore("ledger").get("mlsys_design_ledger"); |
| 178 | + getReq.onsuccess = () => { |
| 179 | + db.close(); |
| 180 | + resolve(getReq.result !== undefined && getReq.result !== null); |
| 181 | + }; |
| 182 | + getReq.onerror = () => { db.close(); reject(getReq.error); }; |
| 183 | + }; |
| 184 | + req.onerror = () => reject(req.error); |
| 185 | + }) |
| 186 | + """ |
| 187 | + ) |
| 188 | + |
| 189 | + if not persisted: |
| 190 | + failures.append(i) |
| 191 | + page.close() |
| 192 | + finally: |
| 193 | + context.close() |
| 194 | + browser.close() |
| 195 | + |
| 196 | + assert not failures, ( |
| 197 | + f"DesignLedger.save_async() failed to durably persist to IndexedDB " |
| 198 | + f"on trial(s) {failures} of {TRIALS} -- it reported success but the " |
| 199 | + f"write was lost. This is the exact silent-data-loss failure mode " |
| 200 | + f"from #1985 / PR #1988. A mocked test cannot catch this; only a " |
| 201 | + f"real Pyodide + IndexedDB check like this one can." |
| 202 | + ) |
0 commit comments