Skip to content

Commit 99af4ea

Browse files
groksrcclaude
andcommitted
fix(test): make setup_logging config-dir regression test Windows-safe
``test_setup_logging_honors_basic_memory_config_dir`` was monkeypatching ``utils.os.name`` to ``"posix"`` to force ``setup_logging`` onto the shared-log-filename branch, which let the test assert on the exact path. That globally redirected ``os.name``, and on Windows CI that broke ``Path(str)`` dispatch inside ``resolve_data_dir``: pathlib.UnsupportedOperation: cannot instantiate 'PosixPath' on your system The regression guard only cares that the log file lives under the redirected ``BASIC_MEMORY_CONFIG_DIR`` — not at ``Path.home()``. It does not care whether the filename is the shared ``basic-memory.log`` (POSIX) or the per-process ``basic-memory-<pid>.log`` (Windows). Drop the ``os.name`` patch, assert on ``log_path.parent`` equalling the custom dir, and sanity-check the filename pattern without pinning the pid suffix. Same intent, portable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Drew Cain <groksrc@gmail.com>
1 parent e39b8e1 commit 99af4ea

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

tests/utils/test_setup_logging.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import os
44
import sys
5+
from pathlib import Path
56

67
from basic_memory import utils
78

@@ -91,13 +92,20 @@ def test_setup_logging_honors_basic_memory_config_dir(monkeypatch, tmp_path) ->
9192
Prior to #742 the log path was hardcoded to ``~/.basic-memory/``, which
9293
split state across instances when users set BASIC_MEMORY_CONFIG_DIR to
9394
isolate config and the database elsewhere.
95+
96+
Asserts on the log *directory* rather than the exact filename because
97+
Windows uses a per-process ``basic-memory-<pid>.log`` while POSIX
98+
shares a single ``basic-memory.log``. The thing this regression guard
99+
cares about is that the log lives under the redirected config dir,
100+
not at ``Path.home() / ".basic-memory"``. Patching ``utils.os.name``
101+
to force one branch would break ``Path(str)`` dispatch on the other
102+
platform, so we stay platform-agnostic.
94103
"""
95104
added_sinks: list[str] = []
96105

97106
custom_dir = tmp_path / "instance-x" / "state"
98107
monkeypatch.setenv("BASIC_MEMORY_ENV", "dev")
99108
monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom_dir))
100-
monkeypatch.setattr(utils.os, "name", "posix")
101109
monkeypatch.setattr(utils.logger, "remove", lambda *args, **kwargs: None)
102110
monkeypatch.setattr(
103111
utils.logger,
@@ -109,7 +117,11 @@ def test_setup_logging_honors_basic_memory_config_dir(monkeypatch, tmp_path) ->
109117

110118
utils.setup_logging(log_to_file=True)
111119

112-
assert added_sinks == [str(custom_dir / "basic-memory.log")]
120+
assert len(added_sinks) == 1
121+
log_path = Path(added_sinks[0])
122+
assert log_path.parent == custom_dir
123+
assert log_path.name.startswith("basic-memory")
124+
assert log_path.suffix == ".log"
113125

114126

115127
def test_setup_logging_test_env_uses_stderr_only(monkeypatch) -> None:

0 commit comments

Comments
 (0)