← Back to team overview

launchpad-reviewers team mailing list archive

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

 

Bayard 'kit' Randel has proposed merging lp:~blr/launchpad/bug-1471426-commentmail-binarypatch-oops into lp:launchpad.

Commit message:
Handle binary patches and associated comments.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1471426 in Launchpad itself: "AttributeError: 'BinaryPatch' object has no attribute 'get_header'"
  https://bugs.launchpad.net/launchpad/+bug/1471426

For more details, see:
https://code.launchpad.net/~blr/launchpad/bug-1471426-commentmail-binarypatch-oops/+merge/263995
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~blr/launchpad/bug-1471426-commentmail-binarypatch-oops into lp:launchpad.
=== modified file 'lib/lp/code/mail/codereviewcomment.py'
--- lib/lp/code/mail/codereviewcomment.py	2015-06-30 08:51:21 +0000
+++ lib/lp/code/mail/codereviewcomment.py	2015-07-07 04:22:50 +0000
@@ -209,6 +209,18 @@
                     dirty_comment = True
             patch = patch['patch']
 
+            if type(patch) is patches.BinaryPatch:
+
+                result_lines.extend(dirty_head)
+                result_lines.append(
+                    '> Binary files {old} and {new} differ'.format(
+                        old=patch.oldname, new=patch.newname))
+                line_count += 1
+                comment = comments.get(str(line_count))
+                if comment:
+                    result_lines.extend(format_comment(comment))
+                continue
+
         for ph in patch.get_header().splitlines():
             line_count += 1  # inc patch headers
             comment = comments.get(str(line_count))

=== modified file 'lib/lp/code/mail/tests/test_codereviewcomment.py'
--- lib/lp/code/mail/tests/test_codereviewcomment.py	2015-06-30 07:58:29 +0000
+++ lib/lp/code/mail/tests/test_codereviewcomment.py	2015-07-07 04:22:50 +0000
@@ -406,6 +406,21 @@
         "+d\n"
         "+e\n")
 
+    binary_diff_text = (
+        "=== added file 'lib/canonical/launchpad/images/foo.png'\n"
+        "Binary files lib/canonical/launchpad/images/foo.png\t"
+        "1970-01-01 00:00:00 +0000 and "
+        "lib/canonical/launchpad/images/foo.png\t"
+        "2015-06-21 22:07:50 +0000 differ\n"
+        "=== modified file 'foo/bar/baz.py'\n"
+        "--- bar\t2009-08-26 15:53:34.000000000 -0400\n"
+        "+++ bar\t1969-12-31 19:00:00.000000000 -0500\n"
+        "@@ -1,3 +0,0 @@\n"
+        "-\xc3\xa5\n"
+        "-b\n"
+        "-c\n")
+
+
     def getSection(self, comments, diff_text=None):
         """Call `build_inline_comments_section` with the test-diff."""
         if diff_text is None:
@@ -427,6 +442,25 @@
             [''],
             footer)
 
+    def test_binary_patch_in_diff(self):
+        # Binary patches with comments are handled appropriately.
+        comments = {'1': 'Updated the png', '2': 'foo'}
+        section = self.getSection(comments, diff_text=self.binary_diff_text)
+        self.assertEqual(
+            map(unicode, [
+                "> === added file 'lib/canonical/launchpad/images/foo.png'",
+                "",
+                "Updated the png",
+                "",
+                ("> Binary files lib/canonical/launchpad/images/foo.png\t"
+                 "1970-01-01 00:00:00 +0000 and "
+                 "lib/canonical/launchpad/images/foo.png\t"
+                 "2015-06-21 22:07:50 +0000 differ"),
+                "",
+                "foo",
+                ""]),
+            section.splitlines()[4:12])
+
     def test_single_line_comment(self):
         # The inline comments are correctly contextualized in the diff.
         # and prefixed with '>>> '


Follow ups