diff options
Diffstat (limited to 'packages/meshbay-hub/tests/test_chat_send.py')
| -rw-r--r-- | packages/meshbay-hub/tests/test_chat_send.py | 94 |
1 files changed, 63 insertions, 31 deletions
diff --git a/packages/meshbay-hub/tests/test_chat_send.py b/packages/meshbay-hub/tests/test_chat_send.py index 4224fdb..8b098c2 100644 --- a/packages/meshbay-hub/tests/test_chat_send.py +++ b/packages/meshbay-hub/tests/test_chat_send.py @@ -1,21 +1,27 @@ """ -Sending a chat message must come back. +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. """ @@ -39,35 +45,61 @@ def probe(): run = subprocess.run(["python3", str(HARNESS)], 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") |