Merge lp:~ursinha/uci-engine/update-ticket-raises-error into lp:uci-engine
- update-ticket-raises-error
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Ursula Junque |
Approved revision: | 648 |
Merged at revision: | 646 |
Proposed branch: | lp:~ursinha/uci-engine/update-ticket-raises-error |
Merge into: | lp:uci-engine |
Diff against target: |
619 lines (+208/-74) 9 files modified
cli/ci_cli/tests/__init__.py (+2/-2) cli/ci_cli/tests/test_cli.py (+81/-19) cli/ci_cli/tests/test_get_ticket_status.py (+6/-6) cli/ci_cli/tests/test_image.py (+11/-11) cli/ci_cli/tests/test_ticket.py (+18/-18) cli/ci_cli/tests/test_utils.py (+61/-3) cli/ci_cli/ticket.py (+18/-7) cli/ci_cli/utils.py (+9/-7) cli/ubuntu-ci (+2/-1) |
To merge this branch: | bzr merge lp:~ursinha/uci-engine/update-ticket-raises-error |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Francis Ginther | Approve | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Review via email: mp+225593@code.launchpad.net |
Commit message
Display better errors in case requests fail during ticket creation
Description of the change
This branch ensures all requests will raise exceptions if their return codes are errors. What happened in bug 1337294 should be a rare problem, and until we understand what happened it's hard to find a definitive solution. If that happens again, now it's explicit that the problem happens while update_ticket, and while we don't have a retry mechanism I'm directing the user to let an admin know what happened, to be able to manually set it to "Queued".
- 647. By Ursula Junque
-
Oops, test case was named wrong
PS Jenkins bot (ps-jenkins) wrote : | # |
Andy Doan (doanac) wrote : | # |
I have a question about what we print out during a failure. Since I'll be off tomorrow, don't let my question block things if you have sorted out.
Ursula Junque (ursinha) wrote : | # |
I've replied inline, thanks Andy.
- 648. By Ursula Junque
-
Fixing variable names on cli utils, there were misleading
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:648
http://
Executed test runs:
Click here to trigger a rebuild:
http://
Francis Ginther (fginther) wrote : | # |
This is a step forward and I'll approve. I would like to make sure we take a closer look at the other post and get calls within the cli (and indeed all code) and make sure we're not dropping relevant error messages.
Ursula Junque (ursinha) wrote : | # |
> This is a step forward and I'll approve. I would like to make sure we take a
> closer look at the other post and get calls within the cli (and indeed all
> code) and make sure we're not dropping relevant error messages.
All of the posts, gets and patches are covered now, using utils.patch, utils.post and utils.get will always raise_for_status and main cli script will catch and print that. I agree we still need to revisit how we handle exceptions to see if we're not losing important information we could be displaying.
Preview Diff
1 | === modified file 'cli/ci_cli/tests/__init__.py' |
2 | --- cli/ci_cli/tests/__init__.py 2014-06-19 23:38:34 +0000 |
3 | +++ cli/ci_cli/tests/__init__.py 2014-07-04 03:45:58 +0000 |
4 | @@ -45,8 +45,8 @@ |
5 | sys.stdout = out |
6 | |
7 | |
8 | -class FakeRequestsReturn: |
9 | - """Object to mock the return of python-requests return.""" |
10 | +class FakeResponse: |
11 | + """Object to mock a python-requests response.""" |
12 | |
13 | def __init__(self, code, content, reason=None): |
14 | self.status_code = code |
15 | |
16 | === modified file 'cli/ci_cli/tests/test_cli.py' |
17 | --- cli/ci_cli/tests/test_cli.py 2014-07-01 17:02:35 +0000 |
18 | +++ cli/ci_cli/tests/test_cli.py 2014-07-04 03:45:58 +0000 |
19 | @@ -29,7 +29,7 @@ |
20 | capture_stderr, |
21 | capture_stdout, |
22 | MainScriptTestCase, |
23 | - FakeRequestsReturn, |
24 | + FakeResponse, |
25 | ) |
26 | from ci_utils.testing import get_test_file_path |
27 | from ci_utils.ticket_states import ( |
28 | @@ -62,7 +62,7 @@ |
29 | "removed_binaries": "myoldpackage", |
30 | } |
31 | mock_requests.side_effect = [ |
32 | - FakeRequestsReturn(200, json.dumps(data)) |
33 | + FakeResponse(200, json.dumps(data)) |
34 | ] |
35 | args = ['status', '-t', '5'] |
36 | with capture_stdout(self.cli.main, args) as cm: |
37 | @@ -101,7 +101,7 @@ |
38 | ] |
39 | } |
40 | mock_requests.side_effect = [ |
41 | - FakeRequestsReturn(200, json.dumps(data)) |
42 | + FakeResponse(200, json.dumps(data)) |
43 | ] |
44 | args = ['status'] |
45 | with capture_stdout(self.cli.main, args) as cm: |
46 | @@ -159,7 +159,7 @@ |
47 | def test_ts_server_returns_urlerror(self, mock_requests): |
48 | """Test cli response to ts server refusing connection (111).""" |
49 | mock_requests.side_effect = [ |
50 | - FakeRequestsReturn( |
51 | + FakeResponse( |
52 | 400, '', socket.error(111, "Connection refused")) |
53 | ] |
54 | args = ['status'] |
55 | @@ -176,7 +176,7 @@ |
56 | def test_ts_server_returns_internal_server_error(self, mock_requests): |
57 | """Test cli response to ts server returning error 500.""" |
58 | mock_requests.side_effect = [ |
59 | - FakeRequestsReturn( |
60 | + FakeResponse( |
61 | 500, "", "INTERNAL SERVER ERROR" |
62 | ) |
63 | ] |
64 | @@ -193,7 +193,7 @@ |
65 | def test_ts_server_returns_url_not_found(self, mock_requests): |
66 | """Test cli response to ts server returning 404 error.""" |
67 | mock_requests.side_effect = [ |
68 | - FakeRequestsReturn(404, '', 'Not Found') |
69 | + FakeResponse(404, '', 'Not Found') |
70 | ] |
71 | args = ['status'] |
72 | with LogCapture() as lc: |
73 | @@ -221,7 +221,7 @@ |
74 | def test_ts_server_returns_other_httperror(self, mock_requests): |
75 | """Test cli response to ts server returning HTTPError.""" |
76 | mock_requests.side_effect = [ |
77 | - FakeRequestsReturn(403, '', 'Forbidden') |
78 | + FakeResponse(403, '', 'Forbidden') |
79 | ] |
80 | args = ['status'] |
81 | with LogCapture() as lc: |
82 | @@ -246,18 +246,18 @@ |
83 | # CLI 'create_ticket' action allows users to upload new sources |
84 | # via gatekeeper (without swift credentials). |
85 | mocked_post.side_effect = [ |
86 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'), |
87 | - FakeRequestsReturn(201, '/ticket.html?ticket_id=1'), |
88 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
89 | + FakeResponse(201, '/ticket.html?ticket_id=1'), |
90 | ] |
91 | mocked_put.side_effect = [ |
92 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
93 | - FakeRequestsReturn(201, '/a_sandbox_object_url/'), |
94 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
95 | - FakeRequestsReturn(201, '/a_sandbox_object_url/'), |
96 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
97 | - FakeRequestsReturn(201, '/a_sandbox_object_url/'), |
98 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
99 | - FakeRequestsReturn(201, '/a_sandbox_object_url/'), |
100 | + FakeResponse(200, 'A_TEMP_URL'), |
101 | + FakeResponse(201, '/a_sandbox_object_url/'), |
102 | + FakeResponse(200, 'A_TEMP_URL'), |
103 | + FakeResponse(201, '/a_sandbox_object_url/'), |
104 | + FakeResponse(200, 'A_TEMP_URL'), |
105 | + FakeResponse(201, '/a_sandbox_object_url/'), |
106 | + FakeResponse(200, 'A_TEMP_URL'), |
107 | + FakeResponse(201, '/a_sandbox_object_url/'), |
108 | ] |
109 | mocked_of.return_value = 'a_content' |
110 | foobar_changes = get_test_file_path('foobar_0.1-1_source.changes') |
111 | @@ -272,7 +272,8 @@ |
112 | # The first output line contains a local absolute host path, so |
113 | # it is ignored. |
114 | self.assertEquals( |
115 | - ['Validating .changes file...', |
116 | + ['Parsing foobar_0.1-1_source.changes...', |
117 | + 'Validating .changes...', |
118 | 'foobar_0.1-1 parsed and validated.', |
119 | 'Created source package upload: http://mocked/api/v1/ticket/1/', |
120 | 'Created subticket: http://mocked/api/v1/ticket/1/', |
121 | @@ -285,7 +286,68 @@ |
122 | 'Created artifact: http://mocked/api/v1/ticket/1/', |
123 | 'Created artifact: http://mocked/api/v1/ticket/1/', |
124 | 'Created artifact: http://mocked/api/v1/ticket/1/'], |
125 | - list([r.msg for r in lc.records])[1:]) |
126 | + list([r.msg for r in lc.records])) |
127 | + |
128 | + @mock.patch('ci_cli.utils.post', |
129 | + return_value='http://mocked/api/v1/ticket/1/') |
130 | + @mock.patch('ci_cli.utils.get') |
131 | + @mock.patch('ci_cli.ticket._open_file') |
132 | + @mock.patch('requests.put') |
133 | + @mock.patch('requests.post') |
134 | + @mock.patch('requests.patch') |
135 | + def test_create_ticket_update_fails( |
136 | + self, mocked_patch, mocked_post, mocked_put, mocked_of, mocked_get, |
137 | + mocked_upost): |
138 | + # CLI 'create_ticket' action allows users to upload new sources |
139 | + # via gatekeeper (without swift credentials). |
140 | + mocked_post.side_effect = [ |
141 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
142 | + FakeResponse(201, '/ticket.html?ticket_id=1'), |
143 | + ] |
144 | + mocked_put.side_effect = [ |
145 | + FakeResponse(200, 'A_TEMP_URL'), |
146 | + FakeResponse(201, '/a_sandbox_object_url/'), |
147 | + FakeResponse(200, 'A_TEMP_URL'), |
148 | + FakeResponse(201, '/a_sandbox_object_url/'), |
149 | + FakeResponse(200, 'A_TEMP_URL'), |
150 | + FakeResponse(201, '/a_sandbox_object_url/'), |
151 | + FakeResponse(200, 'A_TEMP_URL'), |
152 | + FakeResponse(201, '/a_sandbox_object_url/'), |
153 | + ] |
154 | + mocked_of.return_value = 'a_content' |
155 | + foobar_changes = get_test_file_path('foobar_0.1-1_source.changes') |
156 | + args = ['-v2', 'create_ticket', '-t', '"New feature"', '-d', |
157 | + '"New feature description"', '-o', 'someone@example.com', |
158 | + '-a', 'foobar', '-s', foobar_changes] |
159 | + mocked_patch.side_effect = [ |
160 | + FakeResponse(401, {}, "Failed to patch ticket")] |
161 | + with LogCapture() as lc: |
162 | + logger = logging.getLogger() |
163 | + self.cli.main(args, log=logger) |
164 | + ticket_url = urlparse.urljoin( |
165 | + utils.CI_URL, '/ticket.html?ticket_id=1') |
166 | + # The first output line contains a local absolute host path, so |
167 | + # it is ignored. |
168 | + self.assertEquals( |
169 | + ['Parsing foobar_0.1-1_source.changes...', |
170 | + 'Validating .changes...', |
171 | + 'foobar_0.1-1 parsed and validated.', |
172 | + 'Created source package upload: http://mocked/api/v1/ticket/1/', |
173 | + 'Created subticket: http://mocked/api/v1/ticket/1/', |
174 | + 'Uploading foobar_0.1-1.dsc ...', |
175 | + 'Uploading foobar_0.1.orig.tar.gz ...', |
176 | + 'Uploading foobar_0.1-1.debian.tar.gz ...', |
177 | + 'Uploading foobar_0.1-1_source.changes ...', |
178 | + 'Done! Access {} for more information.'.format(ticket_url), |
179 | + 'Created artifact: http://mocked/api/v1/ticket/1/', |
180 | + 'Created artifact: http://mocked/api/v1/ticket/1/', |
181 | + 'Created artifact: http://mocked/api/v1/ticket/1/', |
182 | + 'Created artifact: http://mocked/api/v1/ticket/1/', |
183 | + 'Ticket was created successfully but there was a problem adding ' |
184 | + 'it to the queue: 401 Client Error: Failed to patch ticket. This ' |
185 | + 'should be a transient error, please contact an admin. Your ' |
186 | + 'ticket number is 1.'], |
187 | + list([r.msg for r in lc.records])) |
188 | |
189 | |
190 | class CliArgumentsTestCase(MainScriptTestCase): |
191 | |
192 | === modified file 'cli/ci_cli/tests/test_get_ticket_status.py' |
193 | --- cli/ci_cli/tests/test_get_ticket_status.py 2014-05-26 13:47:38 +0000 |
194 | +++ cli/ci_cli/tests/test_get_ticket_status.py 2014-07-04 03:45:58 +0000 |
195 | @@ -21,7 +21,7 @@ |
196 | from ci_cli.tests import ( |
197 | capture_stdout, |
198 | MainScriptTestCase, |
199 | - FakeRequestsReturn, |
200 | + FakeResponse, |
201 | ) |
202 | from ci_utils.ticket_states import ( |
203 | TicketWorkflowStep, |
204 | @@ -47,7 +47,7 @@ |
205 | "removed_binaries": "myoldpackage", |
206 | } |
207 | mock_requests.side_effect = [ |
208 | - FakeRequestsReturn(200, json.dumps(data)) |
209 | + FakeResponse(200, json.dumps(data)) |
210 | ] |
211 | args = self.cli.parse_arguments(['status', '-t', '100']) |
212 | with capture_stdout(args.func, args) as cm: |
213 | @@ -82,7 +82,7 @@ |
214 | "removed_binaries": "myoldpackage", |
215 | } |
216 | mock_requests.side_effect = [ |
217 | - FakeRequestsReturn(200, json.dumps(data)) |
218 | + FakeResponse(200, json.dumps(data)) |
219 | ] |
220 | args = self.cli.parse_arguments(['status', '-t', '100']) |
221 | with capture_stdout(args.func, args) as cm: |
222 | @@ -117,7 +117,7 @@ |
223 | "removed_binaries": "myoldpackage", |
224 | } |
225 | mock_requests.side_effect = [ |
226 | - FakeRequestsReturn(200, json.dumps(data)) |
227 | + FakeResponse(200, json.dumps(data)) |
228 | ] |
229 | args = self.cli.parse_arguments(['status', '-t', '100']) |
230 | with capture_stdout(args.func, args) as cm: |
231 | @@ -166,7 +166,7 @@ |
232 | ] |
233 | } |
234 | mock_requests.side_effect = [ |
235 | - FakeRequestsReturn(200, json.dumps(data)) |
236 | + FakeResponse(200, json.dumps(data)) |
237 | ] |
238 | args = self.cli.parse_arguments(['status']) |
239 | with capture_stdout(args.func, args) as cm: |
240 | @@ -181,7 +181,7 @@ |
241 | @mock.patch('requests.get') |
242 | def test_get_status_404_response(self, mock_requests): |
243 | mock_requests.side_effect = [ |
244 | - FakeRequestsReturn(404, '', 'Not Found') |
245 | + FakeResponse(404, '', 'Not Found') |
246 | ] |
247 | args = self.cli.parse_arguments(['status', '-t', '99']) |
248 | with self.assertRaises(SystemExit) as cm: |
249 | |
250 | === modified file 'cli/ci_cli/tests/test_image.py' |
251 | --- cli/ci_cli/tests/test_image.py 2014-06-11 02:56:45 +0000 |
252 | +++ cli/ci_cli/tests/test_image.py 2014-07-04 03:45:58 +0000 |
253 | @@ -24,7 +24,7 @@ |
254 | from ci_cli.image import ImageObjectNotFound |
255 | from ci_cli.tests import ( |
256 | MainScriptTestCase, |
257 | - FakeRequestsReturn, |
258 | + FakeResponse, |
259 | ) |
260 | |
261 | |
262 | @@ -83,9 +83,9 @@ |
263 | # the image file is downloaded and stored in the specified |
264 | # path. |
265 | mock_requests.side_effect = [ |
266 | - FakeRequestsReturn(200, json.dumps(artifact_data)), |
267 | - FakeRequestsReturn(200, json.dumps(artifact_tempurls)), |
268 | - FakeRequestsReturn(200, 'IMAGE_CONTENT'), |
269 | + FakeResponse(200, json.dumps(artifact_data)), |
270 | + FakeResponse(200, json.dumps(artifact_tempurls)), |
271 | + FakeResponse(200, 'IMAGE_CONTENT'), |
272 | ] |
273 | args = self.cli.parse_arguments( |
274 | ['get_image', '-t', '4', '-n', self.image_path]) |
275 | @@ -102,8 +102,8 @@ |
276 | # If the corresponding tempurl for a registered image cannot |
277 | # be found ImageObjectNotFound() is raised. |
278 | mock_requests.side_effect = [ |
279 | - FakeRequestsReturn(200, json.dumps(artifact_data)), |
280 | - FakeRequestsReturn(200, json.dumps({})), |
281 | + FakeResponse(200, json.dumps(artifact_data)), |
282 | + FakeResponse(200, json.dumps({})), |
283 | ] |
284 | args = self.cli.parse_arguments( |
285 | ['get_image', '-t', '4', '-n', self.image_path]) |
286 | @@ -131,10 +131,10 @@ |
287 | # If a ticket number is not specified, CLI looks for the |
288 | # latest COMPLETED ticket and downloads its image. |
289 | mock_requests.side_effect = [ |
290 | - FakeRequestsReturn(200, json.dumps(ticket_data)), |
291 | - FakeRequestsReturn(200, json.dumps(artifact_data)), |
292 | - FakeRequestsReturn(200, json.dumps(artifact_tempurls)), |
293 | - FakeRequestsReturn(200, 'IMAGE_CONTENT'), |
294 | + FakeResponse(200, json.dumps(ticket_data)), |
295 | + FakeResponse(200, json.dumps(artifact_data)), |
296 | + FakeResponse(200, json.dumps(artifact_tempurls)), |
297 | + FakeResponse(200, 'IMAGE_CONTENT'), |
298 | ] |
299 | args = self.cli.parse_arguments( |
300 | ['get_image', '-n', self.image_path]) |
301 | @@ -146,7 +146,7 @@ |
302 | # If no ticket is specified and there are no COMPLETED tickets |
303 | # the CLI renders a message indicating the problem. |
304 | mock_requests.side_effect = [ |
305 | - FakeRequestsReturn(200, json.dumps({'objects': []})), |
306 | + FakeResponse(200, json.dumps({'objects': []})), |
307 | ] |
308 | args = self.cli.parse_arguments( |
309 | ['get_image', '-n', self.image_path]) |
310 | |
311 | === modified file 'cli/ci_cli/tests/test_ticket.py' |
312 | --- cli/ci_cli/tests/test_ticket.py 2014-07-01 03:48:44 +0000 |
313 | +++ cli/ci_cli/tests/test_ticket.py 2014-07-04 03:45:58 +0000 |
314 | @@ -19,7 +19,7 @@ |
315 | import unittest |
316 | |
317 | from ci_cli.tests import ( |
318 | - FakeRequestsReturn, |
319 | + FakeResponse, |
320 | MainScriptTestCase, |
321 | ) |
322 | from ci_cli.ticket import ( |
323 | @@ -44,7 +44,7 @@ |
324 | spu = SourcePackageUpload('1', 'http://server/') |
325 | self.assertIsNone(spu.sandbox_url) |
326 | mocked_post.side_effect = [ |
327 | - FakeRequestsReturn(404, 'Zoing!') |
328 | + FakeResponse(404, 'Zoing!') |
329 | ] |
330 | with self.assertRaises(SourcePackageUploadError) as cm: |
331 | spu.create() |
332 | @@ -60,7 +60,7 @@ |
333 | spu = SourcePackageUpload('1', 'http://server/') |
334 | self.assertIsNone(spu.sandbox_url) |
335 | mocked_post.side_effect = [ |
336 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID') |
337 | + FakeResponse(201, '/ticket/1/sandbox/UUID') |
338 | ] |
339 | spu.create() |
340 | self.assertEquals( |
341 | @@ -82,14 +82,14 @@ |
342 | # `upload` raises an error if it cannot get a tempurl from GK |
343 | # of cannot PUT on the swift tempurl. |
344 | mocked_post.side_effect = [ |
345 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'), |
346 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
347 | ] |
348 | spu = SourcePackageUpload('1', 'http://server/') |
349 | spu.create() |
350 | mocked_put.side_effect = [ |
351 | - FakeRequestsReturn(404, 'Not Found'), |
352 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
353 | - FakeRequestsReturn(401, 'Not Authorized'), |
354 | + FakeResponse(404, 'Not Found'), |
355 | + FakeResponse(200, 'A_TEMP_URL'), |
356 | + FakeResponse(401, 'Not Authorized'), |
357 | ] |
358 | with self.assertRaises(SourcePackageUploadError) as cm: |
359 | spu.upload('not_found_filename') |
360 | @@ -111,13 +111,13 @@ |
361 | # `upload` returns the GK response content (created object |
362 | # sandbox path) when it works. |
363 | mocked_post.side_effect = [ |
364 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'), |
365 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
366 | ] |
367 | spu = SourcePackageUpload('1', 'http://server/') |
368 | spu.create() |
369 | mocked_put.side_effect = [ |
370 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
371 | - FakeRequestsReturn(201, '/a_sandbox_object_url/'), |
372 | + FakeResponse(200, 'A_TEMP_URL'), |
373 | + FakeResponse(201, '/a_sandbox_object_url/'), |
374 | ] |
375 | mocked_of.return_value = 'a_content' |
376 | result = spu.upload('a_file') |
377 | @@ -138,8 +138,8 @@ |
378 | # `process` raises `SourcePackageUploadError` with the GK |
379 | # response content (upload remote validation error). |
380 | mocked_post.side_effect = [ |
381 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'), |
382 | - FakeRequestsReturn(400, 'Problem!'), |
383 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
384 | + FakeResponse(400, 'Problem!'), |
385 | ] |
386 | spu = SourcePackageUpload('1', 'http://server/') |
387 | spu.create() |
388 | @@ -153,8 +153,8 @@ |
389 | # `process` returns the GK response content (upload validation |
390 | # results) when it works. |
391 | mocked_post.side_effect = [ |
392 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'), |
393 | - FakeRequestsReturn(201, 'Success!'), |
394 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
395 | + FakeResponse(201, 'Success!'), |
396 | ] |
397 | spu = SourcePackageUpload('1', 'http://server/') |
398 | spu.create() |
399 | @@ -168,12 +168,12 @@ |
400 | # Using `SourcePackageUpload` as a context manager makes your |
401 | # life easier! |
402 | mocked_post.side_effect = [ |
403 | - FakeRequestsReturn(201, '/ticket/1/sandbox/UUID'), |
404 | - FakeRequestsReturn(201, 'Success!'), |
405 | + FakeResponse(201, '/ticket/1/sandbox/UUID'), |
406 | + FakeResponse(201, 'Success!'), |
407 | ] |
408 | mocked_put.side_effect = [ |
409 | - FakeRequestsReturn(200, 'A_TEMP_URL'), |
410 | - FakeRequestsReturn(201, '/a_sandbox_object_url/'), |
411 | + FakeResponse(200, 'A_TEMP_URL'), |
412 | + FakeResponse(201, '/a_sandbox_object_url/'), |
413 | ] |
414 | mocked_of.return_value = 'a_content' |
415 | |
416 | |
417 | === modified file 'cli/ci_cli/tests/test_utils.py' |
418 | --- cli/ci_cli/tests/test_utils.py 2014-06-19 23:38:34 +0000 |
419 | +++ cli/ci_cli/tests/test_utils.py 2014-07-04 03:45:58 +0000 |
420 | @@ -17,10 +17,12 @@ |
421 | |
422 | import json |
423 | import mock |
424 | +import unittest |
425 | |
426 | from ci_cli import utils |
427 | -from ci_cli.tests import FakeRequestsReturn |
428 | +from ci_cli.tests import FakeResponse |
429 | from ci_utils.testing import TestCaseWithGnupg |
430 | +from requests import exceptions |
431 | |
432 | |
433 | config_contents = """auth_url: http://example.com |
434 | @@ -32,6 +34,62 @@ |
435 | """ |
436 | |
437 | |
438 | +class TestBasicRequest(unittest.TestCase): |
439 | + |
440 | + @mock.patch('requests.get') |
441 | + def test_get(self, mocked_get): |
442 | + mocked_get.side_effect = [FakeResponse(200, '{"foo": ["bar", "baz"]}')] |
443 | + result = utils.get("foo") |
444 | + self.assertEquals(result, {u'foo': [u'bar', u'baz']}) |
445 | + mocked_get.assert_called_once() |
446 | + |
447 | + @mock.patch('requests.get') |
448 | + def test_get_fails(self, mocked_get): |
449 | + mocked_get.side_effect = [FakeResponse(502, {}, "Failed get()!")] |
450 | + with self.assertRaises(exceptions.HTTPError) as cm: |
451 | + utils.get("foo") |
452 | + mocked_get.assert_called_once() |
453 | + self.assertEquals( |
454 | + cm.exception.message, "502 Server Error: Failed get()!") |
455 | + |
456 | + @mock.patch('requests.get') |
457 | + def test_get_returns_invalid_content(self, mocked_get): |
458 | + mocked_get.side_effect = [FakeResponse(200, "Ohh, this is invalid")] |
459 | + with self.assertRaises(ValueError) as cm: |
460 | + utils.get("foo") |
461 | + mocked_get.assert_called_once() |
462 | + self.assertEquals( |
463 | + cm.exception.message, "No JSON object could be decoded") |
464 | + |
465 | + @mock.patch('requests.patch') |
466 | + def test_patch(self, mocked_patch): |
467 | + utils.patch("foo", {}, {}) |
468 | + mocked_patch.assert_called_once() |
469 | + |
470 | + @mock.patch('requests.patch') |
471 | + def test_patch_fails(self, mocked_patch): |
472 | + mocked_patch.side_effect = [FakeResponse(502, {}, "Failed patch()!")] |
473 | + with self.assertRaises(exceptions.HTTPError) as cm: |
474 | + utils.patch("foo", {}, {}) |
475 | + mocked_patch.assert_called_once() |
476 | + self.assertEquals( |
477 | + cm.exception.message, "502 Server Error: Failed patch()!") |
478 | + |
479 | + @mock.patch('requests.post') |
480 | + def test_post(self, mocked_post): |
481 | + utils.post("foo", {}, {}) |
482 | + mocked_post.assert_called_once() |
483 | + |
484 | + @mock.patch('requests.post') |
485 | + def test_post_fails(self, mocked_post): |
486 | + mocked_post.side_effect = [FakeResponse(502, {}, "Failed post()!")] |
487 | + with self.assertRaises(exceptions.HTTPError) as cm: |
488 | + utils.post("foo", {}, {}) |
489 | + mocked_post.assert_called_once() |
490 | + self.assertEquals( |
491 | + cm.exception.message, "502 Server Error: Failed post()!") |
492 | + |
493 | + |
494 | class UtilsTestCase(TestCaseWithGnupg): |
495 | def test_parse_id(self): |
496 | location = 'http://www.example.com/api/v1/ticket/5/' |
497 | @@ -50,7 +108,7 @@ |
498 | def test_get_sourcepackage_uri(self, mock_requests): |
499 | data = {"objects": [{"resource_uri": "/api/v1/sourcepackage/4/"}]} |
500 | mock_requests.side_effect = [ |
501 | - FakeRequestsReturn(200, json.dumps(data)) |
502 | + FakeResponse(200, json.dumps(data)) |
503 | ] |
504 | uri = utils.get_sourcepackage_uri('foobar') |
505 | mock_requests.assert_called_once() |
506 | @@ -60,7 +118,7 @@ |
507 | def test_get_sourcepackage_uri_not_found(self, mock_requests): |
508 | data = {"objects": []} |
509 | mock_requests.side_effect = [ |
510 | - FakeRequestsReturn(200, json.dumps(data)) |
511 | + FakeResponse(200, json.dumps(data)) |
512 | ] |
513 | uri = utils.get_sourcepackage_uri('foobar') |
514 | mock_requests.assert_called_once() |
515 | |
516 | === modified file 'cli/ci_cli/ticket.py' |
517 | --- cli/ci_cli/ticket.py 2014-07-01 03:43:40 +0000 |
518 | +++ cli/ci_cli/ticket.py 2014-07-04 03:45:58 +0000 |
519 | @@ -50,6 +50,10 @@ |
520 | """Raised where there is an error during the SPU.""" |
521 | |
522 | |
523 | +class QueueingTicketError(Exception): |
524 | + """Raised when ticket failed to be set to Queued.""" |
525 | + |
526 | + |
527 | class SourcePackageUpload(object): |
528 | """Context manager helper for performing SPUs via Gatekeeper.""" |
529 | |
530 | @@ -145,18 +149,18 @@ |
531 | return '' |
532 | |
533 | def _parse_changes(self, changes_filepath): |
534 | - log.info("Parsing %s..." % changes_filepath) |
535 | + log.info("Parsing {}...".format(os.path.basename(changes_filepath))) |
536 | changes = ChangesProcessor( |
537 | changes_filepath=changes_filepath) |
538 | - log.info("Validating .changes file...") |
539 | + log.info("Validating .changes...") |
540 | self.files = changes.process() |
541 | self.version = changes.source_version |
542 | self.suite = changes.suite |
543 | self.series = self.suite.split("-")[0] |
544 | self.sourcepackage = changes.source_package_name |
545 | self.version = changes.source_version |
546 | - log.info("%s_%s parsed and validated." % (self.sourcepackage, |
547 | - self.version)) |
548 | + log.info("{}_{} parsed and validated.".format( |
549 | + self.sourcepackage, self.version)) |
550 | |
551 | def _process(self): |
552 | self._create_spu() |
553 | @@ -253,9 +257,16 @@ |
554 | "status": TicketWorkflowStepStatus.QUEUED.value, |
555 | } |
556 | urlbase = utils.CI_URL + utils.API_URL + 'updateticketstatus/' |
557 | - utils.patch(urlbase + self.ticket_id + '/', data=data) |
558 | - print("You have successfully submitted a ticket to the Ubuntu CI" |
559 | - " Engine. Your ticket number is %s." % self.ticket_id) |
560 | + try: |
561 | + utils.patch(urlbase + self.ticket_id + '/', data=data) |
562 | + print("You have successfully submitted a ticket to the Ubuntu CI " |
563 | + "Engine. Your ticket number is %s." % self.ticket_id) |
564 | + except Exception as exc: |
565 | + raise QueueingTicketError( |
566 | + "Ticket was created successfully but there was a problem " |
567 | + "adding it to the queue: {}. This should be a transient " |
568 | + "error, please contact an admin. Your ticket number is " |
569 | + "{}.".format(exc, self.ticket_id)) |
570 | |
571 | def add_new_ticket(self, args): |
572 | suites = [] |
573 | |
574 | === modified file 'cli/ci_cli/utils.py' |
575 | --- cli/ci_cli/utils.py 2014-06-20 14:14:15 +0000 |
576 | +++ cli/ci_cli/utils.py 2014-07-04 03:45:58 +0000 |
577 | @@ -42,19 +42,21 @@ |
578 | |
579 | |
580 | def post(url, data, headers=HEADERS, patch=False): |
581 | - req = requests.post(url=url, data=json.dumps(data), headers=HEADERS) |
582 | - resp = req.headers |
583 | - return resp['location'] |
584 | + response = requests.post(url=url, data=json.dumps(data), headers=HEADERS) |
585 | + response.raise_for_status() |
586 | + headers = response.headers |
587 | + return headers['location'] |
588 | |
589 | |
590 | def patch(url, data, headers=HEADERS): |
591 | - requests.patch(url=url, data=json.dumps(data), headers=HEADERS) |
592 | + response = requests.patch(url=url, data=json.dumps(data), headers=HEADERS) |
593 | + response.raise_for_status() |
594 | |
595 | |
596 | def get(url): |
597 | - r = requests.get(url) |
598 | - r.raise_for_status() |
599 | - return r.json() |
600 | + response = requests.get(url) |
601 | + response.raise_for_status() |
602 | + return response.json() |
603 | |
604 | |
605 | def parse_id(location): |
606 | |
607 | === modified file 'cli/ubuntu-ci' |
608 | --- cli/ubuntu-ci 2014-07-01 03:43:40 +0000 |
609 | +++ cli/ubuntu-ci 2014-07-04 03:45:58 +0000 |
610 | @@ -184,7 +184,8 @@ |
611 | else: |
612 | log.error(error) |
613 | except (ticket.SourcePackageUploadError, |
614 | - ticket.MismatchSuitesError) as exc: |
615 | + ticket.MismatchSuitesError, |
616 | + ticket.QueueingTicketError) as exc: |
617 | log.error(exc.message) |
618 | except Exception: |
619 | log.exception('Unexpected exception') |
PASSED: Continuous integration, rev:647 s-jenkins. ubuntu- ci:8080/ job/uci- engine- ci/1013/
http://
Executed test runs:
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/uci- engine- ci/1013/ rebuild
http://