Merge lp:~rharding/charmworld/allow-more-config-types into lp:charmworld

[Richard Harding]

Support float config type in proof.

- if we don't support validating it then ignore it for now
- add tests to each type we validate.

https://codereview.appspot.com/22370044/

R=bac

Support float config type in proof.

- if we don't support validating it then ignore it for now
- add tests to each type we validate.

https://codereview.appspot.com/22370044/

[Richard Harding (rharding)]

Reviewers: mp+194209_code.launchpad.net,

Message:
Please take a look.

Description:
Support float config type in proof.

- if we don't support validating it then ignore it for now
- add tests to each type we validate.

https://code.launchpad.net/~rharding/charmworld/allow-more-config-types/+merge/194209

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/22370044/

Affected files (+85, -20 lines):
   A [revision details]
   M charmworld/lib/proof.py
   M charmworld/lib/tests/test_proof.py

[Brad Crittenden (bac)]

The code LGTM but I'm not sure about the choice to not throw an error on
unknown types.

https://codereview.appspot.com/22370044/diff/1/charmworld/lib/proof.py
File charmworld/lib/proof.py (right):

https://codereview.appspot.com/22370044/diff/1/charmworld/lib/proof.py#newcode43
charmworld/lib/proof.py:43: return
Shouldn't an invalid type be an error or at least a warning?

https://codereview.appspot.com/22370044/

[lp:~rharding/charmworld/allow-more-config-types]

449. By Richard Harding on 2013-11-06

Per review, make it an error vs ignore

450. By Richard Harding on 2013-11-06

lint

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

=== modified file 'charmworld/lib/proof.py'
--- charmworld/lib/proof.py	2013-11-01 18:37:41 +0000
+++ charmworld/lib/proof.py	2013-11-06 19:18:02 +0000
@@ -14,6 +14,7 @@
     def check_config(charm, test_key, test_value):
         type_map = {
             'boolean': (bool,),
+            'float': (float,),
             'int': (int, ),
             'string': (str, unicode),
         }
@@ -31,10 +32,20 @@
                 'The charm has no option for: ' + test_key
             )
 
-        # # The config key is a valid one for this charm. Check that the value
-        # # is of the right type.
+        # The config key is a valid one for this charm. Check that the value
+        # is of the right type.
         specified_type = charm.options[test_key]['type']
-        valid_types = type_map[specified_type]
+
+        try:
+            valid_types = type_map[specified_type]
+        except KeyError:
+            msg = "%s:%s is not a config type recognized."
+            # Throw an error that we don't recognize the config type.
+            raise ProofError(
+                ['Looking at charm id: ' + charm._id],
+                msg % (test_key, specified_type)
+            )
+
         msg = "%s is not of type %s."
         if type(test_value) not in valid_types:
             raise ProofError(

=== modified file 'charmworld/lib/tests/test_proof.py'
--- charmworld/lib/tests/test_proof.py	2013-11-01 17:13:13 +0000
+++ charmworld/lib/tests/test_proof.py	2013-11-06 19:18:02 +0000
@@ -100,18 +100,84 @@
         """Bundles might attempt to set a boolean config value string/int"""
         charm = Mock()
         charm.options = {
-            'name': {
+            'accelerate': {
                 'type': 'boolean',
-                'default': True,
-            }
-        }
-        charm._id = 'precise/test'
-
-        with self.assertRaises(ProofError) as exc:
-            CharmProof.check_config(charm, 'name', 'test')
-
-        self.assertEqual(
-            'name is not of type boolean.',
+            },
+        }
+        charm._id = 'precise/test'
+
+        with self.assertRaises(ProofError) as exc:
+            CharmProof.check_config(charm, 'accelerate', 'test')
+
+        self.assertEqual(
+            'accelerate is not of type boolean.',
+            exc.exception.msg
+        )
+        self.assertEqual(
+            ['Looking at charm id: precise/test'],
+            exc.exception.debug_info
+        )
+
+    def test_check_config_int(self):
+        """Bundles might attempt to set a int config value string"""
+        charm = Mock()
+        charm.options = {
+            'port': {
+                'type': 'int',
+            },
+        }
+        charm._id = 'precise/test'
+
+        with self.assertRaises(ProofError) as exc:
+            CharmProof.check_config(charm, 'port', 'test')
+
+        self.assertEqual(
+            'port is not of type int.',
+            exc.exception.msg
+        )
+        self.assertEqual(
+            ['Looking at charm id: precise/test'],
+            exc.exception.debug_info
+        )
+
+    def test_check_config_float(self):
+        """Bundles might attempt to set a float config value string"""
+        charm = Mock()
+        charm.options = {
+            'some_count': {
+                'type': 'float',
+            },
+        }
+        charm._id = 'precise/test'
+
+        with self.assertRaises(ProofError) as exc:
+            CharmProof.check_config(charm, 'some_count', 'test')
+
+        self.assertEqual(
+            'some_count is not of type float.',
+            exc.exception.msg
+        )
+        self.assertEqual(
+            ['Looking at charm id: precise/test'],
+            exc.exception.debug_info
+        )
+
+    def test_check_config_is_unknown(self):
+        """If we don't know the type then ignore it for now."""
+        charm = Mock()
+        charm.options = {
+            'some_count': {
+                'type': 'foo',
+            },
+        }
+        charm._id = 'precise/test'
+
+        # Throws no errors.
+        with self.assertRaises(ProofError) as exc:
+            CharmProof.check_config(charm, 'some_count', 'test')
+
+        self.assertEqual(
+            'some_count:foo is not a config type recognized.',
             exc.exception.msg
         )
         self.assertEqual(