Merge lp:~blr/launchpad/bug-1471426-commentmail-binarypatch-oops into lp:launchpad

Proposed by Kit Randel
Status: Merged
Merged at revision: 17606
Proposed branch: lp:~blr/launchpad/bug-1471426-commentmail-binarypatch-oops
Merge into: lp:launchpad
Diff against target: 82 lines (+54/-0)
2 files modified
lib/lp/code/mail/codereviewcomment.py (+11/-0)
lib/lp/code/mail/tests/test_codereviewcomment.py (+43/-0)
To merge this branch: bzr merge lp:~blr/launchpad/bug-1471426-commentmail-binarypatch-oops
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+263995@code.launchpad.net

Commit message

Handle binary patches and associated comments.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)
Revision history for this message
William Grant (wgrant) wrote :

Can you also add another comment in the new test, verifying that the line numbers in subsequent patches are correct?

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 2015-06-30 08:51:21 +0000
3+++ lib/lp/code/mail/codereviewcomment.py 2015-07-07 05:32:52 +0000
4@@ -209,6 +209,17 @@
5 dirty_comment = True
6 patch = patch['patch']
7
8+ # call type here as patch is an instance of both Patch and BinaryPatch
9+ if type(patch) is patches.BinaryPatch:
10+ if dirty_comment:
11+ result_lines.extend(dirty_head)
12+ result_lines.append(u'> %s' % str(patch).rstrip('\n'))
13+ line_count += 1
14+ comment = comments.get(str(line_count))
15+ if comment:
16+ result_lines.extend(format_comment(comment))
17+ continue
18+
19 for ph in patch.get_header().splitlines():
20 line_count += 1 # inc patch headers
21 comment = comments.get(str(line_count))
22
23=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
24--- lib/lp/code/mail/tests/test_codereviewcomment.py 2015-06-30 07:58:29 +0000
25+++ lib/lp/code/mail/tests/test_codereviewcomment.py 2015-07-07 05:32:52 +0000
26@@ -406,6 +406,20 @@
27 "+d\n"
28 "+e\n")
29
30+ binary_diff_text = (
31+ "=== added file 'lib/canonical/launchpad/images/foo.png'\n"
32+ "Binary files lib/canonical/launchpad/images/foo.png\t"
33+ "1970-01-01 00:00:00 +0000 and "
34+ "lib/canonical/launchpad/images/foo.png\t"
35+ "2015-06-21 22:07:50 +0000 differ\n"
36+ "=== modified file 'foo/bar/baz.py'\n"
37+ "--- bar\t2009-08-26 15:53:34.000000000 -0400\n"
38+ "+++ bar\t1969-12-31 19:00:00.000000000 -0500\n"
39+ "@@ -1,3 +0,0 @@\n"
40+ "-a\n"
41+ "-b\n"
42+ "-c\n")
43+
44 def getSection(self, comments, diff_text=None):
45 """Call `build_inline_comments_section` with the test-diff."""
46 if diff_text is None:
47@@ -427,6 +441,35 @@
48 [''],
49 footer)
50
51+ def test_binary_patch_in_diff(self):
52+ # Binary patches with comments are handled appropriately.
53+ comments = {'1': 'Updated the png', '2': 'foo', '8': 'bar'}
54+ section = self.getSection(comments, diff_text=self.binary_diff_text)
55+ self.assertEqual(
56+ map(unicode, [
57+ "> === added file 'lib/canonical/launchpad/images/foo.png'",
58+ "",
59+ "Updated the png",
60+ "",
61+ ("> Binary files lib/canonical/launchpad/images/foo.png\t"
62+ "1970-01-01 00:00:00 +0000 and "
63+ "lib/canonical/launchpad/images/foo.png\t"
64+ "2015-06-21 22:07:50 +0000 differ"),
65+ "",
66+ "foo",
67+ "",
68+ "> === modified file 'foo/bar/baz.py'",
69+ "> --- bar\t2009-08-26 15:53:34.000000000 -0400",
70+ "> +++ bar\t1969-12-31 19:00:00.000000000 -0500",
71+ "> @@ -1,3 +0,0 @@",
72+ "> -a",
73+ "> -b",
74+ "",
75+ "bar",
76+ "",
77+ "> -c"]),
78+ section.splitlines()[4:22])
79+
80 def test_single_line_comment(self):
81 # The inline comments are correctly contextualized in the diff.
82 # and prefixed with '>>> '