Merge lp:~allenap/launchpad/config-dir into lp:launchpad

[Gavin Panella]

[r=adeuring][no-qa] Add support for dir() and iter() to CanonicalConfig.

This adds support for dir() and iter() to CanonicalConfig. This means that it's easier, during development, to see what configuration keys are set, and to drill down. The dir() support is especially useful because it means that tab completion will work on configuration keys.

[Abel Deuring (adeuring)]

Look good. Just one suggestion:

+ def test_iter(self):
+ # iter(config) returns an iterator of config sections.
+ config = canonical.config.config
+ self.assertEqual(set(config._config), set(config))

The comment included, the word "config" appears eight times ;) Makes it a bit hard to understand what exact the test does. What about

sections = set(config._config)
self.assertEqual(sections, set(config))

review: Approve

=== modified file 'lib/canonical/config/__init__.py'
--- lib/canonical/config/__init__.py	2011-02-25 17:28:21 +0000
+++ lib/canonical/config/__init__.py	2011-09-16 14:33:25 +0000
@@ -13,19 +13,20 @@
 __metaclass__ = type
 
 
+import logging
 import os
-import logging
 import sys
-from urlparse import urlparse, urlunparse
+from urlparse import (
+    urlparse,
+    urlunparse,
+    )
 
+from lazr.config import ImplicitTypeSchema
+from lazr.config.interfaces import ConfigErrors
 import pkg_resources
 import ZConfig
 
-from lazr.config import ImplicitTypeSchema
-from lazr.config.interfaces import ConfigErrors
-
 from canonical.launchpad.readonly import is_read_only
-
 from lp.services.osutils import open_for_writing
 
 
@@ -259,7 +260,7 @@
         if not ensureSlash:
             return root_url.rstrip('/')
         if not root_url.endswith('/'):
-            return root_url+'/'
+            return root_url + '/'
         return root_url
 
     def __getattr__(self, name):
@@ -278,6 +279,19 @@
         self._getConfig()
         return self._config[key]
 
+    def __dir__(self):
+        """List section names in addition to methods and variables."""
+        self._getConfig()
+        names = dir(self.__class__)
+        names.extend(self.__dict__)
+        names.extend(section.name for section in self._config)
+        return names
+
+    def __iter__(self):
+        """Iterate through configuration sections."""
+        self._getConfig()
+        return iter(self._config)
+
 
 config = CanonicalConfig()
 
@@ -342,11 +356,11 @@
     value = url(value)
     scheme, location, path, parameters, query, fragment = urlparse(value)
     if parameters:
-        raise ValueError, 'URL parameters not allowed'
+        raise ValueError('URL parameters not allowed')
     if query:
-        raise ValueError, 'URL query not allowed'
+        raise ValueError('URL query not allowed')
     if fragment:
-        raise ValueError, 'URL fragments not allowed'
+        raise ValueError('URL fragments not allowed')
     if not value.endswith('/'):
         value = value + '/'
     return value
 
=== modified file 'lib/canonical/config/tests/test_config.py'
--- lib/canonical/config/tests/test_config.py	2011-03-07 23:54:41 +0000
+++ lib/canonical/config/tests/test_config.py	2011-09-16 14:33:25 +0000
@@ -9,13 +9,17 @@
 
 __metaclass__ = type
 
-from doctest import DocTestSuite, NORMALIZE_WHITESPACE, ELLIPSIS
+from doctest import (
+    DocTestSuite,
+    ELLIPSIS,
+    NORMALIZE_WHITESPACE,
+    )
 import os
-import pkg_resources
 import unittest
 
 from lazr.config import ConfigSchema
 from lazr.config.interfaces import ConfigErrors
+import pkg_resources
 import ZConfig
 
 import canonical.config
@@ -34,8 +38,11 @@
 def make_test(config_file, description):
     def test_function():
         root, handlers = ZConfig.loadConfig(schema, config_file)
+    # Hack the config file name into test_function's __name__ so that the test
+    # -vv output is more informative. Unfortunately, FunctionTestCase's
+    # description argument doesn't do what we want.
     test_function.__name__ = description
-    return test_function
+    return unittest.FunctionTestCase(test_function)
 
 
 def make_config_test(config_file, description):
@@ -63,6 +70,24 @@
     return LAZRConfigTestCase
 
 
+class TestCanonicalConfig(unittest.TestCase):
+
+    def test_dir(self):
+        # dir(config) returns methods, variables and section names.
+        config = canonical.config.config
+        names = set(dir(config))
+        self.assertTrue(names.issuperset(dir(config.__class__)))
+        self.assertTrue(names.issuperset(config.__dict__))
+        section_names = set(section.name for section in config._config)
+        self.assertTrue(names.issuperset(section_names))
+
+    def test_iter(self):
+        # iter(config) returns an iterator of sections.
+        config = canonical.config.config
+        sections = set(config._config)
+        self.assertEqual(sections, set(config))
+
+
 def test_suite():
     """Return a suite of canonical.conf and all conf files."""
     # We know we are not using dirnames.
@@ -72,29 +97,27 @@
         'canonical.config',
         optionflags=NORMALIZE_WHITESPACE | ELLIPSIS,
         ))
+    load_testcase = unittest.defaultTestLoader.loadTestsFromTestCase
     # Add a test for every launchpad[.lazr].conf file in our tree.
     for config_dir in canonical.config.CONFIG_ROOT_DIRS:
-        prefix_len = len(os.path.dirname(config_dir)) + 1
         for dirpath, dirnames, filenames in os.walk(config_dir):
             if os.path.basename(dirpath) in EXCLUDED_CONFIGS:
+                del dirnames[:]  # Don't look in subdirectories.
                 continue
             for filename in filenames:
                 if filename == 'launchpad.conf':
                     config_file = os.path.join(dirpath, filename)
-                    description = config_file[prefix_len:]
-                    # Hack the config file name into the test_function's
-                    # __name__ # so that the test -vv output is more
-                    # informative. Unfortunately, FunctionTestCase's
-                    # description argument doesn't do what we want.
-                    suite.addTest(unittest.FunctionTestCase(
-                        make_test(config_file, description)))
+                    description = os.path.relpath(config_file, config_dir)
+                    suite.addTest(make_test(config_file, description))
                 elif filename.endswith('-lazr.conf'):
                     # Test the lazr.config conf files.
                     config_file = os.path.join(dirpath, filename)
-                    description = config_file[prefix_len:]
-                    test = make_config_test(config_file, description)
-                    suite.addTest(test('testConfig'))
+                    description = os.path.relpath(config_file, config_dir)
+                    testcase = make_config_test(config_file, description)
+                    suite.addTest(load_testcase(testcase))
                 else:
                     # This file is not a config that can be validated.
                     pass
+    # Other tests.
+    suite.addTest(load_testcase(TestCanonicalConfig))
     return suite