From 4d79ddd0b4e46499e472b95cb3cc39c573f50c9c Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 6 May 2026 11:54:13 +1000 Subject: [PATCH] pyxtest: add --display for running a test against a manually started server This makes it much easier to debug an individual test since we can now start an X server via valgrind/gdb/whatever and have the test client connect to that server. Assisted-by: Claude:claude-claude-opus-4-6 Part-of: --- test/pyxtest/README.md | 8 +++++ test/pyxtest/conftest.py | 62 +++++++++++++++++++++++++++++++-- test/pyxtest/xserver.py | 75 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 3 deletions(-) diff --git a/test/pyxtest/README.md b/test/pyxtest/README.md index 66f5a3243..0c1eec7ca 100644 --- a/test/pyxtest/README.md +++ b/test/pyxtest/README.md @@ -42,6 +42,14 @@ pytest test/pyxtest/ -v The normal pytest options work as expected (`-k` for test selection, etc.) +Tests can be run against a manually-started server using the `--display` +option: + +```sh +./build/hw/vfb/Xvfb :2 +pytest test/pyxtest --display :2 +``` + ### Running with AddressSanitizer (ASAN) ASAN is a compile-time instrumentation that detects memory errors such as diff --git a/test/pyxtest/conftest.py b/test/pyxtest/conftest.py index ab2e1375a..1f49a7da0 100644 --- a/test/pyxtest/conftest.py +++ b/test/pyxtest/conftest.py @@ -7,7 +7,7 @@ import shutil import pytest from pathlib import Path -from xserver import XServerProcess +from xserver import ExternalXServer, XServerProcess from xclient import RawX11Connection, X11ConnectionError, XlibConnection @@ -33,6 +33,14 @@ def pytest_addoption(parser): parser.addoption( "--server-path", default=None, help="Explicit path to the X server binary" ) + parser.addoption( + "--display", + default=None, + help="Connect to an existing X server instead of starting one. " + "Value is a display number or :N string (e.g. '42' or ':42'). " + "Optionally combine with --server-type to declare the server type " + "for marker-based test filtering.", + ) def pytest_configure(config): @@ -53,10 +61,36 @@ def pytest_configure(config): "markers", "swapped_client: mark test as requiring a byte-swapped client" ) + # Validate --display against conflicting options + display = config.getoption("--display", default=None) + if display is not None: + if config.getoption("--valgrind"): + raise pytest.UsageError("--display and --valgrind are mutually exclusive") + if config.getoption("--server-path"): + raise pytest.UsageError( + "--display and --server-path are mutually exclusive" + ) + + +def _parse_display(value): + """Parse a display string like ':42' or '42' into an integer.""" + value = value.strip().lstrip(":") + try: + return int(value) + except ValueError: + raise pytest.UsageError(f"Invalid display value: {value!r}") + def get_server_types(config) -> list[str]: - """Get the list of server types to test, default to xvfb.""" - return config.getoption("--server-type") or ["xvfb"] + """Get the list of server types to test. + + With ``--display``, defaults to ``["external"]`` if no explicit + ``--server-type`` is given. Otherwise defaults to ``["xvfb"]``. + """ + types = config.getoption("--server-type") or [] + if config.getoption("--display", default=None) is not None: + return types or ["external"] + return types or ["xvfb"] def get_valgrind_suppressions(config) -> Path | None: @@ -209,6 +243,9 @@ def xserver(request, tmp_path): A fresh server per test, killed afterward. With --valgrind, valgrind memory errors cause test failure during teardown. + When ``--display`` is given, no server is started; an + :class:`ExternalXServer` proxy is yielded instead. + For a fixture that targets a specific server type use the xvfb, xwayland, or xorg fixtures instead. """ @@ -220,6 +257,13 @@ def xserver(request, tmp_path): if request.node.get_closest_marker("xorg_only") and server_type != "xorg": pytest.skip("Test only applies to Xorg") + # External server mode: no server lifecycle management + display = request.config.getoption("--display") + if display is not None: + display_num = _parse_display(display) + yield ExternalXServer(display_num, server_type=server_type) + return + kwargs = {} if server_type == "xorg": kwargs["log_file"] = tmp_path / f"{server_type}.log" @@ -230,6 +274,10 @@ def xserver(request, tmp_path): @pytest.fixture def xvfb(request, tmp_path): """Start an Xvfb server for this test.""" + display = request.config.getoption("--display") + if display is not None: + yield ExternalXServer(_parse_display(display), server_type="xvfb") + return if "xvfb" not in get_server_types(request.config): pytest.skip("Xvfb not in --server-type list") yield from _start_server(request, "xvfb") @@ -238,6 +286,10 @@ def xvfb(request, tmp_path): @pytest.fixture def xwayland(request, tmp_path): """Start an Xwayland server for this test.""" + display = request.config.getoption("--display") + if display is not None: + yield ExternalXServer(_parse_display(display), server_type="xwayland") + return if "xwayland" not in get_server_types(request.config): pytest.skip("Xwayland not in --server-type list") yield from _start_server(request, "xwayland") @@ -246,6 +298,10 @@ def xwayland(request, tmp_path): @pytest.fixture def xorg(request, tmp_path): """Start an Xorg server for this test.""" + display = request.config.getoption("--display") + if display is not None: + yield ExternalXServer(_parse_display(display), server_type="xorg") + return if "xorg" not in get_server_types(request.config): pytest.skip("Xorg not in --server-type list") yield from _start_server(request, "xorg", log_file=tmp_path / "xorg.log") diff --git a/test/pyxtest/xserver.py b/test/pyxtest/xserver.py index bc392e322..32660967d 100644 --- a/test/pyxtest/xserver.py +++ b/test/pyxtest/xserver.py @@ -427,3 +427,78 @@ class XServerProcess: if self.asan: flags += " +asan" return f"" + + +class ExternalXServer: + """Proxy for an externally-managed X server (``--display`` mode). + + Used when the user passes ``--display`` to connect to an already-running + server instead of launching one per test. The server's PID is discovered + via ``SO_PEERCRED`` on the Unix socket so that :attr:`is_alive` can + report whether the process is still running. + """ + + def __init__(self, display_num, server_type="external"): + self._display_num = display_num + self.server_type = server_type + self._pid = self._get_server_pid() + + def _get_server_pid(self): + """Get the PID of the X server via SO_PEERCRED on the Unix socket.""" + import socket as _socket + import struct as _struct + + path = f"/tmp/.X11-unix/X{self._display_num}" + try: + sock = _socket.socket(_socket.AF_UNIX, _socket.SOCK_STREAM) + sock.connect(path) + cred = sock.getsockopt( + _socket.SOL_SOCKET, _socket.SO_PEERCRED, _struct.calcsize("iii") + ) + pid, _, _ = _struct.unpack("iii", cred) + sock.close() + return pid + except (OSError, _struct.error): + return None + + @property + def display_num(self): + return self._display_num + + @property + def display(self): + return f":{self._display_num}" + + @property + def is_alive(self) -> bool: + """Check if the external server process is still running.""" + if self._pid is None: + return True # can't check, assume alive + try: + os.kill(self._pid, 0) + return True + except ProcessLookupError: + return False + except PermissionError: + return True # process exists but we can't signal it + + @property + def crash_signal(self) -> int | None: + """Always None -- we cannot determine this for external servers.""" + return None + + def stop(self): + """No-op -- never stop an external server.""" + return [] + + def get_asan_errors(self): + """No ASAN log access for external servers.""" + return [] + + def check_valgrind_errors(self): + """No valgrind access for external servers.""" + return [] + + def __repr__(self): + state = "running" if self.is_alive else "stopped" + return f""