Skip to content

test(tui): improve coverage for tui_gateway/ws.py (fixes #36617) - #49605

Open
Morad37 wants to merge 3 commits into
NousResearch:mainfrom
Morad37:test-improve-coverage-ws-36617
Open

Morad37 wants to merge 3 commits into
NousResearch:mainfrom
Morad37:test-improve-coverage-ws-36617

Conversation

@Morad37

@Morad37 Morad37 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Brings tui_gateway/ws.py coverage from 0% to 86% (150 stmts, 21 missed).

Covers: WSTransport init/write/write_async/_safe_send/close,
_ws_peer_label, _disable_nagle, and key paths in handle_ws (accept,
parse error, dispatch, ready send failure, dispatch crash).

Closes #36617

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels Jun 20, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding focused WebSocket coverage. The coverage goal is still useful, but this test file needs adaptation before it can be salvaged onto current main.

Problems

  • tests/tui_gateway/test_ws.py:26 defines FakeWebSocket without close(), while current tui_gateway/ws.py:452 unconditionally awaits ws.close() in handle_ws() cleanup. The handle_ws cases will therefore fail during teardown rather than complete their assertions.
  • Current main substantially changed this path after the PR base: token batching is in tui_gateway/ws.py:125-221, MCP startup is at :312-317, and disconnect teardown is at :425-450. The tests also need to isolate the new MCP-startup call and cover these current contracts.

Suggested changes

  • Add an awaited close mock to the fake, patch MCP startup in handle_ws tests, and port the reusable cases to the current transport semantics.
  • Add coverage for batching/order and disconnect session teardown alongside the existing parse and dispatch failures.

Automated hermes-sweeper review.



class FakeWebSocket:
"""Minimal stand-in for a starlette WebSocket to avoid starlette imports in test."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle_ws() now always awaits ws.close() in its finally block (tui_gateway/ws.py:452), but this fake does not provide close. Add an async close mock/method so the handle_ws tests can reach their assertions without failing during cleanup.

@teknium1 teknium1 added sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve test coverage: tui_gateway/ws.py

3 participants