fix: Windows CI compat for palace lock tests and path normalization
Addresses the two actionable Copilot comments from the 2nd review pass. tests/test_palace_locks.py (#7, #8) multiprocessing.get_context("fork") is unavailable on Windows, so the cross-process tests would crash the Windows CI runner. Added `_get_mp_context()` that picks "spawn" on Windows and "fork" elsewhere. Spawn re-imports the module in the child; it inherits os.environ (including the monkeypatched HOME), which is all these tests need. mempalace/palace.py (#10) The per-palace lock key was computed from os.path.abspath(palace_path). On Windows the filesystem is case-insensitive, so `C:\\Palace` and `c:\\palace` would hash to different keys and two concurrent mines could touch the same on-disk palace. Switched to `os.path.normcase(os.path.realpath(...))` so: * realpath resolves symlinks and `..` segments * normcase folds case on Windows (no-op on POSIX) Testing pytest tests/test_palace_locks.py tests/test_hooks_cli.py tests/test_backends.py tests/test_cli.py → 98 passed, 0 failed.
This commit is contained in:
committed by
Igor Lins e Silva
parent
99b820cb42
commit
1998aede66
+9
-2
@@ -329,14 +329,21 @@ def mine_palace_lock(palace_path: str):
|
||||
*different* palaces can still run in parallel — we only serialize
|
||||
writes into the same palace, which is the correctness boundary.
|
||||
|
||||
The key is derived from a fully normalized form of the path:
|
||||
`realpath` resolves symlinks and `..` segments, and `normcase` folds
|
||||
case on Windows (which has a case-insensitive filesystem). Without
|
||||
normcase, `C:\\Palace` and `c:\\palace` would hash to different keys
|
||||
on Windows and let two concurrent mines touch the same on-disk palace.
|
||||
|
||||
Non-blocking: if another `mine` is already writing to this palace,
|
||||
raise MineAlreadyRunning so the caller can exit cleanly instead of
|
||||
piling up as a waiting worker.
|
||||
"""
|
||||
lock_dir = os.path.join(os.path.expanduser("~"), ".mempalace", "locks")
|
||||
os.makedirs(lock_dir, exist_ok=True)
|
||||
resolved = os.path.abspath(os.path.expanduser(palace_path))
|
||||
palace_key = hashlib.sha256(resolved.encode()).hexdigest()[:16]
|
||||
resolved = os.path.realpath(os.path.expanduser(palace_path))
|
||||
lock_key_source = os.path.normcase(resolved)
|
||||
palace_key = hashlib.sha256(lock_key_source.encode()).hexdigest()[:16]
|
||||
lock_path = os.path.join(lock_dir, f"mine_palace_{palace_key}.lock")
|
||||
|
||||
lf = open(lock_path, "w")
|
||||
|
||||
@@ -22,6 +22,18 @@ from mempalace.palace import (
|
||||
)
|
||||
|
||||
|
||||
def _get_mp_context():
|
||||
"""Pick a start method that works on every CI runner.
|
||||
|
||||
`fork` is cheaper (no re-import) but is unavailable on Windows, so we fall
|
||||
back to `spawn` there. `spawn` inherits ``os.environ`` (including the
|
||||
monkeypatched ``HOME``) and re-imports the ``mempalace`` package in the
|
||||
child, which is sufficient for the lock-file semantics exercised here.
|
||||
"""
|
||||
start_method = "spawn" if os.name == "nt" else "fork"
|
||||
return multiprocessing.get_context(start_method)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -75,7 +87,7 @@ def test_same_palace_serializes_across_processes(tmp_path, monkeypatch):
|
||||
ready = str(tmp_path / "ready")
|
||||
release = str(tmp_path / "release")
|
||||
|
||||
ctx = multiprocessing.get_context("fork")
|
||||
ctx = _get_mp_context()
|
||||
holder = ctx.Process(target=_hold_lock, args=(palace, ready, release))
|
||||
holder.start()
|
||||
try:
|
||||
@@ -104,7 +116,7 @@ def test_different_palaces_dont_conflict(tmp_path, monkeypatch):
|
||||
ready = str(tmp_path / "ready_a")
|
||||
release = str(tmp_path / "release_a")
|
||||
|
||||
ctx = multiprocessing.get_context("fork")
|
||||
ctx = _get_mp_context()
|
||||
holder = ctx.Process(target=_hold_lock, args=(palace_a, ready, release))
|
||||
holder.start()
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user