From b0b3ccf5aa9ce3b00cc6db76df29b6f9af539b9d Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sun, 21 Dec 2025 13:25:38 +0100 Subject: [PATCH] fix(packaging): stop including legacy pkgmgr.installers package - Restrict setuptools package discovery to src/ (pkgmgr* only) - Drop config/ as a Python package mapping (keep config as plain data dir) - Remove config_defaults fallback paths and use config/ exclusively - Add unit + integration tests for defaults.yaml loading and CLI update copying https://chatgpt.com/share/6947e74f-573c-800f-b93d-5ed341fcd1a3 --- pyproject.toml | 6 +- src/pkgmgr/cli/commands/config.py | 6 +- src/pkgmgr/core/config/load.py | 3 - .../test_config_defaults_integration.py | 124 ++++++++ .../pkgmgr/core/config/test_cli_update.py | 135 +++++++++ tests/unit/pkgmgr/core/config/test_load.py | 271 ++++++++++++++++++ 6 files changed, 534 insertions(+), 11 deletions(-) create mode 100644 tests/integration/test_config_defaults_integration.py create mode 100644 tests/unit/pkgmgr/core/config/test_cli_update.py create mode 100644 tests/unit/pkgmgr/core/config/test_load.py diff --git a/pyproject.toml b/pyproject.toml index c310be8..5aae799 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,11 +43,11 @@ pkgmgr = "pkgmgr.cli:main" # ----------------------------- # Source layout: all packages live under "src/" [tool.setuptools] -package-dir = { "" = "src", "config" = "config" } +package-dir = { "" = "src" } [tool.setuptools.packages.find] -where = ["src", "."] -include = ["pkgmgr*", "config*"] +where = ["src"] +include = ["pkgmgr*"] [tool.setuptools.package-data] "config" = ["defaults.yaml"] diff --git a/src/pkgmgr/cli/commands/config.py b/src/pkgmgr/cli/commands/config.py index 74efdf2..da13d56 100644 --- a/src/pkgmgr/cli/commands/config.py +++ b/src/pkgmgr/cli/commands/config.py @@ -42,9 +42,7 @@ def _find_defaults_source_dir() -> Optional[str]: project root that contains default config files. Preferred locations (in dieser Reihenfolge): - - /config_defaults - /config - - /config_defaults - /config """ import pkgmgr # local import to avoid circular deps @@ -53,9 +51,7 @@ def _find_defaults_source_dir() -> Optional[str]: project_root = pkg_root.parent candidates = [ - pkg_root / "config_defaults", pkg_root / "config", - project_root / "config_defaults", project_root / "config", ] for cand in candidates: @@ -73,7 +69,7 @@ def _update_default_configs(user_config_path: str) -> None: source_dir = _find_defaults_source_dir() if not source_dir: print( - "[WARN] No config_defaults or config directory found in " + "[WARN] No config directory found in " "pkgmgr installation. Nothing to update." ) return diff --git a/src/pkgmgr/core/config/load.py b/src/pkgmgr/core/config/load.py index 986a6f3..b1f2a17 100644 --- a/src/pkgmgr/core/config/load.py +++ b/src/pkgmgr/core/config/load.py @@ -14,9 +14,7 @@ Layering rules: - Falls dort keine passenden Dateien existieren, wird auf die im Paket / Projekt mitgelieferten Config-Verzeichnisse zurückgegriffen: - /config_defaults /config - /config_defaults /config Dabei werden ebenfalls alle *.yml/*.yaml als Layer geladen. @@ -218,7 +216,6 @@ def _load_defaults_from_package_or_project() -> Dict[str, Any]: # Candidate config dirs candidates = [] for root in roots: - candidates.append(root / "config_defaults") candidates.append(root / "config") for cand in candidates: diff --git a/tests/integration/test_config_defaults_integration.py b/tests/integration/test_config_defaults_integration.py new file mode 100644 index 0000000..e92864f --- /dev/null +++ b/tests/integration/test_config_defaults_integration.py @@ -0,0 +1,124 @@ +# tests/integration/test_config_defaults_integration.py +from __future__ import annotations + +import os +import sys +import tempfile +import types +import unittest +from pathlib import Path +from unittest.mock import patch + +import yaml + +from pkgmgr.core.config.load import load_config +from pkgmgr.cli.commands import config as config_cmd + + +class ConfigDefaultsIntegrationTest(unittest.TestCase): + def test_defaults_yaml_is_loaded_and_can_be_copied_to_user_config_dir(self): + """ + Integration test: + - Create a temp "site-packages/pkgmgr" fake install root + - Put defaults under "/config/defaults.yaml" + where project_root == pkg_root.parent (as per your current logic) + - Verify: + A) load_config() picks up defaults from that config folder when user dir has no defaults + B) _update_default_configs() copies defaults.yaml into ~/.config/pkgmgr/ + """ + + with tempfile.TemporaryDirectory() as td: + root = Path(td) + + # Fake HOME for user config + home = root / "home" + user_cfg_dir = home / ".config" / "pkgmgr" + user_cfg_dir.mkdir(parents=True) + user_config_path = str(user_cfg_dir / "config.yaml") + + # Create a user config file that should NOT be overwritten by update + (user_cfg_dir / "config.yaml").write_text( + yaml.safe_dump({"directories": {"user_only": "/home/user"}}), + encoding="utf-8", + ) + + # Fake pkg install layout: + # pkg_root = /site-packages/pkgmgr + # project_root = pkg_root.parent = /site-packages + site_packages = root / "site-packages" + pkg_root = site_packages / "pkgmgr" + pkg_root.mkdir(parents=True) + + # This is the "project_root/config" candidate for both: + # - load.py: roots include pkg_root.parent -> site-packages, so it checks site-packages/config + # - cli/config.py: project_root == pkg_root.parent -> site-packages, so it checks site-packages/config + config_dir = site_packages / "config" + config_dir.mkdir(parents=True) + + defaults_payload = { + "directories": { + "repositories": "/opt/Repositories", + "binaries": "/usr/local/bin", + }, + "repositories": [ + {"provider": "github", "account": "acme", "repository": "demo"} + ], + } + (config_dir / "defaults.yaml").write_text( + yaml.safe_dump(defaults_payload), + encoding="utf-8", + ) + + # Provide fake pkgmgr module so your functions resolve pkg_root correctly + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + with patch.dict(os.environ, {"HOME": str(home)}): + # A) load_config should fall back to site-packages/config/defaults.yaml + merged = load_config(user_config_path) + + self.assertEqual( + merged["directories"]["repositories"], "/opt/Repositories" + ) + self.assertEqual( + merged["directories"]["binaries"], "/usr/local/bin" + ) + + # user-only key must still exist (user config merges over defaults) + self.assertEqual(merged["directories"]["user_only"], "/home/user") + + self.assertIn("repositories", merged) + self.assertTrue( + any( + r.get("provider") == "github" + and r.get("account") == "acme" + and r.get("repository") == "demo" + for r in merged["repositories"] + ) + ) + + # B) update_default_configs should copy defaults.yaml to ~/.config/pkgmgr/ + # (and should not overwrite config.yaml) + before_config_yaml = (user_cfg_dir / "config.yaml").read_text( + encoding="utf-8" + ) + + config_cmd._update_default_configs(user_config_path) + + self.assertTrue((user_cfg_dir / "defaults.yaml").is_file()) + copied_defaults = yaml.safe_load( + (user_cfg_dir / "defaults.yaml").read_text(encoding="utf-8") + ) + self.assertEqual( + copied_defaults["directories"]["repositories"], + "/opt/Repositories", + ) + + after_config_yaml = (user_cfg_dir / "config.yaml").read_text( + encoding="utf-8" + ) + self.assertEqual(after_config_yaml, before_config_yaml) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/pkgmgr/core/config/test_cli_update.py b/tests/unit/pkgmgr/core/config/test_cli_update.py new file mode 100644 index 0000000..5b60334 --- /dev/null +++ b/tests/unit/pkgmgr/core/config/test_cli_update.py @@ -0,0 +1,135 @@ +from __future__ import annotations + +import io +import os +import sys +import tempfile +import types +import unittest +from pathlib import Path +from unittest.mock import patch + +from pkgmgr.cli.commands import config as config_cmd + + +class FindDefaultsSourceDirTests(unittest.TestCase): + def test_prefers_pkg_root_config_over_project_root_config(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + pkg_root = root / "site-packages" / "pkgmgr" + pkg_root.mkdir(parents=True) + + # both exist + (pkg_root / "config").mkdir(parents=True) + (pkg_root.parent / "config").mkdir(parents=True) + + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + found = config_cmd._find_defaults_source_dir() + + self.assertEqual(Path(found).resolve(), (pkg_root / "config").resolve()) + + def test_falls_back_to_project_root_config(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + pkg_root = root / "site-packages" / "pkgmgr" + pkg_root.mkdir(parents=True) + + # only project_root config exists + (pkg_root.parent / "config").mkdir(parents=True) + + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + found = config_cmd._find_defaults_source_dir() + + self.assertEqual( + Path(found).resolve(), (pkg_root.parent / "config").resolve() + ) + + def test_returns_none_when_no_config_dirs_exist(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + pkg_root = root / "site-packages" / "pkgmgr" + pkg_root.mkdir(parents=True) + + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + found = config_cmd._find_defaults_source_dir() + + self.assertIsNone(found) + + +class UpdateDefaultConfigsTests(unittest.TestCase): + def test_copies_yaml_files_skips_config_yaml(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + source_dir = root / "src" + source_dir.mkdir() + + # Create files + (source_dir / "a.yaml").write_text("x: 1\n", encoding="utf-8") + (source_dir / "b.yml").write_text("y: 2\n", encoding="utf-8") + (source_dir / "config.yaml").write_text( + "should_not_copy: true\n", encoding="utf-8" + ) + (source_dir / "notes.txt").write_text("nope\n", encoding="utf-8") + + home = root / "home" + dest_cfg_dir = home / ".config" / "pkgmgr" + dest_cfg_dir.mkdir(parents=True) + user_config_path = str(dest_cfg_dir / "config.yaml") + + # Patch the source dir finder to our temp source_dir + with patch.object( + config_cmd, "_find_defaults_source_dir", return_value=str(source_dir) + ): + with patch.dict(os.environ, {"HOME": str(home)}): + config_cmd._update_default_configs(user_config_path) + + self.assertTrue((dest_cfg_dir / "a.yaml").is_file()) + self.assertTrue((dest_cfg_dir / "b.yml").is_file()) + self.assertFalse( + (dest_cfg_dir / "config.yaml") + .read_text(encoding="utf-8") + .startswith("should_not_copy") + ) + + # Ensure config.yaml was not overwritten (it may exist, but should remain original if we create it) + # We'll strengthen: create an original config.yaml then re-run + (dest_cfg_dir / "config.yaml").write_text( + "original: true\n", encoding="utf-8" + ) + + with patch.object( + config_cmd, "_find_defaults_source_dir", return_value=str(source_dir) + ): + with patch.dict(os.environ, {"HOME": str(home)}): + config_cmd._update_default_configs(user_config_path) + + self.assertEqual( + (dest_cfg_dir / "config.yaml").read_text(encoding="utf-8"), + "original: true\n", + ) + + def test_prints_warning_and_returns_when_no_source_dir(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + home = root / "home" + dest_cfg_dir = home / ".config" / "pkgmgr" + dest_cfg_dir.mkdir(parents=True) + user_config_path = str(dest_cfg_dir / "config.yaml") + + buf = io.StringIO() + with patch.object( + config_cmd, "_find_defaults_source_dir", return_value=None + ): + with patch("sys.stdout", buf): + with patch.dict(os.environ, {"HOME": str(home)}): + config_cmd._update_default_configs(user_config_path) + + out = buf.getvalue() + self.assertIn("[WARN] No config directory found", out) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/pkgmgr/core/config/test_load.py b/tests/unit/pkgmgr/core/config/test_load.py new file mode 100644 index 0000000..aaa3f30 --- /dev/null +++ b/tests/unit/pkgmgr/core/config/test_load.py @@ -0,0 +1,271 @@ +from __future__ import annotations + +import os +import sys +import tempfile +import types +import unittest +from pathlib import Path +from unittest.mock import patch + +import yaml + +from pkgmgr.core.config.load import ( + _deep_merge, + _merge_repo_lists, + _load_layer_dir, + _load_defaults_from_package_or_project, + load_config, +) + + +class DeepMergeTests(unittest.TestCase): + def test_deep_merge_overrides_scalars_and_merges_dicts(self): + base = {"a": 1, "b": {"x": 1, "y": 2}, "c": {"k": 1}} + override = {"a": 2, "b": {"y": 99, "z": 3}, "c": 7} + merged = _deep_merge(base, override) + + self.assertEqual(merged["a"], 2) + self.assertEqual(merged["b"]["x"], 1) + self.assertEqual(merged["b"]["y"], 99) + self.assertEqual(merged["b"]["z"], 3) + self.assertEqual(merged["c"], 7) + + +class MergeRepoListsTests(unittest.TestCase): + def test_merge_repo_lists_adds_new_repo_and_tracks_category(self): + base = [] + new = [{"provider": "github", "account": "a", "repository": "r", "x": 1}] + _merge_repo_lists(base, new, category_name="cat1") + + self.assertEqual(len(base), 1) + self.assertEqual(base[0]["provider"], "github") + self.assertEqual(base[0]["x"], 1) + self.assertIn("category_files", base[0]) + self.assertIn("cat1", base[0]["category_files"]) + + def test_merge_repo_lists_merges_existing_repo_fields(self): + base = [ + { + "provider": "github", + "account": "a", + "repository": "r", + "x": 1, + "d": {"a": 1}, + } + ] + new = [ + { + "provider": "github", + "account": "a", + "repository": "r", + "x": 2, + "d": {"b": 2}, + } + ] + _merge_repo_lists(base, new, category_name="cat2") + + self.assertEqual(len(base), 1) + self.assertEqual(base[0]["x"], 2) + self.assertEqual(base[0]["d"]["a"], 1) + self.assertEqual(base[0]["d"]["b"], 2) + self.assertIn("cat2", base[0]["category_files"]) + + def test_merge_repo_lists_incomplete_key_appends(self): + base = [] + new = [{"foo": "bar"}] # no provider/account/repository + _merge_repo_lists(base, new, category_name="cat") + + self.assertEqual(len(base), 1) + self.assertEqual(base[0]["foo"], "bar") + self.assertIn("cat", base[0].get("category_files", [])) + + +class LoadLayerDirTests(unittest.TestCase): + def test_load_layer_dir_merges_directories_and_repos_across_files_sorted(self): + with tempfile.TemporaryDirectory() as td: + cfg_dir = Path(td) + + # 10_b.yaml should be applied after 01_a.yaml due to name sorting + (cfg_dir / "01_a.yaml").write_text( + yaml.safe_dump( + { + "directories": {"repositories": "/opt/Repos"}, + "repositories": [ + { + "provider": "github", + "account": "a", + "repository": "r1", + "x": 1, + } + ], + } + ), + encoding="utf-8", + ) + (cfg_dir / "10_b.yaml").write_text( + yaml.safe_dump( + { + "directories": {"binaries": "/usr/local/bin"}, + "repositories": [ + { + "provider": "github", + "account": "a", + "repository": "r1", + "x": 2, + }, + {"provider": "github", "account": "a", "repository": "r2"}, + ], + } + ), + encoding="utf-8", + ) + + defaults = _load_layer_dir(cfg_dir, skip_filename="config.yaml") + + self.assertEqual(defaults["directories"]["repositories"], "/opt/Repos") + self.assertEqual(defaults["directories"]["binaries"], "/usr/local/bin") + + # r1 merged: x becomes 2 and has category_files including both stems + repos = defaults["repositories"] + self.assertEqual(len(repos), 2) + r1 = next(r for r in repos if r["repository"] == "r1") + self.assertEqual(r1["x"], 2) + self.assertIn("01_a", r1.get("category_files", [])) + self.assertIn("10_b", r1.get("category_files", [])) + + def test_load_layer_dir_skips_config_yaml(self): + with tempfile.TemporaryDirectory() as td: + cfg_dir = Path(td) + (cfg_dir / "config.yaml").write_text( + yaml.safe_dump({"directories": {"x": 1}}), encoding="utf-8" + ) + (cfg_dir / "defaults.yaml").write_text( + yaml.safe_dump({"directories": {"x": 2}}), encoding="utf-8" + ) + + defaults = _load_layer_dir(cfg_dir, skip_filename="config.yaml") + # only defaults.yaml should apply + self.assertEqual(defaults["directories"]["x"], 2) + + +class DefaultsFromPackageOrProjectTests(unittest.TestCase): + def test_defaults_from_pkg_root_config_wins(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + pkg_root = root / "site-packages" / "pkgmgr" + cfg_dir = pkg_root / "config" + cfg_dir.mkdir(parents=True) + + (cfg_dir / "defaults.yaml").write_text( + yaml.safe_dump( + {"directories": {"repositories": "/opt/Repos"}, "repositories": []} + ), + encoding="utf-8", + ) + + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + defaults = _load_defaults_from_package_or_project() + + self.assertEqual(defaults["directories"]["repositories"], "/opt/Repos") + + def test_defaults_from_repo_root_src_layout(self): + with tempfile.TemporaryDirectory() as td: + repo_root = Path(td) / "repo" + pkg_root = repo_root / "src" / "pkgmgr" + cfg_dir = repo_root / "config" + cfg_dir.mkdir(parents=True) + pkg_root.mkdir(parents=True) + + (cfg_dir / "defaults.yaml").write_text( + yaml.safe_dump( + {"directories": {"binaries": "/usr/local/bin"}, "repositories": []} + ), + encoding="utf-8", + ) + + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + defaults = _load_defaults_from_package_or_project() + + self.assertEqual(defaults["directories"]["binaries"], "/usr/local/bin") + + def test_defaults_returns_empty_when_no_config_found(self): + with tempfile.TemporaryDirectory() as td: + pkg_root = Path(td) / "site-packages" / "pkgmgr" + pkg_root.mkdir(parents=True) + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + defaults = _load_defaults_from_package_or_project() + + self.assertEqual(defaults, {"directories": {}, "repositories": []}) + + +class LoadConfigIntegrationUnitTests(unittest.TestCase): + def test_load_config_prefers_user_dir_defaults_over_package_defaults(self): + with tempfile.TemporaryDirectory() as td: + home = Path(td) / "home" + user_cfg_dir = home / ".config" / "pkgmgr" + user_cfg_dir.mkdir(parents=True) + user_config_path = str(user_cfg_dir / "config.yaml") + + # user dir defaults exist -> should be used, package fallback must not matter + (user_cfg_dir / "aa.yaml").write_text( + yaml.safe_dump({"directories": {"repositories": "/USER/Repos"}}), + encoding="utf-8", + ) + (user_cfg_dir / "config.yaml").write_text( + yaml.safe_dump({"directories": {"binaries": "/USER/bin"}}), + encoding="utf-8", + ) + + with patch.dict(os.environ, {"HOME": str(home)}): + merged = load_config(user_config_path) + + self.assertEqual(merged["directories"]["repositories"], "/USER/Repos") + self.assertEqual(merged["directories"]["binaries"], "/USER/bin") + + def test_load_config_falls_back_to_package_when_user_dir_has_no_defaults(self): + with tempfile.TemporaryDirectory() as td: + home = Path(td) / "home" + user_cfg_dir = home / ".config" / "pkgmgr" + user_cfg_dir.mkdir(parents=True) + user_config_path = str(user_cfg_dir / "config.yaml") + + # Only user config exists, no other yaml defaults + (user_cfg_dir / "config.yaml").write_text( + yaml.safe_dump({"directories": {"x": 1}}), encoding="utf-8" + ) + + # Provide package defaults via fake pkgmgr + pkg_root/config + root = Path(td) / "site-packages" + pkg_root = root / "pkgmgr" + cfg_dir = ( + root / "config" + ) # NOTE: load.py checks multiple roots, including pkg_root.parent (=site-packages) + pkg_root.mkdir(parents=True) + cfg_dir.mkdir(parents=True) + + (cfg_dir / "defaults.yaml").write_text( + yaml.safe_dump( + {"directories": {"repositories": "/PKG/Repos"}, "repositories": []} + ), + encoding="utf-8", + ) + + fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py")) + with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}): + with patch.dict(os.environ, {"HOME": str(home)}): + merged = load_config(user_config_path) + + # directories are merged: defaults then user + self.assertEqual(merged["directories"]["repositories"], "/PKG/Repos") + self.assertEqual(merged["directories"]["x"], 1) + self.assertIn("repositories", merged) + self.assertIsInstance(merged["repositories"], list) + + +if __name__ == "__main__": + unittest.main()