Merge lp:~cjohnston/launchpad/short-ic-emails into lp:launchpad

Proposed by Chris Johnston
Status: Rejected
Rejected by: William Grant
Proposed branch: lp:~cjohnston/launchpad/short-ic-emails
Merge into: lp:launchpad
Diff against target: 1111 lines (+883/-60)
17 files modified
lib/lp/code/mail/codereviewcomment.py (+47/-12)
lib/lp/code/mail/tests/test_codereviewcomment.py (+189/-46)
lib/lp/services/patches.py (+215/-0)
lib/lp/services/tests/data/diff-1 (+30/-0)
lib/lp/services/tests/data/diff-2 (+2/-0)
lib/lp/services/tests/data/diff-3 (+14/-0)
lib/lp/services/tests/data/expected-1 (+35/-0)
lib/lp/services/tests/data/expected-2 (+12/-0)
lib/lp/services/tests/data/expected-3 (+22/-0)
lib/lp/services/tests/data/expected-4 (+14/-0)
lib/lp/services/tests/data/expected-5 (+39/-0)
lib/lp/services/tests/data/expected-6 (+44/-0)
lib/lp/services/tests/data/expected-7 (+55/-0)
lib/lp/services/tests/data/insert_top.patch (+7/-0)
lib/lp/services/tests/data/tricky-diff.patch (+26/-0)
lib/lp/services/tests/test_patches.py (+127/-0)
lib/lp/testing/factory.py (+5/-2)
To merge this branch: bzr merge lp:~cjohnston/launchpad/short-ic-emails
Reviewer Review Type Date Requested Status
William Grant code Needs Fixing
Celso Providelo (community) Approve
Review via email: mp+225095@code.launchpad.net

Commit message

First pass at reducing the amount of code that is emailed out when someone saves diff comments

Description of the change

This branch makes a first pass at reducing the amount of code that is emailed out when someone saves diff comments. The way this is done is by discarding hunks that do not have diff comments.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Needs Fixing (code)
Revision history for this message
Celso Providelo (cprov) wrote :

Chris,

The code seems to be working as expected, code hunks without comments are suppressed in the email.

However, there are few points that needs consideration IMO:

* Code copied from bzrlib needs to be removed (reused from bzrlib, maybe with accessors) or, in last instance, copied to a different and isolated place.

* Test coverage seems to be enough but the testing infrastructure (bzrlib TestCase should not be needed) and test names and descriptions needs update.

* The way we load test inputs and outputs from files is nice to make the tests less noisy, but at the same time makes it very hard to understand. I don't think there is an easy solution here, but surely the problem can be alleviated with better test description(i.e. 'binary file mentions in diff are properly presented.')

* Finally, the core code changes (build_inline_comments_section) can be simplified by replacing the boolean flags with continue and some use some local functions. That would make the code clearer.

review: Needs Information
Revision history for this message
Celso Providelo (cprov) wrote :

lib/lp/services/patches.py is a much better place, the code style there still alien to LP, it reads very differently. But I am not opposed to landing it this way if you think it will be closer to the original bzrlib, thus easier to maintain up to date.

Looks good for landing and get some feedback from users, possibly experimenting with including previous comments too.

Good work!

review: Approve
Revision history for this message
William Grant (wgrant) wrote :

lp.services.patches should say somewhere that it's derived from bzrlib.patches, what the changes are, and why. It might also be possible to push these improvements back to bzrlib.patches in a backward-compatible way later.

The new set of test files doesn't give any tests for inline comments across multiple files, just multiple hunks. That should probably be fixed.

Revision history for this message
William Grant (wgrant) wrote :

Consider '\n'.join(whatever) instead of '{}{}{}'.format(whatever) to get a more realistic diff with blank lines between files.

Revision history for this message
William Grant (wgrant) wrote :

Line numbers jump ahead by one at the boundary between file, as visible in expected-6: the comment in the second file is rendered a line too late, and the comment in the third is two lines late. I assume the trailing newline isn't being counted.

Also, it probably makes sense to have a gap between hunks, at least when one is skipped. In expected-6, the hunk at 70 seems to lead directly into 88, but in the original there's one at 80.

Additionally, does expected-1 give any value over expected-6?

review: Needs Fixing (code)
Revision history for this message
Chris Johnston (cjohnston) wrote :

> Also, it probably makes sense to have a gap between hunks, at least when one
> is skipped. In expected-6, the hunk at 70 seems to lead directly into 88, but
> in the original there's one at 80.

How do you suggest doing the gap? A blank line or a line that starts with '>', or maybe even some sort of text to indicate that a hunk is skipped? Also, how do you want to handle if multiple hunks in a row (or even a whole file) are mising? One line for each hunk missing, one line total, if a file is missing and it's the first file should we start out with a blank line? I almost think that we should see what type of feedback we get the way things are and then iterate again.

> Additionally, does expected-1 give any value over expected-6?

Just the fact that it's a very simple diff with only one file, so just trying to cover one more case where things are different.

Revision history for this message
William Grant (wgrant) wrote :

This is now broken in the opposite direction. If there is no gap between
two files, the comments end up a line too early.

On 16/07/14 03:35, Chris Johnston wrote:
>> Also, it probably makes sense to have a gap between hunks, at least when one
>> is skipped. In expected-6, the hunk at 70 seems to lead directly into 88, but
>> in the original there's one at 80.
>
> How do you suggest doing the gap? A blank line or a line that starts with '>', or maybe even some sort of text to indicate that a hunk is skipped? Also, how do you want to handle if multiple hunks in a row (or even a whole file) are mising? One line for each hunk missing, one line total, if a file is missing and it's the first file should we start out with a blank line? I almost think that we should see what type of feedback we get the way things are and then iterate again.

I think just a blank line makes sense. A line starting with ">" would
indicate that it was quoting the diff, which is false.

Revision history for this message
Chris Johnston (cjohnston) wrote :

I have updated the MP to handle a nl between patches or no nl between patches. I also added a blank line when there is a skipped hunk.

Revision history for this message
William Grant (wgrant) :
Revision history for this message
William Grant (wgrant) wrote :

This breaks three tests in TestInlineCommentsSection, and I have a few other comments.

review: Needs Fixing (code)
Revision history for this message
Chris Johnston (cjohnston) :
Revision history for this message
William Grant (wgrant) :
Revision history for this message
Chris Johnston (cjohnston) :
Revision history for this message
Chris Johnston (cjohnston) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/mail/codereviewcomment.py'
2--- lib/lp/code/mail/codereviewcomment.py 2014-05-21 08:52:53 +0000
3+++ lib/lp/code/mail/codereviewcomment.py 2014-07-29 15:41:46 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Email notifications for code review comments."""
10@@ -26,6 +26,7 @@
11 append_footer,
12 format_address,
13 )
14+from lp.services.patches import parse_patches
15 from lp.services.webapp import canonical_url
16
17
18@@ -171,16 +172,50 @@
19
20 def build_inline_comments_section(comments, diff_text):
21 """Return a formatted text section with contextualized comments."""
22- result_lines = []
23- diff_lines = diff_text.splitlines()
24- for num, line in enumerate(diff_lines, 1):
25- result_lines.append(u'> {0}'.format(line.decode('utf-8', 'replace')))
26- comment = comments.get(str(num))
27- if comment is not None:
28- result_lines.append('')
29- result_lines.extend(comment.splitlines())
30- result_lines.append('')
31-
32- result_text = u'\n'.join(result_lines)
33+ result_diff = []
34+ lines = diff_text.splitlines(True)
35+ patches = parse_patches(lines, allow_dirty=True)
36+ line_count = 0
37+ for patch in patches:
38+ patch_lines = []
39+ patch_has_ic = False
40+ headers = patch.get_header()
41+ has_header_ic = False
42+ header_lines = []
43+ for line in headers.splitlines():
44+ line_count += 1
45+ comment = comments.get(str(line_count))
46+ header_lines.append(u'> {0}'.format(
47+ line.decode('utf-8', 'replace')))
48+ if comment is not None:
49+ header_lines.append('')
50+ header_lines.extend(comment.splitlines())
51+ header_lines.append('')
52+ has_header_ic = True
53+
54+ for p in patch.hunks:
55+ hunk_lines = []
56+ has_ic = False
57+ raw_diff = str(p).splitlines()
58+ for num, line in enumerate(raw_diff, 1):
59+ line_count += 1
60+ hunk_lines.append(u'> {0}'.format(
61+ line.decode('utf-8', 'replace')))
62+ comment = comments.get(str(line_count))
63+ if comment is not None:
64+ hunk_lines.append('')
65+ hunk_lines.extend(comment.splitlines())
66+ hunk_lines.append('')
67+ has_ic = True
68+ if has_ic:
69+ patch_lines += hunk_lines
70+ patch_has_ic = True
71+ else:
72+ if not patch_lines or patch_lines[-1] != u'':
73+ patch_lines.append('')
74+ if has_header_ic or patch_has_ic:
75+ result_diff += header_lines + patch_lines
76+
77+ result_text = u'\n'.join(result_diff)
78
79 return '\n\nDiff comments:\n\n%s\n\n' % result_text
80
81=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
82--- lib/lp/code/mail/tests/test_codereviewcomment.py 2014-05-21 09:33:04 +0000
83+++ lib/lp/code/mail/tests/test_codereviewcomment.py 2014-07-29 15:41:46 +0000
84@@ -1,8 +1,9 @@
85-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
86+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
87 # GNU Affero General Public License version 3 (see the file LICENSE).
88
89 """Test CodeReviewComment emailing functionality."""
90
91+import os
92
93 import testtools
94 import transaction
95@@ -31,6 +32,12 @@
96 from lp.testing.layers import LaunchpadFunctionalLayer
97
98
99+def datafile(filename):
100+ data_path = os.path.join(os.path.dirname(__file__),
101+ "../../../services/tests/data", filename)
102+ return file(data_path, "rb")
103+
104+
105 class TestCodeReviewComment(TestCaseWithFactory):
106 """Test that comments are generated as expected."""
107
108@@ -215,13 +222,14 @@
109 mailer.message.text_contents.splitlines())
110
111 def makeCommentWithInlineComments(self, subject=None, content=None,
112- inline_comments=None):
113+ inline_comments=None, diff_text=None):
114 """Create a `CodeReviewComment` with inline (diff) comments."""
115 bmp = self.factory.makeBranchMergeProposal()
116 bmp.source_branch.subscribe(bmp.registrant,
117 BranchSubscriptionNotificationLevel.NOEMAIL, None,
118 CodeReviewNotificationLevel.FULL, bmp.registrant)
119- previewdiff = self.factory.makePreviewDiff(merge_proposal=bmp)
120+ previewdiff = self.factory.makePreviewDiff(merge_proposal=bmp,
121+ diff_text=diff_text)
122 transaction.commit()
123 if subject is None:
124 subject = 'A comment'
125@@ -235,14 +243,180 @@
126 inline_comments=inline_comments)
127 return comment
128
129- def test_generateEmailWithInlineComments(self):
130- """Review comments emails consider the inline comments.
131-
132- See `build_inline_comments_section` tests for formatting details.
133- """
134- # Enabled corresponding feature flag.
135- self.useContext(feature_flags())
136- set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
137+ # For the test_inline_comments_email_* tests, please see
138+ # `build_inline_comments_section` tests for formatting details.
139+
140+ def test_inline_comments_email_text(self):
141+ # Testing that a single file with three hunks is parsed properly and
142+ # only the header/hunk combinations with comments are provided.
143+
144+ # Enabled corresponding feature flag.
145+ self.useContext(feature_flags())
146+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
147+ diff = datafile("diff-1").read()
148+ expected = datafile("expected-1").read().decode().splitlines()
149+
150+ comment = self.makeCommentWithInlineComments(
151+ inline_comments={'9': u'Cool',
152+ '27': u'Are you sure?'},
153+ diff_text=diff)
154+ mailer = CodeReviewCommentMailer.forCreation(comment)
155+ commenter = comment.branch_merge_proposal.registrant
156+ ctrl = mailer.generateEmail(
157+ commenter.preferredemail.email, commenter)
158+
159+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
160+
161+ def test_inline_comments_email_text_binary(self):
162+ # Testing that a single file with three hunks and a binary file is
163+ # parsed properly and only the binary header with comments are provided
164+
165+ # Enabled corresponding feature flag.
166+ self.useContext(feature_flags())
167+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
168+ diff_1 = datafile("diff-1").read()
169+ diff_2 = datafile("diff-2").read()
170+ diff = u'\n'.join([diff_1, diff_2])
171+ expected = datafile("expected-2").read().decode().splitlines()
172+
173+ comment = self.makeCommentWithInlineComments(
174+ inline_comments={'33': u'Why are you adding this?'},
175+ diff_text=diff)
176+ mailer = CodeReviewCommentMailer.forCreation(comment)
177+ commenter = comment.branch_merge_proposal.registrant
178+ ctrl = mailer.generateEmail(
179+ commenter.preferredemail.email, commenter)
180+
181+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
182+
183+ def test_inline_comments_email_binary(self):
184+ # Testing that a binary file is parsed properly and only the binary
185+ # header with comments are provided
186+
187+ # Enabled corresponding feature flag.
188+ self.useContext(feature_flags())
189+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
190+ diff = datafile("diff-2").read()
191+ expected = datafile("expected-2").read().decode().splitlines()
192+
193+ comment = self.makeCommentWithInlineComments(
194+ inline_comments={'2': u'Why are you adding this?'},
195+ diff_text=diff)
196+ mailer = CodeReviewCommentMailer.forCreation(comment)
197+ commenter = comment.branch_merge_proposal.registrant
198+ ctrl = mailer.generateEmail(
199+ commenter.preferredemail.email, commenter)
200+
201+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
202+
203+ def test_inline_comments_email_text_binary_text(self):
204+ # Testing that a single file with three hunks, a binary file and a
205+ # text file is parsed properly and only the binary header with comments
206+ # are provided
207+
208+ # Enabled corresponding feature flag.
209+ self.useContext(feature_flags())
210+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
211+ diff_1 = datafile("diff-1").read()
212+ diff_3 = datafile("diff-3").read()
213+ diff = u'\n'.join([diff_1, diff_3])
214+ expected = datafile("expected-3").read().decode().splitlines()
215+
216+ comment = self.makeCommentWithInlineComments(
217+ inline_comments={'42': u'What is this?'},
218+ diff_text=diff)
219+ mailer = CodeReviewCommentMailer.forCreation(comment)
220+ commenter = comment.branch_merge_proposal.registrant
221+ ctrl = mailer.generateEmail(
222+ commenter.preferredemail.email, commenter)
223+
224+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
225+
226+ def test_inline_comments_email_text_binary_text_includes_two_files(self):
227+ # Testing that a single file with three hunks, a binary file and a
228+ # text file is parsed properly and a single hunk from the first file
229+ # plus a single hunk from the second file with comments are provided
230+
231+ # Enabled corresponding feature flag.
232+ self.useContext(feature_flags())
233+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
234+ diff_1 = datafile("diff-1").read()
235+ diff_3 = datafile("diff-3").read()
236+ diff = u'\n'.join([diff_1, diff_3])
237+ expected = datafile("expected-5").read().decode().splitlines()
238+
239+ comment = self.makeCommentWithInlineComments(
240+ inline_comments={'18': u'Why?',
241+ '42': u'Good catch!'},
242+ diff_text=diff)
243+ mailer = CodeReviewCommentMailer.forCreation(comment)
244+ commenter = comment.branch_merge_proposal.registrant
245+ ctrl = mailer.generateEmail(
246+ commenter.preferredemail.email, commenter)
247+
248+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
249+
250+ def test_inline_comments_email_text_binary_text_includes_three_files(self):
251+ # Testing that a single file with three hunks, a binary file and a
252+ # text file is parsed properly and two hunks from the first file,
253+ # the second file and a single hunk from the second file with comments
254+ # are provided
255+
256+ # Enabled corresponding feature flag.
257+ self.useContext(feature_flags())
258+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
259+ diff_1 = datafile("diff-1").read()
260+ diff_3 = datafile("diff-3").read()
261+ diff = u'\n'.join([diff_1, diff_3])
262+ expected = datafile("expected-6").read().decode().splitlines()
263+
264+ comment = self.makeCommentWithInlineComments(
265+ inline_comments={'27': u'+1',
266+ '33': u'Do we need this?',
267+ '42': u'Can we change this?'},
268+ diff_text=diff)
269+ mailer = CodeReviewCommentMailer.forCreation(comment)
270+ commenter = comment.branch_merge_proposal.registrant
271+ ctrl = mailer.generateEmail(
272+ commenter.preferredemail.email, commenter)
273+
274+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
275+
276+ def test_inline_comments_email_text_binary_text_no_nl_files(self):
277+ # Testing that a single file with three hunks, a binary file and a
278+ # text file which do not have a new line between each other is parsed
279+ # properly and two hunks from the first file, the second file and a
280+ # single hunk from the second file with comments are provided
281+
282+ # Enabled corresponding feature flag.
283+ self.useContext(feature_flags())
284+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
285+ diff_1 = datafile("diff-1").read()
286+ diff_3 = datafile("diff-3").read()
287+ diff = u''.join([diff_1, diff_3])
288+ expected = datafile("expected-7").read().decode().splitlines()
289+
290+ comment = self.makeCommentWithInlineComments(
291+ inline_comments={'9': u'Good stuff',
292+ '27': u'+1',
293+ '32': u'Do we need this?',
294+ '41': u'Can we change this?'},
295+ diff_text=diff)
296+ mailer = CodeReviewCommentMailer.forCreation(comment)
297+ commenter = comment.branch_merge_proposal.registrant
298+ ctrl = mailer.generateEmail(
299+ commenter.preferredemail.email, commenter)
300+
301+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
302+
303+ def test_inline_comments_email_text_comment_in_header(self):
304+ # Testing that a single file with comments in the header is parsed
305+ # properly and only the headers is provided
306+
307+ # Enabled corresponding feature flag.
308+ self.useContext(feature_flags())
309+ set_feature_flag(u'code.inline_diff_comments.enabled', u'enabled')
310+ expected = datafile("expected-4").read().decode('utf-8').splitlines()
311
312 comment = self.makeCommentWithInlineComments(
313 inline_comments={'2': u'Is this from Planet Earth\xa9 ?'})
314@@ -251,20 +425,7 @@
315 ctrl = mailer.generateEmail(
316 commenter.preferredemail.email, commenter)
317
318- expected_lines = [
319- '',
320- 'Diff comments:',
321- '',
322- "> === zbqvsvrq svyr 'yvo/yc/pbqr/vagresnprf/qvss.cl'",
323- '> --- yvo/yc/pbqr/vagresnprf/qvss.cl '
324- '2009-10-01 13:25:12 +0000',
325- '',
326- u'Is this from Planet Earth\xa9 ?',
327- '',
328- '> +++ yvo/yc/pbqr/vagresnprf/qvss.cl '
329- '2010-02-02 15:48:56 +0000'
330- ]
331- self.assertEqual(expected_lines, ctrl.body.splitlines()[1:10])
332+ self.assertEqual(expected, ctrl.body.splitlines()[:-2])
333
334 def test_generateEmailWithInlineComments_feature_disabled(self):
335 """Inline comments are not considered if the flag is not enabled."""
336@@ -425,24 +586,6 @@
337 diff_text = self.diff_text
338 return build_inline_comments_section(comments, diff_text)
339
340- def test_section_header_and_footer(self):
341- # The inline comments section starts with a 4-lines header
342- # (empty lines and title) and ends with an empty line.
343- section = self.getSection({}).splitlines()
344- header = section[:5]
345- self.assertEqual(
346- ['',
347- '',
348- 'Diff comments:',
349- '',
350- '> --- bar\t2009-08-26 15:53:34.000000000 -0400'],
351- header)
352- footer = section[-2:]
353- self.assertEqual(
354- ['> +e',
355- ''],
356- footer)
357-
358 def test_single_line_comment(self):
359 # The inline comments are correctly contextualized in the diff.
360 # and prefixed with '>>> '
361@@ -452,8 +595,8 @@
362 '',
363 u'\u03b4\u03bf\u03ba\u03b9\u03bc\u03ae',
364 '',
365- '> @@ -1,3 +0,0 @@',
366- u'> -\xe5'],
367+ '',
368+ u''],
369 self.getSection(comments).splitlines()[5:11])
370
371 def test_multi_line_comment(self):
372@@ -465,7 +608,7 @@
373 'Foo',
374 'Bar',
375 '',
376- '> @@ -1,3 +0,0 @@'],
377+ ''],
378 self.getSection(comments).splitlines()[5:11])
379
380 def test_multiple_comments(self):
381
382=== added file 'lib/lp/services/patches.py'
383--- lib/lp/services/patches.py 1970-01-01 00:00:00 +0000
384+++ lib/lp/services/patches.py 2014-07-29 15:41:46 +0000
385@@ -0,0 +1,215 @@
386+# Copyright 2014 Canonical Ltd. This software is licensed under the
387+# GNU Affero General Public License version 3 (see the file LICENSE).
388+
389+# This file is derived from bzrlib.patches in order to extend some
390+# functionality for use by the Launchpad inline comment feature.
391+
392+# In order to reduce the size of a diff being emailed with inline comments
393+# to just the relevant hunks of code, it was needed to break the diff down
394+# and then put the required pieces back together. To do this, the notion of
395+# 'comments' was added to Patch and BinaryPatch in order to return any lines
396+# beginning with '===' which is later added to the file headers and the line
397+# counted.
398+
399+import re
400+
401+from bzrlib.errors import (
402+ BzrError,
403+ MalformedHunkHeader,
404+ MalformedPatchHeader,
405+ PatchSyntax,
406+ )
407+from bzrlib.patches import (
408+ binary_files_re,
409+ ContextLine,
410+ hunk_from_header,
411+ InsertLine,
412+ iter_lines_handle_nl,
413+ parse_line,
414+ RemoveLine,
415+ )
416+
417+
418+binary_regex = re.compile(binary_files_re)
419+
420+
421+class BinaryFiles(BzrError):
422+
423+ _fmt = 'Binary files section encountered.'
424+
425+ def __init__(self, comments, orig_name, mod_name):
426+ self.comments = comments
427+ self.orig_name = orig_name
428+ self.mod_name = mod_name
429+
430+
431+class BinaryPatch(object):
432+ def __init__(self, comments, oldname, newname):
433+ self.oldname = oldname
434+ self.newname = newname
435+ self.comments = comments
436+ self.hunks = []
437+
438+ def __str__(self):
439+ return self.get_header()
440+
441+ def get_header(self):
442+ header = 'Binary files %s and %s differ\n' % (
443+ self.oldname, self.newname)
444+ return ''.join(self.comments + [header])
445+
446+
447+class Patch(BinaryPatch):
448+ def __init__(self, comments, oldname, newname):
449+ BinaryPatch.__init__(self, comments, oldname, newname)
450+ self.hunks = []
451+ self.comments = comments
452+
453+ def __str__(self):
454+ ret = self.get_header()
455+ ret += "".join([str(h) for h in self.hunks])
456+ return ret
457+
458+ def get_header(self):
459+ comments = ''.join(self.comments)
460+ return "%s--- %s\n+++ %s\n" % (comments, self.oldname, self.newname)
461+
462+
463+def get_patch_names(iter_lines):
464+ comments = []
465+ for line in iter_lines:
466+ if (line == "\n" or line.startswith('=== ') or line.startswith('*** ')
467+ or line.startswith('#')):
468+ comments.append(line)
469+ else:
470+ break
471+ try:
472+ match = binary_regex.match(line)
473+ if match is not None:
474+ # For binary packages we need to check and see if there is a \n
475+ # that follows the header info. If so, we need to carry it.
476+ raise BinaryFiles(comments, match.group(1), match.group(2))
477+ if not line.startswith("--- "):
478+ raise MalformedPatchHeader("No orig name", line)
479+ else:
480+ orig_name = line[4:].rstrip("\n")
481+ except StopIteration:
482+ raise MalformedPatchHeader("No orig line", "")
483+ try:
484+ line = iter_lines.next()
485+ if not line.startswith("+++ "):
486+ raise PatchSyntax("No mod name")
487+ else:
488+ mod_name = line[4:].rstrip("\n")
489+ except StopIteration:
490+ raise MalformedPatchHeader("No mod line", "")
491+ return (comments, orig_name, mod_name)
492+
493+
494+def iter_hunks(iter_lines, allow_dirty=False):
495+ '''
496+ :arg iter_lines: iterable of lines to parse for hunks
497+ :kwarg allow_dirty: If True, when we encounter something that is not
498+ a hunk header when we're looking for one, assume the rest of the lines
499+ are not part of the patch (comments or other junk). Default False
500+ '''
501+ hunk = None
502+ for line in iter_lines:
503+ if line == "\n":
504+ if hunk is not None:
505+ hunk.lines.append(line)
506+ yield hunk
507+ hunk = None
508+ continue
509+ if hunk is not None:
510+ yield hunk
511+ try:
512+ hunk = hunk_from_header(line)
513+ except MalformedHunkHeader:
514+ if allow_dirty:
515+ # If the line isn't a hunk header, then we've reached the end
516+ # of this patch and there's "junk" at the end. Ignore the
517+ # rest of this patch.
518+ return
519+ raise
520+ orig_size = 0
521+ mod_size = 0
522+ while orig_size < hunk.orig_range or mod_size < hunk.mod_range:
523+ hunk_line = parse_line(iter_lines.next())
524+ hunk.lines.append(hunk_line)
525+ if isinstance(hunk_line, (RemoveLine, ContextLine)):
526+ orig_size += 1
527+ if isinstance(hunk_line, (InsertLine, ContextLine)):
528+ mod_size += 1
529+ if hunk is not None:
530+ yield hunk
531+
532+
533+def iter_file_patch(iter_lines, allow_dirty=False):
534+ '''
535+ :arg iter_lines: iterable of lines to parse for patches
536+ :kwarg allow_dirty: If True, allow comments and other non-patch text
537+ before the first patch. Note that the algorithm here can only find
538+ such text before any patches have been found. Comments after the
539+ first patch are stripped away in iter_hunks() if it is also passed
540+ allow_dirty=True. Default False.
541+ '''
542+ ### FIXME: Docstring is not quite true. We allow certain comments no
543+ # matter what, If they startwith '===', '***', or '#' Someone should
544+ # reexamine this logic and decide if we should include those in
545+ # allow_dirty or restrict those to only being before the patch is found
546+ # (as allow_dirty does).
547+ saved_lines = []
548+ for line in iter_lines:
549+ # if it's a === or binary it's a new patch, but we might have
550+ # both together.
551+ if line.startswith('=== '):
552+ # assume a new patch until we know differently
553+ if len(saved_lines) > 0:
554+ yield saved_lines
555+ saved_lines = []
556+ saved_lines.append(line)
557+ elif binary_regex.match(line):
558+ # check for preceeding '=== ' line
559+ # a binary patch always yields the previous patch
560+ if len(saved_lines) > 0:
561+ saved_lines.append(line)
562+ else:
563+ yield [line]
564+ else:
565+ saved_lines.append(line)
566+ if len(saved_lines) > 0:
567+ yield saved_lines
568+
569+
570+def parse_patch(iter_lines, allow_dirty=False):
571+ '''
572+ :arg iter_lines: iterable of lines to parse
573+ :kwarg allow_dirty: If True, allow the patch to have trailing junk.
574+ Default False
575+ '''
576+ iter_lines = iter_lines_handle_nl(iter_lines)
577+ try:
578+ (comments, orig_name, mod_name) = get_patch_names(iter_lines)
579+ except BinaryFiles, e:
580+ return BinaryPatch(e.comments, e.orig_name, e.mod_name)
581+ else:
582+ patch = Patch(comments, orig_name, mod_name)
583+ for hunk in iter_hunks(iter_lines, allow_dirty):
584+ patch.hunks.append(hunk)
585+ return patch
586+
587+
588+def parse_patches(iter_lines, allow_dirty=False):
589+ '''
590+ :arg iter_lines: iterable of lines to parse for patches
591+ :kwarg allow_dirty: If True, allow text that's not part of the patch at
592+ selected places. This includes comments before and after a patch
593+ for instance. Default False.
594+ '''
595+ patches = []
596+ for f in iter_file_patch(iter_lines, allow_dirty):
597+ patch = parse_patch(f.__iter__(), allow_dirty)
598+ patches.append(patch)
599+
600+ return patches
601
602=== added directory 'lib/lp/services/tests/data'
603=== added file 'lib/lp/services/tests/data/diff-1'
604--- lib/lp/services/tests/data/diff-1 1970-01-01 00:00:00 +0000
605+++ lib/lp/services/tests/data/diff-1 2014-07-29 15:41:46 +0000
606@@ -0,0 +1,30 @@
607+=== zbqvsvrq svyr 'yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf'
608+--- yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 12:32:44 +0000
609++++ yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 23:47:16 +0000
610+@@ -70,7 +70,7 @@
611+ "jf.bc=trgVayvarPbzzragf&cerivrjqvss_vq=1",
612+ zbpxvb.erdhrfgf[0].pbasvt.qngn);
613+ zbpxvb.ynfg_erdhrfg = zbpxvb.erdhrfgf[0];
614+- ine abj = (arj Qngr).inyhrBs()
615++ ine abj = (arj Qngr).inyhrBs();
616+ choyvfurq_pbzzragf = [
617+ {'yvar_ahzore': '2',
618+ 'crefba': crefba_bow,
619+@@ -80,7 +80,7 @@
620+ 'crefba': crefba_bow,
621+ 'grkg': 'Guvf vf terng.',
622+ 'qngr': (arj Qngr(abj - 12600000)),
623+- },
624++ }
625+ ];
626+ zbpxvb.fhpprff({
627+ erfcbafrGrkg: L.WFBA.fgevatvsl(choyvfurq_pbzzragf),
628+@@ -88,7 +88,7 @@
629+
630+ // Choyvfurq pbzzrag vf qvfcynlrq.
631+ ine svefg_pbzzragf = L.bar('#qvss-yvar-2').arkg().bar('qvi');
632+- ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq')
633++ ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq');
634+ L.Nffreg.nerRdhny(
635+ 'Sbb One (anzr16) jebgr ba 2012-08-12:',
636+ svefg.bar('.obneqPbzzragQrgnvyf').trg('grkg'));
637
638=== added file 'lib/lp/services/tests/data/diff-2'
639--- lib/lp/services/tests/data/diff-2 1970-01-01 00:00:00 +0000
640+++ lib/lp/services/tests/data/diff-2 2014-07-29 15:41:46 +0000
641@@ -0,0 +1,2 @@
642+=== nqqrq svyr 'jrohv/obql_ot.wct'
643+Binary files jrohv/obql_ot.wct1970-01-01 00:00:00 +0000 and jrohv/obql_ot.wct19702014-01-31 16:57:19 +0000 differ
644
645=== added file 'lib/lp/services/tests/data/diff-3'
646--- lib/lp/services/tests/data/diff-3 1970-01-01 00:00:00 +0000
647+++ lib/lp/services/tests/data/diff-3 2014-07-29 15:41:46 +0000
648@@ -0,0 +1,14 @@
649+=== nqqrq svyr 'jrohv/obql_ot.wct'
650+Binary files jrohv/obql_ot.wct1970-01-01 00:00:00 +0000 and jrohv/obql_ot.wct19702014-01-31 16:57:19 +0000 differ
651+=== zbqvsvrq svyr 'yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf'
652+--- yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 12:32:44 +0000
653++++ yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 23:47:16 +0000
654+@@ -3,7 +3,7 @@
655+ ine grfgf = L.anzrfcnpr('qngr.grfg');
656+ grfgf.fhvgr = arj L.Grfg.Fhvgr("qngr grfgf");
657+
658+- ine abj = (arj Qngr).inyhrBs()
659++ ine abj = (arj Qngr).inyhrBs();
660+ grfgf.fhvgr.nqq(arj L.Grfg.Pnfr({
661+ anzr: 'grfg_nccebkvzngrqngr',
662+
663
664=== added file 'lib/lp/services/tests/data/expected-1'
665--- lib/lp/services/tests/data/expected-1 1970-01-01 00:00:00 +0000
666+++ lib/lp/services/tests/data/expected-1 2014-07-29 15:41:46 +0000
667@@ -0,0 +1,35 @@
668+
669+
670+Diff comments:
671+
672+> === zbqvsvrq svyr 'yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf'
673+> --- yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 12:32:44 +0000
674+> +++ yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 23:47:16 +0000
675+> @@ -70,7 +70,7 @@
676+> "jf.bc=trgVayvarPbzzragf&cerivrjqvss_vq=1",
677+> zbpxvb.erdhrfgf[0].pbasvt.qngn);
678+> zbpxvb.ynfg_erdhrfg = zbpxvb.erdhrfgf[0];
679+> - ine abj = (arj Qngr).inyhrBs()
680+> + ine abj = (arj Qngr).inyhrBs();
681+
682+Cool
683+
684+> choyvfurq_pbzzragf = [
685+> {'yvar_ahzore': '2',
686+> 'crefba': crefba_bow,
687+
688+> @@ -88,7 +88,7 @@
689+>
690+> // Choyvfurq pbzzrag vf qvfcynlrq.
691+> ine svefg_pbzzragf = L.bar('#qvss-yvar-2').arkg().bar('qvi');
692+> - ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq')
693+> + ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq');
694+
695+Are you sure?
696+
697+> L.Nffreg.nerRdhny(
698+> 'Sbb One (anzr16) jebgr ba 2012-08-12:',
699+> svefg.bar('.obneqPbzzragQrgnvyf').trg('grkg'));
700+
701+
702+--
703
704=== added file 'lib/lp/services/tests/data/expected-2'
705--- lib/lp/services/tests/data/expected-2 1970-01-01 00:00:00 +0000
706+++ lib/lp/services/tests/data/expected-2 2014-07-29 15:41:46 +0000
707@@ -0,0 +1,12 @@
708+
709+
710+Diff comments:
711+
712+> === nqqrq svyr 'jrohv/obql_ot.wct'
713+> Binary files jrohv/obql_ot.wct1970-01-01 00:00:00 +0000 and jrohv/obql_ot.wct19702014-01-31 16:57:19 +0000 differ
714+
715+Why are you adding this?
716+
717+
718+
719+--
720
721=== added file 'lib/lp/services/tests/data/expected-3'
722--- lib/lp/services/tests/data/expected-3 1970-01-01 00:00:00 +0000
723+++ lib/lp/services/tests/data/expected-3 2014-07-29 15:41:46 +0000
724@@ -0,0 +1,22 @@
725+
726+
727+Diff comments:
728+
729+> === zbqvsvrq svyr 'yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf'
730+> --- yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 12:32:44 +0000
731+> +++ yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 23:47:16 +0000
732+> @@ -3,7 +3,7 @@
733+> ine grfgf = L.anzrfcnpr('qngr.grfg');
734+> grfgf.fhvgr = arj L.Grfg.Fhvgr("qngr grfgf");
735+>
736+> - ine abj = (arj Qngr).inyhrBs()
737+> + ine abj = (arj Qngr).inyhrBs();
738+
739+What is this?
740+
741+> grfgf.fhvgr.nqq(arj L.Grfg.Pnfr({
742+> anzr: 'grfg_nccebkvzngrqngr',
743+>
744+
745+
746+--
747
748=== added file 'lib/lp/services/tests/data/expected-4'
749--- lib/lp/services/tests/data/expected-4 1970-01-01 00:00:00 +0000
750+++ lib/lp/services/tests/data/expected-4 2014-07-29 15:41:46 +0000
751@@ -0,0 +1,14 @@
752+
753+
754+Diff comments:
755+
756+> === zbqvsvrq svyr 'yvo/yc/pbqr/vagresnprf/qvss.cl'
757+> --- yvo/yc/pbqr/vagresnprf/qvss.cl 2009-10-01 13:25:12 +0000
758+
759+Is this from Planet Earth© ?
760+
761+> +++ yvo/yc/pbqr/vagresnprf/qvss.cl 2010-02-02 15:48:56 +0000
762+
763+
764+
765+--
766
767=== added file 'lib/lp/services/tests/data/expected-5'
768--- lib/lp/services/tests/data/expected-5 1970-01-01 00:00:00 +0000
769+++ lib/lp/services/tests/data/expected-5 2014-07-29 15:41:46 +0000
770@@ -0,0 +1,39 @@
771+
772+
773+Diff comments:
774+
775+> === zbqvsvrq svyr 'yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf'
776+> --- yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 12:32:44 +0000
777+> +++ yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 23:47:16 +0000
778+
779+> @@ -80,7 +80,7 @@
780+> 'crefba': crefba_bow,
781+> 'grkg': 'Guvf vf terng.',
782+> 'qngr': (arj Qngr(abj - 12600000)),
783+> - },
784+> + }
785+
786+Why?
787+
788+> ];
789+> zbpxvb.fhpprff({
790+> erfcbafrGrkg: L.WFBA.fgevatvsl(choyvfurq_pbzzragf),
791+
792+> === zbqvsvrq svyr 'yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf'
793+> --- yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 12:32:44 +0000
794+> +++ yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 23:47:16 +0000
795+> @@ -3,7 +3,7 @@
796+> ine grfgf = L.anzrfcnpr('qngr.grfg');
797+> grfgf.fhvgr = arj L.Grfg.Fhvgr("qngr grfgf");
798+>
799+> - ine abj = (arj Qngr).inyhrBs()
800+> + ine abj = (arj Qngr).inyhrBs();
801+
802+Good catch!
803+
804+> grfgf.fhvgr.nqq(arj L.Grfg.Pnfr({
805+> anzr: 'grfg_nccebkvzngrqngr',
806+>
807+
808+
809+--
810
811=== added file 'lib/lp/services/tests/data/expected-6'
812--- lib/lp/services/tests/data/expected-6 1970-01-01 00:00:00 +0000
813+++ lib/lp/services/tests/data/expected-6 2014-07-29 15:41:46 +0000
814@@ -0,0 +1,44 @@
815+
816+
817+Diff comments:
818+
819+> === zbqvsvrq svyr 'yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf'
820+> --- yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 12:32:44 +0000
821+> +++ yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 23:47:16 +0000
822+
823+> @@ -88,7 +88,7 @@
824+>
825+> // Choyvfurq pbzzrag vf qvfcynlrq.
826+> ine svefg_pbzzragf = L.bar('#qvss-yvar-2').arkg().bar('qvi');
827+> - ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq')
828+> + ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq');
829+
830++1
831+
832+> L.Nffreg.nerRdhny(
833+> 'Sbb One (anzr16) jebgr ba 2012-08-12:',
834+> svefg.bar('.obneqPbzzragQrgnvyf').trg('grkg'));
835+>
836+> === nqqrq svyr 'jrohv/obql_ot.wct'
837+> Binary files jrohv/obql_ot.wct1970-01-01 00:00:00 +0000 and jrohv/obql_ot.wct19702014-01-31 16:57:19 +0000 differ
838+
839+Do we need this?
840+
841+> === zbqvsvrq svyr 'yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf'
842+> --- yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 12:32:44 +0000
843+> +++ yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 23:47:16 +0000
844+> @@ -3,7 +3,7 @@
845+> ine grfgf = L.anzrfcnpr('qngr.grfg');
846+> grfgf.fhvgr = arj L.Grfg.Fhvgr("qngr grfgf");
847+>
848+> - ine abj = (arj Qngr).inyhrBs()
849+> + ine abj = (arj Qngr).inyhrBs();
850+
851+Can we change this?
852+
853+> grfgf.fhvgr.nqq(arj L.Grfg.Pnfr({
854+> anzr: 'grfg_nccebkvzngrqngr',
855+>
856+
857+
858+--
859
860=== added file 'lib/lp/services/tests/data/expected-7'
861--- lib/lp/services/tests/data/expected-7 1970-01-01 00:00:00 +0000
862+++ lib/lp/services/tests/data/expected-7 2014-07-29 15:41:46 +0000
863@@ -0,0 +1,55 @@
864+
865+
866+Diff comments:
867+
868+> === zbqvsvrq svyr 'yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf'
869+> --- yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 12:32:44 +0000
870+> +++ yvo/yc/pbqr/wninfpevcg/grfgf/grfg_oenapuzretrcebcbfny.vayvarpbzzragf.wf 2014-07-08 23:47:16 +0000
871+> @@ -70,7 +70,7 @@
872+> "jf.bc=trgVayvarPbzzragf&cerivrjqvss_vq=1",
873+> zbpxvb.erdhrfgf[0].pbasvt.qngn);
874+> zbpxvb.ynfg_erdhrfg = zbpxvb.erdhrfgf[0];
875+> - ine abj = (arj Qngr).inyhrBs()
876+> + ine abj = (arj Qngr).inyhrBs();
877+
878+Good stuff
879+
880+> choyvfurq_pbzzragf = [
881+> {'yvar_ahzore': '2',
882+> 'crefba': crefba_bow,
883+
884+> @@ -88,7 +88,7 @@
885+>
886+> // Choyvfurq pbzzrag vf qvfcynlrq.
887+> ine svefg_pbzzragf = L.bar('#qvss-yvar-2').arkg().bar('qvi');
888+> - ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq')
889+> + ine svefg = svefg_pbzzragf.bar('qvi:svefg-puvyq');
890+
891++1
892+
893+> L.Nffreg.nerRdhny(
894+> 'Sbb One (anzr16) jebgr ba 2012-08-12:',
895+> svefg.bar('.obneqPbzzragQrgnvyf').trg('grkg'));
896+> === nqqrq svyr 'jrohv/obql_ot.wct'
897+> Binary files jrohv/obql_ot.wct1970-01-01 00:00:00 +0000 and jrohv/obql_ot.wct19702014-01-31 16:57:19 +0000 differ
898+
899+Do we need this?
900+
901+> === zbqvsvrq svyr 'yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf'
902+> --- yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 12:32:44 +0000
903+> +++ yvo/yc/ncc/wninfpevcg/grfgf/grfg_qngr.wf 2014-07-08 23:47:16 +0000
904+> @@ -3,7 +3,7 @@
905+> ine grfgf = L.anzrfcnpr('qngr.grfg');
906+> grfgf.fhvgr = arj L.Grfg.Fhvgr("qngr grfgf");
907+>
908+> - ine abj = (arj Qngr).inyhrBs()
909+> + ine abj = (arj Qngr).inyhrBs();
910+
911+Can we change this?
912+
913+> grfgf.fhvgr.nqq(arj L.Grfg.Pnfr({
914+> anzr: 'grfg_nccebkvzngrqngr',
915+>
916+
917+
918+--
919
920=== added file 'lib/lp/services/tests/data/insert_top.patch'
921--- lib/lp/services/tests/data/insert_top.patch 1970-01-01 00:00:00 +0000
922+++ lib/lp/services/tests/data/insert_top.patch 2014-07-29 15:41:46 +0000
923@@ -0,0 +1,7 @@
924+--- orig/pylon/patches.py
925++++ mod/pylon/patches.py
926+@@ -1,3 +1,4 @@
927++#test
928+ import util
929+ import sys
930+ class PatchSyntax(Exception):
931
932=== added file 'lib/lp/services/tests/data/tricky-diff.patch'
933--- lib/lp/services/tests/data/tricky-diff.patch 1970-01-01 00:00:00 +0000
934+++ lib/lp/services/tests/data/tricky-diff.patch 2014-07-29 15:41:46 +0000
935@@ -0,0 +1,26 @@
936+=== orig-7
937+--- orig-7
938++++ mod-7
939+@@ -1,10 +1,10 @@
940+ -- a
941+--- b
942++++ c
943+ xx d
944+ xx e
945+ ++ f
946+-++ g
947++-- h
948+ xx i
949+ xx j
950+ -- k
951+--- l
952++++ m
953+=== orig-8
954+--- orig-8
955++++ mod-8
956+@@ -1 +1 @@
957+--- A
958++++ B
959+@@ -1 +1 @@
960+--- C
961++++ D
962
963=== added file 'lib/lp/services/tests/test_patches.py'
964--- lib/lp/services/tests/test_patches.py 1970-01-01 00:00:00 +0000
965+++ lib/lp/services/tests/test_patches.py 2014-07-29 15:41:46 +0000
966@@ -0,0 +1,127 @@
967+# Copyright 2014 Canonical Ltd. This software is licensed under the
968+# GNU Affero General Public License version 3 (see the file LICENSE).
969+
970+import os
971+import re
972+
973+from bzrlib.errors import MalformedPatchHeader
974+from bzrlib.patches import difference_index
975+
976+from lp.services.patches import (
977+ BinaryPatch,
978+ get_patch_names,
979+ parse_patch,
980+ parse_patches,
981+ Patch,
982+ )
983+from lp.testing import TestCase
984+
985+
986+def datafile(filename):
987+ data_path = os.path.join(os.path.dirname(__file__),
988+ "data", filename)
989+ return file(data_path, "rb")
990+
991+
992+class TestBzrLibCode(TestCase):
993+ """Test code rewritten from bzrlib.patches for diff comment emails"""
994+
995+ def assertContainsRe(self, haystack, needle_re, flags=0):
996+ """Assert that a contains something matching a regular expression."""
997+ if not re.search(needle_re, haystack, flags):
998+ if '\n' in haystack or len(haystack) > 60:
999+ # a long string, format it in a more readable way
1000+ raise AssertionError(
1001+ 'pattern "%s" not found in\n"""\\\n%s"""\n'
1002+ % (needle_re, haystack))
1003+ else:
1004+ raise AssertionError('pattern "%s" not found in "%s"'
1005+ % (needle_re, haystack))
1006+
1007+ def data_lines(self, filename):
1008+ data = datafile(filename)
1009+ try:
1010+ return data.readlines()
1011+ finally:
1012+ data.close()
1013+
1014+ def testValidPatchHeaderWithComments(self):
1015+ """Parse a valid patch header"""
1016+ expected_comments = ["=== modified file 'commands.py'"]
1017+ lines = ("=== modified file 'commands.py'\n--- orig/commands.py\n"
1018+ "+++ mod/dommands.py\n").split('\n')
1019+ (comments, orig, mod) = get_patch_names(lines.__iter__())
1020+ self.assertEqual(comments, expected_comments)
1021+ self.assertEqual(orig, "orig/commands.py")
1022+ self.assertEqual(mod, "mod/dommands.py")
1023+
1024+ def testValidPatchHeader(self):
1025+ """Parse a valid patch header"""
1026+ expected_comments = []
1027+ lines = ("--- orig/commands.py\n+++ mod/dommands.py\n").split('\n')
1028+ (comments, orig, mod) = get_patch_names(lines.__iter__())
1029+ self.assertEqual(comments, expected_comments)
1030+ self.assertEqual(orig, "orig/commands.py")
1031+ self.assertEqual(mod, "mod/dommands.py")
1032+
1033+ def testInvalidPatchHeader(self):
1034+ """Parse an invalid patch header"""
1035+ lines = "-- orig/commands.py\n+++ mod/dommands.py".split('\n')
1036+ self.assertRaises(MalformedPatchHeader, get_patch_names,
1037+ lines.__iter__())
1038+
1039+ def compare_parsed(self, patchtext):
1040+ lines = patchtext.splitlines(True)
1041+ patch = parse_patch(lines.__iter__())
1042+ pstr = str(patch)
1043+ i = difference_index(patchtext, pstr)
1044+ if i is not None:
1045+ print "%i: \"%s\" != \"%s\"" % (i, patchtext[i], pstr[i])
1046+ self.assertEqual(patchtext, str(patch))
1047+
1048+ def testAll(self):
1049+ """Test parsing a whole patch"""
1050+ patchtext = datafile("diff-1").read()
1051+ self.compare_parsed(patchtext)
1052+
1053+ def test_parse_binary(self):
1054+ """Test parsing a whole patch"""
1055+ diff_1 = datafile("diff-1").read()
1056+ diff_2 = datafile("diff-2").read()
1057+ diff = '\n'.join([diff_2, diff_1]).splitlines(True)
1058+ patches = parse_patches(diff)
1059+ self.assertIs(BinaryPatch, patches[0].__class__)
1060+ self.assertIs(Patch, patches[1].__class__)
1061+ self.assertContainsRe(
1062+ str(patches[0]),
1063+ 'Binary files jrohv/obql_ot.wct1970-01-01.* and '
1064+ 'jrohv/obql_ot.wct19702014-01-31.* differ\n')
1065+
1066+ def test_parse_binary_after_normal(self):
1067+ diff_1 = datafile("diff-1").read()
1068+ diff_2 = datafile("diff-2").read()
1069+ diff = '\n'.join([diff_1, diff_2]).splitlines(True)
1070+ patches = parse_patches(diff)
1071+ self.assertIs(BinaryPatch, patches[1].__class__)
1072+ self.assertIs(Patch, patches[0].__class__)
1073+ self.assertContainsRe(
1074+ str(patches[1]),
1075+ 'Binary files jrohv/obql_ot.wct1970-01-01.* and '
1076+ 'jrohv/obql_ot.wct19702014-01-31.* differ\n')
1077+
1078+ def test_roundtrip_binary(self):
1079+ diff = datafile("diff-3").read()
1080+ patches = parse_patches(diff.splitlines(True))
1081+ self.assertEqual(diff, ''.join(str(p) for p in patches))
1082+
1083+ def testParsePatchesWithComments(self):
1084+ """Make sure file names can be extracted from tricky unified diffs
1085+ with comment lines"""
1086+ patchtext = ''.join(self.data_lines("tricky-diff.patch"))
1087+ filenames = [('orig-7', 'mod-7'),
1088+ ('orig-8', 'mod-8')]
1089+ patches = parse_patches(patchtext.splitlines(True))
1090+ patch_files = []
1091+ for patch in patches:
1092+ patch_files.append((patch.oldname, patch.newname))
1093+ self.assertEqual(patch_files, filenames)
1094
1095=== modified file 'lib/lp/testing/factory.py'
1096--- lib/lp/testing/factory.py 2014-07-18 07:43:06 +0000
1097+++ lib/lp/testing/factory.py 2014-07-29 15:41:46 +0000
1098@@ -1540,8 +1540,11 @@
1099 Diff.fromFile(StringIO(diff_text), len(diff_text)))
1100
1101 def makePreviewDiff(self, conflicts=u'', merge_proposal=None,
1102- date_created=None):
1103- diff = self.makeDiff()
1104+ date_created=None, diff_text=None):
1105+ if diff_text is None:
1106+ diff = self.makeDiff()
1107+ else:
1108+ diff = self.makeDiff(diff_text)
1109 if merge_proposal is None:
1110 merge_proposal = self.makeBranchMergeProposal()
1111 preview_diff = PreviewDiff()