Merge lp:~jml/launchpad/use-testtools into lp:launchpad

[Jonathan Lange]

Use testtools, really. Make it an egg, and delete all of the Launchpad code that duplicates testtools.

[Jonathan Lange (jml)]

This is a bit preliminary, because there are still XXX comments and I don't really know how to do this sort of thing, but it's worth getting feedback now.

This branch changes Launchpad to use a testtools release, rather than the branch. It then changes our base test case to inherit from testtools.TestCase rather than unittest.TestCase directly. This lets us delete a whole bunch of code.

While looking at lp/testing/__init__.py, I noticed a few things that could be improved. The ones I could easily fix (like the duplicated code in 'assertRaisesWithContent'), I fixed. Others, I've marked with XXXs.

I haven't run the tests yet, partly since I'm afraid of committing the tarball for testtools to the download-cache.

[Jonathan Lange (jml)]

I got over my fear & committed the tarball for testtools to download-cache, like this:

$ bzr st
added:
  dist/testtools-0.9.2.tar.gz

Testing now.

[Aaron Bentley (abentley)]

This looks totally reasonable to me.

review: Approve

[Gary Poster (gary)]

You have some diff cruft problems visible in lines 193 through 201 of the diff as I see it right now. Other than that it looks good to me.

Gary

review: Approve

=== removed file 'lib/canonical/launchpad/testing/tests/test_testcase.py'
--- lib/canonical/launchpad/testing/tests/test_testcase.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/testing/tests/test_testcase.py	1970-01-01 00:00:00 +0000
@@ -1,163 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for the base TestCase classes."""
-
-__metaclass__ = type
-
-import unittest
-from lp.testing import TestCase
-
-
-class LoggingResult(unittest.TestResult):
-    """TestResult that logs its event to a list."""
-
-    def __init__(self, log):
-        self._events = log
-        super(LoggingResult, self).__init__()
-
-    def startTest(self, test):
-        self._events.append(('startTest', test))
-        super(LoggingResult, self).startTest(test)
-
-    def stopTest(self, test):
-        self._events.append(('stopTest', test))
-        super(LoggingResult, self).stopTest(test)
-
-    def addFailure(self, test, error):
-        self._events.append(('addFailure', test, error))
-        super(LoggingResult, self).addFailure(test, error)
-
-    def addError(self, test, error):
-        self._events.append(('addError', test, error))
-        super(LoggingResult, self).addError(test, error)
-
-    def addSuccess(self, test):
-        self._events.append(('addSuccess', test))
-        super(LoggingResult, self).addSuccess(test)
-
-
-class TestAddCleanup(unittest.TestCase):
-    """Tests for TestCase.addCleanup."""
-
-    class LoggingTest(TestCase):
-        """A test that logs calls to setUp, runTest and tearDown."""
-
-        def setUp(self):
-            self._calls = ['setUp']
-
-        def brokenSetUp(self):
-            # A tearDown that deliberately fails.
-            self._calls = ['brokenSetUp']
-            raise RuntimeError('Deliberate Failure')
-
-        def runTest(self):
-            self._calls.append('runTest')
-
-        def tearDown(self):
-            self._calls.append('tearDown')
-
-    def setUp(self):
-        self._result_calls = []
-        self.test = TestAddCleanup.LoggingTest('runTest')
-        self.logging_result = LoggingResult(self._result_calls)
-
-    def assertErrorLogEqual(self, messages):
-        self.assertEqual(messages, [call[0] for call in self._result_calls])
-
-    def assertTestLogEqual(self, messages):
-        """Assert that the call log equals `messages`."""
-        self.assertEqual(messages, self.test._calls)
-
-    def logAppender(self, message):
-        """Return a cleanup that appends `message` to the tests log.
-
-        Cleanups are callables that are added to a test by addCleanup. To
-        verify that our cleanups run in the right order, we add strings to a
-        list that acts as a log. This method returns a cleanup that will add
-        the given message to that log when run.
-        """
-        self.test._calls.append(message)
-
-    def test_fixture(self):
-        # A normal run of self.test logs 'setUp', 'runTest' and 'tearDown'.
-        # This test doesn't test addCleanup itself, it just sanity checks the
-        # fixture.
-        self.test.run(self.logging_result)
-        self.assertTestLogEqual(['setUp', 'runTest', 'tearDown'])
-
-    def test_cleanup_run_before_tearDown(self):
-        # Cleanup functions added with 'addCleanup' are called before tearDown
-        # runs.
-        self.test.addCleanup(self.logAppender, 'cleanup')
-        self.test.run(self.logging_result)
-        self.assertTestLogEqual(['setUp', 'runTest', 'cleanup', 'tearDown'])
-
-    def test_add_cleanup_called_if_setUp_fails(self):
-        # Cleanup functions added with 'addCleanup' are called even if setUp
-        # fails. Note that tearDown has a different behavior: it is only
-        # called when setUp succeeds.
-        self.test.setUp = self.test.brokenSetUp
-        self.test.addCleanup(self.logAppender, 'cleanup')
-        self.test.run(self.logging_result)
-        self.assertTestLogEqual(['brokenSetUp', 'cleanup'])
-
-    def test_addCleanup_called_in_reverse_order(self):
-        # Cleanup functions added with 'addCleanup' are called in reverse
-        # order.
-        #
-        # One of the main uses of addCleanup is to dynamically create
-        # resources that need some sort of explicit tearDown. Often one
-        # resource will be created in terms of another, e.g.,
-        #     self.first = self.makeFirst()
-        #     self.second = self.makeSecond(self.first)
-        #
-        # When this happens, we generally want to clean up the second resource
-        # before the first one, since the second depends on the first.
-        self.test.addCleanup(self.logAppender, 'first')
-        self.test.addCleanup(self.logAppender, 'second')
-        self.test.run(self.logging_result)
-        self.assertTestLogEqual(
-            ['setUp', 'runTest', 'second', 'first', 'tearDown'])
-
-    def test_tearDown_runs_after_cleanup_failure(self):
-        # tearDown runs even if a cleanup function fails.
-        self.test.addCleanup(lambda: 1/0)
-        self.test.run(self.logging_result)
-        self.assertTestLogEqual(['setUp', 'runTest', 'tearDown'])
-
-    def test_cleanups_continue_running_after_error(self):
-        # All cleanups are always run, even if one or two of them fail.
-        self.test.addCleanup(self.logAppender, 'first')
-        self.test.addCleanup(lambda: 1/0)
-        self.test.addCleanup(self.logAppender, 'second')
-        self.test.run(self.logging_result)
-        self.assertTestLogEqual(
-            ['setUp', 'runTest', 'second', 'first', 'tearDown'])
-
-    def test_error_in_cleanups_are_captured(self):
-        # If a cleanup raises an error, we want to record it and fail the
-        # test, even though we go on to run other cleanups.
-        self.test.addCleanup(lambda: 1/0)
-        self.test.run(self.logging_result)
-        self.assertErrorLogEqual(['startTest', 'addError', 'stopTest'])
-
-    def test_keyboard_interrupt_not_caught(self):
-        # If a cleanup raises KeyboardInterrupt, it gets reraised.
-        def raiseKeyboardInterrupt():
-            raise KeyboardInterrupt()
-        self.test.addCleanup(raiseKeyboardInterrupt)
-        self.assertRaises(
-            KeyboardInterrupt, self.test.run, self.logging_result)
-
-    def test_multipleErrorsReported(self):
-        # Errors from all failing cleanups are reported.
-        self.test.addCleanup(lambda: 1/0)
-        self.test.addCleanup(lambda: 1/0)
-        self.test.run(self.logging_result)
-        self.assertErrorLogEqual(
-            ['startTest', 'addError', 'addError', 'stopTest'])
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/codehosting/puller/tests/test_scheduler.py'
--- lib/lp/codehosting/puller/tests/test_scheduler.py	2009-12-22 23:35:26 +0000
+++ lib/lp/codehosting/puller/tests/test_scheduler.py	2010-01-14 04:34:14 +0000
@@ -640,6 +640,11 @@
 
     layer = TwistedAppServerLayer
 
+    # XXX: Hacky workaround for Trial. It interprets the skip method as a
+    # reason to skip the tests, but the default test result object doesn't
+    # support skipping, hence errors.
+    skip = None
+
     def setUp(self):
         TrialTestCase.setUp(self)
         PullerBranchTestCase.setUp(self)

=== modified file 'lib/lp/registry/windmill/tests/test_add_milestone.py.disabled'
--- lib/lp/registry/windmill/tests/test_add_milestone.py.disabled	2010-01-12 22:43:49 +0000
+++ lib/lp/registry/windmill/tests/test_add_milestone.py.disabled	2010-01-14 04:34:14 +0000
@@ -87,6 +87,7 @@
     layer = RegistryWindmillLayer
 
     def setUp(self):
+        super(TestAddMilestone, self).setUp()
         self.client = WindmillTestClient('AddMilestone')
 
     def test_adding_milestone_on_addrelease_page(self):

=== modified file 'lib/lp/registry/windmill/tests/test_datetime_picker.py'
--- lib/lp/registry/windmill/tests/test_datetime_picker.py	2009-12-10 17:52:56 +0000
+++ lib/lp/registry/windmill/tests/test_datetime_picker.py	2010-01-14 04:34:14 +0000
@@ -22,6 +22,7 @@
     layer = RegistryWindmillLayer
 
     def setUp(self):
+        super(TestDateTimeCalendarWidget, self).setUp()
         self.client = WindmillTestClient('DateTimeCalendarWidget')
 
     def test_datetime_calendar_widget(self):

=== modified file 'lib/lp/registry/windmill/tests/test_plusnew_step1.py'
--- lib/lp/registry/windmill/tests/test_plusnew_step1.py	2009-12-11 19:54:04 +0000
+++ lib/lp/registry/windmill/tests/test_plusnew_step1.py	2010-01-14 04:34:14 +0000
@@ -24,6 +24,7 @@
     layer = RegistryWindmillLayer
 
     def setUp(self):
+        super(TestNewProjectStep1, self).setUp()
         self.client = WindmillTestClient('TestNewProjectStep1')
 
     def test_projects_plusnew_text_fields(self):

=== modified file 'lib/lp/registry/windmill/tests/test_plusnew_step2.py'
--- lib/lp/registry/windmill/tests/test_plusnew_step2.py	2009-12-11 19:54:04 +0000
+++ lib/lp/registry/windmill/tests/test_plusnew_step2.py	2010-01-14 04:34:14 +0000
@@ -22,6 +22,7 @@
     layer = RegistryWindmillLayer
 
     def setUp(self):
+        super(TestNewProjectStep2, self).setUp()
         self.client = WindmillTestClient('TestNewProjectStep2')
 
     def test_projects_plusnew_step_two(self):

=== modified file 'lib/lp/registry/windmill/tests/test_team_index.py'
--- lib/lp/registry/windmill/tests/test_team_index.py	2009-12-18 05:46:01 +0000
+++ lib/lp/registry/windmill/tests/test_team_index.py	2010-01-14 04:34:14 +0000
@@ -24,6 +24,7 @@
     layer = RegistryWindmillLayer
 
     def setUp(self):
+        super(TestTeamIndex, self).setUp()
         self.client = WindmillTestClient(__name__)
 
     def test_addmember(self):

=== modified file 'lib/lp/registry/windmill/tests/test_timeline_graph.py'
--- lib/lp/registry/windmill/tests/test_timeline_graph.py	2009-12-10 17:52:56 +0000
+++ lib/lp/registry/windmill/tests/test_timeline_graph.py	2010-01-14 04:34:14 +0000
@@ -13,12 +13,14 @@
 from lp.registry.windmill.testing import RegistryWindmillLayer
 from lp.testing import TestCaseWithFactory
 
+
 class TestTimelineGraph(TestCaseWithFactory):
     """Test timeline graph widget."""
 
     layer = RegistryWindmillLayer
 
     def setUp(self):
+        super(TestTimelineGraph, self).setUp()
         self.client = WindmillTestClient('TimelineGraph')
 
     def test_timeline_graph(self):

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2009-12-22 23:50:27 +0000
+++ lib/lp/testing/__init__.py	2010-01-14 04:34:14 +0000
@@ -1,24 +1,47 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=W0401,C0301
 
 __metaclass__ = type
+__all__ = [
+    'ANONYMOUS',
+    'capture_events',
+    'FakeTime',
+    'get_lsb_information',
+    'is_logged_in',
+    'login',
+    'login_person',
+    'logout',
+    'map_branch_contents',
+    'normalize_whitespace',
+    'record_statements',
+    'run_with_login',
+    'run_with_storm_debug',
+    'run_script',
+    'TestCase',
+    'TestCaseWithFactory',
+    'test_tales',
+    'time_counter',
+    # XXX: This really shouldn't be exported from here. People should import
+    # it from Zope.
+    'verifyObject',
+    'validate_mock_class',
+    'with_anonymous_login',
+    ]
 
+import copy
 from datetime import datetime, timedelta
-from pprint import pformat
-import copy
 from inspect import getargspec, getmembers, getmro, isclass, ismethod
 import os
+from pprint import pformat
 import shutil
 import subprocess
 import tempfile
 import time
-import unittest
 
 from bzrlib.branch import Branch as BzrBranch
 from bzrlib.bzrdir import BzrDir, format_registry
-from bzrlib.errors import InvalidURLJoin
 from bzrlib.transport import get_transport
 
 import pytz
@@ -26,7 +49,11 @@
 from storm.store import Store
 from storm.tracer import install_tracer, remove_tracer_type
 
+import testtools
 import transaction
+
+from twisted.python.util import mergeFunctionMetadata
+
 from zope.component import getUtility
 import zope.event
 from zope.interface.verify import verifyClass, verifyObject
@@ -34,18 +61,17 @@
     isinstance as zope_isinstance, removeSecurityProxy)
 
 from canonical.launchpad.webapp import errorlog
+from canonical.config import config
+from canonical.launchpad.webapp.interfaces import ILaunchBag
 from lp.codehosting.vfs import branch_id_to_path, get_multi_server
-from canonical.config import config
 # Import the login and logout functions here as it is a much better
 # place to import them from in tests.
-from canonical.launchpad.webapp.interfaces import ILaunchBag
-
 from lp.testing._login import (
     ANONYMOUS, is_logged_in, login, login_person, logout)
+# canonical.launchpad.ftests expects test_tales to be imported from here.
+# XXX: JonathanLange 2010-01-01: Why?!
 from lp.testing._tales import test_tales
 
-from twisted.python.util import mergeFunctionMetadata
-
 # zope.exception demands more of frame objects than twisted.python.failure
 # provides in its fake frames.  This is enough to make it work with them
 # as of 2009-09-16.  See https://bugs.edge.launchpad.net/bugs/425113.
@@ -164,72 +190,9 @@
         debug(False)
 
 
-class TestCase(unittest.TestCase):
+class TestCase(testtools.TestCase):
     """Provide Launchpad-specific test facilities."""
 
-    # Python 2.4 monkeypatch:
-    if getattr(unittest.TestCase, '_exc_info', None) is None:
-        _exc_info = unittest.TestCase._TestCase__exc_info
-        # We would not expect to need to make this property writeable, but
-        # twisted.trial.unittest.TestCase.__init__ chooses to write to it in
-        # the same way that the __init__ of the standard library's
-        # unittest.TestCase.__init__ does, as part of its own method of
-        # arranging for pre-2.5 compatibility.
-        class MonkeyPatchDescriptor:
-            def __get__(self, obj, type):
-                return obj._TestCase__testMethodName
-            def __set__(self, obj, value):
-                obj._TestCase__testMethodName = value
-        _testMethodName = MonkeyPatchDescriptor()
-
-    def __init__(self, *args, **kwargs):
-        unittest.TestCase.__init__(self, *args, **kwargs)
-        self._cleanups = []
-
-    def __str__(self):
-        """Return the fully qualified Python name of the test.
-
-        Zope uses this method to determine how to print the test in the
-        runner. We use the test's id in order to make the test easier to find,
-        and also so that modifications to the id will show up. This is
-        particularly important with bzrlib-style test multiplication.
-        """
-        return self.id()
-
-    def _runCleanups(self, result):
-        """Run the cleanups that have been added with addCleanup.
-
-        See the docstring for addCleanup for more information.
-
-        Returns True if all cleanups ran without error, False otherwise.
-        """
-        ok = True
-        while self._cleanups:
-            function, arguments, keywordArguments = self._cleanups.pop()
-            try:
-                function(*arguments, **keywordArguments)
-            except KeyboardInterrupt:
-                raise
-            except:
-                result.addError(self, self._exc_info())
-                ok = False
-        return ok
-
-    def addCleanup(self, function, *arguments, **keywordArguments):
-        """Add a cleanup function to be called before tearDown.
-
-        Functions added with addCleanup will be called in reverse order of
-        adding after the test method and before tearDown.
-
-        If a function added with addCleanup raises an exception, the error
-        will be recorded as a test error, and the next cleanup will then be
-        run.
-
-        Cleanup functions are always called before a test finishes running,
-        even if setUp is aborted by an exception.
-        """
-        self._cleanups.append((function, arguments, keywordArguments))
-
     def installFixture(self, fixture):
         """Install 'fixture', an object that has a `setUp` and `tearDown`.
 
@@ -346,27 +309,12 @@
         self.assertTrue(zope_isinstance(instance, assert_class),
             '%r is not an instance of %r' % (instance, assert_class))
 
-    def assertIs(self, expected, observed):
-        """Assert that `expected` is the same object as `observed`."""
-        self.assertTrue(expected is observed,
-                        "%r is not %r" % (expected, observed))
-
     def assertIsNot(self, expected, observed, msg=None):
         """Assert that `expected` is not the same object as `observed`."""
         if msg is None:
             msg = "%r is %r" % (expected, observed)
         self.assertTrue(expected is not observed, msg)
 
-    def assertIn(self, needle, haystack):
-        """Assert that 'needle' is in 'haystack'."""
-        self.assertTrue(
-            needle in haystack, '%r not in %r' % (needle, haystack))
-
-    def assertNotIn(self, needle, haystack):
-        """Assert that 'needle' is not in 'haystack'."""
-        self.assertFalse(
-            needle in haystack, '%r in %r' % (needle, haystack))
-
     def assertContentEqual(self, iter1, iter2):
         """Assert that 'iter1' has the same content as 'iter2'."""
         list1 = sorted(iter1)
@@ -374,28 +322,6 @@
         self.assertEqual(
             list1, list2, '%s != %s' % (pformat(list1), pformat(list2)))
 
-    def assertRaises(self, excClass, callableObj, *args, **kwargs):
-        """Assert that a callable raises a particular exception.
-
-        :param excClass: As for the except statement, this may be either an
-            exception class, or a tuple of classes.
-        :param callableObj: A callable, will be passed ``*args`` and
-            ``**kwargs``.
-
-        Returns the exception so that you can examine it.
-        """
-        try:
-            callableObj(*args, **kwargs)
-        except excClass, e:
-            return e
-        else:
-            if getattr(excClass, '__name__', None) is not None:
-                excName = excClass.__name__
-            else:
-                # probably a tuple
-                excName = str(excClass)
-            raise self.failureException, "%s not raised" % excName
-
     def assertRaisesWithContent(self, exception, exception_content,
                                 func, *args):
         """Check if the given exception is raised with given content.
@@ -403,15 +329,8 @@
         If the exception isn't raised or the exception_content doesn't
         match what was raised an AssertionError is raised.
         """
-        exception_name = str(exception).split('.')[-1]
-
-        try:
-            func(*args)
-        except exception, err:
-            self.assertEqual(str(err), exception_content)
-        else:
-            raise AssertionError(
-                "'%s' was not raised" % exception_name)
+        err = self.assertRaises(exception, func, *args)
+        self.assertEqual(exception_content, str(err))
 
     def assertBetween(self, lower_bound, variable, upper_bound):
         """Assert that 'variable' is strictly between two boundaries."""
@@ -425,51 +344,12 @@
         The config values will be restored during test tearDown.
         """
         name = self.factory.getUniqueString()
-        body = '\n'.join(["%s: %s"%(k, v) for k, v in kwargs.iteritems()])
+        body = '\n'.join(["%s: %s" % (k, v) for k, v in kwargs.iteritems()])
         config.push(name, "\n[%s]\n%s\n" % (section, body))
         self.addCleanup(config.pop, name)
 
-    def run(self, result=None):
-        if result is None:
-            result = self.defaultTestResult()
-        result.startTest(self)
-        testMethod = getattr(self, self._testMethodName)
-        try:
-            try:
-                self.setUp()
-            except KeyboardInterrupt:
-                raise
-            except:
-                result.addError(self, self._exc_info())
-                self._runCleanups(result)
-                return
-
-            ok = False
-            try:
-                testMethod()
-                ok = True
-            except self.failureException:
-                result.addFailure(self, self._exc_info())
-            except KeyboardInterrupt:
-                raise
-            except:
-                result.addError(self, self._exc_info())
-
-            cleanupsOk = self._runCleanups(result)
-            try:
-                self.tearDown()
-            except KeyboardInterrupt:
-                raise
-            except:
-                result.addError(self, self._exc_info())
-                ok = False
-            if ok and cleanupsOk:
-                result.addSuccess(self)
-        finally:
-            result.stopTest(self)
-
     def setUp(self):
-        unittest.TestCase.setUp(self)
+        testtools.TestCase.setUp(self)
         from lp.testing.factory import ObjectFactory
         self.factory = ObjectFactory()
 
@@ -673,6 +553,8 @@
         zope.event.subscribers[:] = old_subscribers
 
 
+# XXX: This doesn't seem like a generically-useful testing function. Perhaps
+# it should go in a sub-module or something? -- jml
 def get_lsb_information():
     """Returns a dictionary with the LSB host information.
 
@@ -770,6 +652,8 @@
     return " ".join(string.split())
 
 
+# XXX: This doesn't seem to be a generically useful testing function. Perhaps
+# it should go into a sub-module? -- jml
 def map_branch_contents(branch_url):
     """Return all files in branch at `branch_url`.
 
@@ -794,6 +678,7 @@
 
     return contents
 
+
 def validate_mock_class(mock_class):
     """Validate method signatures in mock classes derived from real classes.
 

=== removed symlink 'lib/testtools'
=== target was u'../sourcecode/testtools/testtools/'
=== modified file 'utilities/sourcedeps.conf'
--- utilities/sourcedeps.conf	2010-01-14 03:48:53 +0000
+++ utilities/sourcedeps.conf	2010-01-14 04:34:14 +0000
@@ -13,5 +13,5 @@
 subunit lp:~launchpad-pqm/subunit/trunk;revno=61
 subvertpy lp:~launchpad-pqm/subvertpy/trunk;revno=2040
 testresources lp:~launchpad-pqm/testresources/dev;revno=16
-canonical-identity-provider lp:~launchpad-pqm/canonical-identity-provider/trunk;revno=8900 optional
+canonical-identity-provider lp:~launchpad-pqm/canonical-identity-provider/trunk;revno=8901 optional
 shipit lp:~launchpad-pqm/shipit/trunk;revno=8901 optional