diff options
Diffstat (limited to 'packages/meshbay-hub/tests/test_chat_send.py')
| -rw-r--r-- | packages/meshbay-hub/tests/test_chat_send.py | 158 |
1 files changed, 96 insertions, 62 deletions
diff --git a/packages/meshbay-hub/tests/test_chat_send.py b/packages/meshbay-hub/tests/test_chat_send.py index 250c9a3..5db8f26 100644 --- a/packages/meshbay-hub/tests/test_chat_send.py +++ b/packages/meshbay-hub/tests/test_chat_send.py @@ -1,38 +1,29 @@ """ -Sending a chat message must come back — and must go out encrypted. +Sending a chat message must come back — accepted or refused. -The node answers a chat message with a bare `{"type": "ack"}` — no request id, -no type of its own — so `_dispatch` had nothing to match it on and left it to -the arrival-order guess at the end of the function. That guess is wrong as soon -as anything else this browser asked for is still waiting: the ack was handed to -*that* request, and the send waited out `_sendAndWait`'s 30s timeout. Since the -composer is disabled while a send is in flight, the Chat tab stopped taking -clicks and keys, the message never appeared — and it was there on the next -visit, because the node had stored it and answered. +The node's replies to a chat message name no request. The acceptance is a bare +`{"type": "ack"}`; the refusal is a bare `{"type": "error"}`, and it is not a +special case — `_dispatch_message`'s catch-all answers *every* failure that +way, and 238 of webrtc_server.py's 240 error sends name nothing either. So +`_dispatch` had nothing to match either reply on and left both to the +arrival-order guess at the end of the function. -An outstanding request is the ordinary case, not a rare one: the node refuses -an unknown file_id with a bare `error`, which names no request either, so a -Videos tab that asked about a file the index no longer has leaves a -`media_meta_req` in `_pending` for a full 30s. +That guess is wrong as soon as anything else this browser asked for is still +waiting, which is the ordinary case rather than a rare one: a `music_meta_req` +sits in `_pending` for as long as the third-party lookup behind it takes, and +that was measured live at over 100 seconds with the service failing. The reply +went to *that* request, and the send waited out `_sendAndWait`'s 30s timeout. +Since the composer is disabled while a send is in flight, the Chat tab stopped +taking clicks and keys, and the message never appeared. -None of that is visible in `chat-app.js`, where every line is correct, so this +The ack half was fixed by matching on request type. The refusal half could not +be: an `error` has no type of its own to match on. `req_id` is what closed it — +the caller's id, stamped on the reply by the node — so this now drives both +shapes of answer. + +None of it is visible in `chat-app.js`, where every line is correct, so this drives the real panel over the real transport in a browser rather than reading either source. - -Extended for MNP 2.0, where a send seals and signs before it goes anywhere. -That turned out to matter twice on its first run: - - * `chat_keys_resp` answers a `chat_keys_req` under a different type string, - so it fell through to the arrival-order guess and was handed to the very - `media_meta_req` this probe leaves outstanding — the original defect, one - feature later, in a message type that did not exist when it was written. - * `_asText` had been deleted along with an unrelated helper beside it. Its - only caller is inside `_openChatMessage`, whose rejection the panel - swallows, so the whole conversation rendered empty with nothing in the - console and the node answering perfectly. - -Neither is visible in any source file, and neither would have been caught by a -test that reads one. """ import json import shutil @@ -52,7 +43,7 @@ pytestmark = pytest.mark.skipif( @pytest.fixture(scope="module") def probe(): - # `sys.executable`, not a bare "python3": the harness now imports + # `sys.executable`, not a bare "python3": the harness imports # `meshbay_common` to seal the chat keys the way the node does, and the # system interpreter has neither that nor msgpack. The other probes get # away with "python3" because they import nothing from this project. @@ -60,51 +51,81 @@ def probe(): capture_output=True, timeout=180) assert run.returncode == 0, run.stderr.decode()[-2000:] data = json.loads(run.stdout.decode()) - return data, {s["label"]: s for s in data["steps"]} + steps = {sc["name"]: {s["label"]: s for s in sc["steps"]} + for sc in data["scenarios"]} + return data, steps + +@pytest.mark.parametrize("reply", ["ack", "error"]) +def test_the_composer_comes_back(probe, reply): + """The one thing a person sees: the tab is usable again. -def test_the_composer_comes_back(probe): - """The one thing a person sees: the tab is usable again.""" + Both answers have to release it. A refusal that reaches nobody leaves the + composer disabled exactly as long as an acceptance that reaches nobody — + the composer is not waiting for good news, it is waiting for an answer. + """ _, steps = probe - assert steps["stale request pending"]["composerDisabled"] is False, ( + assert steps[reply]["older request pending"]["composerDisabled"] is False, ( "the composer was already unusable before the send") - assert steps["after send"]["composerDisabled"] is False, ( + assert steps[reply]["after send"]["composerDisabled"] is False, ( "the composer is still disabled well inside the 30s request timeout -- " "the send never came back, which is what reads as a frozen Chat tab") -def test_the_message_is_displayed(probe): +def test_an_accepted_message_is_displayed(probe): """A sent message appears at once, not on the next visit to the tab.""" _, steps = probe - before = steps["stale request pending"]["bubbles"] - assert steps["after send"]["bubbles"] == before + 1, ( + before = steps["ack"]["older request pending"]["bubbles"] + assert steps["ack"]["after send"]["bubbles"] == before + 1, ( "the message was not added to the conversation") - assert steps["after send"]["lastText"] == "hello" - assert steps["after send"]["composerValue"] == "", ( + assert steps["ack"]["after send"]["lastText"] == "hello" + assert steps["ack"]["after send"]["composerValue"] == "", ( "the text came back into the composer, so the send was treated as failed") -def test_the_ack_is_not_handed_to_another_request(probe): - """The other half of the same defect: whatever was waiting got the ack and - carried on with a reply to a question it never asked.""" +def test_a_refused_message_is_not_displayed_as_sent(probe): + """The other direction, and the one routing this correctly makes possible. + + While a refusal reached the wrong caller it did not matter what `sendChat` + would have done with it. Now that it arrives, a message the node rejected + must not appear in the conversation as though it had been stored — it must + come back into the composer, where a person can see it did not go. + """ + _, steps = probe + before = steps["error"]["older request pending"]["bubbles"] + assert steps["error"]["after send"]["bubbles"] == before, ( + "a refused message was added to the conversation anyway") + assert steps["error"]["after send"]["composerValue"] == "hello", ( + "the refused text was dropped instead of being handed back") + + +@pytest.mark.parametrize("reply", ["ack", "error"]) +def test_the_reply_is_not_handed_to_another_request(probe, reply): + """The other half of the same defect: whatever was waiting got the reply + and carried on with an answer to a question it never asked.""" data, _ = probe - assert "media_meta resolved with ack" not in data["log"], ( - "the chat ack was routed to the pending media_meta_req -- that request " - "now believes it has an answer, and the chat send is waiting for a " - "reply that already arrived") + stolen = [line for line in data["log"] if line.startswith(f"{reply}: music_meta")] + assert not stolen, ( + f"the chat {reply} was routed to the pending music_meta_req ({stolen}) -- " + "that request now believes it has an answer, and the chat send is " + "waiting for a reply that already arrived") -def test_the_message_goes_out_sealed_and_signed(probe): +@pytest.mark.parametrize("reply", ["ack", "error"]) +def test_the_message_goes_out_sealed_and_signed(probe, reply): """ - What actually left the browser. A composer that let a plaintext message - through would be refused by the node, but the refusal arrives after the - fact and reads as "the message did not send" — so assert the shape here, - where the reason is visible. + What actually left the browser (MNP 2.0). + + Asserted for both scenarios because the composer is what decides to send: + a client that fell back to plaintext when something went wrong would be + refused by the node, but the refusal arrives after the fact and reads as + "the message did not send". There is no plaintext form on the wire. """ data, _ = probe - sent = [line for line in data["log"] if line.startswith("chat_msg ")] - assert sent, ("no chat_msg reached the stand-in node — the send did not " - f"complete. log: {data['log']}") + sent = [line for line in data["log"] + if line.startswith(f"{reply}: chat_msg ")] + assert sent, (f"no chat_msg reached the stand-in node in the {reply} " + f"scenario — the send did not complete. log: {data['log']}") assert "format=1" in sent[0], "the message was not sealed" assert "sig=64" in sent[0], "the message was not signed" assert "ct=" in sent[0] and "ct=0" not in sent[0], "there was no ciphertext" @@ -115,12 +136,25 @@ def test_the_message_goes_out_sealed_and_signed(probe): def test_the_chat_keys_answer_is_not_handed_to_another_request(probe): """ - The original defect's shape, in the message type that carries the group's - chat keys. An unanswered request is the ordinary case, not a rare one, and - the one this probe leaves outstanding swallowed the keys on the first run. + The same defect this file exists for, in the message type that carries the + group's chat keys — which did not exist when it was written, and which a + send now depends on. It went astray on the probe's first encrypted run. """ data, _ = probe - assert not any("media_meta resolved with chat_keys_resp" in line - for line in data["log"]), ( - "chat_keys_resp was routed by arrival order and handed to the stale " - "media_meta_req — the send then waits out its own 30s timeout") + stolen = [line for line in data["log"] if "music_meta resolved with" in line] + assert not stolen, ( + f"a reply was routed to the pending music_meta_req ({stolen}) — the " + "send then waits out its own 30s timeout with the composer disabled") + + +def test_history_still_renders(probe): + """ + Not about sending at all, and here because it broke without a sound: + `_asText` was deleted with an unrelated helper beside it, its only caller + sits inside a promise the panel catches, and every conversation rendered + empty with the node answering perfectly. + """ + _, steps = probe + assert steps["ack"]["older request pending"]["bubbles"] == 5, ( + "the five history messages did not render — the panel swallows a " + "failure in the transport's message reader, so this is silent") |