Launchpad itself

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

  1. registry
  2. Merge into devel
Proposed by Robert Collins
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 11305
Proposed branch: lp:~lifeless/launchpad/registry
Merge into: lp:launchpad
Diff against target: 140 lines (+87/-3)
3 files modified
lib/lp/testing/matchers.py (+42/-0)
lib/lp/testing/tests/test_matchers.py (+43/-2)
versions.cfg (+2/-1)
To merge this branch: bzr merge lp:~lifeless/launchpad/registry
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Approve
Review via email: mp+31830@code.launchpad.net

Commit message

Add a matcher HasQueryCount and a new testtools supporting that.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :
Download full text (12.3 KiB)

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...

Revision history for this message
Robert Collins (lifeless) wrote :

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.

Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve (code)
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Great to see test matchers in action!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/testing/matchers.py'
2--- lib/lp/testing/matchers.py 2010-08-02 20:56:25 +0000
3+++ lib/lp/testing/matchers.py 2010-08-05 10:04:46 +0000
4@@ -5,6 +5,7 @@
5 __all__ = [
6 'DoesNotProvide',
7 'DoesNotCorrectlyProvide',
8+ 'HasQueryCount',
9 'IsNotProxied',
10 'IsProxied',
11 'Provides',
12@@ -16,6 +17,8 @@
13 BrokenImplementation, BrokenMethodImplementation, DoesNotImplement)
14 from zope.security.proxy import builtin_isinstance, Proxy
15
16+from testtools.content import Content
17+from testtools.content_type import ContentType
18 from testtools.matchers import Matcher, Mismatch
19
20
21@@ -89,6 +92,45 @@
22 return None
23
24
25+class HasQueryCount(Matcher):
26+ """Adapt a Binary Matcher to the query count on a QueryCollector.
27+
28+ If there is a mismatch, the queries from the collector are provided as a
29+ test attachment.
30+ """
31+
32+ def __init__(self, count_matcher):
33+ """Create a HasQueryCount that will match using count_matcher."""
34+ self.count_matcher = count_matcher
35+
36+ def __str__(self):
37+ return "HasQueryCount(%s)" % self.count_matcher
38+
39+ def match(self, something):
40+ mismatch = self.count_matcher.match(something.count)
41+ if mismatch is None:
42+ return None
43+ return _MismatchedQueryCount(mismatch, something)
44+
45+
46+class _MismatchedQueryCount(Mismatch):
47+ """The Mismatch for a HasQueryCount matcher."""
48+
49+ def __init__(self, mismatch, query_collector):
50+ self.count_mismatch = mismatch
51+ self.query_collector = query_collector
52+
53+ def describe(self):
54+ return "queries do not match: %s" % (self.count_mismatch.describe(),)
55+
56+ def get_details(self):
57+ result = []
58+ for query in self.query_collector.queries:
59+ result.append(unicode(query).encode('utf8'))
60+ return {'queries': Content(ContentType('text', 'plain',
61+ {'charset': 'utf8'}), lambda:['\n'.join(result)])}
62+
63+
64 class IsNotProxied(Mismatch):
65 """An object is not proxied."""
66
67
68=== modified file 'lib/lp/testing/tests/test_matchers.py'
69--- lib/lp/testing/tests/test_matchers.py 2010-08-02 19:52:59 +0000
70+++ lib/lp/testing/tests/test_matchers.py 2010-08-05 10:04:46 +0000
71@@ -11,8 +11,11 @@
72
73 from lp.testing import TestCase
74 from lp.testing.matchers import (
75- DoesNotCorrectlyProvide, DoesNotProvide, IsNotProxied, IsProxied,
76- Provides, ProvidesAndIsProxied)
77+ DoesNotCorrectlyProvide, DoesNotProvide, HasQueryCount, IsNotProxied,
78+ IsProxied, Provides, ProvidesAndIsProxied)
79+from lp.testing._webservice import QueryCollector
80+
81+from testtools.matchers import Is, Not, LessThan
82
83
84 class ITestInterface(Interface):
85@@ -169,3 +172,41 @@
86 obj = ProxyFactory(object(), checker=NamesChecker())
87 matcher = ProvidesAndIsProxied(ITestInterface)
88 self.assertIsInstance(matcher.match(obj), DoesNotProvide)
89+
90+
91+class TestQueryMatching(TestCase):
92+ """Query matching is a work in progress and can be factored out more.
93+
94+ For now its pretty hard coded to the initial use case and overlaps some
95+ unwritten hypothetical testtools infrastructure - e.g. permitting use of
96+ attrgetter and the like.
97+ """
98+
99+ def test_match(self):
100+ matcher = HasQueryCount(Is(3))
101+ collector = QueryCollector()
102+ collector.count = 3
103+ # not inspected
104+ del collector.queries
105+ self.assertThat(matcher.match(collector), Is(None))
106+
107+ def test_mismatch(self):
108+ matcher = HasQueryCount(LessThan(2))
109+ collector = QueryCollector()
110+ collector.count = 2
111+ collector.queries = [("foo", "bar"), ("baaz", "quux")]
112+ mismatch = matcher.match(collector)
113+ self.assertThat(mismatch, Not(Is(None)))
114+ details = mismatch.get_details()
115+ lines = []
116+ for name, content in details.items():
117+ self.assertEqual("queries", name)
118+ self.assertEqual("text", content.content_type.type)
119+ lines.append(''.join(content.iter_text()))
120+ self.assertEqual(["('foo', 'bar')\n('baaz', 'quux')"],
121+ lines)
122+ self.assertEqual(
123+ "queries do not match: %s" % (LessThan(2).match(2).describe(),),
124+ mismatch.describe())
125+
126+
127
128=== modified file 'versions.cfg'
129--- versions.cfg 2010-07-26 16:00:01 +0000
130+++ versions.cfg 2010-08-05 10:04:46 +0000
131@@ -67,7 +67,8 @@
132 # This is Storm 0.15 with r342 cherry-picked which fixes a memory leak
133 # important for message sharing migration script.
134 storm = 0.15danilo-storm-launchpad-r342
135-testtools = 0.9.2
136+# Has the LessThan matcher.
137+testtools = 0.9.6dev91
138 transaction = 1.0.0
139 Twisted = 10.1.0
140 uuid = 1.30