Skip to content

Commit ae442fd

Browse files
committed
Tests(feat[arena]): Audit two documentation pages on one lent server
why: an artifact could bind several pages but none did, so the capability was unexercised and the cost it exists to avoid — one server per page — was still being paid. Sharing a server also raises the question the single-page gate never had to answer: whether the server a page finishes on is the one it started on. what: - `python-workspace-and-location` audits `workspace_setup.md` and `self_location.md` together. Both take the server the fixture hands them. - `context_managers.md` is excluded by name and reason, not by omission: every example there opens `with Server()`, which under the arena resolves to the lent socket, so leaving the block stops the borrowed server. An import-time guard refuses to load if any artifact ever names an excluded page, and the request is rejected before tmux is touched. - The server's identity — pid, socket and challenge — is proved after each page, not only at the end, and a change names the page it happened after. - Teardown reaps only the sessions the run created, diffed against a baseline taken before it, and only while the identity still matches. It never stops the server. - A destructive test reproduces the failure this is for: a wrapper that stops the server between pages makes the run exit nonzero with no evidence, naming the page. Its control runs the two pages and proves identity between them.
1 parent b38cadc commit ae442fd

3 files changed

Lines changed: 414 additions & 19 deletions

File tree

conftest.py

Lines changed: 176 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import pytest
2222
from _pytest.doctest import DoctestItem
2323

24-
from libtmux._arena import ArenaSpec
24+
from libtmux._arena import ARENA_EXCLUDED_SOURCES, ArenaSpec
2525
from libtmux._internal.control_mode import ControlMode
2626
from libtmux.client import Client
2727
from libtmux.pane import Pane
@@ -33,6 +33,11 @@
3333
pytest_plugins = ["pytester"]
3434

3535
ARENA_EVIDENCE_PREFIX = "LIBTMUX_ARENA_EVIDENCE="
36+
# One round-trip answers identity and the challenge together: pid and
37+
# socket_path prove *which* server answered, @libtmux_arena_challenge proves
38+
# it is still configured as the one that was lent (a silently spawned
39+
# replacement starts with no such option set).
40+
ARENA_IDENTITY_FORMAT = "#{pid}\t#{socket_path}\t#{@libtmux_arena_challenge}"
3641
ARENA_SPEC_KEY: pytest.StashKey[ArenaSpec] = pytest.StashKey()
3742
# A set rather than one path: an artifact may audit several pages, and the
3843
# supervisor then expects one evidence record per page.
@@ -49,6 +54,16 @@
4954
ARENA_PASSED_KEY: pytest.StashKey[dict[pathlib.Path, frozenset[str]]] = (
5055
pytest.StashKey()
5156
)
57+
# The identity captured once, before any page runs, so every later query has
58+
# something fixed to compare against rather than just "looks fine to itself".
59+
ARENA_IDENTITY_KEY: pytest.StashKey[tuple[int, str, str]] = pytest.StashKey()
60+
# Session names already on the lend before this run touched it, so teardown
61+
# can reap exactly what the run's own examples left behind and nothing else.
62+
ARENA_BASELINE_SESSIONS_KEY: pytest.StashKey[frozenset[str]] = pytest.StashKey()
63+
# The last node id collected per source, in the order pytest will actually
64+
# run them -- how the per-page identity check knows a page has finished
65+
# without guessing at item counts.
66+
ARENA_LAST_NODEID_KEY: pytest.StashKey[dict[pathlib.Path, str]] = pytest.StashKey()
5267

5368

5469
def _arena_spec(config: pytest.Config) -> ArenaSpec | None:
@@ -61,6 +76,75 @@ def _arena_targets(config: pytest.Config) -> frozenset[pathlib.Path] | None:
6176
return config.stash.get(ARENA_TARGETS_KEY, None)
6277

6378

79+
def _query_arena_identity(spec: ArenaSpec) -> tuple[int, str, str]:
80+
"""Return (pid, socket_path, challenge) for the endpoint ``spec`` names.
81+
82+
``display-message`` only ever answers a server already listening on the
83+
requested socket -- unlike ``new-session``, tmux does not spawn one to
84+
service it (verified: a `list-sessions`/`display-message` against a
85+
socket with nothing listening just errors, it never creates the socket
86+
file). So this query itself can never be what silently starts the
87+
replacement server a stopped lend produces.
88+
"""
89+
server = Server(socket_path=spec.socket_path, tmux_bin=spec.tmux_bin)
90+
result = server.cmd("display-message", "-p", ARENA_IDENTITY_FORMAT).stdout
91+
if len(result) != 1:
92+
msg = "arena server identity query returned an unexpected result"
93+
raise RuntimeError(msg)
94+
parts = result[0].split("\t", 2)
95+
if len(parts) != 3 or parts[1] != spec.socket_path or not parts[2]:
96+
msg = "arena server identity does not match the requested endpoint"
97+
raise RuntimeError(msg)
98+
return int(parts[0]), parts[1], parts[2]
99+
100+
101+
def _verify_arena_identity(
102+
spec: ArenaSpec,
103+
baseline: tuple[int, str, str],
104+
where: str,
105+
) -> None:
106+
"""Raise, naming ``where``, if the lent server's identity has moved on.
107+
108+
``where`` is the source whose examples just finished running, or "the
109+
run" for the check just before evidence is published -- so a break is
110+
attributed to the page after which it was detected, rather than only
111+
discovered once every requested page has already run against whatever
112+
answered next.
113+
"""
114+
try:
115+
observed = _query_arena_identity(spec)
116+
except RuntimeError as exc_info:
117+
msg = f"arena server identity check failed after {where!r}: {exc_info}"
118+
raise RuntimeError(msg) from exc_info
119+
if observed != baseline:
120+
msg = (
121+
f"arena server identity changed after {where!r}: expected "
122+
f"pid/socket/challenge {baseline!r}, got {observed!r}"
123+
)
124+
raise RuntimeError(msg)
125+
126+
127+
def _reap_arena_sessions(spec: ArenaSpec, baseline_sessions: frozenset[str]) -> None:
128+
"""Kill every session this run's own examples left behind.
129+
130+
Idempotent: a session already gone (already reaped, or never existed)
131+
is not an error. Scoped to the diff against ``baseline_sessions`` -- the
132+
lend's own sessions from before this run touched it -- so a session that
133+
predates the run, such as its hold session, is never a candidate.
134+
"""
135+
server = Server(socket_path=spec.socket_path, tmux_bin=spec.tmux_bin)
136+
current = {s.session_name for s in server.sessions if s.session_name is not None}
137+
for name in sorted(current - baseline_sessions):
138+
cleanup = server.cmd("kill-session", target=name)
139+
already_gone = cleanup.returncode != 0 and any(
140+
"can't find session" in line or "session not found" in line
141+
for line in cleanup.stderr
142+
)
143+
if cleanup.returncode != 0 and not already_gone:
144+
msg = f"arena teardown could not reap leaked session {name!r}"
145+
raise RuntimeError(msg)
146+
147+
64148
def pytest_addoption(parser: pytest.Parser) -> None:
65149
"""Register the source(s) selected by the arena adapter.
66150
@@ -86,6 +170,22 @@ def pytest_configure(config: pytest.Config) -> None:
86170
return
87171

88172
raw_targets = frozenset(config.getoption("libtmux_arena_target") or [])
173+
excluded = {
174+
source: ARENA_EXCLUDED_SOURCES[source]
175+
for source in raw_targets
176+
if source in ARENA_EXCLUDED_SOURCES
177+
}
178+
if excluded:
179+
# Checked before the artifact's own tuple is even consulted: a
180+
# source like this must be refused by name, not merely absent from
181+
# ARENA_ARTIFACT_TARGETS -- the two look identical from outside a
182+
# mismatch error otherwise.
183+
reasons = "; ".join(
184+
f"{source!r} ({reason})" for source, reason in sorted(excluded.items())
185+
)
186+
msg = f"arena refuses excluded source(s): {reasons}"
187+
raise pytest.UsageError(msg)
188+
89189
expected_relative = frozenset(
90190
p.as_posix() for p in spec.targets_for(pathlib.Path())
91191
)
@@ -97,6 +197,20 @@ def pytest_configure(config: pytest.Config) -> None:
97197
config.stash[ARENA_SPEC_KEY] = spec
98198
config.stash[ARENA_TARGETS_KEY] = targets
99199

200+
# Baseline, captured before any page runs: what "the lent server" and
201+
# "its sessions" mean for the rest of this run. Every later identity
202+
# check compares against this rather than against its own last query, so
203+
# a slow drift across several pages cannot pass by always comparing
204+
# favorably to the most recent (possibly already-wrong) reading.
205+
try:
206+
config.stash[ARENA_IDENTITY_KEY] = _query_arena_identity(spec)
207+
except RuntimeError as exc_info:
208+
raise pytest.UsageError(str(exc_info)) from exc_info
209+
baseline_server = Server(socket_path=spec.socket_path, tmux_bin=spec.tmux_bin)
210+
config.stash[ARENA_BASELINE_SESSIONS_KEY] = frozenset(
211+
s.session_name for s in baseline_server.sessions if s.session_name is not None
212+
)
213+
100214

101215
def pytest_collection_finish(session: pytest.Session) -> None:
102216
"""Reject selections that include anything besides the audited sources."""
@@ -121,6 +235,17 @@ def pytest_collection_finish(session: pytest.Session) -> None:
121235
raise pytest.UsageError(msg)
122236
session.config.stash[ARENA_COLLECTED_KEY] = dict(discovered)
123237

238+
# The last node id per source, in the order pytest is actually about to
239+
# run them (not the order they happened to be discovered in). This is
240+
# how the per-page identity check recognizes "a page just finished"
241+
# without hard-coding how many items any given page collects.
242+
last_nodeid_by_path: dict[pathlib.Path, str] = {}
243+
for item in session.items:
244+
item_path = item.path.resolve()
245+
if item_path in targets:
246+
last_nodeid_by_path[item_path] = item.nodeid
247+
session.config.stash[ARENA_LAST_NODEID_KEY] = last_nodeid_by_path
248+
124249

125250
def pytest_itemcollected(item: pytest.Item) -> None:
126251
"""Record every arena item before pytest applies filters."""
@@ -218,6 +343,26 @@ def add_doctest_fixtures(
218343
msg = "arena session cleanup failed"
219344
raise RuntimeError(msg)
220345

346+
# Prove identity survived this page before the next one starts
347+
# to run against whatever answers next. Only the item that is
348+
# last (in run order) for its source checks -- an earlier item
349+
# in the same page would just repeat a check its own page
350+
# hasn't finished yet, and the page boundary is exactly where a
351+
# doctest-content break (like the excluded context_managers.md's
352+
# `with Server()`) would land.
353+
last_nodeid_by_path = request.config.stash.get(
354+
ARENA_LAST_NODEID_KEY,
355+
{},
356+
)
357+
item_path = request._pyfuncitem.path.resolve()
358+
if last_nodeid_by_path.get(item_path) == request._pyfuncitem.nodeid:
359+
baseline = request.config.stash.get(ARENA_IDENTITY_KEY, None)
360+
if baseline is not None:
361+
source = item_path.relative_to(
362+
request.config.rootpath,
363+
).as_posix()
364+
_verify_arena_identity(spec, baseline, source)
365+
221366

222367
@pytest.fixture(autouse=True)
223368
def set_home(
@@ -248,9 +393,17 @@ def setup_session(
248393

249394

250395
def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None:
251-
"""Publish one evidence record per audited source, after all of them pass.
396+
"""Reap the run's stray sessions, then publish evidence once all pass.
252397
253-
fail-closed is preserved because ``exitstatus`` is only
398+
Reaping runs whenever the identity captured at ``pytest_configure`` still
399+
matches -- regardless of ``exitstatus`` -- so a run that fails for an
400+
unrelated reason does not leave the lend dirtier than it found it.
401+
It is skipped, not attempted against a guess, the moment identity no
402+
longer matches: after a break (or a silent replacement) the socket may
403+
not even be answering for the server this run was lent, and teardown
404+
does not touch a tmux server it cannot first prove is that one.
405+
406+
fail-closed is preserved for evidence because ``exitstatus`` is only
254407
``pytest.ExitCode.OK`` when every collected item passed; since collection
255408
already required every target to contribute at least one item
256409
(``pytest_collection_finish``), ``collected_by_target`` and
@@ -260,38 +413,42 @@ def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None:
260413
"""
261414
spec = _arena_spec(session.config)
262415
targets = _arena_targets(session.config)
263-
if spec is None or targets is None or exitstatus != pytest.ExitCode.OK:
416+
if spec is None or targets is None:
264417
return
418+
419+
baseline_identity = session.config.stash.get(ARENA_IDENTITY_KEY, None)
420+
baseline_sessions = session.config.stash.get(ARENA_BASELINE_SESSIONS_KEY, None)
421+
try:
422+
observed_identity = _query_arena_identity(spec)
423+
except RuntimeError:
424+
observed_identity = None
425+
identity_intact = (
426+
baseline_identity is not None and observed_identity == baseline_identity
427+
)
428+
if identity_intact and baseline_sessions is not None:
429+
_reap_arena_sessions(spec, baseline_sessions)
430+
265431
collected_by_target = session.config.stash.get(ARENA_COLLECTED_KEY, {})
266432
passed_by_target = session.config.stash.get(ARENA_PASSED_KEY, {})
267433
if (
268-
not collected_by_target
434+
exitstatus != pytest.ExitCode.OK
435+
or not collected_by_target
269436
or collected_by_target.keys() != targets
270437
or passed_by_target != collected_by_target
271438
or session.config.getoption("collectonly")
272439
):
273440
return
274441

275-
server = Server(socket_path=spec.socket_path, tmux_bin=spec.tmux_bin)
276-
result = server.cmd(
277-
"display-message",
278-
"-p",
279-
"#{pid}\t#{socket_path}\t#{@libtmux_arena_challenge}",
280-
).stdout
281-
if len(result) != 1:
282-
msg = "arena server identity query returned an unexpected result"
283-
raise RuntimeError(msg)
284-
parts = result[0].split("\t", 2)
285-
if len(parts) != 3 or parts[1] != spec.socket_path or not parts[2]:
442+
if baseline_identity is None or not identity_intact:
286443
msg = "arena server identity does not match the requested endpoint"
287444
raise RuntimeError(msg)
288445
for target in sorted(targets, key=lambda p: p.as_posix()):
289446
evidence = {
290447
"artifact": spec.artifact,
291-
"challenge": parts[2],
448+
"challenge": baseline_identity[2],
292449
"schema": 1,
293-
"server_pid": int(parts[0]),
294-
"socket_path": parts[1],
450+
"server_pid": baseline_identity[0],
451+
"socket_path": baseline_identity[1],
295452
"source": target.relative_to(session.config.rootpath).as_posix(),
296453
}
297454
print(

src/libtmux/_arena.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,55 @@
99
ARENA_ARTIFACT_TARGETS = {
1010
"python-exact-binary": ("docs/topics/workspace_setup.md",),
1111
"python-workspace-setup": ("docs/topics/workspace_setup.md",),
12+
# The first artifact to actually exercise several sources against one
13+
# lent server. Both pages only ever touch the `server`/`session` the
14+
# arena fixture hands them -- neither constructs its own `Server()`, so
15+
# neither can reach the lent daemon's own kill-server (see
16+
# ARENA_EXCLUDED_SOURCES below for the page that does).
17+
"python-workspace-and-location": (
18+
"docs/topics/workspace_setup.md",
19+
"docs/topics/self_location.md",
20+
),
1221
}
1322

23+
# A source that must never run under the arena: its examples stop the lent
24+
# server rather than a private one. Named here so the refusal is a specific,
25+
# visible reason rather than an absence from ARENA_ARTIFACT_TARGETS -- the
26+
# two would otherwise look identical from the outside (a source rejected for
27+
# not matching the requested artifact's tuple looks like a typo, not a
28+
# safety rule).
29+
ARENA_EXCLUDED_SOURCES: dict[str, str] = {
30+
"docs/topics/context_managers.md": (
31+
"every example opens `with Server()`; in arena mode the doctest "
32+
"namespace's `Server` name is bound to a factory pinned to the "
33+
"lent socket path, so leaving the block runs Server.__exit__ -> "
34+
"Server.kill() -> `kill-server` against the borrowed daemon "
35+
"itself, not a private one"
36+
),
37+
}
38+
39+
40+
def _assert_no_excluded_targets(
41+
artifact_targets: t.Mapping[str, tuple[str, ...]],
42+
) -> None:
43+
"""Refuse at import time if any artifact ever names an excluded source.
44+
45+
A mapping edit that adds an excluded page back in would otherwise only
46+
surface the first time someone ran that artifact against a real lent
47+
server -- by which point it may already have stopped it.
48+
"""
49+
conflicts = {
50+
artifact: sorted(overlap)
51+
for artifact, sources in artifact_targets.items()
52+
if (overlap := frozenset(sources) & ARENA_EXCLUDED_SOURCES.keys())
53+
}
54+
if conflicts:
55+
msg = f"artifact(s) name an excluded arena source: {conflicts!r}"
56+
raise AssertionError(msg)
57+
58+
59+
_assert_no_excluded_targets(ARENA_ARTIFACT_TARGETS)
60+
1461

1562
@dataclasses.dataclass(frozen=True)
1663
class ArenaSpec:

0 commit comments

Comments
 (0)