Merge lp:~gz/brz/py3_ignores into lp:brz

Proposed by Martin Packman
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 6811
Proposed branch: lp:~gz/brz/py3_ignores
Merge into: lp:brz
Diff against target: 234 lines (+52/-64)
2 files modified
breezy/ignores.py (+11/-20)
breezy/tests/test_ignores.py (+41/-44)
To merge this branch: bzr merge lp:~gz/brz/py3_ignores
Reviewer Review Type Date Requested Status
Jelmer Vernooij Approve
Review via email: mp+333588@code.launchpad.net

Commit message

Make test_ignores (mostly) pass on Python 3

Description of the change

This leaves one failing test on Python 3:

======================================================================
FAIL: breezy.tests.test_ignores.TestTreeIgnores.test_crlf
----------------------------------------------------------------------
...

Traceback (most recent call last):
  File "/home/martin/brz/tree/breezy/tests/test_ignores.py", line 223, in test_crlf
    self.assertEqual(f.read(), 'myentry1\r\nmyentry2\r\nfoo\r\n')
  File "/home/martin/brz/tree/breezy/tests/__init__.py", line 1312, in assertEqual
    pprint.pformat(a), pprint.pformat(b)))
AssertionError: not equal:
a = 'myentry1\nmyentry2\nfoo\n'
b = 'myentry1\r\nmyentry2\r\nfoo\r\n'

This is a real regression in behaviour, but will want to be fixed by dropping reliance on the deprecated 'U' flag to open and 'newlines' attribute and using an alternative scheme to preserve the newline type of existing ignore files.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve
Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/ignores.py'
2--- breezy/ignores.py 2017-05-22 00:56:52 +0000
3+++ breezy/ignores.py 2017-11-12 13:46:56 +0000
4@@ -68,7 +68,7 @@
5 except UnicodeDecodeError:
6 # Otherwise go though line by line and pick out the 'good'
7 # decodable lines
8- lines = ignore_file.split('\n')
9+ lines = ignore_file.split(b'\n')
10 unicode_lines = []
11 for line_number, line in enumerate(lines):
12 try:
13@@ -128,12 +128,9 @@
14 config.ensure_config_dir_exists()
15
16 # Create an empty file
17- f = open(ignore_path, 'wb')
18- try:
19+ with open(ignore_path, 'wb') as f:
20 for pattern in patterns:
21- f.write(pattern.encode('utf8') + '\n')
22- finally:
23- f.close()
24+ f.write(pattern.encode('utf8') + b'\n')
25
26
27 def add_unique_user_ignores(new_ignores):
28@@ -153,12 +150,9 @@
29 if not to_add:
30 return []
31
32- f = open(config.user_ignore_config_filename(), 'ab')
33- try:
34+ with open(config.user_ignore_config_filename(), 'ab') as f:
35 for pattern in to_add:
36- f.write(pattern.encode('utf8') + '\n')
37- finally:
38- f.close()
39+ f.write(pattern.encode('utf8') + b'\n')
40
41 return to_add
42
43@@ -198,20 +192,17 @@
44 # read in the existing ignores set
45 ifn = tree.abspath(breezy.IGNORE_FILENAME)
46 if tree.has_filename(ifn):
47- f = open(ifn, 'rU')
48- try:
49+ with open(ifn, 'rbU') as f:
50 file_contents = f.read()
51 # figure out what kind of line endings are used
52 newline = getattr(f, 'newlines', None)
53 if isinstance(newline, tuple):
54 newline = newline[0]
55 elif newline is None:
56- newline = os.linesep
57- finally:
58- f.close()
59+ newline = os.linesep.encode()
60 else:
61- file_contents = ""
62- newline = os.linesep
63+ file_contents = b""
64+ newline = os.linesep.encode()
65
66 sio = BytesIO(file_contents)
67 try:
68@@ -223,8 +214,8 @@
69 f = atomicfile.AtomicFile(ifn, 'wb')
70 try:
71 # write the original contents, preserving original line endings
72- f.write(newline.join(file_contents.split('\n')))
73- if len(file_contents) > 0 and not file_contents.endswith('\n'):
74+ f.write(newline.join(file_contents.split(b'\n')))
75+ if len(file_contents) > 0 and not file_contents.endswith(b'\n'):
76 f.write(newline)
77 for pattern in name_pattern_list:
78 if not pattern in ignores:
79
80=== modified file 'breezy/tests/test_ignores.py'
81--- breezy/tests/test_ignores.py 2017-10-27 00:18:42 +0000
82+++ breezy/tests/test_ignores.py 2017-11-12 13:46:56 +0000
83@@ -34,25 +34,25 @@
84
85 def test_parse_fancy(self):
86 ignored = ignores.parse_ignore_file(BytesIO(
87- './rootdir\n'
88- 'randomfile*\n'
89- 'path/from/ro?t\n'
90- 'unicode\xc2\xb5\n' # u'\xb5'.encode('utf8')
91- 'dos\r\n'
92- '\n' # empty line
93- '#comment\n'
94- ' xx \n' # whitespace
95- '!RE:^\\.z.*\n'
96- '!!./.zcompdump\n'
97+ b'./rootdir\n'
98+ b'randomfile*\n'
99+ b'path/from/ro?t\n'
100+ b'unicode\xc2\xb5\n' # u'\xb5'.encode('utf8')
101+ b'dos\r\n'
102+ b'\n' # empty line
103+ b'#comment\n'
104+ b' xx \n' # whitespace
105+ b'!RE:^\\.z.*\n'
106+ b'!!./.zcompdump\n'
107 ))
108- self.assertEqual({'./rootdir',
109- 'randomfile*',
110- 'path/from/ro?t',
111+ self.assertEqual({u'./rootdir',
112+ u'randomfile*',
113+ u'path/from/ro?t',
114 u'unicode\xb5',
115- 'dos',
116- ' xx ',
117- '!RE:^\\.z.*',
118- '!!./.zcompdump',
119+ u'dos',
120+ u' xx ',
121+ u'!RE:^\\.z.*',
122+ u'!!./.zcompdump',
123 }, ignored)
124
125 def test_parse_empty(self):
126@@ -62,13 +62,13 @@
127 def test_parse_non_utf8(self):
128 """Lines with non utf 8 characters should be discarded."""
129 ignored = ignores.parse_ignore_file(BytesIO(
130- 'utf8filename_a\n'
131- 'invalid utf8\x80\n'
132- 'utf8filename_b\n'
133+ b'utf8filename_a\n'
134+ b'invalid utf8\x80\n'
135+ b'utf8filename_b\n'
136 ))
137 self.assertEqual({
138- 'utf8filename_a',
139- 'utf8filename_b',
140+ u'utf8filename_a',
141+ u'utf8filename_b',
142 }, ignored)
143
144
145@@ -82,15 +82,12 @@
146 self.assertEqual(set(ignores.USER_DEFAULTS), user_ignores)
147
148 self.assertPathExists(ignore_path)
149- f = open(ignore_path, 'rb')
150- try:
151+ with open(ignore_path, 'rb') as f:
152 entries = ignores.parse_ignore_file(f)
153- finally:
154- f.close()
155 self.assertEqual(set(ignores.USER_DEFAULTS), entries)
156
157 def test_use_existing(self):
158- patterns = ['*.o', '*.py[co]', u'\xe5*']
159+ patterns = [u'*.o', u'*.py[co]', u'\xe5*']
160 ignores._set_user_ignores(patterns)
161
162 user_ignores = ignores.get_user_ignores()
163@@ -173,40 +170,40 @@
164
165
166 class TestTreeIgnores(TestCaseWithTransport):
167-
168+
169 def assertPatternsEquals(self, patterns):
170- contents = open(".bzrignore", 'rU').read().strip().split('\n')
171+ with open(".bzrignore", "rb") as f:
172+ contents = f.read().decode("utf-8").splitlines()
173 self.assertEqual(sorted(patterns), sorted(contents))
174
175 def test_new_file(self):
176 tree = self.make_branch_and_tree(".")
177- ignores.tree_ignores_add_patterns(tree, ["myentry"])
178+ ignores.tree_ignores_add_patterns(tree, [u"myentry"])
179 self.assertTrue(tree.has_filename(".bzrignore"))
180 self.assertPatternsEquals(["myentry"])
181
182 def test_add_to_existing(self):
183 tree = self.make_branch_and_tree(".")
184- self.build_tree_contents([('.bzrignore', "myentry1\n")])
185+ self.build_tree_contents([('.bzrignore', b"myentry1\n")])
186 tree.add([".bzrignore"])
187- ignores.tree_ignores_add_patterns(tree, ["myentry2", "foo"])
188+ ignores.tree_ignores_add_patterns(tree, [u"myentry2", u"foo"])
189 self.assertPatternsEquals(["myentry1", "myentry2", "foo"])
190
191 def test_adds_ending_newline(self):
192 tree = self.make_branch_and_tree(".")
193- self.build_tree_contents([('.bzrignore', "myentry1")])
194+ self.build_tree_contents([('.bzrignore', b"myentry1")])
195 tree.add([".bzrignore"])
196- ignores.tree_ignores_add_patterns(tree, ["myentry2"])
197+ ignores.tree_ignores_add_patterns(tree, [u"myentry2"])
198 self.assertPatternsEquals(["myentry1", "myentry2"])
199- text = open(".bzrignore", 'r').read()
200- self.assertTrue(text.endswith('\r\n') or
201- text.endswith('\n') or
202- text.endswith('\r'))
203+ with open(".bzrignore") as f:
204+ text = f.read()
205+ self.assertTrue(text.endswith(('\r\n', '\n', '\r')))
206
207 def test_does_not_add_dupe(self):
208 tree = self.make_branch_and_tree(".")
209- self.build_tree_contents([('.bzrignore', "myentry\n")])
210+ self.build_tree_contents([('.bzrignore', b"myentry\n")])
211 tree.add([".bzrignore"])
212- ignores.tree_ignores_add_patterns(tree, ["myentry"])
213+ ignores.tree_ignores_add_patterns(tree, [u"myentry"])
214 self.assertPatternsEquals(["myentry"])
215
216 def test_non_ascii(self):
217@@ -215,13 +212,13 @@
218 u"myentry\u1234\n".encode('utf-8'))])
219 tree.add([".bzrignore"])
220 ignores.tree_ignores_add_patterns(tree, [u"myentry\u5678"])
221- self.assertPatternsEquals([u"myentry\u1234".encode('utf-8'),
222- u"myentry\u5678".encode('utf-8')])
223+ self.assertPatternsEquals([u"myentry\u1234", u"myentry\u5678"])
224
225 def test_crlf(self):
226 tree = self.make_branch_and_tree(".")
227- self.build_tree_contents([('.bzrignore', "myentry1\r\n")])
228+ self.build_tree_contents([('.bzrignore', b"myentry1\r\n")])
229 tree.add([".bzrignore"])
230 ignores.tree_ignores_add_patterns(tree, ["myentry2", "foo"])
231- self.assertEqual(open('.bzrignore', 'rb').read(), 'myentry1\r\nmyentry2\r\nfoo\r\n')
232+ with open('.bzrignore') as f:
233+ self.assertEqual(f.read(), 'myentry1\r\nmyentry2\r\nfoo\r\n')
234 self.assertPatternsEquals(["myentry1", "myentry2", "foo"])

Subscribers

People subscribed via source and target branches