configglue team mailing list archive

[Ricardo Kirkner <ricardo.kirkner@xxxxxxxxxxxxx>]

Ricardo Kirkner has proposed merging lp:~ricardokirkner/configglue/574615-missing-sections-reported-unclearly into lp:configglue.

Requested reviews:
  Configglue developers (configglue)

For more details, see:
https://code.launchpad.net/~ricardokirkner/configglue/574615-missing-sections-reported-unclearly/+merge/69583

Overview
========

Better error handling and reporting for missing and extra sections during configuration validation.
-- 
https://code.launchpad.net/~ricardokirkner/configglue/574615-missing-sections-reported-unclearly/+merge/69583
Your team Configglue developers is requested to review the proposed merge of lp:~ricardokirkner/configglue/574615-missing-sections-reported-unclearly into lp:configglue.

=== modified file 'configglue/glue.py'
--- configglue/glue.py	2011-07-26 23:56:48 +0000
+++ configglue/glue.py	2011-07-28 01:34:29 +0000
@@ -66,8 +66,10 @@
             kwargs = {}
             if option.help:
                 kwargs['help'] = option.help
-            if not option.fatal:
+            try:
                 kwargs['default'] = parser.get(section.name, option.name)
+            except (NoSectionError, NoOptionError):
+                pass
             kwargs['action'] = option.action
             args = ['--' + long_name(option)]
             if option.short_name:

=== modified file 'configglue/parser.py'
--- configglue/parser.py	2011-07-17 22:32:16 +0000
+++ configglue/parser.py	2011-07-28 01:34:29 +0000
@@ -80,27 +80,37 @@
         errors = []
         try:
             # validate structure
-            parsed_sections = set(self.sections())
+            config_sections = set(self.sections())
             schema_sections = set(s.name for s in self.schema.sections())
             skip_sections = self.extra_sections
-            if '__noschema__' in parsed_sections:
-                skip_sections.add('__noschema__')
-            schema_sections.update(skip_sections)
-            sections_match = (parsed_sections == schema_sections)
-            if not sections_match:
-                unmatched_sections = list(parsed_sections - schema_sections)
-                if len(unmatched_sections) > 0:
-                    error_msg = "Sections in configuration are missing from schema: %s"
-                    error_value = ', '.join(unmatched_sections)
-                    errors.append(error_msg % error_value)
-                unmatched_sections = list(schema_sections - parsed_sections)
-                if len(unmatched_sections) > 0:
-                    error_msg = "Sections in schema are missing from configuration: %s"
-                    error_value = ', '.join(unmatched_sections)
-                    errors.append(error_msg % error_value)
-            valid &= sections_match
+            magic_sections = set(['__main__', '__noschema__'])
+            # test1: no undefined implicit sections
+            unmatched_sections = (skip_sections - config_sections)
+            if unmatched_sections:
+                error_msg = "Undefined sections in configuration: %s"
+                error_value = ', '.join(unmatched_sections)
+                errors.append(error_msg % error_value)
+                valid = False
+            # remove sections to skip from config sections
+            config_sections.difference_update(skip_sections)
+            # test2: no missing schema sections
+            unmatched_sections = (
+                schema_sections - magic_sections - config_sections)
+            if unmatched_sections:
+                error_msg = "Sections in schema are missing from configuration: %s"
+                error_value = ', '.join(unmatched_sections)
+                errors.append(error_msg % error_value)
+                valid = False
+            # test3: no extra sections that are not implicit sections
+            unmatched_sections = (
+                config_sections - magic_sections - schema_sections)
+            if unmatched_sections:
+                error_msg = "Sections in configuration are missing from schema: %s"
+                error_value = ', '.join(unmatched_sections)
+                errors.append(error_msg % error_value)
+                valid = False
 
-            for name in parsed_sections.union(schema_sections):
+            for name in config_sections.union(schema_sections):
                 if name not in skip_sections:
                     if not self.schema.has_section(name):
                         # this should have been reported before

=== modified file 'configglue/tests/test_parser.py'
--- configglue/tests/test_parser.py	2011-07-23 18:31:46 +0000
+++ configglue/tests/test_parser.py	2011-07-28 01:34:29 +0000
@@ -684,6 +684,21 @@
             {'bar': 0, 'baz': False})
         self.assertEqual(parser.extra_sections, set([]))
 
+    def test_extra_sections_missing_section(self):
+        """Test parse dict with missing referenced section."""
+        class MySchema(Schema):
+            foo = DictOption()
+
+        config = StringIO(textwrap.dedent("""
+            [__main__]
+            foo = dict1
+            """))
+        parser = SchemaConfigParser(MySchema())
+        parser.readfp(config)
+        parser.parse_all()
+
+        self.assertEqual(parser.extra_sections, set(['dict1']))
+
     def test_multiple_extra_sections(self):
         """Test parsing multiple extra sections."""
         class MySchema(Schema):
@@ -1221,6 +1236,23 @@
                 {'baz': [{'whoosh': '3'}, {'swoosh': '4'}]}]}})
         self.assertTrue(parser.is_valid())
 
+    def test_extra_sections_with_missing_section(self):
+        """Test parser.is_valid with dict referencing missing section."""
+        class MySchema(Schema):
+            foo = DictOption()
+
+        config = StringIO(textwrap.dedent("""
+            [__main__]
+            foo = dict1
+            """))
+        parser = SchemaConfigParser(MySchema())
+        parser.readfp(config)
+        parser.parse_all()
+
+        self.assertRaises(NoSectionError, parser.values)
+        # config is not valid
+        self.assertFalse(parser.is_valid())
+
     def test_multiple_extra_sections(self):
         """Test parser.is_valid with multiple extra sections."""
         class MySchema(Schema):

=== modified file 'configglue/tests/test_schemaconfig.py'
--- configglue/tests/test_schemaconfig.py	2011-07-26 23:56:48 +0000
+++ configglue/tests/test_schemaconfig.py	2011-07-28 01:34:29 +0000
@@ -33,8 +33,12 @@
     configglue,
     schemaconfigglue,
 )
-from configglue.parser import SchemaConfigParser
+from configglue.parser import (
+    NoSectionError,
+    SchemaConfigParser,
+)
 from configglue.schema import (
+    DictOption,
     IntOption,
     Option,
     Schema,
@@ -178,6 +182,25 @@
         self.assertEqual(self.parser.values(),
                          {'foo': {'bar': 2}, '__main__': {'baz': 0}})
 
+    def test_glue_missing_section(self):
+        """Test schemaconfigglue with missing section."""
+        class MySchema(Schema):
+            foo = DictOption()
+
+        config = StringIO("[__main__]\nfoo = bar")
+        parser = SchemaConfigParser(MySchema())
+        parser.readfp(config)
+
+        # hitting the parser directly raises an exception
+        self.assertRaises(NoSectionError, parser.values)
+        self.assertFalse(parser.is_valid())
+
+        # which is nicely handled by the glue code, so as not to crash it
+        op, options, args = schemaconfigglue(parser)
+
+        # there is no value for 'foo' due to the missing section
+        self.assertEqual(options, {'foo': None})
+
     @patch('configglue.glue.os')
     def test_glue_environ(self, mock_os):
         mock_os.environ = {'CONFIGGLUE_FOO_BAR': '42', 'CONFIGGLUE_BAZ': 3}
@@ -510,7 +533,7 @@
 
         configglue(Schema, [], validate=True)
 
-        # validation was not invoked
+        # validation was invoked
         self.assertEqual(mock_is_valid.called, True)
 
     @patch('configglue.parser.SchemaConfigParser.is_valid')