Skip to content

Commit aadb165

Browse files
committed
fix(pytest_plugin): unlink socket file on fixture teardown
The `server` and `TestServer` fixtures called `server.kill()` on teardown but never unlinked the socket file under `/tmp/tmux-<uid>/`. tmux does not reliably `unlink(2)` its own socket on non-graceful exit — so every test run that used these fixtures left behind stale socket entries that accumulated indefinitely (10k+ observed on long-lived dev machines). Introduce `_reap_test_server(socket_name)` helper that kills the daemon (if alive) *and* unconditionally unlinks the computed socket path. `Server(socket_name=...)` does not populate `socket_path`, so the helper recomputes `$TMUX_TMPDIR/tmux-<uid>/<name>` the same way tmux does. Both fixture finalizers delegate to it. Cleanup errors are suppressed via `contextlib.suppress` so a finalizer failure never masks the real test failure, and races with pytest-xdist workers (concurrent access to the same directory) stay benign. Kills ride through `LibTmuxException`/`OSError`; unlinks use `missing_ok=True` and swallow `OSError`. Added regression tests: - `_reap_test_server` unlinks a real socket file after killing a live daemon. - Reaping a never-created socket is a silent no-op. - Reaping `None` is tolerated for symmetry with `Server.socket_name`'s nullable type. Fixes #660
1 parent 027c919 commit aadb165

3 files changed

Lines changed: 118 additions & 9 deletions

File tree

CHANGES

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,18 @@ $ uvx --from 'libtmux' --prerelease allow python
4040
_Notes on the upcoming release will go here._
4141
<!-- END PLACEHOLDER - ADD NEW CHANGELOG ENTRIES BELOW THIS LINE -->
4242

43+
### Fixes
44+
45+
- `pytest_plugin`: the `server` and `TestServer` fixture finalizers now
46+
unlink the tmux socket file from `/tmp/tmux-<uid>/` in addition to
47+
calling `server.kill()`. tmux does not reliably `unlink(2)` its
48+
socket on non-graceful exit, so `/tmp/tmux-<uid>/` would otherwise
49+
accumulate stale `libtmux_test*` entries across test runs (10k+
50+
observed on long-lived dev machines). A new internal helper
51+
`_reap_test_server` centralizes the kill + unlink flow and
52+
suppresses cleanup-time errors so a finalizer failure can no longer
53+
mask the real test failure (#660).
54+
4355
### Documentation
4456

4557
- Visual improvements to API docs from [gp-sphinx](https://gp-sphinx.git-pull.com)-based Sphinx packages (#658)

src/libtmux/pytest_plugin.py

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,40 @@
2424
USING_ZSH = "zsh" in os.getenv("SHELL", "")
2525

2626

27+
def _reap_test_server(socket_name: str | None) -> None:
28+
"""Kill the tmux daemon on ``socket_name`` and unlink the socket file.
29+
30+
Invoked from the :func:`server` and :func:`TestServer` fixture
31+
finalizers to guarantee teardown even when the daemon has already
32+
exited (``kill`` is a no-op then) and the socket file was left on
33+
disk. tmux does not reliably ``unlink(2)`` its socket on
34+
non-graceful exit, so ``/tmp/tmux-<uid>/`` otherwise accumulates
35+
stale entries across test runs.
36+
37+
Conservative: suppresses ``LibTmuxException`` / ``OSError`` on both
38+
the kill and the unlink. A finalizer that raises replaces the real
39+
test failure with a cleanup error, and cleanup failures are not
40+
actionable (socket already gone, permissions changed, race with a
41+
concurrent pytest-xdist worker).
42+
"""
43+
if not socket_name:
44+
return
45+
46+
with contextlib.suppress(exc.LibTmuxException, OSError):
47+
srv = Server(socket_name=socket_name)
48+
if srv.is_alive():
49+
srv.kill()
50+
51+
# ``Server(socket_name=...)`` does not populate ``socket_path`` —
52+
# the Server class only derives the path when neither ``socket_name``
53+
# nor ``socket_path`` was supplied. Recompute the location tmux uses
54+
# so we can unlink the file regardless of daemon state.
55+
tmux_tmpdir = pathlib.Path(os.environ.get("TMUX_TMPDIR", "/tmp"))
56+
socket_path = tmux_tmpdir / f"tmux-{os.geteuid()}" / socket_name
57+
with contextlib.suppress(OSError):
58+
socket_path.unlink(missing_ok=True)
59+
60+
2761
@pytest.fixture(scope="session")
2862
def home_path(tmp_path_factory: pytest.TempPathFactory) -> pathlib.Path:
2963
"""Temporary `/home/` path."""
@@ -141,7 +175,7 @@ def server(
141175
server = Server(socket_name=f"libtmux_test{next(namer)}")
142176

143177
def fin() -> None:
144-
server.kill()
178+
_reap_test_server(server.socket_name)
145179

146180
request.addfinalizer(fin)
147181

@@ -295,11 +329,9 @@ def socket_name_factory() -> str:
295329
return f"libtmux_test{next(namer)}"
296330

297331
def fin() -> None:
298-
"""Kill all servers created with these sockets."""
332+
"""Kill all servers created with these sockets and unlink their sockets."""
299333
for socket_name in created_sockets:
300-
server = Server(socket_name=socket_name)
301-
if server.is_alive():
302-
server.kill()
334+
_reap_test_server(socket_name)
303335

304336
request.addfinalizer(fin)
305337

tests/test_pytest_plugin.py

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,19 @@
22

33
from __future__ import annotations
44

5+
import contextlib
6+
import os
7+
import pathlib
58
import textwrap
69
import time
710
import typing as t
811

9-
if t.TYPE_CHECKING:
10-
import pathlib
12+
from libtmux.pytest_plugin import _reap_test_server
13+
from libtmux.server import Server
1114

15+
if t.TYPE_CHECKING:
1216
import pytest
1317

14-
from libtmux.server import Server
15-
1618

1719
def test_plugin(
1820
pytester: pytest.Pytester,
@@ -156,3 +158,66 @@ def test_test_server_multiple(TestServer: t.Callable[..., Server]) -> None:
156158
assert any(s.session_name == "test2" for s in server2.sessions)
157159
assert not any(s.session_name == "test1" for s in server2.sessions)
158160
assert not any(s.session_name == "test2" for s in server1.sessions)
161+
162+
163+
def _libtmux_socket_dir() -> pathlib.Path:
164+
"""Resolve the tmux socket directory tmux uses for this uid."""
165+
tmux_tmpdir = pathlib.Path(os.environ.get("TMUX_TMPDIR", "/tmp"))
166+
return tmux_tmpdir / f"tmux-{os.geteuid()}"
167+
168+
169+
def test_reap_test_server_unlinks_socket_file() -> None:
170+
"""``_reap_test_server`` kills the daemon *and* unlinks the socket.
171+
172+
Regression for #660: tmux does not reliably ``unlink(2)`` its socket
173+
on non-graceful exit. Before this fix the plugin's finalizer only
174+
called ``server.kill()``, so ``/tmp/tmux-<uid>/`` accumulated stale
175+
``libtmux_test*`` socket files over time.
176+
177+
This test boots a real tmux daemon on a unique socket, confirms the
178+
socket file exists, invokes the reaper, and asserts the file is
179+
gone.
180+
"""
181+
server = Server(socket_name="libtmux_test_reap_unlink")
182+
server.new_session(session_name="reap_probe")
183+
socket_path = _libtmux_socket_dir() / "libtmux_test_reap_unlink"
184+
try:
185+
assert socket_path.exists(), (
186+
f"expected tmux to have created {socket_path}, but it is missing"
187+
)
188+
189+
_reap_test_server("libtmux_test_reap_unlink")
190+
191+
assert not socket_path.exists(), (
192+
f"_reap_test_server should have unlinked {socket_path}"
193+
)
194+
finally:
195+
# Belt-and-braces: if the assertion above fired before the
196+
# unlink, don't leak the socket the next run of this test.
197+
with contextlib.suppress(OSError):
198+
socket_path.unlink(missing_ok=True)
199+
200+
201+
def test_reap_test_server_is_noop_when_socket_missing() -> None:
202+
"""Reaping a non-existent socket succeeds silently.
203+
204+
Finalizers run even when the fixture failed before the daemon ever
205+
started; the reaper must tolerate the case where the socket file
206+
never existed.
207+
"""
208+
bogus_name = "libtmux_test_reap_never_existed_xyz"
209+
socket_path = _libtmux_socket_dir() / bogus_name
210+
assert not socket_path.exists()
211+
212+
# Should not raise.
213+
_reap_test_server(bogus_name)
214+
215+
216+
def test_reap_test_server_tolerates_none() -> None:
217+
"""``_reap_test_server(None)`` is a no-op, not a crash.
218+
219+
The ``server`` fixture's finalizer passes ``server.socket_name``,
220+
which is typed ``str | None``. Tolerate ``None`` for symmetry with
221+
other nullable paths in the API.
222+
"""
223+
_reap_test_server(None)

0 commit comments

Comments
 (0)