Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ help:
@echo "servers-down: stop the docker VNC test servers"
@echo "test-servers: run functional tests against the VNC test servers"
@echo "test-os-server: run functional tests against this OS's VNC server"
@echo "test-api: run the in-process vncdotool.api lifecycle suite"
@echo "screenshots: screenshot each running VNC test server into a gallery"
@echo "docs: build documentation"
@echo "release: tag and push current version to trigger PyPI release"
Expand Down
9 changes: 6 additions & 3 deletions docs/testing-framework-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ questions.
**In-process API suite** (small, separate): the library API needs live
coverage of its *lifecycle*, not of server compatibility — `api.connect`,
error propagation, timeouts, `api.shutdown` cleanliness — against a single
known-good container. One reactor per process means this suite runs in its
own process invocation. It does not fan out across the fleet: server
compatibility is already proven by the subprocess grid.
known-good container. One reactor per process means exactly one module
(`test_api_lifecycle.py`) may ever touch `vncdotool.api` in-process; it is
safe inside the shared functional discover because every other module is
subprocess-only and never touches that reactor, regardless of run order —
`make test-api` also runs it alone. It does not fan out across the fleet:
server compatibility is already proven by the subprocess grid.

### 3. Capture kit (discovery for unhosted servers)

Expand Down
187 changes: 187 additions & 0 deletions tests/functional/test_api_lifecycle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
"""In-process coverage of ``vncdotool.api``'s *lifecycle* -- context-manager
use, reconnection, error propagation, timeouts, ``shutdown()`` cleanliness.
Server compatibility belongs to the subprocess grid in test_servers.py.

``api.connect()`` starts the Twisted reactor in a daemon thread, and a
reactor cannot be restarted once stopped, so one process gets one reactor
for its whole lifetime: this is the only module allowed to call it. A
second in-process module would either race this one for it or leave its
own reactor thread wedged on exit -- new in-process tests belong in
*this* file, sharing *this* reactor.

``setUpModule`` makes the first connection itself, so the reactor is
running before any test method does -- no test depends on being first.
"""

import queue
import socket
import threading
import time
import unittest

from twisted.internet import reactor
from twisted.internet.error import ConnectError
from twisted.internet.protocol import Factory, Protocol

from vncdotool import api

from .vncservers import DOCKER_SERVERS, HOST, SUBPROCESS_TIMEOUT_HEADROOM, connect, port_open

LIBVNC = next(s for s in DOCKER_SERVERS if s.name == "libvncserver-example")

# Same headroom the subprocess grid adds: the bare 5s server budget was
# observed to flake under load.
HAPPY_TIMEOUT = LIBVNC.timeout + SUBPROCESS_TIMEOUT_HEADROOM

SHORT_TIMEOUT = 2.0

# join() budget for the closed-port helper thread: long enough to fail
# promptly, short enough that a real hang doesn't stall the suite.
HANG_GUARD_TIMEOUT = 10.0

# Bound for a call dispatched onto the reactor thread via callFromThread --
# a local listenTCP/stopListening, not network I/O, so this is generous
# only relative to how fast that actually is.
REACTOR_CALL_TIMEOUT = 2.0


def setUpModule() -> None:
if not port_open(HOST, LIBVNC.port):
raise RuntimeError(
f"libvncserver-example not reachable on {HOST}:{LIBVNC.port} -- "
"start the servers first with `make servers-up`"
)
with connect(LIBVNC, timeout=HAPPY_TIMEOUT) as client:
client.refreshScreen()


def tearDownModule() -> None:
# Exactly once per process: without it the interpreter hangs on the
# non-daemon worker threads Twisted runs under the reactor.
api.shutdown()


class TestApiLifecycle(unittest.TestCase):
"""Lifecycle cases against the libvncserver-example:5935 container.

Independent of each other and of run order: the one case that could
wedge the shared reactor (the closed-port case) is bounded by its own
timeout and helper thread, not by being run last.
"""

def test_context_manager_connect(self) -> None:
"""The documented ``with api.connect(...) as client:`` pattern works."""
with connect(LIBVNC, timeout=HAPPY_TIMEOUT) as client:
client.keyPress("x")

def test_sequential_reconnects(self) -> None:
"""``shutdown()`` is what's terminal, not ``disconnect()``: otherwise no
long-running application could reconnect after a single drop.
"""
for _ in range(2):
with connect(LIBVNC, timeout=HAPPY_TIMEOUT) as client:
client.refreshScreen()

def test_timeout_raises_timeout_error(self) -> None:
"""``api.connect()`` is fire-and-forget and never raises here; the
timeout comes from the first call that has to wait on the
never-completing handshake.
"""
listener = _SilentListener()
listener.start()
self.addCleanup(listener.stop)

client = api.connect(f"{HOST}::{listener.port}")
client.timeout = SHORT_TIMEOUT
self.addCleanup(client.disconnect)

start = time.monotonic()
with self.assertRaises(TimeoutError):
client.refreshScreen()
elapsed = time.monotonic() - start

# Tight bound: the timeout path costs mild thread-wakeup slack at
# most. Padding this further would hide a real regression instead
# of catching one.
self.assertLess(elapsed, SHORT_TIMEOUT + 1.0)

def test_closed_port_raises_promptly(self) -> None:
"""Bounded twice over -- a per-client timeout, and a helper thread
joined with a deadline -- so even a wedge fails only this test,
not the suite.

Twisted's ``ConnectionRefusedError`` is a ``ConnectError``, not the
stdlib ``OSError`` of the same name, so assert on that base class.
"""
closed_port = _closed_port()

outcome: dict = {}

def attempt() -> None:
try:
client = api.connect(f"{HOST}::{closed_port}")
client.timeout = SHORT_TIMEOUT
try:
client.refreshScreen()
outcome["result"] = "no exception"
except Exception as exc: # noqa: BLE001 - captured for the assertion below
outcome["exception"] = exc
finally:
client.disconnect()
except Exception as exc: # noqa: BLE001 - connect() itself is not expected to raise
outcome["exception"] = exc

worker = threading.Thread(target=attempt, name="closed-port-attempt", daemon=True)
worker.start()
worker.join(timeout=HANG_GUARD_TIMEOUT)

if worker.is_alive():
self.fail(
f"connect()/refreshScreen() against a closed port did not "
f"return within {HANG_GUARD_TIMEOUT}s -- this would have "
"hung the process"
)

self.assertIn("exception", outcome, f"expected an exception, got: {outcome}")
# The family, not the exact class: any prompt, clear connection
# failure satisfies the lifecycle guarantee.
self.assertIsInstance(outcome["exception"], ConnectError)


class _SilentProtocol(Protocol):
"""Accepts the connection and never sends or reads anything."""


class _SilentListener:
"""A TCP listener on api.connect()'s own reactor that accepts and then
never speaks, forcing a protocol-level timeout rather than a
connection-refused. This process gets exactly one reactor, so this
reuses it rather than starting a second one via raw sockets.
"""

def start(self) -> None:
ready: "queue.Queue[None]" = queue.Queue()

def _listen() -> None:
self._port = reactor.listenTCP(0, Factory.forProtocol(_SilentProtocol), interface=HOST)
ready.put(None)

reactor.callFromThread(_listen)
ready.get(timeout=REACTOR_CALL_TIMEOUT)
self.port = self._port.getHost().port

def stop(self) -> None:
reactor.callFromThread(self._port.stopListening)


def _closed_port() -> int:
"""A port nothing listens on, found by binding and releasing it."""
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.bind((HOST, 0))
port = s.getsockname()[1]
s.close()
return port


if __name__ == "__main__":
unittest.main()
4 changes: 4 additions & 0 deletions tests/functional/vncservers.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ def connect(server: VNCServer, timeout: Optional[float] = None) -> api.ThreadedV

Remember that the returned client is a context manager, and that
``api.shutdown()`` still has to be called once before the process exits.
And that shutdown is terminal for the whole process: after it, no
further ``connect()`` can ever work again (the reactor cannot restart),
which is why only ONE test module -- test_api_lifecycle.py -- may use
this in-process; everything else shells out via run_vncdo().
"""
client = api.connect(
f"{HOST}::{server.port}",
Expand Down
7 changes: 7 additions & 0 deletions tests/servers/servers.mk
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ test-servers:
test-os-server:
$(PYTHON) -m unittest discover $(UNITTEST_ARGS) -s tests/functional -t . -p 'test_os_servers.py'

# The in-process vncdotool.api lifecycle suite, against libvncserver-example
# alone. Runs on its own because one process gets one Twisted reactor -- see
# tests/functional/test_api_lifecycle.py.
.PHONY: test-api
test-api:
$(PYTHON) -m unittest discover $(UNITTEST_ARGS) -s tests/functional -t . -p 'test_api_lifecycle.py'

# Screenshot every running test server into $(SCREENSHOT_DIR), including an
# index.html gallery of them all, for eyeballing what the servers render.
.PHONY: screenshots
Expand Down
Loading