Skip to content

Commit 365afbd

Browse files
committed
test: per-server session lock for parallel runs
Concurrent pytest runs (e.g. from several worktrees) share the docker, embedded and external test servers, so they clobbered each other's calendars, and one run's teardown could stop a container another run still used. start() now takes an exclusive flock per server, held until the process exits; a second run on the same server waits, runs on different servers proceed in parallel. Random test order can make two runs deadlock, so the wait is capped (default 30 min), after which tests on that server fail at once. get_davclient()'s test-server fallback does not wait but moves on to the next server. Prompt: we now have multiple pytest runs initiated by claude, in different workbranches, but towards the same set of docker test calendar servers. This may clobber up. Followup-Prompt: Perhaps one lock file per test server. It can be very useful to run tests towards different servers in parallell, and that should be safe enough Followup-Prompt: go [implement the per-server lock: holder pid in the lock file, wait timeout, setup hook always calling start()] Assisted-By: Claude Opus 5.5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
1 parent fa5ead6 commit 365afbd

5 files changed

Lines changed: 265 additions & 8 deletions

File tree

‎caldav/config.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,8 @@ def _get_test_server_config(
413413

414414
for server in _collect_test_servers(name, config_file):
415415
was_already_started = server._started
416+
# Another process using this server: try the next one rather than wait
417+
server.claim_timeout = 0
416418
try:
417419
server.start()
418420
except Exception as e:

‎tests/README.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,30 @@ pytest -k "test_to_utc"
3434
pytest tests/test_caldav_unit.py
3535
```
3636

37+
### Parallel Test Runs
38+
39+
Test runs share the servers (fixed ports, users and calendar names), so
40+
two runs against the same server would clobber each other. Each process
41+
takes an exclusive per-server lock (`$TMPDIR/caldav-test-server-<name>.session.lock`)
42+
when it first starts using a server and holds it until it exits. A second run
43+
against the same server waits; runs against different servers proceed in
44+
parallel:
45+
46+
```bash
47+
pytest tests/test_caldav.py -k Baikal &
48+
pytest tests/test_caldav.py -k Nextcloud &
49+
```
50+
51+
The lock file lives in the temp directory, so runs only see each other's
52+
locks when they share `TMPDIR`.
53+
54+
The wait is capped at `PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT` seconds
55+
(default 1800), as two runs taking servers in different orders could
56+
otherwise deadlock. After that, the tests for that server fail at once,
57+
naming the pid holding the lock; the run continues with the other servers.
58+
`get_davclient()` with `PYTHON_CALDAV_USE_TEST_SERVER` does not wait at all:
59+
it moves on to the next available server.
60+
3761
## Test Configuration
3862

3963
Test configuration uses YAML or JSON files. The configuration loader searches

‎tests/test_servers/base.py‎

Lines changed: 103 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,13 @@
77
- DockerTestServer: For servers that run in Docker containers
88
"""
99

10+
import os
11+
import threading
1012
from abc import ABC, abstractmethod
1113
from collections.abc import Iterator
1214
from contextlib import contextmanager
13-
from typing import Any
15+
from pathlib import Path
16+
from typing import IO, Any
1417

1518
try:
1619
import niquests as requests
@@ -21,6 +24,22 @@
2124
DEFAULT_HTTP_TIMEOUT = 5
2225
MAX_STARTUP_WAIT_SECONDS = 60
2326
STARTUP_POLL_INTERVAL = 0.05
27+
SESSION_LOCK_TIMEOUT_SECONDS = 1800
28+
29+
# Per-server session locks held by this process, keyed by server name.
30+
# flock conflicts between two opens even within one process, so each
31+
# lock file is opened only once.
32+
_session_locks: dict[str, IO[str]] = {}
33+
# Servers whose lock timed out; later claims fail at once instead of
34+
# waiting again for every remaining test.
35+
_session_lock_failures: dict[str, RuntimeError] = {}
36+
_session_locks_mutex = threading.Lock()
37+
38+
39+
def _lock_path(name: str, suffix: str = "") -> Path:
40+
import tempfile
41+
42+
return Path(tempfile.gettempdir()) / f"caldav-test-server-{name.lower()}{suffix}.lock"
2443

2544

2645
class TestServer(ABC):
@@ -106,11 +125,83 @@ def features(self) -> Any:
106125

107126
return resolve_features(self.config.get("features", []))
108127

128+
# Seconds to wait for another process's session lock; None means
129+
# $PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT or SESSION_LOCK_TIMEOUT_SECONDS.
130+
claim_timeout: float | None = None
131+
132+
@staticmethod
133+
def claim_server(name: str, timeout: float | None = None) -> bool:
134+
"""Take an exclusive lock on server ``name`` for the rest of this process.
135+
136+
Test runs sharing a server would clobber each other's calendars (and
137+
one run's teardown may stop a container another run still uses), so
138+
a second process waits here until the first one exits. Runs against
139+
different servers proceed in parallel. The OS drops the lock when
140+
the process dies, so a crashed run leaves nothing stale behind.
141+
142+
Waits at most ``timeout`` seconds (default
143+
``$PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT``, or 30 minutes) - two runs
144+
taking servers in different orders could otherwise deadlock. A
145+
timeout is remembered, so the remaining tests for that server fail
146+
at once.
147+
148+
Returns True if the lock was taken just now, False if this process
149+
already held it. Anything learned about the server before a fresh
150+
claim may be stale, since its previous user may have stopped it.
151+
"""
152+
import fcntl
153+
import time
154+
155+
with _session_locks_mutex:
156+
if name in _session_locks:
157+
return False
158+
if name in _session_lock_failures:
159+
raise _session_lock_failures[name]
160+
if timeout is None:
161+
timeout = float(
162+
os.environ.get(
163+
"PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT", SESSION_LOCK_TIMEOUT_SECONDS
164+
)
165+
)
166+
lock_file = open(_lock_path(name, ".session"), "a+")
167+
deadline = time.monotonic() + timeout
168+
announced = False
169+
while True:
170+
try:
171+
fcntl.flock(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB)
172+
break
173+
except BlockingIOError:
174+
lock_file.seek(0)
175+
holder = lock_file.read().strip() or "unknown"
176+
if time.monotonic() > deadline:
177+
lock_file.close()
178+
error = RuntimeError(
179+
f"Test server {name} is in use by pid {holder}; "
180+
f"gave up after {timeout:.0f}s"
181+
)
182+
_session_lock_failures[name] = error
183+
raise error from None
184+
if not announced:
185+
print(f"[WAIT] {name} is in use by pid {holder} - waiting")
186+
announced = True
187+
time.sleep(1)
188+
lock_file.truncate(0)
189+
lock_file.write(str(os.getpid()))
190+
lock_file.flush()
191+
_session_locks[name] = lock_file
192+
return True
193+
194+
def claim(self) -> bool:
195+
"""``claim_server()`` for this server, honouring ``claim_timeout``."""
196+
return self.claim_server(self.name, self.claim_timeout)
197+
109198
@abstractmethod
110199
def start(self) -> None:
111200
"""
112201
Start the server if not already running.
113202
203+
Implementations must call ``self.claim()`` first.
204+
114205
This method should be idempotent - calling it multiple times
115206
should not cause issues.
116207
@@ -206,15 +297,15 @@ def get_server_params(self) -> dict[str, Any]:
206297
# Pass through scheduling_users if configured (for TestScheduling generation)
207298
if "scheduling_users" in self.config:
208299
params["scheduling_users"] = self.config["scheduling_users"]
300+
# setup is always needed, since start() also takes the session lock
301+
params["setup"] = lambda _: self.start()
209302
# Check if server is already running (either started by us or externally)
210303
already_running = self._started or self.is_accessible()
211304
if already_running:
212305
# Server is already running - mark as started but don't add teardown
213306
# to avoid stopping a server that was running before tests started
214307
self._started = True
215308
else:
216-
# Server needs to be started - add setup/teardown callbacks
217-
params["setup"] = lambda _: self.start()
218309
params["teardown"] = lambda _: self.stop()
219310
return params
220311

@@ -340,7 +431,13 @@ def start(self) -> None:
340431
Raises:
341432
RuntimeError: If Docker is not available or container fails to start
342433
"""
343-
if self._started or self.is_accessible():
434+
if self.claim():
435+
# Whatever was seen before the lock (e.g. at collection) may be
436+
# stale: the previous holder may have stopped the container.
437+
self._started = False
438+
if self._started:
439+
return
440+
if self.is_accessible():
344441
self._started = True # Mark as started even if already running
345442
# Don't set _started_by_us - we didn't start it this time
346443
print(f"[OK] {self.name} is already running")
@@ -371,11 +468,8 @@ def _lock(self) -> Iterator[None]:
371468
repo share the same containers and ports.
372469
"""
373470
import fcntl
374-
import tempfile
375-
from pathlib import Path
376471

377-
lock_path = Path(tempfile.gettempdir()) / f"caldav-test-server-{self.name.lower()}.lock"
378-
with open(lock_path, "w") as lock_file:
472+
with open(_lock_path(self.name), "w") as lock_file:
379473
fcntl.flock(lock_file, fcntl.LOCK_EX)
380474
try:
381475
yield
@@ -482,6 +576,7 @@ def start(self) -> None:
482576
# No accessibility check here - the caldav library handles URL
483577
# normalization, RFC6764 discovery, and will provide proper error
484578
# messages if the server is unreachable
579+
self.claim()
485580
self._started = True
486581

487582
def stop(self) -> None:

‎tests/test_servers/embedded.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ def is_accessible(self) -> bool:
3939
return self._core.is_accessible()
4040

4141
def start(self) -> None:
42+
self.claim()
4243
self._core.start()
4344
self._started = self._core._started
4445

@@ -73,6 +74,7 @@ def is_accessible(self) -> bool:
7374
return self._core.is_accessible()
7475

7576
def start(self) -> None:
77+
self.claim()
7678
self._core.start()
7779
self._started = self._core._started
7880

‎tests/test_servers/test_base.py‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,38 @@
55

66
import pytest
77

8+
from . import base
89
from .base import DockerTestServer
910

1011

12+
@pytest.fixture(autouse=True)
13+
def isolated_session_locks(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
14+
"""Keep session locks taken by these tests out of the real tempdir."""
15+
monkeypatch.setattr("tempfile.tempdir", str(tmp_path))
16+
monkeypatch.setattr(base, "_session_locks", {})
17+
monkeypatch.setattr(base, "_session_lock_failures", {})
18+
yield
19+
for f in base._session_locks.values():
20+
f.close()
21+
22+
23+
@pytest.fixture
24+
def held_elsewhere(tmp_path: Path):
25+
"""Hold a server's session lock through a separate open file, as another process would."""
26+
import fcntl
27+
28+
files = []
29+
30+
def hold(name: str) -> None:
31+
f = open(tmp_path / f"caldav-test-server-{name.lower()}.session.lock", "a+")
32+
fcntl.flock(f, fcntl.LOCK_EX)
33+
files.append(f)
34+
35+
yield hold
36+
for f in files:
37+
f.close()
38+
39+
1140
class UnreachableDockerServer(DockerTestServer):
1241
"""A docker server whose start.sh fails and which never answers."""
1342

@@ -64,3 +93,108 @@ def is_accessible(self) -> bool:
6493

6594
assert counter.read_text().count("run") == 1
6695
assert sum(s._started_by_us for s in servers) == 1
96+
97+
98+
class TestSessionLock:
99+
"""Test runs sharing a server must not overlap; different servers may."""
100+
101+
CLAIM = (
102+
"import sys; sys.path.insert(0, {root!r}); "
103+
"from tests.test_servers.base import TestServer; "
104+
"TestServer.claim_server({name!r})"
105+
)
106+
107+
def _claim_in_subprocess(self, tmp_path: Path, name: str) -> subprocess.CompletedProcess:
108+
import os
109+
import sys
110+
111+
root = str(Path(__file__).parents[2])
112+
env = dict(os.environ, TMPDIR=str(tmp_path), PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT="1")
113+
return subprocess.run(
114+
[sys.executable, "-c", self.CLAIM.format(root=root, name=name)],
115+
env=env,
116+
capture_output=True,
117+
text=True,
118+
timeout=60,
119+
)
120+
121+
def test_same_server_blocks_other_process(self, tmp_path: Path) -> None:
122+
import os
123+
124+
base.TestServer.claim_server("Alpha")
125+
base.TestServer.claim_server("Alpha") # re-entrant within one process
126+
blocked = self._claim_in_subprocess(tmp_path, "Alpha")
127+
assert blocked.returncode != 0
128+
assert f"pid {os.getpid()}" in blocked.stderr
129+
assert self._claim_in_subprocess(tmp_path, "Beta").returncode == 0
130+
131+
def test_start_claims_server(self, tmp_path: Path) -> None:
132+
server = base.ExternalTestServer({"name": "Gamma", "url": "http://localhost:1/"})
133+
server.start()
134+
assert self._claim_in_subprocess(tmp_path, "Gamma").returncode != 0
135+
136+
def test_setup_hook_set_for_running_server(self) -> None:
137+
"""test_caldav only reaches start() (and thus the lock) via the setup hook."""
138+
server = base.ExternalTestServer({"name": "Delta", "url": "http://localhost:1/"})
139+
assert "setup" in server.get_server_params()
140+
141+
def test_timeout_is_remembered(self, held_elsewhere, monkeypatch: pytest.MonkeyPatch) -> None:
142+
"""test_caldav claims per test; a timed-out server must not wait again each time."""
143+
import time
144+
145+
held_elsewhere("Epsilon")
146+
monkeypatch.setenv("PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT", "1")
147+
with pytest.raises(RuntimeError, match="in use"):
148+
base.TestServer.claim_server("Epsilon")
149+
t0 = time.monotonic()
150+
with pytest.raises(RuntimeError, match="in use"):
151+
base.TestServer.claim_server("Epsilon")
152+
assert time.monotonic() - t0 < 0.5
153+
154+
def test_claim_timeout_attribute_skips_wait(self, held_elsewhere) -> None:
155+
"""get_davclient()'s fallback must move on at once from a busy server."""
156+
import time
157+
158+
held_elsewhere("Zeta")
159+
server = base.ExternalTestServer({"name": "Zeta", "url": "http://localhost:1/"})
160+
server.claim_timeout = 0
161+
t0 = time.monotonic()
162+
with pytest.raises(RuntimeError, match="in use"):
163+
server.start()
164+
assert time.monotonic() - t0 < 0.5
165+
166+
def test_get_davclient_falls_through_busy_server(
167+
self, held_elsewhere, monkeypatch: pytest.MonkeyPatch
168+
) -> None:
169+
import time
170+
171+
from caldav import config
172+
173+
held_elsewhere("Busy")
174+
monkeypatch.setenv("PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT", "30")
175+
servers = [
176+
base.ExternalTestServer({"name": n, "url": f"http://localhost:1/{n}/"})
177+
for n in ("Busy", "Free")
178+
]
179+
monkeypatch.setattr(config, "_collect_test_servers", lambda name, config_file: servers)
180+
t0 = time.monotonic()
181+
params = config._get_test_server_config(None, environment=False)
182+
assert time.monotonic() - t0 < 5
183+
assert params is not None and "Free" in params["url"]
184+
185+
def test_waiting_run_restarts_container_stopped_meanwhile(self, tmp_path: Path) -> None:
186+
"""Collection saw the container up, but its owner stopped it before we got the lock."""
187+
counter = tmp_path / "runs"
188+
start_script = tmp_path / "start.sh"
189+
start_script.write_text(f"#!/bin/sh\necho run >> {counter}\n")
190+
start_script.chmod(0o755)
191+
192+
class CountingServer(UnreachableDockerServer):
193+
def is_accessible(self) -> bool:
194+
return counter.exists()
195+
196+
server = CountingServer({"name": "Eta", "docker_dir": str(tmp_path)})
197+
server.verify_docker = lambda: True # type: ignore[method-assign]
198+
server._started = True # as get_server_params() leaves it at collection
199+
server.start()
200+
assert counter.read_text().count("run") == 1

0 commit comments

Comments
 (0)