cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

I like the idea; and the fix is needed. I just have a question about whether we can find a way to generalize that validate_or_fail method and avoid duplication.

Diff comments:

> diff --git a/cloudinit/config/tests/test_snap.py b/cloudinit/config/tests/test_snap.py
> index 492d2d4..cb0e5bb 100644
> --- a/cloudinit/config/tests/test_snap.py
> +++ b/cloudinit/config/tests/test_snap.py
> @@ -340,41 +341,36 @@ class TestSchema(CiTestCase):
>              {'snap': {'assertions': {'01': 'also valid'}}}, schema)
>          self.assertEqual('', self.logs.getvalue())
>  
> +    def _validate_or_fail(self, cfg, msg):

This is good test utility method. Now that you've crossed the rule-of-three, where we now have 3 examples of different consumers of the method, it might be worth generalizing this method and creating a Mixin class that we can subclass from.

 It seems like a general method could be

SchemaTestCaseMixin():

   def validate_schema(self, key, cfg, msg):
       """
       @param key: Top-level config key (i.e. 'runcmd') under which cfg lives.
       @param cfg: Dict of cfg values provided under key.
       @param msg: Failure message raised if schema validation fails.
       """

Then we can declare class TestSchema(CiTestCase, SchemaTestCaseMixin): in test_snap.py and leverage something like this

http://paste.ubuntu.com/p/cGHdzmHNj2

what do you think?

> +        try:
> +            validate_cloudconfig_schema(
> +                {'snap': cfg}, schema, strict=True)
> +        except SchemaValidationError as e:
> +            self.fail(msg)
> +
>      def test_duplicates_are_fine_array_array(self):
>          """Duplicated commands array/array entries are allowed."""
> -        byebye = ["echo", "bye"]
> -        try:
> -            cfg = {'snap': {'commands': [byebye, byebye]}}
> -            validate_cloudconfig_schema(cfg, schema, strict=True)
> -        except schema.SchemaValidationError as e:
> -            self.fail("command entries can be duplicate.")
> +        self._validate_or_fail(
> +            {'commands': [["echo", "bye"], ["echo", "bye"]]},
> +            "command entries can be duplicate.")
>  
>      def test_duplicates_are_fine_array_string(self):
>          """Duplicated commands array/string entries are allowed."""
> -        byebye = "echo bye"
> -        try:
> -            cfg = {'snap': {'commands': [byebye, byebye]}}
> -            validate_cloudconfig_schema(cfg, schema, strict=True)
> -        except schema.SchemaValidationError as e:
> -            self.fail("command entries can be duplicate.")
> +        self._validate_or_fail(
> +            {'commands': ["echo bye", "echo bye"]},
> +            "command entries can be duplicate.")
>  
>      def test_duplicates_are_fine_dict_array(self):
>          """Duplicated commands dict/array entries are allowed."""
> -        byebye = ["echo", "bye"]
> -        try:
> -            cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
> -            validate_cloudconfig_schema(cfg, schema, strict=True)
> -        except schema.SchemaValidationError as e:
> -            self.fail("command entries can be duplicate.")
> +        self._validate_or_fail(
> +            {'commands': {'00': ["echo", "bye"], '01': ["echo", "bye"]}},
> +            "command entries can be duplicate.")
>  
>      def test_duplicates_are_fine_dict_string(self):
>          """Duplicated commands dict/string entries are allowed."""
> -        byebye = "echo bye"
> -        try:
> -            cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
> -            validate_cloudconfig_schema(cfg, schema, strict=True)
> -        except schema.SchemaValidationError as e:
> -            self.fail("command entries can be duplicate.")
> +        self._validate_or_fail(
> +            {'commands': {'00': "echo bye", '01': "echo bye"}},
> +            "command entries can be duplicate.")
>  
>  
>  class TestHandle(CiTestCase):


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/343609
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/jsonschema-follow-on into cloud-init:master.