[trac1004] add tests and additional fixes

also do a few fixes and address a couple of comments;
- made getRelatedLoggers public (so we can test it)
- use set instead of vector
- don't allow '*foo' etc.
- added tests
- added tests for the cfgmgr b10logging plugin

3 files changed, 152 insertions, 4 deletions

diff --git a/src/bin/cfgmgr/plugins/b10logging.py b/src/bin/cfgmgr/plugins/b10logging.py
index 6af3f66655..e288c6d3c0 100644
--- a/src/bin/cfgmgr/plugins/b10logging.py
+++ b/src/bin/cfgmgr/plugins/b10logging.py
@@ -48,6 +48,19 @@ def check(config):
         for logger in config['loggers']:
             # name should always be present
             name = logger['name']
+            # report an error if name starts with * but not *.,
+            # or if * is not the first character.
+            # TODO: we might want to also warn or error if the
+            # logger name is not an existing module, but we can't
+            # really tell that from here at this point
+            star_pos = name.find('*')
+            if star_pos > 0 or\
+               name == '*.' or\
+               (star_pos == 0 and len(name) > 1 and name[1] != '.'):
+                errors.append("Bad logger name: '" + name + "': * can "
+                              "only be used instead of the full "
+                              "first-level name, e.g. '*' or "
+                              "'*.subsystem'")
 
             if 'severity' in logger and\
                logger['severity'].lower() not in ALLOWED_SEVERITIES:
@@ -71,11 +84,11 @@ def check(config):
                                'output' in output_option and\
                                output_option['output'] not in ALLOWED_STREAMS:
                                 errors.append("bad output for logger " + name +
-                                              ": " + output_option['stream'] +
+                                              ": " + output_option['output'] +
                                               ", must be stdout or stderr")
                             elif destination == "file" and\
-                                 'output' not in output_option or\
-                                 output_option['output'] == "":
+                                 ('output' not in output_option or\
+                                  output_option['output'] == ""):
                                     errors.append("destination set to file but "
                                                   "output not set to any "
                                                   "filename for logger "
diff --git a/src/bin/cfgmgr/plugins/tests/Makefile.am b/src/bin/cfgmgr/plugins/tests/Makefile.am
index 725d391968..07b7a858c2 100644
--- a/src/bin/cfgmgr/plugins/tests/Makefile.am
+++ b/src/bin/cfgmgr/plugins/tests/Makefile.am
@@ -1,5 +1,5 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
-PYTESTS = tsig_keys_test.py
+PYTESTS = tsig_keys_test.py logging_test.py
 
 EXTRA_DIST = $(PYTESTS)
 
diff --git a/src/bin/cfgmgr/plugins/tests/logging_test.py b/src/bin/cfgmgr/plugins/tests/logging_test.py
new file mode 100644
index 0000000000..818a5964a7
--- /dev/null
+++ b/src/bin/cfgmgr/plugins/tests/logging_test.py
@@ -0,0 +1,135 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+# Make sure we can load the module, put it into path
+import sys
+import os
+sys.path.extend(os.environ["B10_TEST_PLUGIN_DIR"].split(':'))
+
+import b10logging
+import unittest
+
+class LoggingConfCheckTest(unittest.TestCase):
+    def test_load(self):
+        """
+        Checks the entry point returns the correct values.
+        """
+        (spec, check) = b10logging.load()
+        # It returns the checking function
+        self.assertEqual(check, b10logging.check)
+        # The plugin stores it's spec
+        self.assertEqual(spec, b10logging.spec)
+
+    def test_logger_conf(self):
+        self.assertEqual(None,
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'DEBUG',
+                                            'debuglevel': 50,
+                                            'output_options':
+                                            [{'destination': 'file',
+                                              'output': '/some/file'
+                                            }]
+                                           },
+                                           {'name': 'b10-resolver',
+                                            'severity': 'WARN',
+                                            'additive': True,
+                                            'output_options':
+                                            [{'destination': 'console',
+                                              'output': 'stderr',
+                                              'flush': True
+                                            }]
+                                           },
+                                           {'name': 'b10-resolver.resolver',
+                                            'severity': 'ERROR',
+                                            'output_options': []
+                                           },
+                                           {'name': '*.cache',
+                                            'severity': 'INFO'
+                                           }
+                                          ]}))
+    def do_bad_name_test(self, name):
+        err_str = "Bad logger name: '" + name + "': * can only be "\
+                  "used instead of the full first-level name, e.g. "\
+                  "'*' or '*.subsystem'"
+        self.assertEqual(err_str,
+                         b10logging.check({'loggers':
+                                          [{'name': name,
+                                            'severity': 'DEBUG'},
+                                          ]}))
+        
+    def test_logger_bad_name(self):
+        self.do_bad_name_test("*.")
+        self.do_bad_name_test("*foo")
+        self.do_bad_name_test("*foo.lib")
+        self.do_bad_name_test("foo*")
+        self.do_bad_name_test("foo*.lib")
+
+    def test_logger_bad_severity(self):
+        self.assertEqual('bad severity value for logger *: BADVAL',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'BADVAL'}]}))
+
+    def test_logger_bad_destination(self):
+        self.assertEqual('bad destination for logger *: baddest',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'baddest' }
+                                            ]}]}))
+
+    def test_logger_bad_console_output(self):
+        self.assertEqual('bad output for logger *: bad_output, must be stdout or stderr',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'console',
+                                              'output': 'bad_output'
+                                            }
+                                            ]}]}))
+
+    def test_logger_bad_file_output(self):
+        self.assertEqual('destination set to file but output not set to any filename for logger *',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'file' }
+                                            ]}]}))
+
+    def test_logger_bad_syslog_output(self):
+        self.assertEqual('destination set to syslog but output not set to any facility for logger *',
+                         b10logging.check({'loggers':
+                                          [{'name': '*',
+                                            'severity': 'INFO',
+                                            'output_options': [
+                                            { 'destination': 'syslog' }
+                                            ]}]}))
+
+    def test_logger_bad_type(self):
+        self.assertEqual('123 should be a string',
+                         b10logging.check({'loggers':
+                                          [{'name': 123,
+                                            'severity': 'INFO'}]}))
+        self.assertEqual('123 should be a string',
+                         b10logging.check({'loggers':
+                                          [{'name': 'bind10',
+                                            'severity': 123}]}))
+
+if __name__ == '__main__':
+        unittest.main()