Skip to content

Commit 8bf4738

Browse files
committed
Merge PR 776: test, build: per-server test-server lock; v[0-9]* version tags
#776 Concurrent pytest runs now take an exclusive per-server lock, so parallel runs no longer clobber shared test servers; get_davclient()'s test-server fallback skips a server another process holds. hatch-vcs takes the version only from v[0-9]* tags. Commits: - dc853e0 test: per-server session lock for parallel runs - 156b1b3 build: only take versions from v[0-9]* tags - 8f5199f docs: note search-cache delay misses other clients - ca601f1 test: test-server fallback without session locks Assisted-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
2 parents 723d85e + ca601f1 commit 8bf4738

7 files changed

Lines changed: 386 additions & 8 deletions

File tree

‎caldav/config.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,10 +413,17 @@ def _get_test_server_config(
413413

414414
for server in _collect_test_servers(name, config_file):
415415
was_already_started = server._started
416+
# Servers from caldav.testing (pip-installed caldav) have no session lock
417+
claim = getattr(server, "claim_server", None)
418+
claimed = False
416419
try:
420+
# Another process using this server: try the next one rather than wait
421+
claimed = bool(claim and claim(server.name, 0))
417422
server.start()
418423
except Exception as e:
419424
logging.warning("Failed to start test server %s: %s", server.name, e)
425+
if claimed:
426+
server.release_server(server.name)
420427
continue
421428
logging.info("Using test server: %s at %s", server.name, server.url)
422429
return _test_server_to_params(server, was_already_started)

‎docs/design/TODO.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,7 @@ Known downsides: no Unreleased section visible on master (consider `sphinxcontri
4444
## An error response with a non-XML body labelled as XML
4545

4646
A response whose `Content-Type` says XML but whose body is not XML (an HTML error page sent as `application/xml`) makes `DAVResponse.__init__` raise an lxml parse error, on any status. The caller never gets the `DAVError` (with `body` and `preconditions`, https://github.com/python-caldav/caldav/issues/756) that the status calls for. Only the async client's 401/403 path was fixed (https://github.com/python-caldav/caldav/issues/738). Fix: tolerate a parse failure on a 4xx/5xx response and keep the raw body. Found in the review of the #756 fix.
47+
48+
## Search-cache delay misses writes from other clients
49+
50+
`tests/search_cache_delay.py` only resets its clock on writes through the one wrapped client. Writes through another `DAVClient` in the same test (e.g. the scheduling users' clients) are not counted, so a filtered search right after one may wait less than the server's cache lag, and get flaky on servers like Purelymail or Bedework. Fix: wrap `DAVClient.request` at class level for the duration of the test, or document the limitation. Found in the review of https://github.com/python-caldav/caldav/pull/776.

‎pyproject.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ build-backend = "hatchling.build"
44

55
[tool.hatch.version]
66
source = "vcs"
7+
# Only v<digit>* tags are releases. The default describe matches any tag
8+
# containing a digit, so a non-release tag with a number in it could
9+
# become the version and break the build.
10+
raw-options = { scm = { git = { describe_command = "git describe --dirty --tags --long --abbrev=40 --match v[0-9]*" } } }
711

812
[tool.hatch.build.hooks.vcs]
913
version-file = "caldav/_version.py"

‎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: 124 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,97 @@ def features(self) -> Any:
106125

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

128+
@staticmethod
129+
def claim_server(name: str, timeout: float | None = None) -> bool:
130+
"""Take an exclusive lock on server ``name`` for the rest of this process.
131+
132+
Test runs sharing a server would clobber each other's calendars (and
133+
one run's teardown may stop a container another run still uses), so
134+
a second process waits here until the first one exits. Runs against
135+
different servers proceed in parallel. The OS drops the lock when
136+
the process dies, so a crashed run leaves nothing stale behind.
137+
138+
Waits at most ``timeout`` seconds (default
139+
``$PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT``, or 30 minutes) - two runs
140+
taking servers in different orders could otherwise deadlock. A
141+
timeout is remembered, so the remaining tests for that server fail
142+
at once - except with ``timeout=0``, a mere probe.
143+
144+
Returns True if the lock was taken just now, False if this process
145+
already held it. Anything learned about the server before a fresh
146+
claim may be stale, since its previous user may have stopped it.
147+
148+
Without ``fcntl`` (Windows) nothing is locked between processes.
149+
"""
150+
import time
151+
152+
try:
153+
import fcntl
154+
except ImportError:
155+
fcntl = None # type: ignore[assignment]
156+
157+
with _session_locks_mutex:
158+
if name in _session_locks:
159+
return False
160+
if name in _session_lock_failures:
161+
raise _session_lock_failures[name]
162+
if timeout is None:
163+
timeout = float(
164+
os.environ.get(
165+
"PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT", SESSION_LOCK_TIMEOUT_SECONDS
166+
)
167+
)
168+
lock_file = open(_lock_path(name, ".session"), "a+")
169+
try:
170+
deadline = time.monotonic() + timeout
171+
announced = False
172+
while fcntl:
173+
try:
174+
fcntl.flock(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB)
175+
break
176+
except BlockingIOError:
177+
lock_file.seek(0)
178+
holder = lock_file.read().strip() or "unknown"
179+
if time.monotonic() > deadline:
180+
error = RuntimeError(
181+
f"Test server {name} is in use by pid {holder}; "
182+
f"gave up after {timeout:.0f}s"
183+
)
184+
if timeout:
185+
_session_lock_failures[name] = error
186+
raise error from None
187+
if not announced:
188+
print(f"[WAIT] {name} is in use by pid {holder} - waiting")
189+
announced = True
190+
time.sleep(1)
191+
lock_file.truncate(0)
192+
lock_file.write(str(os.getpid()))
193+
lock_file.flush()
194+
except BaseException:
195+
lock_file.close()
196+
raise
197+
_session_locks[name] = lock_file
198+
return True
199+
200+
@staticmethod
201+
def release_server(name: str) -> None:
202+
"""Give up a claim taken by ``claim_server()``, letting other runs have the server."""
203+
with _session_locks_mutex:
204+
lock_file = _session_locks.pop(name, None)
205+
if lock_file:
206+
lock_file.close()
207+
208+
def claim(self) -> bool:
209+
"""``claim_server()`` for this server."""
210+
return self.claim_server(self.name)
211+
109212
@abstractmethod
110213
def start(self) -> None:
111214
"""
112215
Start the server if not already running.
113216
217+
Implementations must call ``self.claim()`` first.
218+
114219
This method should be idempotent - calling it multiple times
115220
should not cause issues.
116221
@@ -206,15 +311,15 @@ def get_server_params(self) -> dict[str, Any]:
206311
# Pass through scheduling_users if configured (for TestScheduling generation)
207312
if "scheduling_users" in self.config:
208313
params["scheduling_users"] = self.config["scheduling_users"]
314+
# setup is always needed, since start() also takes the session lock
315+
params["setup"] = lambda _: self.start()
209316
# Check if server is already running (either started by us or externally)
210317
already_running = self._started or self.is_accessible()
211318
if already_running:
212319
# Server is already running - mark as started but don't add teardown
213320
# to avoid stopping a server that was running before tests started
214321
self._started = True
215322
else:
216-
# Server needs to be started - add setup/teardown callbacks
217-
params["setup"] = lambda _: self.start()
218323
params["teardown"] = lambda _: self.stop()
219324
return params
220325

@@ -340,7 +445,13 @@ def start(self) -> None:
340445
Raises:
341446
RuntimeError: If Docker is not available or container fails to start
342447
"""
343-
if self._started or self.is_accessible():
448+
if self.claim():
449+
# Whatever was seen before the lock (e.g. at collection) may be
450+
# stale: the previous holder may have stopped the container.
451+
self._started = False
452+
if self._started:
453+
return
454+
if self.is_accessible():
344455
self._started = True # Mark as started even if already running
345456
# Don't set _started_by_us - we didn't start it this time
346457
print(f"[OK] {self.name} is already running")
@@ -371,11 +482,8 @@ def _lock(self) -> Iterator[None]:
371482
repo share the same containers and ports.
372483
"""
373484
import fcntl
374-
import tempfile
375-
from pathlib import Path
376485

377-
lock_path = Path(tempfile.gettempdir()) / f"caldav-test-server-{self.name.lower()}.lock"
378-
with open(lock_path, "w") as lock_file:
486+
with open(_lock_path(self.name), "w") as lock_file:
379487
fcntl.flock(lock_file, fcntl.LOCK_EX)
380488
try:
381489
yield
@@ -412,6 +520,13 @@ def _start_container(self) -> None:
412520

413521
raise RuntimeError(f"{self.name} failed to start after {MAX_STARTUP_WAIT_SECONDS}s")
414522

523+
def get_server_params(self) -> dict[str, Any]:
524+
params = super().get_server_params()
525+
# A run that waited for the lock may have restarted a container that
526+
# was up at collection; stop() leaves alone one we did not start.
527+
params["teardown"] = lambda _: self.stop()
528+
return params
529+
415530
def stop(self) -> None:
416531
"""Stop the Docker container and cleanup.
417532
@@ -482,6 +597,7 @@ def start(self) -> None:
482597
# No accessibility check here - the caldav library handles URL
483598
# normalization, RFC6764 discovery, and will provide proper error
484599
# messages if the server is unreachable
600+
self.claim()
485601
self._started = True
486602

487603
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

0 commit comments

Comments
 (0)