Merge ~jfguedez/charm-juju-lint:bug/1942947 into charm-juju-lint:master

[Jose Guedez]

Collect default charm settings, add unit tests

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Edin S (exsdev)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 31f543f9e3880535fb16f282eedd20546c223efc

diff --git a/scripts/auto_lint.py b/scripts/auto_lint.py
index 6446a01..944dcf5 100755
--- a/scripts/auto_lint.py
+++ b/scripts/auto_lint.py
@@ -21,6 +21,7 @@ License: Apache License 2.0
 
 from base64 import b64decode
 import json
+import logging
 from os.path import join
 import subprocess
 from sys import exit
@@ -160,13 +161,21 @@ async def add_charm_config_to_juju_status(juju_status, model):
         model_app = model.applications[app_name]
         model_app_config = await model_app.get_config()
 
-        # collect only the manual settings to match bundle behaviour
-        # which is what juju-lint expects for charm configuration
-        status_app_config = {
-            c: d["value"]
-            for c, d in model_app_config.items()
-            if d["source"] != "default"
-        }
+        # Collect all manual/default config settings
+        # this similar to juju export-bundle --include-charm-defaults
+        status_app_config = {}
+        for k, v in model_app_config.items():
+            if v["source"] != "unset":
+                try:
+                    status_app_config[k] = v["value"]
+                except KeyError as error:
+                    logging.error(
+                        'missing info for app: "{}", key: "{}", val: "{}"'.format(
+                            app_name, k, v
+                        )
+                    )
+                    raise error
+
         juju_status["applications"][app_name]["options"] = status_app_config
 
     return juju_status
diff --git a/tests/unit/fixtures.py b/tests/unit/fixtures.py
index 344586f..5e25c17 100644
--- a/tests/unit/fixtures.py
+++ b/tests/unit/fixtures.py
@@ -419,3 +419,35 @@ class MockModel:
             get_config=AsyncMock(return_value={"swap": {"value": "", "source": "user"}})
         ),
     }
+
+
+class MockModelUnset:
+    applications = {
+        "juju-lint": Mock(
+            get_config=AsyncMock(return_value={"model-uuid": {"source": "unset"}})
+        ),
+        "nagios": Mock(
+            get_config=AsyncMock(
+                return_value={"check_timeout": {"value": "10", "source": "default"}}
+            )
+        ),
+        "nrpe": Mock(
+            get_config=AsyncMock(return_value={"swap": {"value": "", "source": "user"}})
+        ),
+    }
+
+
+class MockModelMissingValue:
+    applications = {
+        "juju-lint": Mock(
+            get_config=AsyncMock(return_value={"model-uuid": {"source": "user"}})
+        ),
+        "nagios": Mock(
+            get_config=AsyncMock(
+                return_value={"check_timeout": {"value": "10", "source": "default"}}
+            )
+        ),
+        "nrpe": Mock(
+            get_config=AsyncMock(return_value={"swap": {"value": "", "source": "user"}})
+        ),
+    }
diff --git a/tests/unit/test_auto_lint.py b/tests/unit/test_auto_lint.py
index a0f5e05..00189ae 100644
--- a/tests/unit/test_auto_lint.py
+++ b/tests/unit/test_auto_lint.py
@@ -85,8 +85,37 @@ class TestAutoLint(unittest.TestCase):
         juju_status = loop.run(
             add_charm_config_to_juju_status(self.juju_status, fixtures.MockModel)
         )
-        for app_dict in juju_status["applications"].values():
-            assert "options" in app_dict
+        for app_name, app_dict in juju_status["applications"].items():
+            options = app_dict["options"]
+            if app_name == "juju-lint":
+                self.assertEqual(options["model-uuid"], "1")
+            elif app_name == "nagios":
+                self.assertEqual(options["check_timeout"], "10")
+            elif app_name == "nrpe":
+                self.assertEqual(options["swap"], "")
+            else:
+                raise Exception("unexpected application: {}".format(app_name))
+
+    def test_add_config_to_status_unset(self):
+        """Tests that the unset options are ignored."""
+        juju_status = loop.run(
+            add_charm_config_to_juju_status(self.juju_status, fixtures.MockModelUnset)
+        )
+        for app_name, app_dict in juju_status["applications"].items():
+            if app_name == "juju-lint":
+                self.assertEqual(app_dict["options"], {})
+
+    def test_add_config_to_status_value_present(self):
+        """Tests handling the case where an option is missing a value."""
+
+        def test_helper():
+            loop.run(
+                add_charm_config_to_juju_status(
+                    self.juju_status, fixtures.MockModelMissingValue
+                )
+            )
+
+        self.assertRaises(KeyError, test_helper)
 
     @mock.patch("scripts.auto_lint.write_file")
     @mock.patch("scripts.auto_lint.subprocess.check_output")