diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-08 23:43:46 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-08 23:43:46 +0200 |
| commit | 77615ddb5fead3e74751a94847d3bcc99fc0a96d (patch) | |
| tree | 72e596fdc56567925100b18460793a978bb29c63 /packages/meshbay-hub/tests | |
| parent | 1ba91bb4f38f6338d1c86678ebcd19195db2a120 (diff) | |
| download | meshbay-77615ddb5fead3e74751a94847d3bcc99fc0a96d.tar.gz | |
fix(hub): the transfers row exists from the click
Clicking Download produced nothing — no row, no icon, no panel — for as long as
it took to open somewhere to write, and then several rows at once. The streamed
path waits for the worker twice; a Save As dialog waits for a person. The row
was created after that, so the slowest part of a download happened with nothing
on screen to say it had begun.
The store gains a `prepare` step, distinct from `run`, and the order is now:
row, then target, then slot.
That last part is why the obvious fix was wrong. Taking the slot first would let
the row appear immediately, and it was tried this morning: a granted slot has to
be taken up within the node's deadline, opening a target can outlast it, and
three downloads became one. (The diagnosis at the time blamed that ordering for
revocations which were in fact a missing `touch()` call — the revert was right
for the wrong reason.) `makeLease` is called after `prepare` succeeds, never
before.
Three behaviours fall out, each with a test:
- a dismissed dialog leaves nothing behind. `prepare` returning false drops
the row: nothing started, so nothing should remain on screen to explain it;
- the row takes the name the file was actually saved under, once known;
- a refusal above the memory ceiling fails the row that is already there,
rather than creating one to kill it.
`preparing` counts as live everywhere — badge, cancel, clearFinished, and
`_busy`, since closing a transport under a preparing transfer strands it exactly
as under a queued one. Six places asked "is this finished?" and were drifting
apart; there is one definition now.
Two mistakes in the tests, worth the note: one counted positions in an output
array by hand and was one out, which reads exactly like a failing assertion
about the code — the values are tagged now, not indexed. And
test_zip_size_limit.py's stub did not run `prepare`, so it no longer reached the
size check the file is about; it now behaves like the real store.
819 hub, 1169 node, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST
Diffstat (limited to 'packages/meshbay-hub/tests')
| -rw-r--r-- | packages/meshbay-hub/tests/test_transfers.py | 116 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_zip_size_limit.py | 17 |
2 files changed, 131 insertions, 2 deletions
diff --git a/packages/meshbay-hub/tests/test_transfers.py b/packages/meshbay-hub/tests/test_transfers.py index e805afc..7e21409 100644 --- a/packages/meshbay-hub/tests/test_transfers.py +++ b/packages/meshbay-hub/tests/test_transfers.py @@ -435,3 +435,119 @@ def test_the_slot_is_asked_for_after_there_is_somewhere_to_write(): assert fn.index("_openDownloadTarget") < fn.index("openTransfer"), ( "downloadEntry asks for a transfer slot before it has anywhere to " "write — the grant expires before the download can use it") + + +# ── the row exists from the click ─────────────────────────────────────────── + +def test_the_row_appears_before_the_target_is_open(tmp_path): + """ + Opening a target is the slow part — the streamed path waits for the worker + twice, a Save As dialog waits for a person — and the row used to be created + only after it returned. Three clicks produced no panel at all, not even the + icon, and then several rows at once. + """ + out = _run(""" + const t = new TransferStore(); + let release; + const opened = new Promise(r => { release = r; }); + t.start({ kind: 'download', name: 'film.mkv', total: 10, + prepare: async () => { await opened; return { name: 'saved.mkv' }; }, + run: async () => { say('ran'); } }); + const shot = (when) => say(when + '=' + t.list().length + ':' + + t.list().map(i => i.status + '/' + i.name).join(',')); + shot('click'); + release(); + await new Promise(r => setTimeout(r, 10)); + shot('after'); + """, tmp_path) + # Tagged, not indexed. An earlier version counted pushes by hand and was one + # out, which reads exactly like a failing assertion about the code. + seen = dict(line.split("=", 1) for line in out + if isinstance(line, str) and "=" in line) + assert {"click", "after"} <= set(seen), f"probe produced: {out}" + assert seen["click"] == "1:preparing/film.mkv", ( + f"no row, or the wrong one, at the moment of the click: {seen['click']}") + assert seen["after"] == "1:done/saved.mkv", ( + f"the row must keep the name it was saved under: {seen['after']}") + + +def test_a_dismissed_dialog_leaves_nothing_behind(tmp_path): + """Dismissing a Save As dialog is not a failure and not a cancellation: + nothing was started, so nothing should be left on screen explaining it.""" + out = _run(""" + const t = new TransferStore(); + t.start({ kind: 'download', name: 'film.mkv', total: 10, + prepare: async () => false, + run: async () => { say('ran'); } }); + say('at click:', t.list().length); + await new Promise(r => setTimeout(r, 10)); + say('after:', t.list().length, out.includes('ran')); + """, tmp_path) + assert out[1] == 1 + assert out[3] == 0, "a dismissed dialog left a row behind" + assert out[4] is False + + +def test_the_slot_is_only_asked_for_once_there_is_somewhere_to_write(tmp_path): + """ + A granted slot must be taken up within the node's deadline, and opening a + target can outlast it. Asking first cost two of three downloads. + """ + out = _run(""" + const t = new TransferStore(); + let release; + const opened = new Promise(r => { release = r; }); + let asked = false; + t.start({ kind: 'download', name: 'f', total: 10, + prepare: async () => { await opened; return true; }, + makeLease: () => { asked = true; return { + state: 'granted', ahead: 0, tr: 'x', + acquire: () => Promise.resolve(), release: () => {} }; }, + run: async () => {} }); + say('while preparing, asked?', asked); + release(); + await new Promise(r => setTimeout(r, 10)); + say('after preparing, asked?', asked, t.list()[0].status); + """, tmp_path) + assert out[1] is False, "the slot was taken before there was a target" + assert out[3] is True + assert out[4] == "done" + + +def test_a_target_that_cannot_be_opened_fails_the_row_it_already_has(tmp_path): + """The refusal above the memory ceiling lands in the panel, on the row that + is already there, rather than in a console nobody opens.""" + out = _run(""" + const t = new TransferStore(); + t.start({ kind: 'download', name: 'film.mkv', total: 10, + prepare: async () => { throw new Error('too large for memory'); }, + run: async () => { say('ran'); } }); + await new Promise(r => setTimeout(r, 10)); + const it = t.list()[0]; + say(it.status, it.error, out.includes('ran')); + """, tmp_path) + assert out[0] == "failed" + assert "too large" in out[1] + assert out[2] is False + + +def test_a_transport_is_not_closed_under_a_preparing_transfer(tmp_path): + """It has no lease yet and has moved no bytes, but closing its transport + would strand it exactly like a queued one.""" + out = _run(""" + const t = new TransferStore(); + let release; + const opened = new Promise(r => { release = r; }); + let closed = false; + const transport = { close() { closed = true; } }; + t.start({ kind: 'download', name: 'f', total: 10, transport, + prepare: async () => { await opened; return true; }, + run: async () => {} }); + t.releaseWhenIdle(transport); + say('closed while preparing:', closed); + release(); + await new Promise(r => setTimeout(r, 10)); + say('closed after:', closed); + """, tmp_path) + assert out[1] is False + assert out[3] is True diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index e970c55..28e3a0c 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -60,7 +60,7 @@ 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; -const out = {{ errors: [], started: 0, asked: 0 }}; +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 // asks first. Answering yes is what lets the at-the-limit case get as far as @@ -80,7 +80,18 @@ if ({picker_js}) {{ const M = await import('{(sandbox / "file-utils.js").as_posix()}'); -const transfers = {{ start: () => {{ out.started += 1; }} }}; +// Faithful enough to the real store: it runs `prepare` and honours what it +// returns. The target is opened there now — the row exists from the click and +// the slow part happens behind it — so a stub that only counts calls would +// never reach the size check this file is about. +const transfers = {{ start: (opts) => {{ + out.started += 1; + if (!opts.prepare) return; + Promise.resolve() + .then(() => opts.prepare()) + .then((ready) => {{ if (ready === false) {{ out.started -= 1; out.dropped += 1; }} }}) + .catch((e) => {{ out.started -= 1; out.errors.push(e.message); }}); +}} }}; // A transport hands out transfer slots now (transfers.py's leases). The stub // grants at once, which is what a node with no caps does: what this file is // about is the archive limit, not the queue. @@ -100,6 +111,8 @@ await M.downloadDirectory(transfers, transport, null, entries, 'album', {{ setError: (m) => out.errors.push(m), }}); +// `prepare` runs on a microtask, so let it. +await new Promise(r => setTimeout(r, 10)); out.limit = M.ZIP_MAX_BYTES; console.log(JSON.stringify(out)); """, encoding="utf-8") |