Launchpad itself

Merge ~cjwatson/launchpad:py3-unified-diff into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. py3-unified-diff
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: e01bf5426177768f272806e12ec58c91182ba763
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:py3-unified-diff
Merge into: launchpad:master
Diff against target: 142 lines (+35/-16)
3 files modified
lib/lp/code/browser/tests/test_branchmergeproposal.py (+13/-4)
lib/lp/code/model/tests/test_diff.py (+8/-8)
lib/lp/code/stories/branches/xx-branchmergeproposals.txt (+14/-4)
Reviewer Review Type Date Requested Status
Ioana Lasc (community) Approve
Review via email: mp+397942@code.launchpad.net

Commit message

Fix various uses of unified_diff on bytes

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
2index 908cfa4..e8a22f4 100644
3--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
4+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
5@@ -17,6 +17,7 @@ from datetime import (
6 )
7 from difflib import unified_diff
8 import doctest
9+from functools import partial
10 import hashlib
11 import re
12
13@@ -141,6 +142,14 @@ from lp.testing.views import (
14 )
15
16
17+if six.PY3:
18+ from difflib import diff_bytes
19+
20+ unified_diff_bytes = partial(diff_bytes, unified_diff)
21+else:
22+ unified_diff_bytes = unified_diff
23+
24+
25 class GitHostingClientMixin:
26
27 def setUp(self):
28@@ -1402,7 +1411,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
29 def test_preview_diff_utf8(self):
30 """A preview_diff in utf-8 should be decoded as utf-8."""
31 text = ''.join(six.unichr(x) for x in range(255))
32- diff_bytes = ''.join(unified_diff('', text)).encode('utf-8')
33+ diff_bytes = ''.join(unified_diff([''], [text])).encode('utf-8')
34 self.setPreviewDiff(diff_bytes)
35 transaction.commit()
36 view = create_initialized_view(self.bmp, '+index')
37@@ -1413,7 +1422,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
38 def test_preview_diff_all_chars(self):
39 """preview_diff should work on diffs containing all possible bytes."""
40 text = b''.join(six.int2byte(x) for x in range(255))
41- diff_bytes = b''.join(unified_diff(b'', text))
42+ diff_bytes = b''.join(unified_diff_bytes([b''], [text]))
43 self.setPreviewDiff(diff_bytes)
44 transaction.commit()
45 view = create_initialized_view(self.bmp, '+index')
46@@ -1425,7 +1434,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
47 # The preview_diff will recover from a timeout set to get the
48 # librarian content.
49 text = b''.join(six.int2byte(x) for x in range(255))
50- diff_bytes = b''.join(unified_diff(b'', text))
51+ diff_bytes = b''.join(unified_diff_bytes([b''], [text]))
52 preview_diff = self.setPreviewDiff(diff_bytes)
53 transaction.commit()
54
55@@ -1445,7 +1454,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
56 # librarian content. (This can happen e.g. on staging replicas of
57 # the production database.)
58 text = b''.join(six.int2byte(x) for x in range(255))
59- diff_bytes = b''.join(unified_diff(b'', text))
60+ diff_bytes = b''.join(unified_diff_bytes([b''], [text]))
61 preview_diff = self.setPreviewDiff(diff_bytes)
62 transaction.commit()
63
64diff --git a/lib/lp/code/model/tests/test_diff.py b/lib/lp/code/model/tests/test_diff.py
65index d6ed025..f51497e 100644
66--- a/lib/lp/code/model/tests/test_diff.py
67+++ b/lib/lp/code/model/tests/test_diff.py
68@@ -204,30 +204,30 @@ class TestDiff(DiffTestCase):
69 def test_text_reads_librarian_content(self):
70 # IDiff.text will read at most config.diff.max_read_size bytes from
71 # the librarian.
72- content = b''.join(unified_diff(b'', b"1234567890" * 10))
73- diff = self._create_diff(content)
74+ content = ''.join(unified_diff('', "1234567890" * 10))
75+ diff = self._create_diff(content.encode('UTF-8'))
76 self.assertEqual(content, diff.text)
77 self.assertTrue(diff.diff_text.restricted)
78
79 def test_oversized_normal(self):
80 # A diff smaller than config.diff.max_read_size is not oversized.
81- content = b''.join(unified_diff(b'', b"1234567890" * 10))
82- diff = self._create_diff(content)
83+ content = ''.join(unified_diff('', "1234567890" * 10))
84+ diff = self._create_diff(content.encode('UTF-8'))
85 self.assertFalse(diff.oversized)
86
87 def test_text_read_limited_by_config(self):
88 # IDiff.text will read at most config.diff.max_read_size bytes from
89 # the librarian.
90 self.pushConfig("diff", max_read_size=25)
91- content = b''.join(unified_diff(b'', b"1234567890" * 10))
92- diff = self._create_diff(content)
93+ content = ''.join(unified_diff('', "1234567890" * 10))
94+ diff = self._create_diff(content.encode('UTF-8'))
95 self.assertEqual(content[:25], diff.text)
96
97 def test_oversized_for_big_diff(self):
98 # A diff larger than config.diff.max_read_size is oversized.
99 self.pushConfig("diff", max_read_size=25)
100- content = b''.join(unified_diff(b'', b"1234567890" * 10))
101- diff = self._create_diff(content)
102+ content = ''.join(unified_diff('', "1234567890" * 10))
103+ diff = self._create_diff(content.encode('UTF-8'))
104 self.assertTrue(diff.oversized)
105
106 def test_timeout(self):
107diff --git a/lib/lp/code/stories/branches/xx-branchmergeproposals.txt b/lib/lp/code/stories/branches/xx-branchmergeproposals.txt
108index 0220042..6c8a697 100644
109--- a/lib/lp/code/stories/branches/xx-branchmergeproposals.txt
110+++ b/lib/lp/code/stories/branches/xx-branchmergeproposals.txt
111@@ -554,8 +554,7 @@ Create merge proposal with a preview diff, and go to its index page.
112 >>> from zope.security.proxy import removeSecurityProxy
113 >>> from difflib import unified_diff
114 >>> from lp.code.model.diff import PreviewDiff
115- >>> diff_text = b''.join(
116- ... unified_diff(b'', [b'Fake Diff' + u'\u1010'.encode('utf-8')]))
117+ >>> diff_text = ''.join(unified_diff('', [u'Fake Diff\u1010']))
118 >>> bmp = factory.makeBranchMergeProposal()
119 >>> preview_diff = PreviewDiff.create(
120 ... bmp, diff_text, u'a', u'b', None, u'')
121@@ -568,8 +567,19 @@ Create merge proposal with a preview diff, and go to its index page.
122
123 The text of the review diff is in the page.
124
125- >>> print(repr(extract_text(get_review_diff())))
126- 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'
127+ >>> print(backslashreplace(extract_text(get_review_diff())))
128+ Preview Diff
129+ [H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
130+ Download diff
131+ Side-by-side diff
132+ 1
133+ ---
134+ 2
135+ +++
136+ 3
137+ @@ -0,0 +1 @@
138+ 4
139+ +Fake Diff\u1010
140
141 There is also a link to the diff URL, which is the preview diff URL plus
142 "+files/preview.diff". It redirects logged in users to the file in the

Subscribers

People subscribed via source and target branches

to status/vote changes: