pr comments

ops/testing.py

@@ -116,14 +116,13 @@ def __init__(
         self._framework = None
         self._hooks_enabled = True
         self._relation_id_counter = 0
-        self._backend = _TestingModelBackend(self._unit_name, self._meta)
+        config_ = self._get_config(config)
+        self._backend = _TestingModelBackend(self._unit_name, self._meta, config_)
         self._model = model.Model(self._meta, self._backend)
         self._storage = storage.SQLiteStorage(':memory:')
         self._oci_resources = {}
         self._framework = framework.Framework(
             self._storage, self._charm_dir, self._meta, self._model)
-        self._defaults = self._load_config_defaults(config)
-        self._update_config(key_values=self._defaults)
 
         # TODO: If/when we decide to allow breaking changes for a release,
         # change SIMULATE_CAN_CONNECT default value to True and remove the
@@ -325,10 +324,10 @@ def _create_meta(self, charm_metadata, action_metadata):
 
         return charm.CharmMeta.from_yaml(charm_metadata, action_metadata)
 
-    def _load_config_defaults(self, charm_config):
-        """Load default values from config.yaml.
+    def _get_config(self, charm_config: typing.Optional[dict]):
+        """If the user passed a config to Harness, use it.
 
-        Handle the case where a user doesn't supply explicit config snippets.
+        Otherwise, attempt to load one from charm_dir/config.yaml.
         """
         filename = inspect.getfile(self._charm_cls)
         charm_dir = pathlib.Path(filename).parents[1]
@@ -344,8 +343,10 @@ def _load_config_defaults(self, charm_config):
         elif isinstance(charm_config, str):
             charm_config = dedent(charm_config)
         charm_config = yaml.safe_load(charm_config)
-        charm_config = charm_config.get('options', {})
-        return {key: value.get('default', None) for key, value in charm_config.items()}
+
+        if not isinstance(charm_config, dict):
+            raise TypeError(charm_config)
+        return charm_config
 
     def add_oci_resource(self, resource_name: str,
                          contents: typing.Mapping[str, str] = None) -> None:
@@ -924,23 +925,23 @@ def _update_config(
         config = self._backend._config
         if key_values is not None:
             for key, value in key_values.items():
-                if key in self._defaults:
+                if key in config._defaults:
                     if value is not None:
-                        config[key] = value
+                        config._config_set(key, value)
                 else:
                     raise ValueError("unknown config option: '{}'".format(key))
 
         for key in unset:
             # When the key is unset, revert to the default if one exists
-            default = self._defaults.get(key, None)
+            default = config._defaults.get(key, None)
             if default is not None:
-                config[key] = default
+                config._config_set(key, default)
             else:
                 config.pop(key, None)
 
     def update_config(
             self,
-            key_values: typing.Mapping[str, str] = None,
+            key_values: typing.Mapping[str, typing.Union[str, float, int, bool]] = None,
             unset: typing.Iterable[str] = (),
     ) -> None:
         """Update the config as seen by the charm.
@@ -1075,6 +1076,75 @@ def __init__(self, resource_name):
         self.name = resource_name
 
 
+@_record_calls
+class _TestingConfig(dict):
+    """Represents the Juju Config."""
+    _supported_types = {
+        'string': str,
+        'boolean': bool,
+        'int': int,
+        'float': float
+    }
+
+    def __init__(self, config: dict):
+        super().__init__()
+        self._spec = config
+        self._defaults = self._load_defaults(config)
+
+        for key, value in self._defaults.items():
+            if value is None:
+                continue
+            self._config_set(key, value)
+
+    @staticmethod
+    def _load_defaults(charm_config: dict):
+        """Load default values from config.yaml.
+
+        Handle the case where a user doesn't supply explicit config snippets.
+        """
+        if not charm_config:
+            return {}
+        charm_config = charm_config.get('options', {})
+        return {key: value.get('default', None) for key, value in charm_config.items()}
+
+    def _config_set(self, key, value):
+        # this is only called by the harness itself
+        # we don't do real serialization/deserialization, but we do check that the value
+        # has the expected type.
+        if type(value) not in self._supported_types.values():
+            raise RuntimeError('cannot set {} to {}; type {} is not '
+                               'supported by juju'.format(key, value, type(value)))
+
+        option = self._spec.get('options', {}).get(key)
+        if not option:
+            raise RuntimeError('Unknown config option {}; '
+                               'not declared in `config.yaml`.'
+                               'Check https://juju.is/docs/sdk/config for the '
+                               'spec.'.format(key))
+
+        declared_type = option.get('type')
+        if not declared_type:
+            raise RuntimeError('Incorrectly formatted `options.yaml`, option {} '
+                               'is expected to declare a `type`.'.format(key))
+
+        if declared_type not in self._supported_types:
+            raise RuntimeError(
+                'Incorrectly formatted `options.yaml`: `type` needs to be one '
+                'of [{}], not {}.'.format(', '.join(self._supported_types), declared_type))
+
+        if type(value) != self._supported_types[declared_type]:
+            raise RuntimeError('Config option {} is supposed to be of type '
+                               '{}, not `{}`.'.format(key, declared_type,
+                                                      type(value).__name__))
+
+        # call 'normal' setattr.
+        dict.__setitem__(self, key, value)
+
+    def __setitem__(self, key, value):
+        # if a charm attempts to config[foo] = bar:
+        raise TypeError("'ConfigData' object does not support item assignment")
+
+
 @_copy_docstrings(model._ModelBackend)
 @_record_calls
 class _TestingModelBackend:
@@ -1085,7 +1155,7 @@ class _TestingModelBackend:
     as the only public methods of this type are for implementing ModelBackend.
     """
 
-    def __init__(self, unit_name, meta):
+    def __init__(self, unit_name, meta, config):
         self.unit_name = unit_name
         self.app_name = self.unit_name.split('/')[0]
         self.model_name = None
@@ -1100,7 +1170,7 @@ def __init__(self, unit_name, meta):
         self._relation_data = {}  # {relation_id: {name: data}}
         # {relation_id: {"app": app_name, "units": ["app/0",...]}
         self._relation_app_and_units = {}
-        self._config = {}
+        self._config = _TestingConfig(config)
         self._is_leader = False
         self._resources_map = {}  # {resource_name: resource_content}
         self._pod_spec = None

test/test_model.py

@@ -58,7 +58,7 @@ def setUp(self):
             bar:
                 type: int
             qux:
-                type: bool
+                type: boolean
         ''')
         self.addCleanup(self.harness.cleanup)
         self.relation_id_db0 = self.harness.add_relation('db0', 'db')

test/test_testing.py

@@ -993,7 +993,7 @@ def test_no_event_on_no_diff_update_relation_unit_bag(self):
         self.assertEqual(viewer.changes, [{'initial': 'data'}])
 
     def test_empty_config_raises(self):
-        with self.assertRaises(AttributeError):
+        with self.assertRaises(TypeError):
             Harness(RecordingCharm, config='')
 
     def test_update_config(self):
@@ -1033,12 +1033,66 @@ def test_update_config_undefined_option(self):
         with self.assertRaises(ValueError):
             harness.update_config(key_values={'nonexistent': 'foo'})
 
+    def test_update_config_bad_type(self):
+        harness = Harness(RecordingCharm, config='''
+            options:
+                a:
+                    description: a config option
+                    type: boolean
+                    default: false
+            ''')
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+        with self.assertRaises(RuntimeError):
+            # cannot cast to bool
+            harness.update_config(key_values={'a': 'foo'})
+
+        with self.assertRaises(RuntimeError):
+            # cannot cast to float
+            harness.update_config(key_values={'a': 42.42})
+
+        with self.assertRaises(RuntimeError):
+            # cannot cast to int
+            harness.update_config(key_values={'a': 42})
+
+        # can cast to bool!
+        harness.update_config(key_values={'a': False})
+
+    def test_bad_config_option_type(self):
+        with self.assertRaises(RuntimeError):
+            Harness(RecordingCharm, config='''
+                options:
+                    a:
+                        description: a config option
+                        type: gibberish
+                        default: False
+                ''')
+
+    def test_no_config_option_type(self):
+        with self.assertRaises(RuntimeError):
+            Harness(RecordingCharm, config='''
+                options:
+                    a:
+                        description: a config option
+                        default: False
+                ''')
+
+    def test_uncastable_config_option_type(self):
+        with self.assertRaises(RuntimeError):
+            Harness(RecordingCharm, config='''
+                options:
+                    a:
+                        description: a config option
+                        type: boolean
+                        default: peek-a-bool!
+                ''')
+
     def test_update_config_unset_boolean(self):
         harness = Harness(RecordingCharm, config='''
             options:
                 a:
                     description: a config option
-                    type: bool
+                    type: boolean
                     default: False
             ''')
         self.addCleanup(harness.cleanup)
@@ -1249,8 +1303,8 @@ def test_config_from_directory(self):
         self.assertEqual(harness.model.config['opt_float'], 1.0)
         self.assertIsInstance(harness.model.config['opt_float'], float)
         self.assertFalse('opt_null' in harness.model.config)
-        self.assertIsNone(harness._defaults['opt_null'])
-        self.assertIsNone(harness._defaults['opt_no_default'])
+        self.assertIsNone(harness._backend._config._defaults['opt_null'])
+        self.assertIsNone(harness._backend._config._defaults['opt_no_default'])
 
     def test_set_model_name(self):
         harness = Harness(CharmBase, meta='''