diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-08-18 16:34:12 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-08-18 16:34:12 +0200 |
| commit | eedbca3f0d47af39b4dd8812683e5a14ae4e48e6 (patch) | |
| tree | c99304c1f935e63a40e0546ef63ff30879d5aa9b /packages/meshbay-hub/tests/test_resume_position.py | |
| parent | c384878aa0d6ef7a33bda23887dc264b94386725 (diff) | |
| download | meshbay-eedbca3f0d47af39b4dd8812683e5a14ae4e48e6.tar.gz | |
fix: two waits with no deadline, resume positions per account, group settings tab
**Joining a group could hang.** Reported after a first attempt that never
finished and a later one that worked — the shape of a network wait with no
deadline, and there were two.
Signaling here is non-trickle: the offer is not sent until ICE gathering says
it is done. A STUN server that is slow, filtered, or resolved through a DNS that
is not answering means `icegatheringstatechange` never reaches `complete`, and
`connect()` never returns. Same shape as the fullscreen denial fixed yesterday:
a promise that never settles leaves no error to find. Gathering now has four
seconds, after which the offer goes out with what it has — host candidates are
already there, which is enough on a LAN, and giving up instead would turn a slow
STUN server into a refusal to connect.
The second: `hub:fetch` in the desktop client had no timeout, so a host that
accepts a connection and then says nothing holds the request for as long as the
OS allows. `hub:probe` had one; the handler that carries signaling did not. Now
thirty seconds — longer than the hub's own fifteen-second signaling wait, so it
cannot abort a call that was about to succeed — and it says the hub did not
answer rather than "fetch failed".
**Resume positions belonged to the machine, not the account.** Stored as
`mb:pos:<file>`, so a second account signing in on the same computer was offered
"resume where you left off" in a film it had never opened. Wrong on its own
terms, and a small disclosure of what the other person watches, since the offer
only appears for files someone has actually been through. The account is in the
key now. Positions written before this are deleted rather than re-keyed: there
is no record of whose they were, and guessing hands them to whoever signs in
next, which is the bug.
**The staggered rules in the members table.** `display: flex` on the actions
`<td>` — a flex table cell stops being a table cell, so it no longer stretches
to its row and its bottom border is drawn wherever its own content ends.
Measured: in a row whose other cells were `top 76, height 40`, that cell was
`top 77, height 30`, its rule nine pixels above the rest. It is a table cell
again, held open by a zero-width strut so the owner's row — which has no remove
button — stays as tall as the others. Every cell now shares its row's top and
bottom exactly, at 420px and 900px.
**Members became Settings.** It was a list with three unrelated forms stacked
above it, laid out with inline styles on whichever element needed them, and the
group's own controls somewhere else entirely — leaving or deleting a group sat
in the page header beside the title. Now one tab in sections: invitations,
operator pairing, your devices on this node, leaving or deleting, and the roster
last, since it is the only part with no upper bound.
One consequence worth stating: the tab bar no longer waits for the node.
Membership is hub-side, and gating it on a live connection would have made
"leave this group" unreachable exactly when a node is down — which is when
someone most wants it. Files and chat still need the node and say so.
**A download button in the viewer**, beside the close button and in the same
style, for both the video player and the file preview.
844 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'packages/meshbay-hub/tests/test_resume_position.py')
| -rw-r--r-- | packages/meshbay-hub/tests/test_resume_position.py | 158 |
1 files changed, 158 insertions, 0 deletions
diff --git a/packages/meshbay-hub/tests/test_resume_position.py b/packages/meshbay-hub/tests/test_resume_position.py new file mode 100644 index 0000000..ef239c5 --- /dev/null +++ b/packages/meshbay-hub/tests/test_resume_position.py @@ -0,0 +1,158 @@ +""" +"Resume where you left off" belongs to an account, not to a machine. + +The position was stored as `mb:pos:<file>` in localStorage — per *device*. Sign +in with a second account on the same computer and the player offered to resume a +film that account had never opened. Wrong on its own terms, and a small +disclosure of what the other person watches: the offer only appears for files +someone has actually been through. + +The functions are lifted out of `app.js` and run for real, rather than having +their source inspected, because the thing worth holding is the behaviour of two +accounts sharing one storage — which no assertion about the source text says. +""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +APP = STATIC / "app.js" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not APP.exists(), + reason="node or the SPA sources are not available") + +WANTED = ("resumeKey", "readResumePosition", "writeResumePosition", + "purgeUnscopedResumePositions") + + +def _extract() -> str: + """The real source of the functions under test, and nothing else. + + `app.js` imports preact and cannot be loaded outside a browser, so the + declarations are sliced out by brace matching. A rename breaks this loudly, + which is the intent — a silently skipped test is worse than a failing one. + """ + source = APP.read_text(encoding="utf-8") + out = [ + f"const RESUME_MIN_S = {_const(source, 'RESUME_MIN_S')};", + f"const RESUME_MAX_FRACTION = {_const(source, 'RESUME_MAX_FRACTION')};", + ] + for name in WANTED: + start = source.index(f"function {name}(") + depth, i = 0, source.index("{", start) + while True: + if source[i] == "{": + depth += 1 + elif source[i] == "}": + depth -= 1 + if depth == 0: + break + i += 1 + out.append(source[start:i + 1]) + return "\n".join(out) + + +def _const(source: str, name: str) -> str: + match = re.search(rf"^const {name} = ([^;]+);", source, re.M) + assert match, f"{name} is gone or was renamed" + return match.group(1) + + +def _run(body: str, tmp_path: Path): + script = tmp_path / "case.mjs" + script.write_text( + "const store = new Map();\n" + "globalThis.localStorage = {\n" + " get length() { return store.size; },\n" + " key: i => Array.from(store.keys())[i] ?? null,\n" + " getItem: k => (store.has(k) ? store.get(k) : null),\n" + " setItem: (k, v) => store.set(k, String(v)),\n" + " removeItem: k => store.delete(k),\n" + "};\n" + # Whoever is signed in, swapped by the cases below. + "let AUTH = null;\n" + "function loadAuth() { return AUTH; }\n" + f"{_extract()}\n" + "const out = [];\n" + "const say = (...a) => out.push(...a);\n" + "const keys = () => Array.from(store.keys()).sort();\n" + f"{body}\n" + "console.log(JSON.stringify(out));\n", + encoding="utf-8") + proc = subprocess.run(["node", str(script)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + return json.loads(proc.stdout) + + +# ── The bug ───────────────────────────────────────────────────────────────── + +def test_a_second_account_is_not_offered_the_firsts_position(tmp_path): + """The report: a brand-new account was offered a resume point.""" + assert _run(""" +AUTH = { userId: 'alice' }; +writeResumePosition('film1', 600, 7200); +say(readResumePosition('film1')); + +AUTH = { userId: 'bob' }; +say(readResumePosition('film1')); +""", tmp_path) == [600, 0] + + +def test_each_account_keeps_its_own_place_in_the_same_film(tmp_path): + """Two people watching one film on one machine is the ordinary case, and + neither should move the other's bookmark.""" + assert _run(""" +AUTH = { userId: 'alice' }; writeResumePosition('film1', 600, 7200); +AUTH = { userId: 'bob' }; writeResumePosition('film1', 1800, 7200); +AUTH = { userId: 'alice' }; say(readResumePosition('film1')); +AUTH = { userId: 'bob' }; say(readResumePosition('film1')); +""", tmp_path) == [600, 1800] + + +def test_nothing_is_written_when_nobody_is_signed_in(tmp_path): + assert _run(""" +AUTH = null; +writeResumePosition('film1', 600, 7200); +say(keys().length, readResumePosition('film1')); +""", tmp_path) == [0, 0] + + +def test_positions_written_before_the_fix_are_dropped(tmp_path): + """ + They cannot be re-keyed: there is no record of whose they were, and guessing + hands them to whoever signs in next, which is the bug itself. + """ + assert _run(""" +localStorage.setItem('mb:pos:film1', '600'); // the old shape +localStorage.setItem('mb:pos:alice:film2', '900'); // the new one +localStorage.setItem('mb_auth', '{}'); // nothing to do with this +purgeUnscopedResumePositions(); +say(...keys()); +""", tmp_path) == ["mb:pos:alice:film2", "mb_auth"] + + +# ── What must still hold ──────────────────────────────────────────────────── + +def test_a_glance_at_the_opening_is_not_a_bookmark(tmp_path): + assert _run(""" +AUTH = { userId: 'alice' }; +writeResumePosition('film1', 12, 7200); +say(readResumePosition('film1')); +""", tmp_path) == [0] + + +def test_a_film_watched_to_the_end_stops_offering_to_resume(tmp_path): + """And the earlier bookmark goes with it, rather than sitting there + offering the last thirty seconds forever.""" + assert _run(""" +AUTH = { userId: 'alice' }; +writeResumePosition('film1', 3600, 7200); +writeResumePosition('film1', 7150, 7200); +say(readResumePosition('film1'), keys().length); +""", tmp_path) == [0, 0] |