From e4b95b5465e24c83f9e65546bcc1fba47a227362 Mon Sep 17 00:00:00 2001
From: Johannes Kirschbauer <hsjobeki@gmail.com>
Date: Fri, 30 May 2025 10:23:32 +0200
Subject: [PATCH] Test(InventoryPersistence): improve error message

---
 .../clan_lib/persist/inventory_store_test.py  | 46 ++++++++++++++++++-
 pkgs/clan-cli/clan_lib/persist/util.py        |  2 +-
 pkgs/clan-cli/clan_lib/persist/util_test.py   |  7 +--
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/pkgs/clan-cli/clan_lib/persist/inventory_store_test.py b/pkgs/clan-cli/clan_lib/persist/inventory_store_test.py
index 93c729846..82727d3a9 100644
--- a/pkgs/clan-cli/clan_lib/persist/inventory_store_test.py
+++ b/pkgs/clan-cli/clan_lib/persist/inventory_store_test.py
@@ -119,7 +119,10 @@ def test_simple_read_write(setup_test_files: Path) -> None:
     invalid_data = {"protected": "foo"}
     with pytest.raises(ClanError) as e:
         store.write(invalid_data, "test", commit=False)  # type: ignore
-    assert str(e.value) == "Key 'protected' is not writeable."
+    assert (
+        str(e.value)
+        == "Key 'protected' is not writeable. It seems its value is statically defined in nix."
+    )
 
     # Test the data is not touched
     assert store.read() == data
@@ -133,7 +136,7 @@ def test_simple_read_write(setup_test_files: Path) -> None:
 
 @pytest.mark.with_core
 @pytest.mark.parametrize("setup_test_files", ["deferred.nix"], indirect=True)
-def test_read_deferred(setup_test_files: Path) -> None:
+def test_simple_deferred(setup_test_files: Path) -> None:
     files = list(setup_test_files.iterdir())
     nix_file = next(f for f in files if f.suffix == ".nix")
     json_file = next(f for f in files if f.suffix == ".json")
@@ -175,6 +178,45 @@ def test_read_deferred(setup_test_files: Path) -> None:
     assert store.read() == {"foo": {"a": {}, "b": {}}}
 
 
+# TODO: Add this feature. We want it, but its more complex than expected.
+# @pytest.mark.with_core
+# @pytest.mark.parametrize("setup_test_files", ["deferred.nix"], indirect=True)
+# def test_conflicts_deferred(setup_test_files: Path) -> None:
+#     files = list(setup_test_files.iterdir())
+#     nix_file = next(f for f in files if f.suffix == ".nix")
+#     json_file = next(f for f in files if f.suffix == ".json")
+
+#     assert nix_file.exists()
+#     assert json_file.exists()
+
+#     store = InventoryStore(
+#         flake=MockFlake(nix_file),
+#         inventory_file_name=json_file.name,
+#         # Needed to allow auto-transforming deferred modules
+#         _allowed_path_transforms=["foo.*"],
+#         _keys=[],  # disable toplevel filtering
+#     )
+
+#     data = store.read()
+#     assert data == {"foo": {"a": {}, "b": {}}}
+
+#     # Create a new "deferredModule" "a" which collides with existing foo.a
+#     set_value_by_path(data, "foo.a", {"timeout": "1s"})  # type: ignore
+#     with pytest.raises(ClanError) as e:
+#         store.write(data, "test", commit=False)
+#     assert (
+#         str(e.value)
+#         == "Key 'foo.a' is not writeable. It seems its value is statically defined in nix."
+#     )
+
+#     # Giving an empty value to a deferred module does not throw an error
+#     # This is deduplicated with the module in nix and does not need to create a new module
+#     set_value_by_path(data, "foo.a", {})
+#     store.write(data, "test", commit=False)
+#     # unchanged data
+#     assert store.read() == {"foo": {"a": {}, "b": {}}}
+
+
 @pytest.mark.with_core
 @pytest.mark.parametrize("setup_test_files", ["lists.nix"], indirect=True)
 def test_manipulate_list(setup_test_files: Path) -> None:
diff --git a/pkgs/clan-cli/clan_lib/persist/util.py b/pkgs/clan-cli/clan_lib/persist/util.py
index ada021490..382827dc1 100644
--- a/pkgs/clan-cli/clan_lib/persist/util.py
+++ b/pkgs/clan-cli/clan_lib/persist/util.py
@@ -201,7 +201,7 @@ def calc_patches(
         if old != new:
             # If there is a change, check if the key is writeable
             if not is_writeable_key(key):
-                msg = f"Key '{key}' is not writeable."
+                msg = f"Key '{key}' is not writeable. It seems its value is statically defined in nix."
                 raise ClanError(msg)
 
             if any(key.startswith(d) for d in delete_set):
diff --git a/pkgs/clan-cli/clan_lib/persist/util_test.py b/pkgs/clan-cli/clan_lib/persist/util_test.py
index b6d90275a..b0aacf6bb 100644
--- a/pkgs/clan-cli/clan_lib/persist/util_test.py
+++ b/pkgs/clan-cli/clan_lib/persist/util_test.py
@@ -362,7 +362,7 @@ def test_update_parent_non_writeable() -> None:
     with pytest.raises(ClanError) as error:
         calc_patches(data_disk, update, all_values=data_eval, writeables=writeables)
 
-    assert str(error.value) == "Key 'foo.bar' is not writeable."
+    assert "Key 'foo.bar' is not writeable." in str(error.value)
 
 
 def test_update_list() -> None:
@@ -428,10 +428,7 @@ def test_update_list_duplicates() -> None:
     with pytest.raises(ClanError) as error:
         calc_patches(data_disk, update, all_values=data_eval, writeables=writeables)
 
-    assert (
-        str(error.value)
-        == "Key 'foo' contains duplicates: ['A']. This not supported yet."
-    )
+    assert "Key 'foo' contains list duplicates: ['A']" in str(error.value)
 
 
 def test_dont_persist_defaults() -> None: