Skip to content

Commit 13f5652

Browse files
committed
fix: make app-folder resolution and its tests cross-platform
Fixes the four Windows CI failures (all pre-existing: this PR had only touched base.py and tests/test_base.py) and one real Windows bug found while investigating. Code fix -- Windows 'cache' collided with 'data'/'state': APP_FOLDER_STANDARDS described the Windows cache root as FolderSpec("LOCALAPPDATA", join(%LOCALAPPDATA%, "Temp")). Since the fallback is only used when the env var is *absent*, and LOCALAPPDATA is always set on Windows, the "Temp" default was dead code: cache resolved to %LOCALAPPDATA%, exactly the data/state root. Clearing an app's cache would have deleted the user's data. FolderSpec gains a `subpath` field that expresses "a kind that lives inside another kind's root", so cache is now the documented %LOCALAPPDATA%\Temp. The table also snapshotted os.getenv() into its own fallbacks at import time, which made those fallbacks untestable and unreachable. They are now declarative literals (~\AppData\Roaming etc.), so a missing env var yields a real absolute root instead of "" -- an empty root would have degraded os.path.join(root, app_name) into a *relative* path. The single os.name branch is now app_folder_standards(os_name), so either platform's table can be resolved from any platform, and system_default_for_app_data_folder takes it via an optional keyword. That makes Windows behaviour testable on Linux/macOS -- see the new tests/test_platform_standards.py, which loops over both platforms. Test fixes -- assertions that were POSIX-shaped, not wrong behaviour: - test_app_data.py steered app folders with XDG_CONFIG_HOME/XDG_DATA_HOME. XDG is a POSIX standard and is correctly ignored on Windows, so the redirection silently did nothing there: two tests failed on the location assertion and five others passed while writing into the runner's real user profile. They now use config2py's own CONFIG2PY_<KIND>_DIR override, which is honoured on every platform, and compare resolved Path objects rather than strings. Location is now asserted in 5 tests, up from 2. - test_sync_store.py::test_store_repr asserted a separator literal (`temp_file in repr(store)`); pathlib's repr always spells paths with forward slashes, so the native Windows form never appears. It now asserts the store's filepath identity plus the separator-free filename. Doctest fix: get_app_folder's doctest expected '.../.config/config2py'. Rather than skipping it, it now asserts the properties that hold on every platform: the result is absolute, is named after the app, and sits directly inside the 'config' root. That is strictly more coverage than the ELLIPSIS form. Also corrected get_app_rootdir's docstring, which advertised the override variables under wrong names (CONFIG2PY_*_FOLDER instead of CONFIG2PY_*_DIR) and implied XDG works everywhere. pytest config: testpaths named "tests", which does not exist (tests live in config2py/tests/), so pytest warned and fell back to recursive discovery from the CWD. Set to ["config2py"]. doctest_optionflags now match what the wads run-tests-uv action passes on the command line (ELLIPSIS, IGNORE_EXCEPTION_DETAIL) -- it overrides the file, so NORMALIZE_WHITESPACE here meant local runs and CI applied different rules. Collected tests: 106 -> 127 (+20 platform tests, +1 doctest), no warnings. Dependents gate (29 suites) unchanged before/after: 25 pass, 2 pre-existing failures (smart-cv, yp), 1 no-tests, 1 tests-disabled. Claude-Session: https://claude.ai/code/session_01Kug7UUbVeCQgruvNXUq63c
1 parent c7bd1a6 commit 13f5652

5 files changed

Lines changed: 256 additions & 41 deletions

File tree

config2py/tests/test_app_data.py

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,42 @@
77

88
import pytest
99

10-
from config2py.util import ensure_seeded, AppData
10+
from config2py.util import ensure_seeded, AppData, config2py_env_var
11+
12+
13+
def _redirect_app_root(folder_kind: str, target):
14+
"""Redirect a config2py app-root folder kind at *target* for the test.
15+
16+
Uses config2py's own ``CONFIG2PY_<KIND>_DIR`` override, which is honoured on
17+
every platform. The XDG_* variables must NOT be used here: they are a POSIX
18+
standard and are ignored on Windows, so tests keyed on them silently write
19+
into the real user profile instead of the temp dir.
20+
"""
21+
env_var = getattr(config2py_env_var, folder_kind)
22+
return patch.dict(os.environ, {env_var: str(target)})
23+
24+
25+
def _same_path(a, b) -> bool:
26+
"""Compare two paths by identity-on-disk, not by their string spelling.
27+
28+
``resolve()`` on both sides normalises the differences that make naive
29+
string comparison fail per-platform: macOS ``/var`` -> ``/private/var``
30+
symlinks, and Windows 8.3 short names (``RUNNER~1`` -> ``runneradmin``).
31+
"""
32+
return Path(a).resolve() == Path(b).resolve()
1133

1234

1335
# ---------------------------------------------------------------------------
1436
# Helpers — mock importlib.resources for isolated testing
1537
# ---------------------------------------------------------------------------
1638

39+
1740
def _mock_importlib_files(seed_store: dict):
1841
"""Return a mock for importlib.resources.files that reads from *seed_store*.
1942
2043
*seed_store* maps ``(subpackage, filename)`` to ``bytes`` content.
2144
"""
45+
2246
def fake_files(package_path: str):
2347
# Extract subpackage from e.g. "mypkg._seed_data.resources"
2448
parts = package_path.split(".")
@@ -36,6 +60,7 @@ def __truediv__(self, filename):
3660
return mock_ref
3761

3862
return FakeTraversable()
63+
3964
return fake_files
4065

4166

@@ -110,6 +135,7 @@ def test_returns_path_object(self, tmp_path, mock_seeds_for_ensure):
110135
def test_custom_seed_data_dir(self, tmp_path):
111136
"""Ensure the seed_data_dir parameter is used in the package path."""
112137
calls = []
138+
113139
def fake_files(pkg_path):
114140
calls.append(pkg_path)
115141
mock = MagicMock()
@@ -121,7 +147,10 @@ def fake_files(pkg_path):
121147
target = tmp_path / "file.txt"
122148
with patch("importlib.resources.files", side_effect=fake_files):
123149
ensure_seeded(
124-
target, "mypkg", "resources", "file.txt",
150+
target,
151+
"mypkg",
152+
"resources",
153+
"file.txt",
125154
seed_data_dir="my_seeds",
126155
)
127156
assert calls[0] == "mypkg.my_seeds.resources"
@@ -145,21 +174,22 @@ def test_custom_package_name(self):
145174
assert app.package_name == "my_app"
146175

147176
def test_app_folder_creates_directory(self, tmp_path):
148-
with patch.dict(os.environ, {"XDG_DATA_HOME": str(tmp_path)}):
177+
with _redirect_app_root("data", tmp_path):
149178
app = AppData("testapp")
150179
folder = app.app_folder(folder_kind="data")
151180
assert folder.is_dir()
152181
assert folder.name == "testapp"
182+
assert _same_path(folder, tmp_path / "testapp")
153183

154184
def test_app_folder_config(self, tmp_path):
155-
with patch.dict(os.environ, {"XDG_CONFIG_HOME": str(tmp_path)}):
185+
with _redirect_app_root("config", tmp_path):
156186
app = AppData("testapp")
157187
folder = app.app_folder(folder_kind="config")
158188
assert folder.is_dir()
159-
assert folder == tmp_path / "testapp"
189+
assert _same_path(folder, tmp_path / "testapp")
160190

161191
def test_get_resource_seeds_when_missing(self, tmp_path):
162-
with patch.dict(os.environ, {"XDG_DATA_HOME": str(tmp_path)}):
192+
with _redirect_app_root("data", tmp_path):
163193
with patch(
164194
"importlib.resources.files",
165195
side_effect=_mock_importlib_files(SEED_STORE),
@@ -168,10 +198,12 @@ def test_get_resource_seeds_when_missing(self, tmp_path):
168198
path = app.get_resource("hello.txt")
169199
assert path.exists()
170200
assert path.read_bytes() == b"hello world\nline two\n"
171-
assert "resources" in str(path)
201+
assert _same_path(
202+
path, tmp_path / "testapp" / "resources" / "hello.txt"
203+
)
172204

173205
def test_get_resource_preserves_user_edits(self, tmp_path):
174-
with patch.dict(os.environ, {"XDG_DATA_HOME": str(tmp_path)}):
206+
with _redirect_app_root("data", tmp_path):
175207
with patch(
176208
"importlib.resources.files",
177209
side_effect=_mock_importlib_files(SEED_STORE),
@@ -184,7 +216,7 @@ def test_get_resource_preserves_user_edits(self, tmp_path):
184216
assert path2.read_text() == "edited by user"
185217

186218
def test_get_config_seeds_when_missing(self, tmp_path):
187-
with patch.dict(os.environ, {"XDG_CONFIG_HOME": str(tmp_path)}):
219+
with _redirect_app_root("config", tmp_path):
188220
with patch(
189221
"importlib.resources.files",
190222
side_effect=_mock_importlib_files(SEED_STORE),
@@ -194,16 +226,17 @@ def test_get_config_seeds_when_missing(self, tmp_path):
194226
assert path.exists()
195227
data = json.loads(path.read_text())
196228
assert data["tempo"] == 120
229+
assert _same_path(path, tmp_path / "testapp" / "defaults.json")
197230

198231
def test_get_artifact_dir_creates_subdir(self, tmp_path):
199-
with patch.dict(os.environ, {"XDG_DATA_HOME": str(tmp_path)}):
232+
with _redirect_app_root("data", tmp_path):
200233
app = AppData("testapp")
201234
midi_dir = app.get_artifact_dir("midi")
202235
assert midi_dir.is_dir()
203-
assert midi_dir == tmp_path / "testapp" / "artifacts" / "midi"
236+
assert _same_path(midi_dir, tmp_path / "testapp" / "artifacts" / "midi")
204237

205238
def test_get_artifact_dir_multiple_kinds(self, tmp_path):
206-
with patch.dict(os.environ, {"XDG_DATA_HOME": str(tmp_path)}):
239+
with _redirect_app_root("data", tmp_path):
207240
app = AppData("testapp")
208241
for kind in ("midi", "audio", "exports"):
209242
d = app.get_artifact_dir(kind)
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
"""Cross-platform tests for the app-folder standards table.
2+
3+
``config2py.util.app_folder_standards`` is the single place where config2py
4+
branches on the operating system. Because it takes the ``os.name`` to resolve
5+
for as an argument, these tests exercise *both* platforms' tables from whatever
6+
platform happens to be running -- Windows behaviour is verified on Linux/macOS
7+
and vice versa.
8+
"""
9+
10+
import os
11+
from typing import get_args
12+
13+
import pytest
14+
15+
from config2py.util import (
16+
APP_FOLDER_STANDARDS,
17+
AppFolderKind,
18+
app_folder_standards,
19+
config2py_env_var,
20+
get_app_rootdir,
21+
system_default_for_app_data_folder,
22+
)
23+
24+
#: The ``os.name`` values config2py distinguishes between.
25+
OS_NAMES = ("nt", "posix")
26+
27+
#: Every folder kind config2py knows about.
28+
FOLDER_KINDS = tuple(APP_FOLDER_STANDARDS)
29+
30+
#: Environment values standing in for a real Windows user profile.
31+
WINDOWS_ENV = {
32+
"APPDATA": r"C:\Users\someone\AppData\Roaming",
33+
"LOCALAPPDATA": r"C:\Users\someone\AppData\Local",
34+
"TEMP": r"C:\Users\someone\AppData\Local\Temp",
35+
}
36+
37+
38+
@pytest.mark.parametrize("os_name", OS_NAMES)
39+
def test_every_kind_is_specified_on_every_platform(os_name):
40+
"""Both platform tables cover exactly the kinds the type allows."""
41+
standards = app_folder_standards(os_name)
42+
assert set(standards) == set(get_args(AppFolderKind))
43+
44+
45+
@pytest.mark.parametrize("os_name", OS_NAMES)
46+
@pytest.mark.parametrize("folder_kind", FOLDER_KINDS)
47+
def test_defaults_are_nonempty(os_name, folder_kind, monkeypatch):
48+
"""A missing platform env var must still yield a usable root, never ''.
49+
50+
An empty root silently degrades ``os.path.join(root, app_name)`` into a
51+
*relative* path, which would scatter app folders into the CWD.
52+
"""
53+
standards = app_folder_standards(os_name)
54+
for spec in standards.values():
55+
monkeypatch.delenv(spec.env_var, raising=False)
56+
resolved = system_default_for_app_data_folder(folder_kind, standards=standards)
57+
assert resolved
58+
assert not resolved.endswith(("/", "\\"))
59+
60+
61+
def test_windows_cache_is_not_the_data_folder(monkeypatch):
62+
"""Regression: on Windows 'cache' must not collide with 'data'/'state'.
63+
64+
Both are rooted at %LOCALAPPDATA%, so 'cache' has to live in a sub-folder.
65+
When they collided, clearing an app's cache would delete the user's data.
66+
"""
67+
standards = app_folder_standards("nt")
68+
for name, value in WINDOWS_ENV.items():
69+
monkeypatch.setenv(name, value)
70+
resolved = {
71+
kind: system_default_for_app_data_folder(kind, standards=standards)
72+
for kind in standards
73+
}
74+
assert resolved["cache"] != resolved["data"]
75+
assert resolved["cache"] != resolved["state"]
76+
# ... and it is the documented %LOCALAPPDATA%\Temp, i.e. *inside* the root
77+
assert os.path.basename(resolved["cache"]) == "Temp"
78+
assert os.path.dirname(resolved["cache"]) == resolved["data"]
79+
80+
81+
@pytest.mark.parametrize("os_name", OS_NAMES)
82+
def test_posix_only_xdg_vars_are_absent_from_the_windows_table(os_name):
83+
"""XDG_* is a POSIX standard; the Windows table must not name those vars."""
84+
standards = app_folder_standards(os_name)
85+
env_vars = {spec.env_var for spec in standards.values()}
86+
if os_name == "nt":
87+
assert not any(v.startswith("XDG_") for v in env_vars)
88+
else:
89+
assert all(v.startswith("XDG_") for v in env_vars)
90+
91+
92+
@pytest.mark.parametrize("folder_kind", FOLDER_KINDS)
93+
def test_config2py_override_wins_on_every_platform(folder_kind, tmp_path, monkeypatch):
94+
"""``CONFIG2PY_<KIND>_DIR`` is the platform-neutral override.
95+
96+
Unlike XDG_*, it is honoured on Windows too -- which is what makes it the
97+
right knob for tests and for users who need to relocate app folders.
98+
"""
99+
target = tmp_path / folder_kind
100+
monkeypatch.setenv(getattr(config2py_env_var, folder_kind), str(target))
101+
rootdir = get_app_rootdir(folder_kind, ensure_exists=True)
102+
assert os.path.realpath(rootdir) == os.path.realpath(str(target))

config2py/tests/test_sync_store.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,14 @@ def test_store_repr():
270270
try:
271271
file_store = FileStore(temp_file)
272272
assert "FileStore" in repr(file_store)
273-
assert temp_file in repr(file_store)
273+
# The repr embeds ``repr(self.filepath)``, and pathlib's repr always
274+
# spells the path with forward slashes (``PurePath.__repr__`` uses
275+
# ``as_posix()``) -- so on Windows it never contains the native
276+
# backslash form of ``temp_file``. Assert the *identity* of the path the
277+
# store is bound to, plus the separator-free filename in the repr,
278+
# rather than comparing separator-laden strings.
279+
assert file_store.filepath == Path(temp_file)
280+
assert Path(temp_file).name in repr(file_store)
274281

275282
file_store_with_path = FileStore(temp_file, key_path="section")
276283
assert "key_path" in repr(file_store_with_path)

0 commit comments

Comments
 (0)