Code review comment for lp:~jcsackett/launchpad/shortlist-696913

Revision history for this message
j.c.sackett (jcsackett) wrote :

I have made the changes outlined above.

I don't believe that this will be an API incompatibility.

1=== modified file 'lib/lp/blueprints/tests/test_hasspecifications.py'
2--- lib/lp/blueprints/tests/test_hasspecifications.py 2011-01-11 22:53:57 +0000
3+++ lib/lp/blueprints/tests/test_hasspecifications.py 2011-01-12 18:05:13 +0000
4@@ -5,18 +5,15 @@
5
6 __metaclass__ = type
7
8-from lazr.lifecycle.snapshot import Snapshot
9-
10 from canonical.testing.layers import DatabaseFunctionalLayer
11 from lp.blueprints.interfaces.specification import (
12 SpecificationDefinitionStatus,
13 )
14-from lp.registry.interfaces.product import IProduct
15-from lp.registry.interfaces.distribution import IDistribution
16-from lp.registry.interfaces.distroseries import IDistroSeries
17-from lp.registry.interfaces.productseries import IProductSeries
18-from lp.registry.interfaces.projectgroup import IProjectGroup
19+from lp.blueprints.interfaces.specificationtarget import (
20+ IHasSpecifications,
21+ )
22 from lp.testing import TestCaseWithFactory
23+from lp.testing.matcher import DoesNotSnapshot
24
25
26 class HasSpecificationsTests(TestCaseWithFactory):
27@@ -191,42 +188,30 @@
28
29 layer = DatabaseFunctionalLayer
30
31- def setUp(self):
32- super(HasSpecificationsSnapshotTestCase, self).setUp()
33-
34- def check_omissions(self, target, providing):
35- """snapshots of objects with IHasSpecification should not snapshot
36- marked attributes.
37-
38- wrap an export with 'donotsnapshot' to force the snapshot to not
39- include that attribute.
40- """
41- snapshot = Snapshot(target, providing=providing)
42- omitted = [
43+ def check_skipped(self, target, providing):
44+ """Asserts that fields marked doNotSnapshot are skipped."""
45+ skipped = [
46 'all_specifications',
47 'valid_specifications',
48 ]
49- for attribute in omitted:
50- self.assertFalse(
51- hasattr(snapshot, attribute),
52- "snapshot should not include %s." % attribute)
53+ self.assertThat(target, DoesNotSnapshot(skipped, IHasSpecifications))
54
55 def test_product(self):
56 product = self.factory.makeProduct()
57- self.check_omissions(product, IProduct)
58+ self.check_skipped(product)
59
60 def test_distribution(self):
61 distribution = self.factory.makeDistribution()
62- self.check_omissions(distribution, IDistribution)
63+ self.check_skipped(distribution)
64
65 def test_productseries(self):
66 productseries = self.factory.makeProductSeries()
67- self.check_omissions(productseries, IProductSeries)
68+ self.check_skipped(productseries)
69
70 def test_distroseries(self):
71 distroseries = self.factory.makeDistroSeries()
72- self.check_omissions(distroseries, IDistroSeries)
73+ self.check_skipped(distroseries)
74
75 def test_projectgroup(self):
76 projectgroup = self.factory.makeProject()
77- self.check_omissions(projectgroup, IProjectGroup)
78+ self.check_skipped(projectgroup)
79
80=== modified file 'lib/lp/registry/tests/test_milestone.py'
81--- lib/lp/registry/tests/test_milestone.py 2011-01-11 23:43:04 +0000
82+++ lib/lp/registry/tests/test_milestone.py 2011-01-12 18:05:13 +0000
83@@ -7,7 +7,6 @@
84
85 import unittest
86
87-from lazr.lifecycle.snapshot import Snapshot
88 from zope.component import getUtility
89
90 from canonical.launchpad.ftests import (
91@@ -21,15 +20,13 @@
92 )
93 from lp.app.errors import NotFoundError
94 from lp.registry.interfaces.distribution import IDistributionSet
95-from lp.registry.interfaces.distribution import IDistribution
96-from lp.registry.interfaces.distroseries import IDistroSeries
97-from lp.registry.interfaces.milestone import IMilestoneSet
98-from lp.registry.interfaces.product import (
99- IProduct,
100- IProductSet,
101+from lp.registry.interfaces.milestone import (
102+ IHasMilestones,
103+ IMilestoneSet,
104 )
105-from lp.registry.interfaces.projectgroup import IProjectGroup
106+from lp.registry.interfaces.product import IProductSet
107 from lp.testing import TestCaseWithFactory
108+from lp.testing.matchers import DoesNotSnapshot
109
110
111 class MilestoneTest(unittest.TestCase):
112@@ -96,47 +93,26 @@
113
114 layer = DatabaseFunctionalLayer
115
116- def setUp(self):
117- super(HasMilestonesSnapshotTestCase, self).setUp()
118-
119- def check_omissions(self, target, providing):
120- """snapshots of objects with IHasSpecification should not snapshot
121- marked attributes.
122-
123- wrap an export with 'donotsnapshot' to force the snapshot to not
124- include that attribute.
125- """
126- snapshot = Snapshot(target, providing=providing)
127- omitted = [
128+ def check_skipped(self, target):
129+ """Asserts that fields marked doNotSnapshot are skipped."""
130+ skipped = [
131 'milestones',
132 'all_milestones',
133 ]
134- for attribute in omitted:
135- self.assertFalse(
136- hasattr(snapshot, attribute),
137- "snapshot should not include %s." % attribute)
138+ self.assertThat(target, DoesNotSnapshot(skipped, IHasMilestones))
139
140 def test_product(self):
141 product = self.factory.makeProduct()
142- self.check_omissions(product, IProduct)
143+ self.check_skipped(product)
144
145 def test_distribution(self):
146 distribution = self.factory.makeDistribution()
147- self.check_omissions(distribution, IDistribution)
148+ self.check_skipped(distribution)
149
150 def test_distroseries(self):
151 distroseries = self.factory.makeDistroSeries()
152- self.check_omissions(distroseries, IDistroSeries)
153+ self.check_skipped(distroseries)
154
155 def test_projectgroup(self):
156 projectgroup = self.factory.makeProject()
157- self.check_omissions(projectgroup, IProjectGroup)
158-
159-
160-def test_suite():
161- """Return the test suite for the tests in this module."""
162- return unittest.TestLoader().loadTestsFromName(__name__)
163-
164-
165-if __name__ == '__main__':
166- unittest.main()
167+ self.check_skipped(projectgroup)
168
169=== modified file 'lib/lp/registry/tests/test_productseries.py'
170--- lib/lp/registry/tests/test_productseries.py 2011-01-11 22:53:57 +0000
171+++ lib/lp/registry/tests/test_productseries.py 2011-01-12 18:05:13 +0000
172@@ -5,9 +5,6 @@
173
174 __metaclass__ = type
175
176-from unittest import TestLoader
177-
178-from lazr.lifecycle.snapshot import Snapshot
179 from zope.component import getUtility
180
181 from canonical.launchpad.ftests import login
182@@ -23,6 +20,7 @@
183 IProductSeriesSet,
184 )
185 from lp.testing import TestCaseWithFactory
186+from lp.testing.matchers import DoesNotSnapshot
187 from lp.translations.interfaces.translations import (
188 TranslationsBranchImportMode,
189 )
190@@ -310,27 +308,13 @@
191
192 layer = DatabaseFunctionalLayer
193
194- def setUp(self):
195- super(ProductSeriesSnapshotTestCase, self).setUp()
196-
197 def test_productseries(self):
198- """snapshots of objects with IHasSpecification should not snapshot
199- marked attributes.
200-
201- wrap an export with 'donotsnapshot' to force the snapshot to not
202- include that attribute.
203- """
204+ """Asserts that fields marked doNotSnapshot are skipped."""
205 productseries = self.factory.makeProductSeries()
206- snapshot = Snapshot(productseries, providing=IProductSeries)
207- omitted = [
208+ skipped = [
209 'milestones',
210 'all_milestones',
211 ]
212- for attribute in omitted:
213- self.assertFalse(
214- hasattr(snapshot, attribute),
215- "snapshot should not include %s." % attribute)
216-
217-
218-def test_suite():
219- return TestLoader().loadTestsFromName(__name__)
220+ self.assertThat(
221+ productseries,
222+ DoesNotSnapshot(skipped, IProductSeries))
223
224=== modified file 'lib/lp/testing/matchers.py'
225--- lib/lp/testing/matchers.py 2010-12-13 21:40:56 +0000
226+++ lib/lp/testing/matchers.py 2011-01-12 18:05:13 +0000
227@@ -13,6 +13,7 @@
228 'ProvidesAndIsProxied',
229 ]
230
231+from lazr.lifecycle.snapshot import Snapshot
232 from testtools.content import Content
233 from testtools.content_type import UTF8_TEXT
234 from testtools.matchers import (
235@@ -141,7 +142,7 @@
236 result = []
237 for query in self.query_collector.queries:
238 result.append(unicode(query).encode('utf8'))
239- return {'queries': Content(UTF8_TEXT, lambda:['\n'.join(result)])}
240+ return {'queries': Content(UTF8_TEXT, lambda: ['\n'.join(result)])}
241
242
243 class IsNotProxied(Mismatch):
244@@ -265,3 +266,38 @@
245 mismatches.append(mismatch)
246 if mismatches:
247 return MismatchesAll(mismatches)
248+
249+
250+class WasSnapshotted(Mismatch):
251+
252+ def __init__(self, matchee, attribute):
253+ self.matchee = matchee
254+ self.attribute = attribute
255+
256+ def describe(self):
257+ return "Snapshot of %s should not include %s" % (
258+ self.matchee, self.attribute)
259+
260+
261+class DoesNotSnapshot(Matcher):
262+
263+ def __init__(self, attr_list, interface, error_msg=None):
264+ self.attr_list = attr_list
265+ self.interface = interface
266+ self.error_msg = error_msg
267+
268+ def __str__(self):
269+ return "Does not include %s when Snapshot is provided %s." % (
270+ ', '.join(self.attr_list), self.interface)
271+
272+ def match(self, matchee):
273+ snapshot = Snapshot(matchee, providing=self.interface)
274+ mismatches = list()
275+ for attribute in self.attr_list:
276+ if hasattr(snapshot, attribute):
277+ mismatches.append(WasSnapshotted(matchee, attribute))
278+
279+ if mismatches == []:
280+ return None
281+ else:
282+ return MismatchesAll(mismatches)

« Back to merge proposal