Merge ~cjwatson/txpkgupload:py3-context-managers into txpkgupload:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 0cb57cfcf1ab4c1e8efe83c46b500e4139ccf4b0
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/txpkgupload:py3-context-managers
Merge into: txpkgupload:master
Diff against target: 283 lines (+58/-39)
5 files modified
setup.py (+7/-5)
src/txpkgupload/filesystem.py (+10/-10)
src/txpkgupload/tests/filesystem.txt (+32/-17)
src/txpkgupload/tests/test_plugin.py (+7/-4)
src/txpkgupload/tests/test_twistedsftp.py (+2/-3)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+393037@code.launchpad.net

Commit message

Use context managers to squash ResourceWarnings

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/setup.py b/setup.py
2index b4f4bf7..c52a978 100755
3--- a/setup.py
4+++ b/setup.py
5@@ -24,9 +24,8 @@ def generate(*docname_or_string):
6 res = []
7 for value in docname_or_string:
8 if value.endswith('.txt'):
9- f = open(value)
10- value = f.read()
11- f.close()
12+ with open(value) as f:
13+ value = f.read()
14 res.append(value)
15 if not value.endswith('\n'):
16 res.append('')
17@@ -34,7 +33,10 @@ def generate(*docname_or_string):
18 # end generic helpers
19
20
21-__version__ = open("src/txpkgupload/version.txt").read().strip()
22+with open("src/txpkgupload/version.txt") as f:
23+ __version__ = f.read().strip()
24+with open("README.txt") as f:
25+ description = f.readline().strip()
26
27 setup(
28 name='txpkgupload',
29@@ -45,7 +47,7 @@ setup(
30 zip_safe=False,
31 maintainer='LAZR Developers',
32 maintainer_email='lazr-developers@lists.launchpad.net',
33- description=open('README.txt').readline().strip(),
34+ description=description,
35 long_description=generate('src/txpkgupload/NEWS.txt'),
36 license='AGPL v3',
37 install_requires=[
38diff --git a/src/txpkgupload/filesystem.py b/src/txpkgupload/filesystem.py
39index 37c8146..495bd7b 100644
40--- a/src/txpkgupload/filesystem.py
41+++ b/src/txpkgupload/filesystem.py
42@@ -230,21 +230,21 @@ class UploadFileSystem:
43 elif start or end:
44 open_flag = "r+b"
45 if not os.path.exists(full_path):
46- open(full_path, 'wb')
47+ with open(full_path, 'wb'):
48+ pass
49
50 else:
51 open_flag = 'wb'
52- outstream = open(full_path, open_flag)
53- if start:
54- outstream.seek(start)
55- chunk = instream.read()
56- while chunk:
57- outstream.write(chunk)
58+ with open(full_path, open_flag) as outstream:
59+ if start:
60+ outstream.seek(start)
61 chunk = instream.read()
62- if not end:
63- outstream.truncate()
64+ while chunk:
65+ outstream.write(chunk)
66+ chunk = instream.read()
67+ if not end:
68+ outstream.truncate()
69 instream.close()
70- outstream.close()
71
72 def writable(self, path):
73 """Return boolean indicating whether a file at path is writable."""
74diff --git a/src/txpkgupload/tests/filesystem.txt b/src/txpkgupload/tests/filesystem.txt
75index f64d035..97b2286 100644
76--- a/src/txpkgupload/tests/filesystem.txt
77+++ b/src/txpkgupload/tests/filesystem.txt
78@@ -20,7 +20,8 @@ First we need to setup our test environment.
79 >>> testfile = "testfile"
80 >>> full_testfile = os.path.join(rootpath, testfile)
81 >>> testfile_contents = b"contents of the file"
82- >>> open(full_testfile, 'wb').write(testfile_contents)
83+ >>> with open(full_testfile, 'wb') as f:
84+ ... f.write(testfile_contents)
85
86 >>> testdir = "testdir"
87 >>> full_testdir = os.path.join(rootpath, testdir)
88@@ -144,7 +145,8 @@ directory.
89 >>> expected = [exp]
90 >>> for i in [ "foo", "bar" ]:
91 ... filename = os.path.join(rootpath, i)
92- ... x = open(filename, 'w')
93+ ... with open(filename, 'w'):
94+ ... pass
95 ... os.chmod(filename, stat.S_IRUSR | stat.S_IWUSR | \
96 ... stat.S_IRGRP | stat.S_IROTH)
97 ... exp = copy.copy(def_exp)
98@@ -285,7 +287,8 @@ file does not exist or is a directory.
99 >>> ufs.remove(testfile)
100 >>> os.path.exists(full_testfile)
101 False
102- >>> open(full_testfile, 'wb').write(b"contents of the file")
103+ >>> with open(full_testfile, 'wb') as f:
104+ ... f.write(b"contents of the file")
105
106 >>> ufs.remove(testdir)
107 Traceback (most recent call last):
108@@ -314,7 +317,8 @@ directory.
109 False
110 >>> os.path.exists(new_full_testfile)
111 True
112- >>> open(new_full_testfile, "rb").read() == testfile_contents
113+ >>> with open(new_full_testfile, "rb") as f:
114+ ... f.read() == testfile_contents
115 True
116 >>> ufs.rename(new_testfile, testfile)
117
118@@ -365,7 +369,8 @@ the given `filter` function returns True for it.
119 testfile
120
121 >>> for i in [ "foo", "bar", "baz", "bat" ]:
122- ... x = open(os.path.join(rootpath, i), 'w')
123+ ... with open(os.path.join(rootpath, i), 'w'):
124+ ... pass
125 >>> names = ufs.names(".", arbitrary_filter)
126 >>> names.sort()
127 >>> names == ['baz', 'foo']
128@@ -380,7 +385,8 @@ writefile
129
130 >>> import io
131 >>> ufs.writefile("upload", io.BytesIO(propaganda))
132- >>> open(os.path.join(rootpath, "upload"), "rb").read() == propaganda
133+ >>> with open(os.path.join(rootpath, "upload"), "rb") as f:
134+ ... f.read() == propaganda
135 True
136 >>> ufs.remove("upload")
137
138@@ -388,7 +394,8 @@ If neither `start` nor `end` are specified, then the file contents
139 are overwritten.
140
141 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"))
142- >>> open(full_testfile, "rb").read() == b"MOO"
143+ >>> with open(full_testfile, "rb") as f:
144+ ... f.read() == b"MOO"
145 True
146 >>> ufs.writefile(testfile, io.BytesIO(testfile_contents))
147
148@@ -408,7 +415,8 @@ If `start` or `end` is not None, then only part of the file is
149 written. The remainder of the file is unchanged.
150
151 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), 9, 12)
152- >>> open(full_testfile, "rb").read() == b"contents MOOthe file"
153+ >>> with open(full_testfile, "rb") as f:
154+ ... f.read() == b"contents MOOthe file"
155 True
156 >>> ufs.writefile(testfile, io.BytesIO(testfile_contents))
157
158@@ -416,7 +424,8 @@ If `end` is None, then the file is truncated after the data are
159 written.
160
161 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), 9)
162- >>> open(full_testfile, "rb").read() == b"contents MOO"
163+ >>> with open(full_testfile, "rb") as f:
164+ ... f.read() == b"contents MOO"
165 True
166 >>> ufs.writefile(testfile, io.BytesIO(testfile_contents))
167
168@@ -424,8 +433,8 @@ If `start` is specified and the file doesn't exist or is shorter
169 than start, the file will contain undefined data before start.
170
171 >>> ufs.writefile("didnt-exist", io.BytesIO(b"MOO"), 9)
172- >>> open(os.path.join(rootpath, "didnt-exist"), "rb").read() == (
173- ... b"\x00\x00\x00\x00\x00\x00\x00\x00\x00MOO")
174+ >>> with open(os.path.join(rootpath, "didnt-exist"), "rb") as f:
175+ ... f.read() == b"\x00\x00\x00\x00\x00\x00\x00\x00\x00MOO"
176 True
177 >>> ufs.remove("didnt-exist")
178
179@@ -433,32 +442,37 @@ If `end` is not None and there isn't enough data in `instream` to fill
180 out the file, then the missing data is undefined.
181
182 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), 9, 15)
183- >>> open(full_testfile, "rb").read() == b"contents MOOthe file"
184+ >>> with open(full_testfile, "rb") as f:
185+ ... f.read() == b"contents MOOthe file"
186 True
187 >>> ufs.writefile(testfile, io.BytesIO(testfile_contents))
188
189 If `end` is less than or the same as `start no data is writen to the file.
190
191 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), 9, 4)
192- >>> open(full_testfile, "rb").read() == b"contents of the file"
193+ >>> with open(full_testfile, "rb") as f:
194+ ... f.read() == b"contents of the file"
195 True
196
197 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), 9, 9)
198- >>> open(full_testfile, "rb").read() == b"contents of the file"
199+ >>> with open(full_testfile, "rb") as f:
200+ ... f.read() == b"contents of the file"
201 True
202
203 If `append` is true the file is appended to rather than being
204 overwritten.
205
206 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), append=True)
207- >>> open(full_testfile, "rb").read() == b"contents of the fileMOO"
208+ >>> with open(full_testfile, "rb") as f:
209+ ... f.read() == b"contents of the fileMOO"
210 True
211 >>> ufs.writefile(testfile, io.BytesIO(testfile_contents))
212
213 Additionally, if `append` is true, `start` and `end` are ignored.
214
215 >>> ufs.writefile(testfile, io.BytesIO(b"MOO"), 10, 13, append=True)
216- >>> open(full_testfile, "rb").read() == b"contents of the fileMOO"
217+ >>> with open(full_testfile, "rb") as f:
218+ ... f.read() == b"contents of the fileMOO"
219 True
220 >>> ufs.writefile(testfile, io.BytesIO(testfile_contents))
221
222@@ -470,7 +484,8 @@ path:
223 >>> ufs.writefile("foo/bar", io.BytesIO(b"fake"))
224 >>> os.path.exists(os.path.join(rootpath, "foo/bar"))
225 True
226- >>> open(os.path.join(rootpath, "foo/bar"), "rb").read() == b"fake"
227+ >>> with open(os.path.join(rootpath, "foo/bar"), "rb") as f:
228+ ... f.read() == b"fake"
229 True
230
231
232diff --git a/src/txpkgupload/tests/test_plugin.py b/src/txpkgupload/tests/test_plugin.py
233index c3601c8..0f6968e 100644
234--- a/src/txpkgupload/tests/test_plugin.py
235+++ b/src/txpkgupload/tests/test_plugin.py
236@@ -572,7 +572,8 @@ class TestPkgUploadServiceMakerMixin:
237 yield self.server.waitForClose()
238
239 wanted_path = self._uploadPath('foo/bar/baz')
240- fs_content = open(os.path.join(wanted_path)).read()
241+ with open(os.path.join(wanted_path)) as f:
242+ fs_content = f.read()
243 self.assertEqual(fs_content, "fake contents")
244 # Expected mode is -rw-rwSr--.
245 self.assertEqual(
246@@ -613,7 +614,8 @@ class TestPkgUploadServiceMakerMixin:
247 for upload in files:
248 wanted_path = self._uploadPath(
249 "~ppa-user/ppa/ubuntu/%s" % upload)
250- fs_content = open(os.path.join(wanted_path)).read()
251+ with open(os.path.join(wanted_path)) as f:
252+ fs_content = f.read()
253 self.assertEqual(fs_content, upload)
254 # Expected mode is -rw-rwSr--.
255 self.assertEqual(
256@@ -661,8 +663,9 @@ class TestPkgUploadServiceMakerMixin:
257 # Check the contents of files on each session.
258 expected_contents = ['ONE', 'TWO', 'THREE', 'FOUR']
259 for index in range(4):
260- content = open(os.path.join(
261- self.server.fsroot, upload_dirs[index], "test")).read()
262+ with open(os.path.join(
263+ self.server.fsroot, upload_dirs[index], "test")) as f:
264+ content = f.read()
265 self.assertEqual(content, expected_contents[index])
266
267
268diff --git a/src/txpkgupload/tests/test_twistedsftp.py b/src/txpkgupload/tests/test_twistedsftp.py
269index 01b5e20..a1b3093 100644
270--- a/src/txpkgupload/tests/test_twistedsftp.py
271+++ b/src/txpkgupload/tests/test_twistedsftp.py
272@@ -65,9 +65,8 @@ class TestSFTPServer(testtools.TestCase):
273 upload_file = self.sftp_server.openFile('foo/bar', None, None)
274 upload_file.writeChunk(0, b"This is a test")
275 file_name = os.path.join(self.sftp_server._current_upload, 'foo/bar')
276- test_file = open(file_name, 'r')
277- self.assertEqual(test_file.read(), "This is a test")
278- test_file.close()
279+ with open(file_name, 'r') as test_file:
280+ self.assertEqual(test_file.read(), "This is a test")
281 self.assertPermissions(0o100644, file_name)
282 dir_name = os.path.join(self.sftp_server._current_upload, 'bar/foo')
283 os.makedirs(dir_name)

Subscribers

People subscribed via source and target branches

to all changes: