Merge lp:~lifeless/launchpad/registry into lp:launchpad

[Robert Collins]

Factor out QueryCounter from the rosetta tests for reuse and cleanup a couple of registry tests.

Some minor test cleanups from my performance day dive.

[Michael Hudson-Doyle (mwhudson)]

Have you seen assertStatementCount on lp.testing.TestCase? It's a bit different, but more direct and general (I imagine get_request_statements() uses a storm trace hook at some level), and I wonder if that should be the approach we aim to promote and generalize?

The test cleanups look fine.

review: Needs Information

[Robert Collins (lifeless)]

13:19 < lifeless> I looked for it
13:19 < lifeless> but found QueryCounter
13:19 < lifeless> oh yay two totally different ways of doing the same thing.
13:19 < lifeless> uhm
13:20 < lifeless> sigh, thanks.
13:20 < lifeless> I think my refactoring is not harmful
13:20 < lifeless> in fact, I think QueryCounter is useful because its
aimed directly at the request, not at the storm layer
13:20 < lifeless> so its useful for 'is this page request going to be ok'
13:20 < lifeless> which, record_statements on allmembers is not a good
answers for

[Michael Hudson-Doyle (mwhudson)]

We talked about this a bit on IRC.

Happy for the branch to land as is. A colon seems to have become misplaced in QueryCounter.__doc__, fix that and you're good.

review: Approve

=== modified file 'lib/lp/registry/tests/test_listteammembers.py'
--- lib/lp/registry/tests/test_listteammembers.py	2010-04-07 20:36:28 +0000
+++ lib/lp/registry/tests/test_listteammembers.py	2010-08-05 00:26:50 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-import unittest
-
 from canonical.testing import LaunchpadZopelessLayer
 
 from lp.registry.scripts import listteammembers
@@ -107,7 +105,3 @@
         self.assertRaises(
             listteammembers.NoSuchTeamError, listteammembers.process_team,
             'nosuchteam-matey')
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2010-07-14 16:08:24 +0000
+++ lib/lp/registry/tests/test_person.py	2010-08-05 00:26:50 +0000
@@ -3,7 +3,6 @@
 
 __metaclass__ = type
 
-import unittest
 from datetime import datetime
 import pytz
 import time
@@ -35,7 +34,7 @@
 from lp.bugs.interfaces.bugtask import IllegalRelatedBugTasksParams
 from lp.answers.model.answercontact import AnswerContact
 from lp.blueprints.model.specification import Specification
-from lp.testing import login_person, logout, TestCaseWithFactory
+from lp.testing import login_person, logout, TestCase, TestCaseWithFactory
 from lp.testing.views import create_initialized_view
 from lp.registry.interfaces.person import PrivatePersonLinkageError
 from canonical.testing.layers import DatabaseFunctionalLayer, reconnect_stores
@@ -214,12 +213,14 @@
         self.assertEqual('(\\u0170-tester)>', displayname)
 
 
-class TestPersonSet(unittest.TestCase):
+class TestPersonSet(TestCase):
     """Test `IPersonSet`."""
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        TestCase.setUp(self)
         login(ANONYMOUS)
+        self.addCleanup(logout)
         self.person_set = getUtility(IPersonSet)
 
     def test_isNameBlacklisted(self):
@@ -299,12 +300,14 @@
         self.assertEqual(expected, second_account.openid_identifier)
 
 
-class TestCreatePersonAndEmail(unittest.TestCase):
+class TestCreatePersonAndEmail(TestCase):
     """Test `IPersonSet`.createPersonAndEmail()."""
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        TestCase.setUp(self)
         login(ANONYMOUS)
+        self.addCleanup(logout)
         self.person_set = getUtility(IPersonSet)
 
     def test_duplicated_name_not_accepted(self):
@@ -545,7 +548,3 @@
         names = [entry['project'].name for entry in results]
         self.assertEqual(
             ['cc', 'bb', 'aa', 'dd', 'ee'], names)
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/_webservice.py'
--- lib/lp/testing/_webservice.py	2010-04-28 14:22:03 +0000
+++ lib/lp/testing/_webservice.py	2010-08-05 00:26:50 +0000
@@ -13,10 +13,14 @@
 
 
 import transaction
+from zope.app.publication.interfaces import IEndRequestEvent
+from zope.app.testing import ztapi
 from zope.component import getUtility
+
 from launchpadlib.credentials import AccessToken, Credentials
 from launchpadlib.launchpad import Launchpad
 
+from canonical.launchpad.webapp.adapter import get_request_statements
 from canonical.launchpad.webapp.interaction import ANONYMOUS
 from canonical.launchpad.webapp.interfaces import OAuthPermission
 from canonical.launchpad.interfaces import (
@@ -122,3 +126,42 @@
     transaction.commit()
     version = version or Launchpad.DEFAULT_VERSION
     return Launchpad(credentials, service_root, version=version)
+
+
+class QueryCollector:
+    """Collect database calls made in web requests.
+
+    These are only retrievable at the end of a request, and for tests it is
+    useful to be able to make aassertions about the calls made during a request
+    : this class provides a tool to gather them in a simple fashion.
+
+    :ivar count: The count of db queries the last web request made.
+    :ivar queries: The list of queries made. See
+        canonical.launchpad.webapp.adapter.get_request_statements for more
+        information.
+    """
+
+    def __init__(self):
+        self._active = False
+        self.count = None
+        self.queries = None
+
+    def register(self):
+        """Start counting queries.
+        
+        Be sure to call unregister when finished with the collector.
+
+        After each web request the count and queries attributes are updated.
+        """
+        ztapi.subscribe((IEndRequestEvent, ), None, self)
+        self._active = True
+
+    def __call__(self, event):
+        if self._active:
+            self.queries = get_request_statements()
+            self.count = len(self.queries)
+
+    def unregister(self):
+        self._active = False
+
+

=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate-performance.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-translate-performance.txt	2009-07-01 20:45:39 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-translate-performance.txt	2010-08-05 00:26:50 +0000
@@ -5,32 +5,9 @@
 translation suggestions.  It's hard to keep database performance for this page
 under control.
 
-  >>> from canonical.launchpad.webapp.adapter import get_request_statements
-  >>> from zope.app.testing import ztapi
-  >>> from zope.app.publication.interfaces import IEndRequestEvent
-
-  >>> class QueryCounter:
-  ...     """Count database statements performed in last request.
-  ...
-  ...     We can't get at this count after the browser has completed its
-  ...     request, because that will reset the tracking logic.  Instead, we
-  ...     subscribe to the Zope "request completed" event and grab our count
-  ...     when that event comes along.
-  ...     """
-  ...     _active = True
-  ...     count = None
-  ...
-  ...     def __init__(self):
-  ...         ztapi.subscribe((IEndRequestEvent, ), None, self)
-  ...
-  ...     def __call__(self, event):
-  ...         if self._active:
-  ...             self.count = len(get_request_statements())
-  ...
-  ...     def unregister(self):
-  ...         self._active = False
-
-  >>> query_counter = QueryCounter()
+  >>> from lp.testing._webservice import QueryCollector
+  >>> query_counter = QueryCollector()
+  >>> query_counter.register()
 
 === Anonymous access ===