Merge ~cjwatson/launchpad:py3-unstable-json into launchpad:master
- Git
- lp:~cjwatson/launchpad
- py3-unstable-json
- Merge into 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) |
Related bugs: |
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 559e1e9c31c0052
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/answers/browser/tests/test_questiontarget.py b/lib/lp/answers/browser/tests/test_questiontarget.py |
2 | index 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): |
83 | diff --git a/lib/lp/bugs/browser/tests/test_bugsubscription_views.py b/lib/lp/bugs/browser/tests/test_bugsubscription_views.py |
84 | index 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)) |
185 | diff --git a/lib/lp/services/webapp/tests/test_publisher.py b/lib/lp/services/webapp/tests/test_publisher.py |
186 | index 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. |
Looks good!