Merge lp:~allenap/maas/test-bling into lp:maas/trunk

[Gavin Panella]

This brings testresources and testtools into maas, creating a base class from these. It also adds some templates to use when creating a new module or test.

[lp:~allenap/maas/test-bling]

28. By Gavin Panella on 2012-01-18

Merge trunk, resolving 1 conflict.

29. By Gavin Panella on 2012-01-18

Use templates/test.py and maas.testing.TestCase in test_macaddressfield.

30. By Gavin Panella on 2012-01-18

Fix lint.

[Raphaël Badin (rvb)]

Looks good. I confess I'm no test expert so I'm happy if the testsuite runs when I launch /bin/test. But I'm not sure about [2].

[1]

62 # Use our custom test runner, which makes sure that a local database
63 # cluster is running in the branch.
64 -TEST_RUNNER='maas.testing.runner.CustomTestRunner'
65 +TEST_RUNNER='maas.testing.runner.TestRunner'

Why the rename? It is indeed a custom runner isn't it?

[2]

339 +from maas.testing import TestCase
340 +
341 +
342 +class TestSomething(TestCase):

This is in the template, but you have not updated the existing tests. Why is that?

Also, I sense this is a problem because this will require maasserver's tests to import from maas.testing. This is not a no-go but maas is a simple app that should be independent of the containing project.

What do you think?

[3]

Maybe we should add the copyright headers in a separate branch on all the files at once.

[4]

222 -from django.test import TestCase
223 -
224 -from maasserver.models import MACAddress, Node
225 +from maas.testing import TestCase
226 from maasserver.macaddress import validate_mac
227 +from maasserver.models import (
228 + MACAddress,
229 + Node,
230 + )

How about stealing the lp code to do that if the script is not huge? I really don't want to do that manually each time add an import.

review: Needs Information

[Gavin Panella (allenap)]

Thanks for the great review :)

> [1]
>
> 62 # Use our custom test runner, which makes sure that a local database
> 63 # cluster is running in the branch.
> 64 -TEST_RUNNER='maas.testing.runner.CustomTestRunner'
> 65 +TEST_RUNNER='maas.testing.runner.TestRunner'
>
> Why the rename? It is indeed a custom runner isn't it?

Yes, but so is anything that's a subclass. I first changed it to
MaaSTestRunner then realised that it was already in the maas namespace
so dropped the prefix. Perhaps I should have just left it alone right
from the start :)

> [2]
>
> 339 +from maas.testing import TestCase
> 340 +
> 341 +
> 342 +class TestSomething(TestCase):
>
> This is in the template, but you have not updated the existing
> tests. Why is that?

I think I have...?

> Also, I sense this is a problem because this will require
> maasserver's tests to import from maas.testing. This is not a no-go
> but maas is a simple app that should be independent of the
> containing project.

You have a good point. I thought about this a bit...

 * Move preexisting stuff in maas to maas.web
 * Move maasserver to maas.api
 * Leaves room for maas.metadata, etc.
 * maas.testing remains where it is, available for all.

(In other words, .api and .metadata are applications used by .web.)

> [3]
>
> Maybe we should add the copyright headers in a separate branch on
> all the files at once.

Ah, perhaps, but we can get there one bit at a time too :)

> [4]
>
> 222 -from django.test import TestCase
> 223 -
> 224 -from maasserver.models import MACAddress, Node
> 225 +from maas.testing import TestCase
> 226 from maasserver.macaddress import validate_mac
> 227 +from maasserver.models import (
> 228 + MACAddress,
> 229 + Node,
> 230 + )
>
> How about stealing the lp code to do that if the script is not huge?
> I really don't want to do that manually each time add an import.

I have been thinking about packaging that up for a while. I will steal
it for now, but we really ought to get it out there. Fwiw, it can be
run from the LP tree without fuss; it operates on the working
directory, not on the tree in which it resides.

[lp:~allenap/maas/test-bling]

31. By Gavin Panella on 2012-01-18

Borrow format-imports from Launchpad.

32. By Gavin Panella on 2012-01-18

Update python_standard_libs for 2.7.

33. By Gavin Panella on 2012-01-18

Also run pep8.

34. By Gavin Panella on 2012-01-18

Fix lint.

35. By Gavin Panella on 2012-01-18

Merge trunk.

36. By Gavin Panella on 2012-01-19

Merge trunk, resolving several conflicts.

37. By Gavin Panella on 2012-01-19

Add empty __init__.py to maasserver.testing, and up-call in NodeTest.setUp().

38. By Gavin Panella on 2012-01-19

Options for tags/TAGS generation.

=== added file '.ctags'
--- .ctags	1970-01-01 00:00:00 +0000
+++ .ctags	2012-01-19 10:35:31 +0000
@@ -0,0 +1,4 @@
+--python-kinds=-iv
+--exclude=*-min.js
+--exclude=*-debug.js
+--extra=+f

=== modified file 'Makefile'
--- Makefile	2012-01-18 17:05:52 +0000
+++ Makefile	2012-01-19 10:35:31 +0000
@@ -14,8 +14,9 @@
 test: bin/test
 	bin/test
 
+lint: sources = setup.py src templates utilities
 lint:
-	pyflakes src
+	@(pyflakes $(sources); pep8 --repeat $(sources)) | sort -g
 
 check: clean test
 

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-01-17 16:03:30 +0000
+++ buildout.cfg	2012-01-19 10:35:31 +0000
@@ -24,9 +24,12 @@
   django
   django-debug-toolbar
   django-piston
+  fixtures
   psycopg2
   rabbitfixture
   South
+  testresources
+  testtools
 project = maas
 projectegg = maas
 test = maasserver

=== modified file 'setup.py'
--- setup.py	2012-01-18 17:41:01 +0000
+++ setup.py	2012-01-19 10:35:31 +0000
@@ -4,11 +4,16 @@
 
 """Distutils installer for maas."""
 
+import os
+
 import distribute_setup
 distribute_setup.use_setuptools()
-import os
-
-from setuptools import setup, find_packages
+
+from setuptools import (
+    find_packages,
+    setup,
+    )
+
 
 def read(fname):
     return open(os.path.join(os.path.dirname(__file__), fname)).read().strip()
@@ -21,17 +26,17 @@
     url="https://launchpad.net/maas",
     license="GPL",
     description="Metal as as Service",
-    long_description = read('README.txt'),
+    long_description=read('README.txt'),
 
     author="MaaS Developers",
     author_email="[e-mail]",
 
-    packages = find_packages('src'),
-    package_dir = {'': 'src'},
-
-    install_requires = ['setuptools'],
-
-    classifiers = [
+    packages=find_packages('src'),
+    package_dir={'': 'src'},
+
+    install_requires=['setuptools'],
+
+    classifiers=[
         'Development Status :: 4 - Beta',
         'Framework :: Django',
         'Intended Audience :: Developers',

=== modified file 'src/maas/development.py'
--- src/maas/development.py	2012-01-17 16:45:13 +0000
+++ src/maas/development.py	2012-01-19 10:35:31 +0000
@@ -1,13 +1,13 @@
 
 # Django development settings for maas project.
 
+import os
+
 from maas.settings import *
 
-import os
-
 # Use our custom test runner, which makes sure that a local database
 # cluster is running in the branch.
-TEST_RUNNER='maas.testing.runner.CustomTestRunner'
+TEST_RUNNER = 'maas.testing.runner.TestRunner'
 
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
@@ -117,8 +117,8 @@
 ROOT_URLCONF = 'maas.urls'
 
 TEMPLATE_DIRS = (
-    # Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
-    # Always use forward slashes, even on Windows.
+    # Put strings here, like "/home/html/django_templates" or
+    # "C:/www/django/templates". Always use forward slashes, even on Windows.
     # Don't forget to use absolute paths, not relative paths.
     os.path.join(os.path.dirname(__file__), "templates"),
 )
@@ -150,9 +150,9 @@
     'version': 1,
     'disable_existing_loggers': False,
     'handlers': {
-        'console':{
-            'level':'DEBUG',
-            'class':'logging.StreamHandler',
+        'console': {
+            'level': 'DEBUG',
+            'class': 'logging.StreamHandler',
         }
     },
     'loggers': {

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-01-16 17:03:28 +0000
+++ src/maas/settings.py	2012-01-19 10:35:31 +0000
@@ -3,6 +3,7 @@
 
 import os
 
+
 ADMINS = (
     # ('Your Name', '[e-mail]'),
 )
@@ -123,8 +124,8 @@
 ROOT_URLCONF = 'maas.urls'
 
 TEMPLATE_DIRS = (
-    # Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
-    # Always use forward slashes, even on Windows.
+    # Put strings here, like "/home/html/django_templates" or
+    # "C:/www/django/templates". Always use forward slashes, even on Windows.
     # Don't forget to use absolute paths, not relative paths.
     os.path.join(os.path.dirname(__file__), "templates"),
 )

=== modified file 'src/maas/testing/__init__.py'
--- src/maas/testing/__init__.py	2012-01-17 16:45:13 +0000
+++ src/maas/testing/__init__.py	2012-01-19 10:35:31 +0000
@@ -0,0 +1,37 @@
+__all__ = []
+__metaclass__ = type
+
+import django.test
+import testresources
+import testtools
+
+
+class TestCase(testtools.TestCase, django.test.TestCase):
+    """`TestCase` for Metal as a Service.
+
+    Supports test resources and fixtures.
+    """
+
+    # testresources.ResourcedTestCase does something similar to this class
+    # (with respect to setUpResources and tearDownResources) but it explicitly
+    # up-calls to unittest.TestCase instead of using super() even though it is
+    # not guaranteed that the next class in the inheritance chain is
+    # unittest.TestCase.
+
+    resources = ()
+
+    def setUp(self):
+        super(TestCase, self).setUp()
+        self.setUpResources()
+
+    def setUpResources(self):
+        testresources.setUpResources(
+            self, self.resources, testresources._get_result())
+
+    def tearDown(self):
+        self.tearDownResources()
+        super(TestCase, self).tearDown()
+
+    def tearDownResources(self):
+        testresources.tearDownResources(
+            self, self.resources, testresources._get_result())

=== modified file 'src/maas/testing/runner.py'
--- src/maas/testing/runner.py	2012-01-18 14:39:59 +0000
+++ src/maas/testing/runner.py	2012-01-19 10:35:31 +0000
@@ -1,11 +1,18 @@
 from subprocess import check_call
+
 from django.test.simple import DjangoTestSuiteRunner
-
-
-class CustomTestRunner(DjangoTestSuiteRunner):
+from testresources import OptimisingTestSuite
+
+
+class TestRunner(DjangoTestSuiteRunner):
     """Custom test runner; ensures that the test database cluster is up."""
 
+    def build_suite(self, test_labels, extra_tests=None, **kwargs):
+        suite = super(TestRunner, self).build_suite(
+            test_labels, extra_tests, **kwargs)
+        return OptimisingTestSuite(suite)
+
     def setup_databases(self, *args, **kwargs):
         """Fire up the db cluster, then punt to original implementation."""
         check_call(['bin/maasdb', 'start', './db/', 'disposable'])
-        return super(CustomTestRunner, self).setup_databases(*args, **kwargs)
+        return super(TestRunner, self).setup_databases(*args, **kwargs)

=== modified file 'src/maas/urls.py'
--- src/maas/urls.py	2012-01-16 16:15:44 +0000
+++ src/maas/urls.py	2012-01-19 10:35:31 +0000
@@ -1,6 +1,7 @@
 
+from django.conf import settings
 from django.conf.urls.defaults import *
-from django.conf import settings
+
 
 urlpatterns = patterns('',
     url(r'^', include('maasserver.urls')),

=== modified file 'src/maasserver/macaddress.py'
--- src/maasserver/macaddress.py	2012-01-18 14:27:42 +0000
+++ src/maasserver/macaddress.py	2012-01-19 10:35:31 +0000
@@ -8,8 +8,8 @@
 mac_re = re.compile(r'^([0-9a-fA-F]{2}[:-]){5}[0-9a-fA-F]{2}$')
 
 validate_mac = RegexValidator(
-    regex = mac_re,
-    message = u"Enter a valid MAC address (e.g. AA:BB:CC:DD:EE:FF).")
+    regex=mac_re,
+    message=u"Enter a valid MAC address (e.g. AA:BB:CC:DD:EE:FF).")
 
 
 class MACAddressField(Field):

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-01-18 19:16:57 +0000
+++ src/maasserver/models.py	2012-01-19 10:35:31 +0000
@@ -2,9 +2,8 @@
 import re
 from uuid import uuid1
 
+from django.contrib import admin
 from django.db import models
-from django.contrib import admin
-
 from maasserver.macaddress import MACAddressField
 
 

=== added file 'src/maasserver/testing/__init__.py'
=== modified file 'src/maasserver/tests/__init__.py'
--- src/maasserver/tests/__init__.py	2012-01-17 15:35:41 +0000
+++ src/maasserver/tests/__init__.py	2012-01-19 10:35:31 +0000
@@ -1,12 +1,10 @@
-import os
-import unittest
-
-_test_suite = unittest.TestSuite()
-for module in os.listdir(os.path.dirname(__file__)):
-    if module == '__init__.py' or module[-3:] != '.py':
-        continue
-    mod = __import__(module[:-3], locals(), globals())
-    _test_suite.addTests(unittest.TestLoader().loadTestsFromModule(mod))
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from os.path import dirname
+
+from django.utils.unittest import defaultTestLoader
+
 
 def suite():
-    return _test_suite
+    return defaultTestLoader.discover(dirname(__file__))

=== modified file 'src/maasserver/tests/test_macaddressfield.py'
--- src/maasserver/tests/test_macaddressfield.py	2012-01-18 19:16:57 +0000
+++ src/maasserver/tests/test_macaddressfield.py	2012-01-19 10:35:31 +0000
@@ -1,10 +1,15 @@
-"""
-Test MACAddressField.
-"""
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test MACAddressField."""
+
+from __future__ import print_function
+
+__metaclass__ = type
+__all__ = []
 
 from django.core.exceptions import ValidationError
-from django.test import TestCase
-
+from maas.testing import TestCase
 from maasserver.models import MACAddress
 from maasserver.macaddress import validate_mac
 from maasserver.testing.factory import factory

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-01-18 19:16:57 +0000
+++ src/maasserver/tests/test_models.py	2012-01-19 10:35:31 +0000
@@ -1,16 +1,25 @@
-"""
-Test maasserver models.
-"""
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test maasserver models."""
+
+from __future__ import print_function
+
+__metaclass__ = type
+__all__ = []
 
 from django.core.exceptions import ValidationError
-from django.test import TestCase
-
-from maasserver.models import Node, MACAddress
+from maas.testing import TestCase
+from maasserver.models import (
+    MACAddress,
+    Node,
+    )
 
 
 class NodeTest(TestCase):
 
     def setUp(self):
+        super(NodeTest, self).setUp()
         self.node = Node()
         self.node.save()
 

=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py	2012-01-18 17:10:34 +0000
+++ src/maasserver/urls.py	2012-01-19 10:35:31 +0000
@@ -3,7 +3,10 @@
 from django.views.generic import ListView
 from piston.resource import Resource
 from maasserver.models import Node
-from maasserver.views import NodeView, NodesCreateView
+from maasserver.views import (
+    NodesCreateView,
+    NodeView,
+    )
 from maasserver.api import NodeHandler, NodeMacsHandler
 
 

=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py	2012-01-16 08:33:18 +0000
+++ src/maasserver/views.py	2012-01-19 10:35:31 +0000
@@ -1,7 +1,8 @@
-from django.http import HttpResponse
-from django.views.generic import ListView, CreateView
 from django.shortcuts import get_object_or_404
-
+from django.views.generic import (
+    CreateView,
+    ListView,
+    )
 from maasserver.models import Node
 
 

=== added directory 'templates'
=== added file 'templates/README'
--- templates/README	1970-01-01 00:00:00 +0000
+++ templates/README	2012-01-19 10:35:31 +0000
@@ -0,0 +1,3 @@
+This directory contains templates for files that may need to be added
+to this project, like HTML pages, Python modules, Python tests, and so
+on.

=== added file 'templates/module.py'
--- templates/module.py	1970-01-01 00:00:00 +0000
+++ templates/module.py	2012-01-19 10:35:31 +0000
@@ -0,0 +1,9 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""..."""
+
+from __future__ import print_function
+
+__metaclass__ = type
+__all__ = []

=== added file 'templates/test.py'
--- templates/test.py	1970-01-01 00:00:00 +0000
+++ templates/test.py	2012-01-19 10:35:31 +0000
@@ -0,0 +1,19 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""..."""
+
+from __future__ import print_function
+
+__metaclass__ = type
+__all__ = []
+
+from maas.testing import TestCase
+
+
+class TestSomething(TestCase):
+
+    #resources = [...]
+
+    def test_something(self):
+        self.assertTrue(1)

=== added directory 'utilities'
=== added file 'utilities/format-imports'
--- utilities/format-imports	1970-01-01 00:00:00 +0000
+++ utilities/format-imports	2012-01-19 10:35:31 +0000
@@ -0,0 +1,406 @@
+#!/usr/bin/python
+#
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+""" Format import sections in python files
+
+= Usage =
+
+format-imports <file or directory> ...
+
+= Operation =
+
+The script will process each filename on the command line. If the file is a
+directory it recurses into it an process all *.py files found in the tree.
+It will output the paths of all the files that have been changed.
+
+For Launchpad it was applied to the "lib/canonical/launchpad" and the "lib/lp"
+subtrees. Running it with those parameters on a freshly branched LP tree
+should not produce any output, meaning that all the files in the tree should
+be formatted correctly.
+
+The script identifies the import section of each file as a block of lines
+that start with "import" or "from" or are indented with at least one space or
+are blank lines. Comment lines are also included if they are followed by an
+import statement. An inital __future__ import and a module docstring are
+explicitly skipped.
+
+The import section is rewritten as three subsections, each separated by a
+blank line. Any of the sections may be empty.
+ 1. Standard python library modules
+ 2. Import statements explicitly ordered to the top (see below)
+ 3. Third-party modules, meaning anything not fitting one of the other
+    subsection criteria
+ 4. Local modules that begin with "canonical" or "lp".
+
+Each section is sorted alphabetically by module name. Each module is put
+on its own line, i.e.
+{{{
+  import os, sys
+}}}
+becomes
+{{{
+  import os
+  import sys
+}}}
+Multiple import statements for the same module are conflated into one
+statement, or two if the module was imported alongside an object inside it,
+i.e.
+{{{
+  import sys
+  from sys import stdin
+}}}
+
+Statements that import more than one objects are put on multiple lines in
+list style, i.e.
+{{{
+  from sys import (
+      stdin,
+      stdout,
+      )
+}}}
+Objects are sorted alphabetically and case-insensitively. One-object imports
+are only formatted in this manner if the statement exceeds 78 characters in
+length.
+
+Comments stick with the import statement that followed them. Comments at the
+end of one-line statements are moved to be be in front of it, .i.e.
+{{{
+  from sys import exit # Have a way out
+}}}
+becomes
+{{{
+  # Have a way out
+  from sys import exit
+}}}
+
+= Format control =
+
+Two special comments allow to control the operation of the formatter.
+
+When an import statement is immediately preceded by a comment that starts
+with the word "FIRST", it is placed into the second subsection (see above).
+
+When the first import statement is directly preceded by a comment that starts
+with the word "SKIP", the entire file is exempt from formatting.
+
+= Known bugs =
+
+Make sure to always check the result of the re-formatting to see if you have
+been bitten by one of these.
+
+Comments inside multi-line import statements break the formatter. A statement
+like this will be ignored:
+{{{
+  from lp.app.interfaces import (
+      # Don't do this.
+      IMyInterface,
+      IMyOtherInterface, # Don't do this either
+      )
+}}}
+Actually, this will make the statement and all following to be ignored:
+{{{
+  from lp.app.interfaces import (
+  # Breaks indentation rules anyway.
+      IMyInterface,
+      IMyOtherInterface,
+      )
+}}}
+
+If a single-line statement has both a comment in front of it and at the end
+of the line, only the end-line comment will survive. This could probably
+easily be fixed to concatenate the too.
+{{{
+  # I am a gonner.
+  from lp.app.interfaces import IMyInterface # I will survive!
+}}}
+
+Line continuation characters are recognized and resolved but
+not re-introduced. This may leave the re-formatted text with a line that
+is over the length limit.
+{{{
+    from lp.app.verylongnames.orverlydeep.modulestructure.leavenoroom \
+        import object
+}}}
+"""
+
+__metaclass__ = type
+
+# SKIP this file when reformatting.
+import os
+import re
+import sys
+from textwrap import dedent
+
+sys.path[0:0] = [os.path.dirname(__file__)]
+from python_standard_libs import python_standard_libs
+
+# python_standard_libs is only used for membership tests.
+python_standard_libs = frozenset(python_standard_libs)
+
+# To search for escaped newline chars.
+escaped_nl_regex = re.compile("\\\\\n", re.M)
+import_regex = re.compile("^import +(?P<module>.+)$", re.M)
+from_import_single_regex = re.compile(
+    "^from (?P<module>.+) +import +"
+    "(?P<objects>[*]|[a-zA-Z0-9_, ]+)"
+    "(?P<comment>#.*)?$", re.M)
+from_import_multi_regex = re.compile(
+    "^from +(?P<module>.+) +import *[(](?P<objects>[a-zA-Z0-9_, \n]+)[)]$",
+    re.M)
+comment_regex = re.compile(
+    "(?P<comment>(^#.+\n)+)(^import|^from) +(?P<module>[a-zA-Z0-9_.]+)", re.M)
+split_regex = re.compile(",\s*")
+module_base_regex = re.compile("([^. ]+)")
+
+# Module docstrings are multiline (""") strings that are not indented and are
+# followed at some point by an import .
+module_docstring_regex = re.compile(
+    '(?P<docstring>^["]{3}[^"]+["]{3}\n).*^(import |from .+ import)',
+    re.M | re.S)
+# The imports section starts with an import state that is not a __future__
+# import and consists of import lines, indented lines, empty lines and
+# comments which are followed by an import line. Sometimes we even find
+# lines that contain a single ")"... :-(
+imports_section_regex = re.compile(
+    "(^#.+\n)*^(import|(from ((?!__future__)\S+) import)).*\n"
+    "(^import .+\n|^from .+\n|^[\t ]+.+\n|(^#.+\n)+((^import|^from) "
+    ".+\n)|^\n|^[)]\n)*",
+    re.M)
+
+
+def format_import_lines(module, objects):
+    """Generate correct from...import strings."""
+    if len(objects) == 1:
+        statement = "from %s import %s" % (module, objects[0])
+        if len(statement) < 79:
+            return statement
+    return "from %s import (\n    %s,\n    )" % (
+        module, ",\n    ".join(objects))
+
+
+def find_imports_section(content):
+    """Return that part of the file that contains the import statements."""
+    # Skip module docstring.
+    match = module_docstring_regex.search(content)
+    if match is None:
+        startpos = 0
+    else:
+        startpos = match.end('docstring')
+
+    match = imports_section_regex.search(content, startpos)
+    if match is None:
+        return (None, None)
+    startpos = match.start()
+    endpos = match.end()
+    if content[startpos:endpos].startswith('# SKIP'):
+        # Skip files explicitely.
+        return(None, None)
+    return (startpos, endpos)
+
+
+class ImportStatement:
+    """Holds information about an import statement."""
+
+    def __init__(self, objects=None, comment=None):
+        self.import_module = objects is None
+        if objects is None:
+            self.objects = None
+        else:
+            self.objects = sorted(objects, key=str.lower)
+        self.comment = comment
+
+    def addObjects(self, new_objects):
+        """More objects in this statement; eliminate duplicates."""
+        if self.objects is None:
+            # No objects so far.
+            self.objects = new_objects
+        else:
+            # Use set to eliminate double objects.
+            more_objects = set(self.objects + new_objects)
+            self.objects = sorted(list(more_objects), key=str.lower)
+
+    def setComment(self, comment):
+        """Add a comment to the statement."""
+        self.comment = comment
+
+
+def parse_import_statements(import_section):
+    """Split the import section into statements.
+
+    Returns a dictionary with the module as the key and the objects being
+    imported as a sorted list of strings."""
+    imports = {}
+    # Search for escaped newlines and remove them.
+    searchpos = 0
+    while True:
+        match = escaped_nl_regex.search(import_section, searchpos)
+        if match is None:
+            break
+        start = match.start()
+        end = match.end()
+        import_section = import_section[:start] + import_section[end:]
+        searchpos = start
+    # Search for simple one-line import statements.
+    searchpos = 0
+    while True:
+        match = import_regex.search(import_section, searchpos)
+        if match is None:
+            break
+        # These imports are marked by a "None" value.
+        # Multiple modules in one statement are split up.
+        for module in split_regex.split(match.group('module').strip()):
+            imports[module] = ImportStatement()
+        searchpos = match.end()
+    # Search for "from ... import" statements.
+    for pattern in (from_import_single_regex, from_import_multi_regex):
+        searchpos = 0
+        while True:
+            match = pattern.search(import_section, searchpos)
+            if match is None:
+                break
+            import_objects = split_regex.split(
+                match.group('objects').strip(" \n,"))
+            module = match.group('module').strip()
+            # Only one pattern has a 'comment' group.
+            comment = match.groupdict().get('comment', None)
+            if module in imports:
+                # Catch double import lines.
+                imports[module].addObjects(import_objects)
+            else:
+                imports[module] = ImportStatement(import_objects)
+            if comment is not None:
+                imports[module].setComment(comment)
+            searchpos = match.end()
+    # Search for comments in import section.
+    searchpos = 0
+    while True:
+        match = comment_regex.search(import_section, searchpos)
+        if match is None:
+            break
+        module = match.group('module').strip()
+        comment = match.group('comment').strip()
+        imports[module].setComment(comment)
+        searchpos = match.end()
+
+    return imports
+
+LOCAL_PACKAGES = (
+    'canonical', 'lp', 'launchpad_loggerhead', 'devscripts',
+    # database/* have some implicit relative imports.
+    'fti', 'replication', 'preflight', 'security', 'upgrade',
+    )
+
+def format_imports(imports):
+    """Group and order imports, return the new import statements."""
+    early_section = {}
+    standard_section = {}
+    first_section = {}
+    thirdparty_section = {}
+    local_section = {}
+    # Group modules into sections.
+    for module, statement in imports.iteritems():
+        module_base = module_base_regex.findall(module)[0]
+        comment = statement.comment
+        if module_base == '_pythonpath':
+            early_section[module] = statement
+        elif comment is not None and comment.startswith("# FIRST"):
+            first_section[module] = statement
+        elif module_base in LOCAL_PACKAGES:
+            local_section[module] = statement
+        elif module_base in python_standard_libs:
+            standard_section[module] = statement
+        else:
+            thirdparty_section[module] = statement
+
+    all_import_lines = []
+    # Sort within each section and generate statement strings.
+    sections = (
+        early_section,
+        standard_section,
+        first_section,
+        thirdparty_section,
+        local_section,
+        )
+    for section in sections:
+        import_lines = []
+        for module in sorted(section.keys(), key=str.lower):
+            if section[module].comment is not None:
+                import_lines.append(section[module].comment)
+            if section[module].import_module:
+                import_lines.append("import %s" % module)
+            if section[module].objects is not None:
+                import_lines.append(
+                    format_import_lines(module, section[module].objects))
+        if len(import_lines) > 0:
+            all_import_lines.append('\n'.join(import_lines))
+    # Sections are separated by two blank lines.
+    return '\n\n'.join(all_import_lines)
+
+
+def reformat_importsection(filename):
+    """Replace the given file with a reformatted version of it."""
+    pyfile = file(filename).read()
+    import_start, import_end = find_imports_section(pyfile)
+    if import_start is None:
+        # Skip files with no import section.
+        return False
+    imports_section = pyfile[import_start:import_end]
+    imports = parse_import_statements(imports_section)
+
+    next_char = pyfile[import_end:import_end + 1]
+
+    if next_char == '':
+        number_of_newlines = 1
+    elif next_char != '#':
+        # Two newlines before anything but comments.
+        number_of_newlines = 3
+    else:
+        number_of_newlines = 2
+
+    new_imports = format_imports(imports) + ("\n" * number_of_newlines)
+    if new_imports == imports_section:
+        # No change, no need to write a new file.
+        return False
+
+    new_file = open(filename, "w")
+    new_file.write(pyfile[:import_start])
+    new_file.write(new_imports)
+    new_file.write(pyfile[import_end:])
+
+    return True
+
+
+def process_file(fpath):
+    """Process the file with the given path."""
+    changed = reformat_importsection(fpath)
+    if changed:
+        print fpath
+
+
+def process_tree(dpath):
+    """Walk a directory tree and process all *.py files."""
+    for dirpath, dirnames, filenames in os.walk(dpath):
+        for filename in filenames:
+            if filename.endswith('.py'):
+                process_file(os.path.join(dirpath, filename))
+
+
+if __name__ == "__main__":
+    if len(sys.argv) == 1 or sys.argv[1] in ("-h", "-?", "--help"):
+        sys.stderr.write(dedent("""\
+        usage: format-imports <file or directory> ...
+
+        Type "format-imports --docstring | less" to see the documentation.
+        """))
+        sys.exit(1)
+    if sys.argv[1] == "--docstring":
+        sys.stdout.write(__doc__)
+        sys.exit(2)
+    for filename in sys.argv[1:]:
+        if os.path.isdir(filename):
+            process_tree(filename)
+        else:
+            process_file(filename)
+    sys.exit(0)

=== added file 'utilities/format-new-and-modified-imports'
--- utilities/format-new-and-modified-imports	1970-01-01 00:00:00 +0000
+++ utilities/format-new-and-modified-imports	2012-01-19 10:35:31 +0000
@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+#
+# Reformat imports in new and modified files. Call without arguments
+# to operate on uncommitted changes. Arguments will be passed to bzr
+# status, so to operate on all new and modified files relative to the
+# submit branch, use:
+#
+#   format-new-and-modified-imports -r submit:
+#
+
+bzr status --short "$@" | \
+    awk '/^.[MN] .*[.]py$/ { print $NF }' | \
+    xargs -r "$(dirname "$0")/format-imports"

=== added file 'utilities/python_standard_libs.py'
--- utilities/python_standard_libs.py	1970-01-01 00:00:00 +0000
+++ utilities/python_standard_libs.py	2012-01-19 10:35:31 +0000
@@ -0,0 +1,319 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+""" A list of top-level standard python library names.
+
+This list is used by format-imports to determine if a module is in this group
+or not.
+The list is taken from http://docs.python.org/release/2.5.4/lib/modindex.html
+but modules specific to other OSs have been taken out. It may need to be
+updated from time to time.
+"""
+
+
+# Run this to generate a new module list.
+if __name__ == '__main__':
+    from lxml import html
+    from sys import version_info, stdout
+    modindex_url = (
+        "http://docs.python.org/release/"
+        "{0}.{1}.{2}/modindex.html").format(*version_info)
+    root = html.parse(modindex_url).getroot()
+    modules = set(
+        node.text.split(".", 1)[0]  # The "base" module name.
+        for node in root.cssselect("table tt"))
+    stdout.write("python_standard_libs = [\n")
+    for module in sorted(modules, key=str.lower):
+        stdout.write("    %r,\n" % module)
+    stdout.write("    ]\n")
+
+
+python_standard_libs = [
+    '__builtin__',
+    '__future__',
+    '__main__',
+    '_winreg',
+    'abc',
+    'aepack',
+    'aetools',
+    'aetypes',
+    'aifc',
+    'al',
+    'AL',
+    'anydbm',
+    'applesingle',
+    'argparse',
+    'array',
+    'ast',
+    'asynchat',
+    'asyncore',
+    'atexit',
+    'audioop',
+    'autoGIL',
+    'base64',
+    'BaseHTTPServer',
+    'Bastion',
+    'bdb',
+    'binascii',
+    'binhex',
+    'bisect',
+    'bsddb',
+    'buildtools',
+    'bz2',
+    'calendar',
+    'Carbon',
+    'cd',
+    'cfmfile',
+    'cgi',
+    'CGIHTTPServer',
+    'cgitb',
+    'chunk',
+    'cmath',
+    'cmd',
+    'code',
+    'codecs',
+    'codeop',
+    'collections',
+    'ColorPicker',
+    'colorsys',
+    'commands',
+    'compileall',
+    'compiler',
+    'ConfigParser',
+    'contextlib',
+    'Cookie',
+    'cookielib',
+    'copy',
+    'copy_reg',
+    'cPickle',
+    'cProfile',
+    'crypt',
+    'cStringIO',
+    'csv',
+    'ctypes',
+    'curses',
+    'datetime',
+    'dbhash',
+    'dbm',
+    'decimal',
+    'DEVICE',
+    'difflib',
+    'dircache',
+    'dis',
+    'distutils',
+    'dl',
+    'doctest',
+    'DocXMLRPCServer',
+    'dumbdbm',
+    'dummy_thread',
+    'dummy_threading',
+    'EasyDialogs',
+    'email',
+    'encodings',
+    'errno',
+    'exceptions',
+    'fcntl',
+    'filecmp',
+    'fileinput',
+    'findertools',
+    'FL',
+    'fl',
+    'flp',
+    'fm',
+    'fnmatch',
+    'formatter',
+    'fpectl',
+    'fpformat',
+    'fractions',
+    'FrameWork',
+    'ftplib',
+    'functools',
+    'future_builtins',
+    'gc',
+    'gdbm',
+    'gensuitemodule',
+    'getopt',
+    'getpass',
+    'gettext',
+    'gl',
+    'GL',
+    'glob',
+    'grp',
+    'gzip',
+    'hashlib',
+    'heapq',
+    'hmac',
+    'hotshot',
+    'htmlentitydefs',
+    'htmllib',
+    'HTMLParser',
+    'httplib',
+    'ic',
+    'icopen',
+    'imageop',
+    'imaplib',
+    'imgfile',
+    'imghdr',
+    'imp',
+    'importlib',
+    'imputil',
+    'inspect',
+    'io',
+    'itertools',
+    'jpeg',
+    'json',
+    'keyword',
+    'lib2to3',
+    'linecache',
+    'locale',
+    'logging',
+    'macerrors',
+    'MacOS',
+    'macostools',
+    'macpath',
+    'macresource',
+    'mailbox',
+    'mailcap',
+    'marshal',
+    'math',
+    'md5',
+    'mhlib',
+    'mimetools',
+    'mimetypes',
+    'MimeWriter',
+    'mimify',
+    'MiniAEFrame',
+    'mmap',
+    'modulefinder',
+    'msilib',
+    'msvcrt',
+    'multifile',
+    'multiprocessing',
+    'mutex',
+    'Nav',
+    'netrc',
+    'new',
+    'nis',
+    'nntplib',
+    'numbers',
+    'operator',
+    'optparse',
+    'os',
+    'ossaudiodev',
+    'parser',
+    'pdb',
+    'pickle',
+    'pickletools',
+    'pipes',
+    'PixMapWrapper',
+    'pkgutil',
+    'platform',
+    'plistlib',
+    'popen2',
+    'poplib',
+    'posix',
+    'posixfile',
+    'pprint',
+    'profile',
+    'pstats',
+    'pty',
+    'pwd',
+    'py_compile',
+    'pyclbr',
+    'pydoc',
+    'Queue',
+    'quopri',
+    'random',
+    're',
+    'readline',
+    'repr',
+    'resource',
+    'rexec',
+    'rfc822',
+    'rlcompleter',
+    'robotparser',
+    'runpy',
+    'sched',
+    'ScrolledText',
+    'select',
+    'sets',
+    'sgmllib',
+    'sha',
+    'shelve',
+    'shlex',
+    'shutil',
+    'signal',
+    'SimpleHTTPServer',
+    'SimpleXMLRPCServer',
+    'site',
+    'smtpd',
+    'smtplib',
+    'sndhdr',
+    'socket',
+    'SocketServer',
+    'spwd',
+    'sqlite3',
+    'ssl',
+    'stat',
+    'statvfs',
+    'string',
+    'StringIO',
+    'stringprep',
+    'struct',
+    'subprocess',
+    'sunau',
+    'sunaudiodev',
+    'SUNAUDIODEV',
+    'symbol',
+    'symtable',
+    'sys',
+    'sysconfig',
+    'syslog',
+    'tabnanny',
+    'tarfile',
+    'telnetlib',
+    'tempfile',
+    'termios',
+    'test',
+    'textwrap',
+    'thread',
+    'threading',
+    'time',
+    'timeit',
+    'Tix',
+    'Tkinter',
+    'token',
+    'tokenize',
+    'trace',
+    'traceback',
+    'ttk',
+    'tty',
+    'turtle',
+    'types',
+    'unicodedata',
+    'unittest',
+    'urllib',
+    'urllib2',
+    'urlparse',
+    'user',
+    'UserDict',
+    'UserList',
+    'UserString',
+    'uu',
+    'uuid',
+    'videoreader',
+    'W',
+    'warnings',
+    'wave',
+    'weakref',
+    'webbrowser',
+    'whichdb',
+    'winsound',
+    'wsgiref',
+    'xdrlib',
+    'xml',
+    'xmlrpclib',
+    'zipfile',
+    'zipimport',
+    'zlib',
+    ]