diff options
Diffstat (limited to 'packages/meshbay-hub/tests')
| -rw-r--r-- | packages/meshbay-hub/tests/test_downloads.py | 66 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_memory_ceiling.py | 53 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_transfers.py | 4 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_zip_size_limit.py | 4 |
4 files changed, 123 insertions, 4 deletions
diff --git a/packages/meshbay-hub/tests/test_downloads.py b/packages/meshbay-hub/tests/test_downloads.py index e68396f..81ab9d3 100644 --- a/packages/meshbay-hub/tests/test_downloads.py +++ b/packages/meshbay-hub/tests/test_downloads.py @@ -154,7 +154,10 @@ def test_a_length_is_only_promised_when_it_is_known(tmp_path): # the ability to refuse an oversized download (test_memory_ceiling.py). # What this test is about -- the `0` -- did not move. app = (STATIC / "file-utils.js").read_text() - zip_call = app[app.index("_openDownloadTarget(suggested"):] + # Anchored on the argument list, not on the function name: the call became + # `_openTargetInTurn(suggested, …)` when target openings were serialised. + # The `0` this test is about did not move. + zip_call = app[app.index("(suggested, totalBytes"):] zip_call = zip_call[:zip_call.index(");") + 2] assert zip_call.rstrip().endswith(", 0);"), ( "the zip download announces a Content-Length it will not match") @@ -363,3 +366,64 @@ def test_the_worker_is_kept_alive_while_it_streams(): # event, but the reply is what tells the page it is talking to the worker # that holds its stream. assert "mbdl-ping" in sw and "mbdl-pong" in sw + + +def test_targets_are_opened_one_at_a_time(tmp_path): + """ + A browser shows one file picker at a time and grants one per user gesture, + so four downloads asking at once get one dialog and three failures. + + That used to be prevented by accident: `downloadEntry` awaited the target + inline and files-app.js's `for (…) await downloadFile(e)` serialised them. + Opening the target inside `prepare` — so the row appears at the click rather + than tens of seconds later — removed the accident, and four pickers raced. + Reported from Chrome: one file downloaded, a prompt for the second, the + other two timed out. Firefox and Electron never noticed, because neither + opens a picker at all, which is why this reached one browser only. + + The queue is on the *targets*, never on the rows: every download still + appears the moment it is asked for. + + Queueing alone was not enough: a second dialog with no gesture behind it + still waits for a human, and the two behind it wait for the dialog. So + everything that has to wait its turn is also marked `batched`, which the + opener reads as "do not ask" — see the streamed-path branch below. + """ + src = (STATIC / "file-utils.js").read_text() + fn = src[src.index("function _openTargetInTurn"):] + fn = fn[:fn.index("\n}\n") + 2] + + script = tmp_path / "case.mjs" + script.write_text(""" +const out = []; +let live = 0, peak = 0; +// Stands in for _openDownloadTarget: records how many are open at once. +const asked = []; +const _openDownloadTarget = async (name, size, opts, swSize, flags) => { + live += 1; peak = Math.max(peak, live); + asked.push(flags && flags.batched); + await new Promise(r => setTimeout(r, 20)); + live -= 1; + if (name === 'boom') throw new Error('refused'); + return { name }; +}; +let _targetQueue = Promise.resolve(); +let _targetsInFlight = 0; +""" + fn + """ +const results = await Promise.allSettled( + ['a', 'boom', 'c', 'd'].map(n => _openTargetInTurn(n))); +out.push(peak); +out.push(results.map(r => r.status).join(',')); +out.push(asked); +console.log(JSON.stringify(out)); +""") + proc = subprocess.run(["node", str(script)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + peak, statuses, batched = json.loads(proc.stdout) + assert peak == 1, f"{peak} targets were being opened at once" + assert batched == [False, True, True, True], ( + "only the first of a batch holds the user's gesture; the rest must be " + "opened without asking") + # And one refusal must not stop the rest: a chain that breaks on a rejection + # leaves every later download unable to open anything at all. + assert statuses == "fulfilled,rejected,fulfilled,fulfilled" diff --git a/packages/meshbay-hub/tests/test_memory_ceiling.py b/packages/meshbay-hub/tests/test_memory_ceiling.py index 8430627..d46cae3 100644 --- a/packages/meshbay-hub/tests/test_memory_ceiling.py +++ b/packages/meshbay-hub/tests/test_memory_ceiling.py @@ -66,12 +66,16 @@ def target_fn(): def _run(target_fn, tmp_path, *, size, native=False, granted=False, - streamed=False, picker=False, mode="auto"): + streamed=False, picker=False, mode="auto", batched=False): """Drive the real function against one browser shape.""" script = tmp_path / "case.mjs" script.write_text(f""" // Stubs for everything the lifted function reaches. `formatSize` and `t` only // build the message; the assertions are about which branch was taken. +// +// stdout carries the outcome and nothing else, so the function's own logging +// goes to stderr -- where it is still shown when a case fails. +console.info = (...a) => console.error(...a); const formatSize = (n) => `${{n}} B`; const t = (key, vars) => key + ' ' + JSON.stringify(vars); const platform = {{ @@ -107,7 +111,8 @@ if ({json.dumps(picker)}) {{ let outcome; try {{ - const r = await _openDownloadTarget('film.mkv', {size}); + const r = await _openDownloadTarget('film.mkv', {size}, {{}}, {size}, + {{ batched: {json.dumps(batched)} }}); outcome = r === null ? {{ kind: 'memory' }} : r === false ? {{ kind: 'cancelled' }} : {{ kind: 'stream', name: r.name }}; @@ -180,6 +185,50 @@ def test_a_browser_with_a_picker_is_offered_one_instead_of_being_refused( assert out == {"kind": "stream", "name": "p"} +# ── One dialog per gesture, not one per file ──────────────────────────────── + +def test_the_first_of_a_batch_still_asks_where_to_save(target_fn, tmp_path): + """The preference is not being taken away. Someone who asked to choose the + folder chooses it, for the download they actually clicked.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=True, + streamed=True) + assert out == {"kind": "stream", "name": "p"} + + +def test_the_rest_of_a_batch_stream_instead_of_asking(target_fn, tmp_path): + """A browser grants one picker per user gesture and selecting four files is + one gesture. Chrome showed the dialog for the second file anyway and then + waited for a human, so the third and fourth sat behind it until they timed + out — reported as three downloads frozen. + + There is no gesture left to spend, so nothing is lost by streaming: the file + still lands on disk, in the browser's own download folder. Only the choice + of folder goes, and it was not on offer. + """ + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=True, + streamed=True, batched=True) + assert out == {"kind": "stream", "name": "s"} + + +def test_a_batched_download_falls_back_to_the_dialog_rather_than_failing( + target_fn, tmp_path): + """When the worker does not answer, asking is better than refusing: a + dialog that has to be answered is still a download, and the alternative + here is losing the file. A preference must not cost a capability, and + neither must the fix for one.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=True, + streamed=False, batched=True) + assert out == {"kind": "stream", "name": "p"} + + +def test_batching_never_pushes_a_large_file_into_memory(target_fn, tmp_path): + """Firefox shape — no picker at all. Nothing about the batch flag may reach + the memory floor above the ceiling.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=False, + streamed=False, batched=True) + assert out["kind"] == "refused", out + + # ── The one that outlives today's branches ────────────────────────────────── def test_no_unguarded_memory_floor(target_fn): diff --git a/packages/meshbay-hub/tests/test_transfers.py b/packages/meshbay-hub/tests/test_transfers.py index 7e21409..c48e38c 100644 --- a/packages/meshbay-hub/tests/test_transfers.py +++ b/packages/meshbay-hub/tests/test_transfers.py @@ -432,7 +432,9 @@ def test_the_slot_is_asked_for_after_there_is_somewhere_to_write(): src = (STATIC / "file-utils.js").read_text() fn = src[src.index("async function downloadEntry"):] fn = fn[:fn.index("\n}\n")] - assert fn.index("_openDownloadTarget") < fn.index("openTransfer"), ( + # `_openTargetInTurn` since target openings were serialised — same call, + # queued. What is pinned is that it comes before the slot is asked for. + assert fn.index("_openTargetInTurn") < fn.index("openTransfer"), ( "downloadEntry asks for a transfer slot before it has anywhere to " "write — the grant expires before the download can use it") diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index 28e3a0c..9471b8a 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -60,6 +60,10 @@ globalThis.localStorage = {{ // Node 22 defines `navigator` itself, so it is left alone; `window` is what // platform.js reaches for to decide it is not running in the desktop app. globalThis.window = globalThis; +// stdout carries the outcome and nothing else, so file-utils' own logging goes +// to stderr -- where it is still shown when a case fails. It logs before every +// save dialog, which is exactly what this harness provokes. +console.info = (...a) => console.error(...a); const out = {{ errors: [], started: 0, asked: 0, dropped: 0 }}; // Reached only once the size check has passed: with no File System Access API // under Node, downloadDirectory falls through to its build-in-memory path and |