launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-unified-diff into launchpad:master.

Commit message:
Fix various uses of unified_diff on bytes

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/397942

We have to do this a bit differently for Python 3 compatibility: unified_diff only accepts text now, although we can wrap it in diff_bytes if comparing bytes is necessary.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-unified-diff into launchpad:master.

diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index 908cfa4..e8a22f4 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -17,6 +17,7 @@ from datetime import (
     )
 from difflib import unified_diff
 import doctest
+from functools import partial
 import hashlib
 import re
 
@@ -141,6 +142,14 @@ from lp.testing.views import (
     )
 
 
+if six.PY3:
+    from difflib import diff_bytes
+
+    unified_diff_bytes = partial(diff_bytes, unified_diff)
+else:
+    unified_diff_bytes = unified_diff
+
+
 class GitHostingClientMixin:
 
     def setUp(self):
@@ -1402,7 +1411,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
     def test_preview_diff_utf8(self):
         """A preview_diff in utf-8 should be decoded as utf-8."""
         text = ''.join(six.unichr(x) for x in range(255))
-        diff_bytes = ''.join(unified_diff('', text)).encode('utf-8')
+        diff_bytes = ''.join(unified_diff([''], [text])).encode('utf-8')
         self.setPreviewDiff(diff_bytes)
         transaction.commit()
         view = create_initialized_view(self.bmp, '+index')
@@ -1413,7 +1422,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
     def test_preview_diff_all_chars(self):
         """preview_diff should work on diffs containing all possible bytes."""
         text = b''.join(six.int2byte(x) for x in range(255))
-        diff_bytes = b''.join(unified_diff(b'', text))
+        diff_bytes = b''.join(unified_diff_bytes([b''], [text]))
         self.setPreviewDiff(diff_bytes)
         transaction.commit()
         view = create_initialized_view(self.bmp, '+index')
@@ -1425,7 +1434,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
         # The preview_diff will recover from a timeout set to get the
         # librarian content.
         text = b''.join(six.int2byte(x) for x in range(255))
-        diff_bytes = b''.join(unified_diff(b'', text))
+        diff_bytes = b''.join(unified_diff_bytes([b''], [text]))
         preview_diff = self.setPreviewDiff(diff_bytes)
         transaction.commit()
 
@@ -1445,7 +1454,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
         # librarian content.  (This can happen e.g. on staging replicas of
         # the production database.)
         text = b''.join(six.int2byte(x) for x in range(255))
-        diff_bytes = b''.join(unified_diff(b'', text))
+        diff_bytes = b''.join(unified_diff_bytes([b''], [text]))
         preview_diff = self.setPreviewDiff(diff_bytes)
         transaction.commit()
 
diff --git a/lib/lp/code/model/tests/test_diff.py b/lib/lp/code/model/tests/test_diff.py
index d6ed025..f51497e 100644
--- a/lib/lp/code/model/tests/test_diff.py
+++ b/lib/lp/code/model/tests/test_diff.py
@@ -204,30 +204,30 @@ class TestDiff(DiffTestCase):
     def test_text_reads_librarian_content(self):
         # IDiff.text will read at most config.diff.max_read_size bytes from
         # the librarian.
-        content = b''.join(unified_diff(b'', b"1234567890" * 10))
-        diff = self._create_diff(content)
+        content = ''.join(unified_diff('', "1234567890" * 10))
+        diff = self._create_diff(content.encode('UTF-8'))
         self.assertEqual(content, diff.text)
         self.assertTrue(diff.diff_text.restricted)
 
     def test_oversized_normal(self):
         # A diff smaller than config.diff.max_read_size is not oversized.
-        content = b''.join(unified_diff(b'', b"1234567890" * 10))
-        diff = self._create_diff(content)
+        content = ''.join(unified_diff('', "1234567890" * 10))
+        diff = self._create_diff(content.encode('UTF-8'))
         self.assertFalse(diff.oversized)
 
     def test_text_read_limited_by_config(self):
         # IDiff.text will read at most config.diff.max_read_size bytes from
         # the librarian.
         self.pushConfig("diff", max_read_size=25)
-        content = b''.join(unified_diff(b'', b"1234567890" * 10))
-        diff = self._create_diff(content)
+        content = ''.join(unified_diff('', "1234567890" * 10))
+        diff = self._create_diff(content.encode('UTF-8'))
         self.assertEqual(content[:25], diff.text)
 
     def test_oversized_for_big_diff(self):
         # A diff larger than config.diff.max_read_size is oversized.
         self.pushConfig("diff", max_read_size=25)
-        content = b''.join(unified_diff(b'', b"1234567890" * 10))
-        diff = self._create_diff(content)
+        content = ''.join(unified_diff('', "1234567890" * 10))
+        diff = self._create_diff(content.encode('UTF-8'))
         self.assertTrue(diff.oversized)
 
     def test_timeout(self):
diff --git a/lib/lp/code/stories/branches/xx-branchmergeproposals.txt b/lib/lp/code/stories/branches/xx-branchmergeproposals.txt
index 0220042..6c8a697 100644
--- a/lib/lp/code/stories/branches/xx-branchmergeproposals.txt
+++ b/lib/lp/code/stories/branches/xx-branchmergeproposals.txt
@@ -554,8 +554,7 @@ Create merge proposal with a preview diff, and go to its index page.
     >>> from zope.security.proxy import removeSecurityProxy
     >>> from difflib import unified_diff
     >>> from lp.code.model.diff import PreviewDiff
-    >>> diff_text = b''.join(
-    ...     unified_diff(b'', [b'Fake Diff' + u'\u1010'.encode('utf-8')]))
+    >>> diff_text = ''.join(unified_diff('', [u'Fake Diff\u1010']))
     >>> bmp = factory.makeBranchMergeProposal()
     >>> preview_diff = PreviewDiff.create(
     ...     bmp, diff_text, u'a', u'b', None, u'')
@@ -568,8 +567,19 @@ Create merge proposal with a preview diff, and go to its index page.
 
 The text of the review diff is in the page.
 
-    >>> print(repr(extract_text(get_review_diff())))
-    u'Preview Diff\n[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk\nDownload diff\nSide-by-side diff\n1\n---\n2\n+++\n3\n@@ -0,0 +1 @@\n4\n+Fake Diff\u1010'
+    >>> print(backslashreplace(extract_text(get_review_diff())))
+    Preview Diff
+    [H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
+    Download diff
+    Side-by-side diff
+    1
+    ---
+    2
+    +++
+    3
+    @@ -0,0 +1 @@
+    4
+    +Fake Diff\u1010
 
 There is also a link to the diff URL, which is the preview diff URL plus
 "+files/preview.diff". It redirects logged in users to the file in the