From d8dfdbdd9b123c7604683ccd28db10648cd159b6 Mon Sep 17 00:00:00 2001 From: Rehanpatel00900 Date: Mon, 31 Aug 2026 15:44:39 +0530 Subject: [PATCH 1/5] Fix Windows appauthor path doubling and add Windows-specific platform tests - platformdirs defaulted appauthor to the app name, producing doubled paths like AppData\Roaming\modeldock\modeldock on Windows. Pass appauthor=False explicitly in user_config_dir/user_cache_dir/user_data_dir. - Add tests/unit/test_platform.py covering platform.py's Windows path resolution, both mocked (cross-platform) and real (Windows-only, skipped elsewhere) assertions, including a regression test for the doubling bug. Closes #115 --- src/modeldock/common/platform.py | 6 +- tests/unit/test_platform.py | 131 +++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_platform.py diff --git a/src/modeldock/common/platform.py b/src/modeldock/common/platform.py index 57f3ee8..f49895f 100644 --- a/src/modeldock/common/platform.py +++ b/src/modeldock/common/platform.py @@ -19,17 +19,17 @@ def app_name() -> str: def user_config_dir() -> Path: """Return the per-user config directory for ModelDock.""" - return Path(platformdirs.user_config_dir(app_name(), roaming=True)) + return Path(platformdirs.user_config_dir(app_name(), appauthor=False, roaming=True)) def user_cache_dir() -> Path: """Return the per-user cache directory for ModelDock.""" - return Path(platformdirs.user_cache_dir(app_name())) + return Path(platformdirs.user_cache_dir(app_name(), appauthor=False)) def user_data_dir() -> Path: """Return the per-user data directory for ModelDock.""" - return Path(platformdirs.user_data_dir(app_name())) + return Path(platformdirs.user_data_dir(app_name(), appauthor=False)) def system_config_dir() -> Path: diff --git a/tests/unit/test_platform.py b/tests/unit/test_platform.py new file mode 100644 index 0000000..1e30558 --- /dev/null +++ b/tests/unit/test_platform.py @@ -0,0 +1,131 @@ +"""Unit tests for common/platform.py, including Windows-specific path handling.""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +import pytest + +from modeldock.common import platform as md_platform + + +# --------------------------------------------------------------------------- +# Mocked tests — run on every OS, force Windows-style platformdirs output. +# --------------------------------------------------------------------------- + +class TestWindowsPathsMocked: + """Force platformdirs to return Windows-style paths and check our wrappers.""" + + def test_user_config_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + md_platform.platformdirs, + "user_config_dir", + lambda name, appauthor=False, roaming=True: r"C:\Users\test\AppData\Roaming\modeldock", + ) + result = md_platform.user_config_dir() + assert isinstance(result, Path) + assert result == Path(r"C:\Users\test\AppData\Roaming\modeldock") + + def test_user_cache_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + md_platform.platformdirs, + "user_cache_dir", + lambda name, appauthor=False: r"C:\Users\test\AppData\Local\modeldock\Cache", + ) + result = md_platform.user_cache_dir() + assert isinstance(result, Path) + assert result == Path(r"C:\Users\test\AppData\Local\modeldock\Cache") + + def test_user_data_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + md_platform.platformdirs, + "user_data_dir", + lambda name, appauthor=False: r"C:\Users\test\AppData\Local\modeldock", + ) + result = md_platform.user_data_dir() + assert isinstance(result, Path) + assert result == Path(r"C:\Users\test\AppData\Local\modeldock") + + def test_system_config_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + md_platform.platformdirs, + "site_config_dir", + lambda name: r"C:\ProgramData\modeldock", + ) + result = md_platform.system_config_dir() + assert isinstance(result, Path) + assert result == Path(r"C:\ProgramData\modeldock") + + def test_default_cache_dir_windows_no_override( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("MODELDOCK_CACHE_DIR", raising=False) + monkeypatch.setattr( + md_platform.platformdirs, + "user_cache_dir", + lambda name, appauthor=False: r"C:\Users\test\AppData\Local\modeldock", + ) + result = md_platform.default_cache_dir() + assert result == Path(r"C:\Users\test\AppData\Local\modeldock") / "models" + + def test_default_cache_dir_windows_env_override( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("MODELDOCK_CACHE_DIR", r"D:\ModelDockCache") + result = md_platform.default_cache_dir() + assert result == Path(r"D:\ModelDockCache") + + def test_is_windows_true(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(md_platform.os, "name", "nt") + assert md_platform.is_windows() is True + + def test_is_windows_false_on_posix(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(md_platform.os, "name", "posix") + assert md_platform.is_windows() is False + + +# --------------------------------------------------------------------------- +# Real tests — only meaningful (and only run) on an actual Windows host, +# e.g. the Windows job in the CI matrix. Exercises real platformdirs +# behavior rather than mocks. +# --------------------------------------------------------------------------- + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only path behavior") +class TestWindowsPathsReal: + def test_is_windows_true_on_real_windows(self) -> None: + assert md_platform.is_windows() is True + + def test_user_config_dir_under_real_appdata(self) -> None: + result = md_platform.user_config_dir() + appdata = os.environ["APPDATA"] + assert str(result).lower().startswith(appdata.lower()) + + def test_user_config_dir_not_doubled(self) -> None: + """Regression test: appauthor must not default to appname + (previously produced AppData\\Roaming\\modeldock\\modeldock).""" + result = md_platform.user_config_dir() + assert str(result).lower().count("modeldock") == 1 + + def test_user_cache_dir_under_real_localappdata(self) -> None: + result = md_platform.user_cache_dir() + localappdata = os.environ["LOCALAPPDATA"] + assert str(result).lower().startswith(localappdata.lower()) + + def test_user_cache_dir_not_doubled(self) -> None: + result = md_platform.user_cache_dir() + assert str(result).lower().count("modeldock") == 1 + + def test_paths_use_backslash_separators(self) -> None: + result = md_platform.user_config_dir() + # Real Windows paths render with backslashes via str(Path). + assert "\\" in str(result) + + def test_default_cache_dir_env_override_with_windows_drive_letter( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("MODELDOCK_CACHE_DIR", r"E:\Custom\Cache") + result = md_platform.default_cache_dir() + assert result == Path(r"E:\Custom\Cache") + assert result.drive == "E:" \ No newline at end of file From e262ac461a4765498bfcd92be272ce12229b8841 Mon Sep 17 00:00:00 2001 From: utkarsha741 Date: Wed, 2 Sep 2026 12:15:28 +0530 Subject: [PATCH 2/5] fix: sort imports in test_platform.py --- tests/unit/test_platform.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_platform.py b/tests/unit/test_platform.py index 1e30558..ea463dd 100644 --- a/tests/unit/test_platform.py +++ b/tests/unit/test_platform.py @@ -1,15 +1,12 @@ -"""Unit tests for common/platform.py, including Windows-specific path handling.""" + from __future__ import annotations -from __future__ import annotations + import os + import sys + from pathlib import Path -import os -import sys -from pathlib import Path - -import pytest - -from modeldock.common import platform as md_platform + import pytest + from modeldock.common import platform as md_platform # --------------------------------------------------------------------------- # Mocked tests — run on every OS, force Windows-style platformdirs output. @@ -128,4 +125,4 @@ def test_default_cache_dir_env_override_with_windows_drive_letter( monkeypatch.setenv("MODELDOCK_CACHE_DIR", r"E:\Custom\Cache") result = md_platform.default_cache_dir() assert result == Path(r"E:\Custom\Cache") - assert result.drive == "E:" \ No newline at end of file + assert result.drive == "E:" From 527118ff12dc26c0d4955010d196a5efc1ada461 Mon Sep 17 00:00:00 2001 From: utkarsha741 Date: Thu, 3 Sep 2026 12:28:52 +0530 Subject: [PATCH 3/5] Update test_platform.py for resolving checks --- tests/unit/test_platform.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_platform.py b/tests/unit/test_platform.py index ea463dd..08fb619 100644 --- a/tests/unit/test_platform.py +++ b/tests/unit/test_platform.py @@ -1,12 +1,12 @@ - from __future__ import annotations +from __future__ import annotations - import os - import sys - from pathlib import Path +import os +import sys +from pathlib import Path - import pytest +import pytest - from modeldock.common import platform as md_platform +from modeldock.common import platform as md_platform # --------------------------------------------------------------------------- # Mocked tests — run on every OS, force Windows-style platformdirs output. From bff54f4dff7fbb07b295f714a5798d86d953c706 Mon Sep 17 00:00:00 2001 From: utkarsha741 Date: Thu, 3 Sep 2026 13:59:33 +0530 Subject: [PATCH 4/5] Update test_platform.py for checks --- tests/unit/test_platform.py | 60 +++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/tests/unit/test_platform.py b/tests/unit/test_platform.py index 08fb619..fa11c27 100644 --- a/tests/unit/test_platform.py +++ b/tests/unit/test_platform.py @@ -1,7 +1,10 @@ +"""Unit tests for common/platform.py, including Windows-specific path handling.""" + from __future__ import annotations import os import sys +from unittest.mock import MagicMock from pathlib import Path import pytest @@ -12,46 +15,39 @@ # Mocked tests — run on every OS, force Windows-style platformdirs output. # --------------------------------------------------------------------------- + class TestWindowsPathsMocked: """Force platformdirs to return Windows-style paths and check our wrappers.""" def test_user_config_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr( - md_platform.platformdirs, - "user_config_dir", - lambda name, appauthor=False, roaming=True: r"C:\Users\test\AppData\Roaming\modeldock", - ) + mock_fn = MagicMock(return_value=r"C:\Users\test\AppData\Roaming\modeldock") + monkeypatch.setattr(md_platform.platformdirs, "user_config_dir", mock_fn) result = md_platform.user_config_dir() + mock_fn.assert_called_once_with("modeldock", appauthor=False, roaming=True) assert isinstance(result, Path) assert result == Path(r"C:\Users\test\AppData\Roaming\modeldock") def test_user_cache_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr( - md_platform.platformdirs, - "user_cache_dir", - lambda name, appauthor=False: r"C:\Users\test\AppData\Local\modeldock\Cache", - ) + mock_fn = MagicMock(return_value=r"C:\Users\test\AppData\Local\modeldock\Cache") + monkeypatch.setattr(md_platform.platformdirs, "user_cache_dir", mock_fn) result = md_platform.user_cache_dir() + mock_fn.assert_called_once_with("modeldock", appauthor=False) assert isinstance(result, Path) assert result == Path(r"C:\Users\test\AppData\Local\modeldock\Cache") def test_user_data_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr( - md_platform.platformdirs, - "user_data_dir", - lambda name, appauthor=False: r"C:\Users\test\AppData\Local\modeldock", - ) + mock_fn = MagicMock(return_value=r"C:\Users\test\AppData\Local\modeldock") + monkeypatch.setattr(md_platform.platformdirs, "user_data_dir", mock_fn) result = md_platform.user_data_dir() + mock_fn.assert_called_once_with("modeldock", appauthor=False) assert isinstance(result, Path) assert result == Path(r"C:\Users\test\AppData\Local\modeldock") def test_system_config_dir_windows(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr( - md_platform.platformdirs, - "site_config_dir", - lambda name: r"C:\ProgramData\modeldock", - ) + mock_fn = MagicMock(return_value=r"C:\ProgramData\modeldock") + monkeypatch.setattr(md_platform.platformdirs, "site_config_dir", mock_fn) result = md_platform.system_config_dir() + mock_fn.assert_called_once_with("modeldock", appauthor=False) assert isinstance(result, Path) assert result == Path(r"C:\ProgramData\modeldock") @@ -59,11 +55,8 @@ def test_default_cache_dir_windows_no_override( self, monkeypatch: pytest.MonkeyPatch ) -> None: monkeypatch.delenv("MODELDOCK_CACHE_DIR", raising=False) - monkeypatch.setattr( - md_platform.platformdirs, - "user_cache_dir", - lambda name, appauthor=False: r"C:\Users\test\AppData\Local\modeldock", - ) + mock_fn = MagicMock(return_value=r"C:\Users\test\AppData\Local\modeldock") + monkeypatch.setattr(md_platform.platformdirs, "user_cache_dir", mock_fn) result = md_platform.default_cache_dir() assert result == Path(r"C:\Users\test\AppData\Local\modeldock") / "models" @@ -89,15 +82,18 @@ def test_is_windows_false_on_posix(self, monkeypatch: pytest.MonkeyPatch) -> Non # behavior rather than mocks. # --------------------------------------------------------------------------- + @pytest.mark.skipif(sys.platform != "win32", reason="Windows-only path behavior") class TestWindowsPathsReal: + """Exercise real (unmocked) platformdirs output on an actual Windows host.""" + def test_is_windows_true_on_real_windows(self) -> None: assert md_platform.is_windows() is True def test_user_config_dir_under_real_appdata(self) -> None: result = md_platform.user_config_dir() - appdata = os.environ["APPDATA"] - assert str(result).lower().startswith(appdata.lower()) + appdata = os.environ.get("APPDATA", "") + assert appdata and str(result).lower().startswith(appdata.lower()) def test_user_config_dir_not_doubled(self) -> None: """Regression test: appauthor must not default to appname @@ -114,10 +110,16 @@ def test_user_cache_dir_not_doubled(self) -> None: result = md_platform.user_cache_dir() assert str(result).lower().count("modeldock") == 1 + def test_user_data_dir_not_doubled(self) -> None: + """Regression test: appauthor must not default to appname for the + data dir either (mirrors the config/cache doubling bug).""" + result = md_platform.user_data_dir() + assert str(result).lower().count("modeldock") == 1 + def test_paths_use_backslash_separators(self) -> None: - result = md_platform.user_config_dir() # Real Windows paths render with backslashes via str(Path). - assert "\\" in str(result) + assert "\\" in str(md_platform.user_config_dir()) + assert "\\" in str(md_platform.user_cache_dir()) def test_default_cache_dir_env_override_with_windows_drive_letter( self, monkeypatch: pytest.MonkeyPatch From 0e21201bd522867921d8086f788bf9badbe5e548 Mon Sep 17 00:00:00 2001 From: utkarsha741 Date: Thu, 3 Sep 2026 14:00:23 +0530 Subject: [PATCH 5/5] Update platform.py for checks --- src/modeldock/common/platform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modeldock/common/platform.py b/src/modeldock/common/platform.py index f49895f..6542fd4 100644 --- a/src/modeldock/common/platform.py +++ b/src/modeldock/common/platform.py @@ -34,7 +34,7 @@ def user_data_dir() -> Path: def system_config_dir() -> Path: """Return the system-wide config directory (may not exist).""" - return Path(platformdirs.site_config_dir(app_name())) + return Path(platformdirs.site_config_dir(app_name(), appauthor=False)) def default_cache_dir() -> Path: