Merge ~cjwatson/launchpad:py3-unstable-json into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: f79caef83f15f26ad5e59bc94b9870bb651c49c6
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:py3-unstable-json
Merge into: launchpad:master
Diff against target: 332 lines (+59/-42)
3 files modified
lib/lp/answers/browser/tests/test_questiontarget.py (+8/-8)
lib/lp/bugs/browser/tests/test_bugsubscription_views.py (+12/-11)
lib/lp/services/webapp/tests/test_publisher.py (+39/-23)
Reviewer Review Type Date Requested Status
Cristian Gonzalez (community) Approve
Review via email: mp+397873@code.launchpad.net

Commit message

Don't assume stable JSON dict serialisation (take 2)

Description of the change

Like 559e1e9c31c005200977039897a93d450f3cfc6b, but in some more tests.

To post a comment you must log in.
Revision history for this message
Cristian Gonzalez (cristiangsp) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/answers/browser/tests/test_questiontarget.py b/lib/lp/answers/browser/tests/test_questiontarget.py
2index cd510b6..241a1a8 100644
3--- a/lib/lp/answers/browser/tests/test_questiontarget.py
4+++ b/lib/lp/answers/browser/tests/test_questiontarget.py
5@@ -7,13 +7,13 @@ from __future__ import absolute_import, print_function, unicode_literals
6
7 __metaclass__ = type
8
9+import json
10 import os
11
12 from lazr.restful.interfaces import (
13 IJSONRequestCache,
14 IWebServiceClientRequest,
15 )
16-from simplejson import dumps
17 import six
18 from six.moves.urllib.parse import quote
19 from zope.component import getUtility
20@@ -276,7 +276,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
21 def test_data_no_answer_contacts(self):
22 question = self.factory.makeQuestion()
23 view = create_view(question.target, '+portlet-answercontacts-details')
24- self.assertEqual(dumps([]), view.answercontact_data_js)
25+ self.assertEqual([], json.loads(view.answercontact_data_js))
26
27 def test_data_person_answercontact(self):
28 # answercontact_data_js returns JSON string of a list
29@@ -302,7 +302,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
30 }
31 }
32 self.assertEqual(
33- dumps([expected_result]), view.answercontact_data_js)
34+ [expected_result], json.loads(view.answercontact_data_js))
35
36 def test_data_team_answer_contact(self):
37 # For a team answer contacts, answercontact_data_js has is_team set
38@@ -329,7 +329,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
39 }
40 }
41 self.assertEqual(
42- dumps([expected_result]), view.answercontact_data_js)
43+ [expected_result], json.loads(view.answercontact_data_js))
44
45 def test_data_team_answercontact_owner_looks(self):
46 # For a team subscription, answercontact_data_js has can_edit
47@@ -357,7 +357,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
48 }
49 with person_logged_in(contact.teamowner):
50 self.assertEqual(
51- dumps([expected_result]), view.answercontact_data_js)
52+ [expected_result], json.loads(view.answercontact_data_js))
53
54 def test_data_team_subscription_member_looks(self):
55 # For a team subscription, answercontact_data_js has can_edit
56@@ -387,7 +387,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
57 }
58 with person_logged_in(contact.teamowner):
59 self.assertEqual(
60- dumps([expected_result]), view.answercontact_data_js)
61+ [expected_result], json.loads(view.answercontact_data_js))
62
63 def test_data_target_owner_answercontact_looks(self):
64 # Answercontact_data_js has can_edit set to true for target owner.
65@@ -413,7 +413,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
66 }
67 with person_logged_in(distro.owner):
68 self.assertEqual(
69- dumps([expected_result]), view.answercontact_data_js)
70+ [expected_result], json.loads(view.answercontact_data_js))
71
72 def test_data_subscription_lp_admin(self):
73 # For a subscription, answercontact_data_js has can_edit
74@@ -443,7 +443,7 @@ class QuestionTargetPortletAnswerContactsWithDetailsTests(
75 admin = getUtility(IPersonSet).find(ADMIN_EMAIL).any()
76 with person_logged_in(admin):
77 self.assertEqual(
78- dumps([expected_result]), view.answercontact_data_js)
79+ [expected_result], json.loads(view.answercontact_data_js))
80
81
82 class TestQuestionTargetPortletAnswerContacts(TestCaseWithFactory):
83diff --git a/lib/lp/bugs/browser/tests/test_bugsubscription_views.py b/lib/lp/bugs/browser/tests/test_bugsubscription_views.py
84index d6566dd..5dd87d0 100644
85--- a/lib/lp/bugs/browser/tests/test_bugsubscription_views.py
86+++ b/lib/lp/bugs/browser/tests/test_bugsubscription_views.py
87@@ -5,8 +5,9 @@
88
89 __metaclass__ = type
90
91+import json
92+
93 from lazr.restful.interfaces import IWebServiceClientRequest
94-from simplejson import dumps
95 from storm.store import Store
96 from testtools.matchers import Equals
97 from zope.component import getUtility
98@@ -528,7 +529,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
99 def test_data_no_subscriptions(self):
100 bug = self._makeBugWithNoSubscribers()
101 harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
102- self.assertEqual(dumps([]), harness.view.subscriber_data_js)
103+ self.assertEqual([], json.loads(harness.view.subscriber_data_js))
104
105 def test_data_person_subscription(self):
106 # subscriber_data_js returns JSON string of a list
107@@ -556,7 +557,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
108 'subscription_level': "Lifecycle",
109 }
110 self.assertEqual(
111- dumps([expected_result]), harness.view.subscriber_data_js)
112+ [expected_result], json.loads(harness.view.subscriber_data_js))
113
114 def test_data_person_subscription_other_subscriber(self):
115 # Ensure subscriber_data_js does the correct thing when the person who
116@@ -586,7 +587,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
117 'subscription_level': "Lifecycle",
118 }
119 self.assertEqual(
120- dumps([expected_result]), harness.view.subscriber_data_js)
121+ [expected_result], json.loads(harness.view.subscriber_data_js))
122
123 def test_data_person_subscription_other_subscriber_query_count(self):
124 # All subscriber data should be retrieved with a single query.
125@@ -634,7 +635,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
126 'subscription_level': "Lifecycle",
127 }
128 self.assertEqual(
129- dumps([expected_result]), harness.view.subscriber_data_js)
130+ [expected_result], json.loads(harness.view.subscriber_data_js))
131
132 def _test_data_private_team_subscription(self, authenticated_user):
133 # For a private team subscription, the team name and url are rendered
134@@ -700,7 +701,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
135 'subscription_level': "Maybe",
136 }]
137 self.assertEqual(
138- dumps(expected_result), view.subscriber_data_js)
139+ expected_result, json.loads(view.subscriber_data_js))
140
141 def test_data_private_team_subscription_authenticated_user(self):
142 # For a logged in user, private team subscriptions are rendered.
143@@ -740,7 +741,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
144 }
145 with person_logged_in(subscriber.teamowner):
146 self.assertEqual(
147- dumps([expected_result]), harness.view.subscriber_data_js)
148+ [expected_result], json.loads(harness.view.subscriber_data_js))
149
150 def test_data_team_subscription_member_looks(self):
151 # For a team subscription, subscriber_data_js has can_edit
152@@ -774,7 +775,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
153 }
154 with person_logged_in(subscriber.teamowner):
155 self.assertEqual(
156- dumps([expected_result]), harness.view.subscriber_data_js)
157+ [expected_result], json.loads(harness.view.subscriber_data_js))
158
159 def test_data_subscription_lp_admin(self):
160 # For a subscription, subscriber_data_js has can_edit
161@@ -807,7 +808,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
162 admin = getUtility(IPersonSet).find(ADMIN_EMAIL).any()
163 with person_logged_in(admin):
164 self.assertEqual(
165- dumps([expected_result]), harness.view.subscriber_data_js)
166+ [expected_result], json.loads(harness.view.subscriber_data_js))
167
168 def test_data_person_subscription_subscriber(self):
169 # For a subscription, subscriber_data_js has can_edit
170@@ -837,7 +838,7 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
171 }
172 with person_logged_in(subscribed_by):
173 self.assertEqual(
174- dumps([expected_result]), harness.view.subscriber_data_js)
175+ [expected_result], json.loads(harness.view.subscriber_data_js))
176
177 def test_data_person_subscription_user_excluded(self):
178 # With the subscriber logged in, they are not included in the results.
179@@ -850,4 +851,4 @@ class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
180 level=BugNotificationLevel.LIFECYCLE)
181 harness = LaunchpadFormHarness(
182 bug, BugPortletSubscribersWithDetails)
183- self.assertEqual(dumps([]), harness.view.subscriber_data_js)
184+ self.assertEqual([], json.loads(harness.view.subscriber_data_js))
185diff --git a/lib/lp/services/webapp/tests/test_publisher.py b/lib/lp/services/webapp/tests/test_publisher.py
186index 40e4fcd..728e228 100644
187--- a/lib/lp/services/webapp/tests/test_publisher.py
188+++ b/lib/lp/services/webapp/tests/test_publisher.py
189@@ -6,13 +6,13 @@ from doctest import (
190 ELLIPSIS,
191 )
192 import io
193+import json
194 from unittest import (
195 TestLoader,
196 TestSuite,
197 )
198
199 from lazr.restful.interfaces import IJSONRequestCache
200-import simplejson
201 from zope.component import getUtility
202 from zope.interface import implementer
203
204@@ -59,7 +59,8 @@ class TestLaunchpadView(TestCaseWithFactory):
205
206 def test_getCacheJSON_non_resource_context(self):
207 view = LaunchpadView(object(), LaunchpadTestRequest())
208- self.assertEqual('{"related_features": {}}', view.getCacheJSON())
209+ self.assertEqual(
210+ {"related_features": {}}, json.loads(view.getCacheJSON()))
211
212 @staticmethod
213 def getCanada():
214@@ -78,7 +79,7 @@ class TestLaunchpadView(TestCaseWithFactory):
215
216 def test_getCacheJSON_resource_context(self):
217 view = LaunchpadView(self.getCanada(), LaunchpadTestRequest())
218- json_dict = simplejson.loads(view.getCacheJSON())['context']
219+ json_dict = json.loads(view.getCacheJSON())['context']
220 self.assertIsCanada(json_dict)
221
222 def test_getCacheJSON_non_resource_object(self):
223@@ -87,15 +88,15 @@ class TestLaunchpadView(TestCaseWithFactory):
224 IJSONRequestCache(request).objects['my_bool'] = True
225 with person_logged_in(self.factory.makePerson()):
226 self.assertEqual(
227- '{"related_features": {}, "my_bool": true}',
228- view.getCacheJSON())
229+ {"related_features": {}, "my_bool": True},
230+ json.loads(view.getCacheJSON()))
231
232 def test_getCacheJSON_resource_object(self):
233 request = LaunchpadTestRequest()
234 view = LaunchpadView(object(), request)
235 IJSONRequestCache(request).objects['country'] = self.getCanada()
236 with person_logged_in(self.factory.makePerson()):
237- json_dict = simplejson.loads(view.getCacheJSON())['country']
238+ json_dict = json.loads(view.getCacheJSON())['country']
239 self.assertIsCanada(json_dict)
240
241 def test_getCacheJSON_context_overrides_objects(self):
242@@ -103,7 +104,7 @@ class TestLaunchpadView(TestCaseWithFactory):
243 view = LaunchpadView(self.getCanada(), request)
244 IJSONRequestCache(request).objects['context'] = True
245 with person_logged_in(self.factory.makePerson()):
246- json_dict = simplejson.loads(view.getCacheJSON())['context']
247+ json_dict = json.loads(view.getCacheJSON())['context']
248 self.assertIsCanada(json_dict)
249
250 def test_getCache_anonymous(self):
251@@ -111,7 +112,7 @@ class TestLaunchpadView(TestCaseWithFactory):
252 view = LaunchpadView(self.getCanada(), request)
253 self.assertIs(None, view.user)
254 IJSONRequestCache(request).objects['my_bool'] = True
255- json_dict = simplejson.loads(view.getCacheJSON())
256+ json_dict = json.loads(view.getCacheJSON())
257 self.assertIsCanada(json_dict['context'])
258 self.assertIn('my_bool', json_dict)
259
260@@ -127,7 +128,7 @@ class TestLaunchpadView(TestCaseWithFactory):
261 # A redirection view by default provides no json cache data.
262 request = LaunchpadTestRequest()
263 view = RedirectionView(None, request)
264- json_dict = simplejson.loads(view.getCacheJSON())
265+ json_dict = json.loads(view.getCacheJSON())
266 self.assertEqual({}, json_dict)
267
268 def test_getCache_redirected_view(self):
269@@ -141,7 +142,7 @@ class TestLaunchpadView(TestCaseWithFactory):
270 test_view = TestView(self.getCanada(), request)
271 view = RedirectionView(None, request, cache_view=test_view)
272 IJSONRequestCache(request).objects['my_bool'] = True
273- json_dict = simplejson.loads(view.getCacheJSON())
274+ json_dict = json.loads(view.getCacheJSON())
275 self.assertIsCanada(json_dict['context'])
276 self.assertIn('my_bool', json_dict)
277
278@@ -327,13 +328,17 @@ class TestLaunchpadView(TestCaseWithFactory):
279 view = TestView(object(), request)
280 with person_logged_in(self.factory.makePerson()):
281 self.assertEqual(
282- '{"related_features": {"test_feature": {'
283- '"url": "http://wiki.lp.dev/LEP/sample", '
284- '"is_beta": true, '
285- '"value": "on", '
286- '"title": "title"'
287- '}}}',
288- view.getCacheJSON())
289+ {
290+ "related_features": {
291+ "test_feature": {
292+ "is_beta": True,
293+ "title": "title",
294+ "url": "http://wiki.lp.dev/LEP/sample",
295+ "value": "on",
296+ },
297+ },
298+ },
299+ json.loads(view.getCacheJSON()))
300
301 def test_json_cache_collects_related_features_from_all_views(self):
302 # A typical page includes data from more than one view,
303@@ -353,12 +358,23 @@ class TestLaunchpadView(TestCaseWithFactory):
304 TestView2(object(), request)
305 with person_logged_in(self.factory.makePerson()):
306 self.assertEqual(
307- '{"related_features": '
308- '{"test_feature_2": {"url": "http://wiki.lp.dev/LEP/sample2",'
309- ' "is_beta": true, "value": "on", "title": "title"}, '
310- '"test_feature": {"url": "http://wiki.lp.dev/LEP/sample", '
311- '"is_beta": true, "value": "on", "title": "title"}}}',
312- view.getCacheJSON())
313+ {
314+ "related_features": {
315+ "test_feature": {
316+ "is_beta": True,
317+ "title": "title",
318+ "url": "http://wiki.lp.dev/LEP/sample",
319+ "value": "on",
320+ },
321+ "test_feature_2": {
322+ "is_beta": True,
323+ "title": "title",
324+ "url": "http://wiki.lp.dev/LEP/sample2",
325+ "value": "on",
326+ },
327+ },
328+ },
329+ json.loads(view.getCacheJSON()))
330
331 def test_view_creation_with_fake_or_none_request(self):
332 # LaunchpadView.__init__() does not crash with a FakeRequest.

Subscribers

People subscribed via source and target branches

to status/vote changes: