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

[Robert Collins]

Add a matcher HasQueryCount and a new testtools supporting that.

Another step along my arc of fixing the /participation API performance. This adds a matcher for query counts and a dependency on a newer testtools to get LessThan.

[Robert Collins (lifeless)]

This is what a test using it looks like:
+class TestAPIPartipication(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_participation_query_limit(self):
+ # A team with 2 members should only query once for all their
+ # attributes.
+ team = self.factory.makeTeam()
+ with person_logged_in(team.teamowner):
+ team.addMember(self.factory.makePerson(), team.teamowner)
+ team.addMember(self.factory.makePerson(), team.teamowner)
+ webservice = LaunchpadWebServiceCaller()
+ collector = QueryCollector()
+ collector.register()
+ self.addCleanup(collector.unregister)
+ url = "/~%s/participants" % team.name
+ logout()
+ response = webservice.get(url, headers={'User-Agent':'AnonNeedsThis'})
+ self.assertEqual(response.status, 200,
+ "Got %d for url %r with response %r" % (
+ response.status, url, response.body))
+ self.assertThat(collector, HasQueryCount(LessThan(24)))

and it generates this output:
running: xvfb-run ./bin/test --subunit -t test_participation_query_limit| testr load
======================================================================
FAIL: lp.registry.tests.test_person.TestAPIPartipication.test_participation_query_limit
----------------------------------------------------------------------
Text attachment: traceback
------------
Traceback (most recent call last):
_StringException: Text attachment: queries
------------
(2, 3, 'launchpad-main-master', 'SELECT OAuthConsumer.date_created, OAuthConsumer.disabled, OAuthConsumer.id, OAuthConsumer."key", OAuthConsumer.secret FROM OAuthConsumer WHERE OAuthConsumer."key" IS NULL')
(4, 4, 'launchpad-main-master', 'SELECT OAuthConsumer.date_created, OAuthConsumer.disabled, OAuthConsumer.id, OAuthConsumer."key", OAuthConsumer.secret FROM OAuthConsumer WHERE OAuthConsumer."key" = %s')
(5, 5, 'launchpad-main-master', 'SELECT OAuthConsumer.date_created, OAuthConsumer.disabled, OAuthConsumer.id, OAuthConsumer."key", OAuthConsumer.secret FROM OAuthConsumer WHERE OAuthConsumer."key" = %s')
(8, 9, 'launchpad-main-master', 'INSERT INTO OAuthConsumer (date_created, disabled, "key", secret) VALUES (CURRENT_TIMESTAMP AT TIME ZONE \'UTC\', %s, %s, %s) RETURNING OAuthConsumer.id')
(11, 11, 'launchpad-main-master', 'SELECT Person.account, Person.addressline1, Person.addressline2, Person.city, Person.country, Person.creation_comment, Person.creation_rationale, Person.datecreated, Person.defaultmembershipperiod, Person.defaultrenewalperiod, Person.displayname, Person.hide_email_addresses, Person.homepage_content, Person.icon, Person.id, Person.logo, Person.mailing_list_auto_subscribe_policy, Person.merged, Person.mugshot, Person.name, Person.organization, Person.personal_standing, Person.personal_standing_reason, Person.phone, Person.postcode, Person.province, Person.registrant, Person.renewal_policy, Person.subscriptionpolicy, Person.teamdescription, Person.teamowner, Person.verbose_bugnotifications, Person.visibility FROM Person WHERE Person.name = %s AND Person.merged IS NULL ORDER BY person_sort_key(Person.displayname, Person.name)')
(12, 13, 'launc...

[Robert Collins (lifeless)]

Oh, and I am aware of assertStatementCount - thats too fragile (its an exact match) for my purposes, which is why this matcher is composable with a more primitive matcher - it can replace assertStatementCount if you use Equals, or be used in a looser way as I do in my example with LessThan. I'm going to look at making assertStatementCount into a trivial wrapper in a later patch.

[Jelmer Vernooij (jelmer)]

review: Approve (code)

[Jelmer Vernooij (jelmer)]

Great to see test matchers in action!

=== modified file 'lib/lp/testing/matchers.py'
--- lib/lp/testing/matchers.py	2010-08-02 20:56:25 +0000
+++ lib/lp/testing/matchers.py	2010-08-05 10:04:46 +0000
@@ -5,6 +5,7 @@
 __all__ = [
     'DoesNotProvide',
     'DoesNotCorrectlyProvide',
+    'HasQueryCount',
     'IsNotProxied',
     'IsProxied',
     'Provides',
@@ -16,6 +17,8 @@
     BrokenImplementation, BrokenMethodImplementation, DoesNotImplement)
 from zope.security.proxy import builtin_isinstance, Proxy
 
+from testtools.content import Content
+from testtools.content_type import ContentType
 from testtools.matchers import Matcher, Mismatch
 
 
@@ -89,6 +92,45 @@
         return None
 
 
+class HasQueryCount(Matcher):
+    """Adapt a Binary Matcher to the query count on a QueryCollector.
+
+    If there is a mismatch, the queries from the collector are provided as a
+    test attachment.
+    """
+
+    def __init__(self, count_matcher):
+        """Create a HasQueryCount that will match using count_matcher."""
+        self.count_matcher = count_matcher
+
+    def __str__(self):
+        return "HasQueryCount(%s)" % self.count_matcher
+
+    def match(self, something):
+        mismatch = self.count_matcher.match(something.count)
+        if mismatch is None:
+            return None
+        return _MismatchedQueryCount(mismatch, something)
+
+
+class _MismatchedQueryCount(Mismatch):
+    """The Mismatch for a HasQueryCount matcher."""
+
+    def __init__(self, mismatch, query_collector):
+        self.count_mismatch = mismatch
+        self.query_collector = query_collector
+
+    def describe(self):
+        return "queries do not match: %s" % (self.count_mismatch.describe(),)
+
+    def get_details(self):
+        result = []
+        for query in self.query_collector.queries:
+            result.append(unicode(query).encode('utf8'))
+        return {'queries': Content(ContentType('text', 'plain',
+            {'charset': 'utf8'}), lambda:['\n'.join(result)])}
+ 
+
 class IsNotProxied(Mismatch):
     """An object is not proxied."""
 

=== modified file 'lib/lp/testing/tests/test_matchers.py'
--- lib/lp/testing/tests/test_matchers.py	2010-08-02 19:52:59 +0000
+++ lib/lp/testing/tests/test_matchers.py	2010-08-05 10:04:46 +0000
@@ -11,8 +11,11 @@
 
 from lp.testing import TestCase
 from lp.testing.matchers import (
-    DoesNotCorrectlyProvide, DoesNotProvide, IsNotProxied, IsProxied,
-    Provides, ProvidesAndIsProxied)
+    DoesNotCorrectlyProvide, DoesNotProvide, HasQueryCount, IsNotProxied,
+    IsProxied, Provides, ProvidesAndIsProxied)
+from lp.testing._webservice import QueryCollector
+
+from testtools.matchers import Is, Not, LessThan
 
 
 class ITestInterface(Interface):
@@ -169,3 +172,41 @@
         obj = ProxyFactory(object(), checker=NamesChecker())
         matcher = ProvidesAndIsProxied(ITestInterface)
         self.assertIsInstance(matcher.match(obj), DoesNotProvide)
+
+
+class TestQueryMatching(TestCase):
+    """Query matching is a work in progress and can be factored out more.
+
+    For now its pretty hard coded to the initial use case and overlaps some
+    unwritten hypothetical testtools infrastructure - e.g. permitting use of
+    attrgetter and the like.
+    """
+
+    def test_match(self):
+        matcher = HasQueryCount(Is(3))
+        collector = QueryCollector()
+        collector.count = 3 
+        # not inspected
+        del collector.queries
+        self.assertThat(matcher.match(collector), Is(None))
+
+    def test_mismatch(self):
+        matcher = HasQueryCount(LessThan(2))
+        collector = QueryCollector()
+        collector.count = 2
+        collector.queries = [("foo", "bar"), ("baaz", "quux")]
+        mismatch = matcher.match(collector)
+        self.assertThat(mismatch, Not(Is(None)))
+        details = mismatch.get_details()
+        lines = []
+        for name, content in details.items():
+            self.assertEqual("queries", name)
+            self.assertEqual("text", content.content_type.type)
+            lines.append(''.join(content.iter_text()))
+        self.assertEqual(["('foo', 'bar')\n('baaz', 'quux')"],
+            lines)
+        self.assertEqual(
+            "queries do not match: %s" % (LessThan(2).match(2).describe(),),
+            mismatch.describe())
+
+

=== modified file 'versions.cfg'
--- versions.cfg	2010-07-26 16:00:01 +0000
+++ versions.cfg	2010-08-05 10:04:46 +0000
@@ -67,7 +67,8 @@
 # This is Storm 0.15 with r342 cherry-picked which fixes a memory leak
 # important for message sharing migration script.
 storm = 0.15danilo-storm-launchpad-r342
-testtools = 0.9.2
+# Has the LessThan matcher.
+testtools = 0.9.6dev91
 transaction = 1.0.0
 Twisted = 10.1.0
 uuid = 1.30