Merge ~cjwatson/launchpad:resource-warnings into launchpad:master
- Git
- lp:~cjwatson/launchpad
- resource-warnings
- Merge into master
Proposed by
Colin Watson
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | ff09e66b4cec763097798dcbb413510f5a3b4872 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~cjwatson/launchpad:resource-warnings |
Merge into: | launchpad:master |
Diff against target: |
637 lines (+163/-122) 12 files modified
lib/lp/bugs/doc/externalbugtracker-debbugs.rst (+8/-10) lib/lp/bugs/scripts/debbugs.py (+7/-2) lib/lp/codehosting/tests/test_rewrite.py (+2/-1) lib/lp/registry/doc/cache-country-mirrors.rst (+3/-2) lib/lp/services/apachelogparser/tests/test_apachelogparser.py (+55/-47) lib/lp/services/twistedsupport/loggingsupport.py (+4/-1) lib/lp/services/twistedsupport/tests/test_loggingsupport.py (+51/-35) lib/lp/soyuz/doc/gina-multiple-arch.rst (+3/-3) lib/lp/soyuz/doc/gina.rst (+16/-16) lib/lp/soyuz/scripts/gina/archive.py (+3/-0) lib/lp/soyuz/scripts/tests/test_gina.py (+7/-4) lib/lp/testing/layers.py (+4/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jürgen Gmach | Approve | ||
Review via email: mp+436127@code.launchpad.net |
Commit message
Fix various ResourceWarnings in tests
Description of the change
I found this collection of miscellaneous `ResourceWarning` fixes when looking through my various local cleanup branches. I'm sure there's more of the same to be found elsewhere, but we might as well make the test suite a little quieter.
To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/bugs/doc/externalbugtracker-debbugs.rst b/lib/lp/bugs/doc/externalbugtracker-debbugs.rst |
2 | index 6fd64aa..b26f84f 100644 |
3 | --- a/lib/lp/bugs/doc/externalbugtracker-debbugs.rst |
4 | +++ b/lib/lp/bugs/doc/externalbugtracker-debbugs.rst |
5 | @@ -175,11 +175,10 @@ cause importance to be set to medium, equivalent to the default normal |
6 | severity in debbugs. |
7 | |
8 | >>> import email |
9 | - >>> summary = email.message_from_file( |
10 | - ... open( |
11 | - ... os.path.join(test_db_location, "db-h", "01", "237001.summary") |
12 | - ... ) |
13 | - ... ) |
14 | + >>> with open( |
15 | + ... os.path.join(test_db_location, "db-h", "01", "237001.summary") |
16 | + ... ) as summary_file: |
17 | + ... summary = email.message_from_file(summary_file) |
18 | >>> "Severity" not in summary |
19 | True |
20 | |
21 | @@ -327,11 +326,10 @@ The Debbugs ExternalBugTracker can import a Debian bug into Launchpad. |
22 | The bug reporter gets taken from the From field in the debbugs bug |
23 | report. |
24 | |
25 | - >>> report = email.message_from_file( |
26 | - ... open( |
27 | - ... os.path.join(test_db_location, "db-h", "35", "322535.report") |
28 | - ... ) |
29 | - ... ) |
30 | + >>> with open( |
31 | + ... os.path.join(test_db_location, "db-h", "35", "322535.report") |
32 | + ... ) as report_file: |
33 | + ... report = email.message_from_file(report_file) |
34 | >>> print(report["From"]) |
35 | Moritz Muehlenhoff <jmm@inutil.org> |
36 | |
37 | diff --git a/lib/lp/bugs/scripts/debbugs.py b/lib/lp/bugs/scripts/debbugs.py |
38 | index a0307fc..4a908cc 100644 |
39 | --- a/lib/lp/bugs/scripts/debbugs.py |
40 | +++ b/lib/lp/bugs/scripts/debbugs.py |
41 | @@ -123,6 +123,7 @@ class Database: |
42 | def __next__(self): |
43 | line = self.index.readline() |
44 | if not line: |
45 | + self.index.close() |
46 | raise StopIteration |
47 | |
48 | match = self.index_record.match(line) |
49 | @@ -184,6 +185,8 @@ class Database: |
50 | message = email.message_from_file(fd) |
51 | except Exception as e: |
52 | raise SummaryParseError("%s: %s" % (summary, str(e))) |
53 | + finally: |
54 | + fd.close() |
55 | |
56 | version = message["format-version"] |
57 | if version is None: |
58 | @@ -223,8 +226,10 @@ class Database: |
59 | except FileNotFoundError: |
60 | raise ReportMissing(report) |
61 | |
62 | - bug.report = fd.read() |
63 | - fd.close() |
64 | + try: |
65 | + bug.report = fd.read() |
66 | + finally: |
67 | + fd.close() |
68 | |
69 | report_msg = email.message_from_bytes(bug.report) |
70 | charset = report_msg.get_content_charset("ascii") |
71 | diff --git a/lib/lp/codehosting/tests/test_rewrite.py b/lib/lp/codehosting/tests/test_rewrite.py |
72 | index 0424f32..9ba726d 100644 |
73 | --- a/lib/lp/codehosting/tests/test_rewrite.py |
74 | +++ b/lib/lp/codehosting/tests/test_rewrite.py |
75 | @@ -350,7 +350,7 @@ class TestBranchRewriterScript(TestCaseWithFactory): |
76 | output_lines.append(nonblocking_readline(proc.stdout, 60).rstrip("\n")) |
77 | |
78 | os.kill(proc.pid, signal.SIGINT) |
79 | - err = proc.stderr.read() |
80 | + _, err = proc.communicate() |
81 | # The script produces logging output, but not to stderr. |
82 | self.assertEqual("", err) |
83 | self.assertEqual(expected_lines, output_lines) |
84 | @@ -373,6 +373,7 @@ class TestBranchRewriterScriptHandlesDisconnects(TestCase): |
85 | universal_newlines=True, |
86 | ) |
87 | |
88 | + self.addCleanup(self.rewriter_proc.communicate) |
89 | self.addCleanup(self.rewriter_proc.terminate) |
90 | |
91 | def request(self, query): |
92 | diff --git a/lib/lp/registry/doc/cache-country-mirrors.rst b/lib/lp/registry/doc/cache-country-mirrors.rst |
93 | index 7c9b82c..78dfe6c 100644 |
94 | --- a/lib/lp/registry/doc/cache-country-mirrors.rst |
95 | +++ b/lib/lp/registry/doc/cache-country-mirrors.rst |
96 | @@ -48,8 +48,9 @@ look good. |
97 | >>> fr_txt_path = os.path.join(directory, "FR.txt") |
98 | >>> print("%o" % stat.S_IMODE(os.stat(fr_txt_path).st_mode)) |
99 | 644 |
100 | - >>> for line in sorted(open(fr_txt_path).read().split("\n")): |
101 | - ... print(line) |
102 | + >>> with open(fr_txt_path) as fr_txt: |
103 | + ... for line in sorted(fr_txt.read().split("\n")): |
104 | + ... print(line) |
105 | ... |
106 | http://archive.ubuntu.com/ubuntu/ |
107 | http://localhost:11375/archive-mirror/ |
108 | diff --git a/lib/lp/services/apachelogparser/tests/test_apachelogparser.py b/lib/lp/services/apachelogparser/tests/test_apachelogparser.py |
109 | index b820eb1..dee5327 100644 |
110 | --- a/lib/lp/services/apachelogparser/tests/test_apachelogparser.py |
111 | +++ b/lib/lp/services/apachelogparser/tests/test_apachelogparser.py |
112 | @@ -36,12 +36,12 @@ class TestLineParsing(TestCase): |
113 | """Test parsing of lines of an apache log file.""" |
114 | |
115 | def test_return_value(self): |
116 | - fd = open( |
117 | + with open( |
118 | os.path.join(here, "apache-log-files", "librarian-oneline.log") |
119 | - ) |
120 | - host, date, status, request = get_host_date_status_and_request( |
121 | - fd.readline() |
122 | - ) |
123 | + ) as fd: |
124 | + host, date, status, request = get_host_date_status_and_request( |
125 | + fd.readline() |
126 | + ) |
127 | self.assertEqual(host, "201.158.154.121") |
128 | self.assertEqual(date, "[13/Jun/2008:18:38:57 +0100]") |
129 | self.assertEqual(status, "200") |
130 | @@ -158,8 +158,11 @@ class Test_get_fd_and_file_size(TestCase): |
131 | very beginning. |
132 | """ |
133 | fd, file_size = get_fd_and_file_size(file_path) |
134 | - self.assertEqual(fd.tell(), 0) |
135 | - self.assertEqual(len(fd.read()), file_size) |
136 | + try: |
137 | + self.assertEqual(fd.tell(), 0) |
138 | + self.assertEqual(len(fd.read()), file_size) |
139 | + finally: |
140 | + fd.close() |
141 | |
142 | def test_regular_file(self): |
143 | file_path = os.path.join( |
144 | @@ -212,58 +215,61 @@ class TestLogFileParsing(TestCase): |
145 | # also been downloaded once (last line of the sample log), but |
146 | # parse_file() always skips the last line as it may be truncated, so |
147 | # it doesn't show up in the dict returned. |
148 | - fd = open( |
149 | + with open( |
150 | os.path.join( |
151 | here, "apache-log-files", "launchpadlibrarian.net.access-log" |
152 | ), |
153 | "rb", |
154 | - ) |
155 | - downloads, parsed_bytes, parsed_lines = parse_file( |
156 | - fd, |
157 | - start_position=0, |
158 | - logger=self.logger, |
159 | - get_download_key=get_path_download_key, |
160 | - ) |
161 | - self.assertEqual( |
162 | - self.logger.getLogBuffer().strip(), |
163 | - "INFO Parsed 5 lines resulting in 3 download stats.", |
164 | - ) |
165 | - date = datetime(2008, 6, 13) |
166 | - self.assertContentEqual( |
167 | - downloads.items(), |
168 | - [ |
169 | - ("/12060796/me-tv-icon-64x64.png", {date: {"AU": 1}}), |
170 | - ("/8196569/mediumubuntulogo.png", {date: {"AR": 1, "JP": 1}}), |
171 | - ("/9096290/me-tv-icon-14x14.png", {date: {"AU": 1}}), |
172 | - ], |
173 | - ) |
174 | + ) as fd: |
175 | + downloads, parsed_bytes, parsed_lines = parse_file( |
176 | + fd, |
177 | + start_position=0, |
178 | + logger=self.logger, |
179 | + get_download_key=get_path_download_key, |
180 | + ) |
181 | + self.assertEqual( |
182 | + self.logger.getLogBuffer().strip(), |
183 | + "INFO Parsed 5 lines resulting in 3 download stats.", |
184 | + ) |
185 | + date = datetime(2008, 6, 13) |
186 | + self.assertContentEqual( |
187 | + downloads.items(), |
188 | + [ |
189 | + ("/12060796/me-tv-icon-64x64.png", {date: {"AU": 1}}), |
190 | + ( |
191 | + "/8196569/mediumubuntulogo.png", |
192 | + {date: {"AR": 1, "JP": 1}}, |
193 | + ), |
194 | + ("/9096290/me-tv-icon-14x14.png", {date: {"AU": 1}}), |
195 | + ], |
196 | + ) |
197 | |
198 | - # The last line is skipped, so we'll record that the file has been |
199 | - # parsed until the beginning of the last line. |
200 | - self.assertNotEqual(parsed_bytes, fd.tell()) |
201 | - self.assertEqual(parsed_bytes, self._getLastLineStart(fd)) |
202 | + # The last line is skipped, so we'll record that the file has been |
203 | + # parsed until the beginning of the last line. |
204 | + self.assertNotEqual(parsed_bytes, fd.tell()) |
205 | + self.assertEqual(parsed_bytes, self._getLastLineStart(fd)) |
206 | |
207 | def test_parsing_last_line(self): |
208 | # When there's only the last line of a given file for us to parse, we |
209 | # assume the file has been rotated and it's safe to parse its last |
210 | # line without worrying about whether or not it's been truncated. |
211 | - fd = open( |
212 | + with open( |
213 | os.path.join( |
214 | here, "apache-log-files", "launchpadlibrarian.net.access-log" |
215 | ), |
216 | "rb", |
217 | - ) |
218 | - downloads, parsed_bytes, parsed_lines = parse_file( |
219 | - fd, |
220 | - start_position=self._getLastLineStart(fd), |
221 | - logger=self.logger, |
222 | - get_download_key=get_path_download_key, |
223 | - ) |
224 | - self.assertEqual( |
225 | - self.logger.getLogBuffer().strip(), |
226 | - "INFO Parsed 1 lines resulting in 1 download stats.", |
227 | - ) |
228 | - self.assertEqual(parsed_bytes, fd.tell()) |
229 | + ) as fd: |
230 | + downloads, parsed_bytes, parsed_lines = parse_file( |
231 | + fd, |
232 | + start_position=self._getLastLineStart(fd), |
233 | + logger=self.logger, |
234 | + get_download_key=get_path_download_key, |
235 | + ) |
236 | + self.assertEqual( |
237 | + self.logger.getLogBuffer().strip(), |
238 | + "INFO Parsed 1 lines resulting in 1 download stats.", |
239 | + ) |
240 | + self.assertEqual(parsed_bytes, fd.tell()) |
241 | |
242 | self.assertContentEqual( |
243 | downloads.items(), |
244 | @@ -526,7 +532,8 @@ class TestParsedFilesDetection(TestCase): |
245 | # A file that has been parsed already but in which new content was |
246 | # added will be parsed again, starting from where parsing stopped last |
247 | # time. |
248 | - first_line = open(self.file_path).readline() |
249 | + with open(self.file_path) as fd: |
250 | + first_line = fd.readline() |
251 | ParsedApacheLog(first_line, len(first_line)) |
252 | |
253 | files_to_parse = list(get_files_to_parse([self.file_path])) |
254 | @@ -576,7 +583,8 @@ class TestParsedFilesDetection(TestCase): |
255 | # stopped last time. (Here we pretend we parsed only the first line) |
256 | gz_name = "launchpadlibrarian.net.access-log.1.gz" |
257 | gz_path = os.path.join(self.root, gz_name) |
258 | - first_line = gzip.open(gz_path).readline() |
259 | + with gzip.open(gz_path) as gz: |
260 | + first_line = gz.readline() |
261 | ParsedApacheLog(first_line, len(first_line)) |
262 | files_to_parse = get_files_to_parse([gz_path]) |
263 | positions = [] |
264 | diff --git a/lib/lp/services/twistedsupport/loggingsupport.py b/lib/lp/services/twistedsupport/loggingsupport.py |
265 | index 2856992..6936fb6 100644 |
266 | --- a/lib/lp/services/twistedsupport/loggingsupport.py |
267 | +++ b/lib/lp/services/twistedsupport/loggingsupport.py |
268 | @@ -80,12 +80,13 @@ class LaunchpadLogFile(DailyLogFile): |
269 | maxRotatedFiles=None, |
270 | compressLast=None, |
271 | ): |
272 | - DailyLogFile.__init__(self, name, directory, defaultMode) |
273 | if maxRotatedFiles is not None: |
274 | self.maxRotatedFiles = int(maxRotatedFiles) |
275 | if compressLast is not None: |
276 | self.compressLast = int(compressLast) |
277 | |
278 | + # Check parameters before calling the superclass's __init__, since |
279 | + # that opens a file that we'd otherwise need to close. |
280 | assert ( |
281 | self.compressLast <= self.maxRotatedFiles |
282 | ), "Only %d rotate files are kept, cannot compress %d" % ( |
283 | @@ -93,6 +94,8 @@ class LaunchpadLogFile(DailyLogFile): |
284 | self.compressLast, |
285 | ) |
286 | |
287 | + super().__init__(name, directory, defaultMode) |
288 | + |
289 | def _compressFile(self, path): |
290 | """Compress the file in the given path using bzip2. |
291 | |
292 | diff --git a/lib/lp/services/twistedsupport/tests/test_loggingsupport.py b/lib/lp/services/twistedsupport/tests/test_loggingsupport.py |
293 | index a929f49..a6ecac7 100644 |
294 | --- a/lib/lp/services/twistedsupport/tests/test_loggingsupport.py |
295 | +++ b/lib/lp/services/twistedsupport/tests/test_loggingsupport.py |
296 | @@ -29,15 +29,21 @@ class TestLaunchpadLogFile(TestCase): |
297 | """ |
298 | # Default behaviour. |
299 | log_file = LaunchpadLogFile("test.log", self.temp_dir) |
300 | - self.assertEqual(5, log_file.maxRotatedFiles) |
301 | - self.assertEqual(3, log_file.compressLast) |
302 | + try: |
303 | + self.assertEqual(5, log_file.maxRotatedFiles) |
304 | + self.assertEqual(3, log_file.compressLast) |
305 | + finally: |
306 | + log_file.close() |
307 | |
308 | # Keeping only compressed rotated logs. |
309 | log_file = LaunchpadLogFile( |
310 | "test.log", self.temp_dir, maxRotatedFiles=1, compressLast=1 |
311 | ) |
312 | - self.assertEqual(1, log_file.maxRotatedFiles) |
313 | - self.assertEqual(1, log_file.compressLast) |
314 | + try: |
315 | + self.assertEqual(1, log_file.maxRotatedFiles) |
316 | + self.assertEqual(1, log_file.compressLast) |
317 | + finally: |
318 | + log_file.close() |
319 | |
320 | # Inconsistent parameters, compression more than kept rotated files. |
321 | self.assertRaises( |
322 | @@ -52,9 +58,8 @@ class TestLaunchpadLogFile(TestCase): |
323 | def createTestFile(self, name, content="nothing"): |
324 | """Create a new file in the test directory.""" |
325 | file_path = os.path.join(self.temp_dir, name) |
326 | - fd = open(file_path, "w") |
327 | - fd.write(content) |
328 | - fd.close() |
329 | + with open(file_path, "w") as fd: |
330 | + fd.write(content) |
331 | return file_path |
332 | |
333 | def listTestFiles(self): |
334 | @@ -71,18 +76,24 @@ class TestLaunchpadLogFile(TestCase): |
335 | the newest first. |
336 | """ |
337 | log_file = LaunchpadLogFile("test.log", self.temp_dir) |
338 | - self.assertEqual(["test.log"], self.listTestFiles()) |
339 | - self.assertEqual([], log_file.listLogs()) |
340 | - |
341 | - self.createTestFile("boing") |
342 | - self.assertEqual([], log_file.listLogs()) |
343 | - |
344 | - self.createTestFile("test.log.2000-12-31") |
345 | - self.createTestFile("test.log.2000-12-30.bz2") |
346 | - self.assertEqual( |
347 | - ["test.log.2000-12-31", "test.log.2000-12-30.bz2"], |
348 | - [os.path.basename(log_path) for log_path in log_file.listLogs()], |
349 | - ) |
350 | + try: |
351 | + self.assertEqual(["test.log"], self.listTestFiles()) |
352 | + self.assertEqual([], log_file.listLogs()) |
353 | + |
354 | + self.createTestFile("boing") |
355 | + self.assertEqual([], log_file.listLogs()) |
356 | + |
357 | + self.createTestFile("test.log.2000-12-31") |
358 | + self.createTestFile("test.log.2000-12-30.bz2") |
359 | + self.assertEqual( |
360 | + ["test.log.2000-12-31", "test.log.2000-12-30.bz2"], |
361 | + [ |
362 | + os.path.basename(log_path) |
363 | + for log_path in log_file.listLogs() |
364 | + ], |
365 | + ) |
366 | + finally: |
367 | + log_file.close() |
368 | |
369 | def testRotate(self): |
370 | """Check `LaunchpadLogFile.rotate`. |
371 | @@ -95,24 +106,29 @@ class TestLaunchpadLogFile(TestCase): |
372 | "test.log", self.temp_dir, maxRotatedFiles=2, compressLast=1 |
373 | ) |
374 | |
375 | - # Monkey-patch DailyLogFile.suffix to be time independent. |
376 | - self.local_index = 0 |
377 | + try: |
378 | + # Monkey-patch DailyLogFile.suffix to be time independent. |
379 | + self.local_index = 0 |
380 | |
381 | - def testSuffix(tupledate): |
382 | - self.local_index += 1 |
383 | - return str(self.local_index) |
384 | + def testSuffix(tupledate): |
385 | + self.local_index += 1 |
386 | + return str(self.local_index) |
387 | |
388 | - log_file.suffix = testSuffix |
389 | + log_file.suffix = testSuffix |
390 | |
391 | - log_file.rotate() |
392 | - self.assertEqual(["test.log", "test.log.1"], self.listTestFiles()) |
393 | + log_file.rotate() |
394 | + self.assertEqual(["test.log", "test.log.1"], self.listTestFiles()) |
395 | |
396 | - log_file.rotate() |
397 | - self.assertEqual( |
398 | - ["test.log", "test.log.1.bz2", "test.log.2"], self.listTestFiles() |
399 | - ) |
400 | + log_file.rotate() |
401 | + self.assertEqual( |
402 | + ["test.log", "test.log.1.bz2", "test.log.2"], |
403 | + self.listTestFiles(), |
404 | + ) |
405 | |
406 | - log_file.rotate() |
407 | - self.assertEqual( |
408 | - ["test.log", "test.log.2.bz2", "test.log.3"], self.listTestFiles() |
409 | - ) |
410 | + log_file.rotate() |
411 | + self.assertEqual( |
412 | + ["test.log", "test.log.2.bz2", "test.log.3"], |
413 | + self.listTestFiles(), |
414 | + ) |
415 | + finally: |
416 | + log_file.close() |
417 | diff --git a/lib/lp/soyuz/doc/gina-multiple-arch.rst b/lib/lp/soyuz/doc/gina-multiple-arch.rst |
418 | index f9c5e1e..70c851a 100644 |
419 | --- a/lib/lp/soyuz/doc/gina-multiple-arch.rst |
420 | +++ b/lib/lp/soyuz/doc/gina-multiple-arch.rst |
421 | @@ -98,17 +98,17 @@ Let's set up the filesystem: |
422 | >>> os.symlink(path, "/tmp/gina_test_archive") |
423 | |
424 | >>> gina_proc = ["scripts/gina.py", "-q", "dapper", "dapper-updates"] |
425 | - >>> proc = subprocess.Popen( |
426 | + >>> proc = subprocess.run( |
427 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
428 | ... ) |
429 | - >>> print(proc.stderr.read()) |
430 | + >>> print(proc.stderr) |
431 | WARNING ... |
432 | WARNING No source package bdftopcf (0.99.0-1) listed for bdftopcf |
433 | (0.99.0-1), scrubbing archive... |
434 | WARNING The archive for dapper-updates/universe doesn't contain a |
435 | directory for powerpc, skipping |
436 | <BLANKLINE> |
437 | - >>> proc.wait() |
438 | + >>> proc.returncode |
439 | 0 |
440 | |
441 | Make the changes visible elsewhere: |
442 | diff --git a/lib/lp/soyuz/doc/gina.rst b/lib/lp/soyuz/doc/gina.rst |
443 | index 5d5f0c9..97d53d5 100644 |
444 | --- a/lib/lp/soyuz/doc/gina.rst |
445 | +++ b/lib/lp/soyuz/doc/gina.rst |
446 | @@ -136,13 +136,13 @@ Let's set up the filesystem: |
447 | And give it a spin: |
448 | |
449 | >>> gina_proc = ["scripts/gina.py", "-q", "hoary", "breezy"] |
450 | - >>> proc = subprocess.Popen( |
451 | + >>> proc = subprocess.run( |
452 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
453 | ... ) |
454 | |
455 | Check STDERR for the errors we expected: |
456 | |
457 | - >>> print(proc.stderr.read()) |
458 | + >>> print(proc.stderr) |
459 | ERROR Error processing package files for clearlooks |
460 | ... |
461 | ...ExecutionError: Error 2 unpacking source |
462 | @@ -200,7 +200,7 @@ Check STDERR for the errors we expected: |
463 | |
464 | The exit status must be 0, for success: |
465 | |
466 | - >>> proc.wait() |
467 | + >>> proc.returncode |
468 | 0 |
469 | >>> transaction.commit() |
470 | |
471 | @@ -567,10 +567,10 @@ been updated for packages in breezy which have changed since the last |
472 | run. |
473 | |
474 | >>> gina_proc = ["scripts/gina.py", "-q", "hoary", "breezy"] |
475 | - >>> proc = subprocess.Popen( |
476 | + >>> proc = subprocess.run( |
477 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
478 | ... ) |
479 | - >>> print(proc.stderr.read()) |
480 | + >>> print(proc.stderr) |
481 | ERROR Error processing package files for clearlooks |
482 | ... |
483 | ...ExecutionError: Error 2 unpacking source |
484 | @@ -615,7 +615,7 @@ run. |
485 | ... |
486 | ...PoolFileNotFound: .../python-sqlite_1.0.1-2ubuntu1_all.deb not found |
487 | <BLANKLINE> |
488 | - >>> proc.wait() |
489 | + >>> proc.returncode |
490 | 0 |
491 | >>> transaction.commit() |
492 | |
493 | @@ -709,10 +709,10 @@ First get a set of existing publishings for both source and binary: |
494 | Now run gina to import packages and convert them to partner: |
495 | |
496 | >>> gina_proc = ["scripts/gina.py", "-q", "partner"] |
497 | - >>> proc = subprocess.Popen( |
498 | + >>> proc = subprocess.run( |
499 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
500 | ... ) |
501 | - >>> proc.wait() |
502 | + >>> proc.returncode |
503 | 0 |
504 | >>> transaction.commit() |
505 | |
506 | @@ -826,10 +826,10 @@ Commit the changes and run the importer script. |
507 | >>> transaction.commit() |
508 | |
509 | >>> gina_proc = ["scripts/gina.py", "-q", "lenny"] |
510 | - >>> proc = subprocess.Popen( |
511 | + >>> proc = subprocess.run( |
512 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
513 | ... ) |
514 | - >>> proc.wait() |
515 | + >>> proc.returncode |
516 | 0 |
517 | |
518 | >>> transaction.commit() |
519 | @@ -866,11 +866,11 @@ Both, 'lenny' and 'hoary' (as partner) will be processed in the same |
520 | batch. |
521 | |
522 | >>> gina_proc = ["scripts/gina.py", "lenny", "partner"] |
523 | - >>> proc = subprocess.Popen( |
524 | + >>> proc = subprocess.run( |
525 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
526 | ... ) |
527 | |
528 | - >>> print(proc.stderr.read()) |
529 | + >>> print(proc.stderr) |
530 | INFO Creating lockfile: /var/lock/launchpad-gina.lock |
531 | ... |
532 | INFO === Processing debian/lenny/release === |
533 | @@ -878,7 +878,7 @@ batch. |
534 | INFO === Processing ubuntu/hoary/release === |
535 | ... |
536 | |
537 | - >>> proc.wait() |
538 | + >>> proc.returncode |
539 | 0 |
540 | |
541 | |
542 | @@ -888,15 +888,15 @@ Other tests |
543 | For kicks, finally, run gina on a configured but incomplete archive: |
544 | |
545 | >>> gina_proc = ["scripts/gina.py", "-q", "bogus"] |
546 | - >>> proc = subprocess.Popen( |
547 | + >>> proc = subprocess.run( |
548 | ... gina_proc, stderr=subprocess.PIPE, universal_newlines=True |
549 | ... ) |
550 | - >>> print(proc.stderr.read()) |
551 | + >>> print(proc.stderr) |
552 | ERROR Failed to analyze archive for bogoland |
553 | ... |
554 | ...MangledArchiveError: No archive directory for bogoland/main |
555 | <BLANKLINE> |
556 | - >>> proc.wait() |
557 | + >>> proc.returncode |
558 | 1 |
559 | |
560 | |
561 | diff --git a/lib/lp/soyuz/scripts/gina/archive.py b/lib/lp/soyuz/scripts/gina/archive.py |
562 | index 19ab69d..f283190 100644 |
563 | --- a/lib/lp/soyuz/scripts/gina/archive.py |
564 | +++ b/lib/lp/soyuz/scripts/gina/archive.py |
565 | @@ -125,10 +125,13 @@ class ArchiveFilesystemInfo: |
566 | |
567 | def cleanup(self): |
568 | os.unlink(self.sources_tagfile) |
569 | + self.srcfile.close() |
570 | if self.source_only: |
571 | return |
572 | os.unlink(self.binaries_tagfile) |
573 | + self.binfile.close() |
574 | os.unlink(self.di_tagfile) |
575 | + self.difile.close() |
576 | |
577 | |
578 | class ArchiveComponentItems: |
579 | diff --git a/lib/lp/soyuz/scripts/tests/test_gina.py b/lib/lp/soyuz/scripts/tests/test_gina.py |
580 | index ad9061d..3c37297 100644 |
581 | --- a/lib/lp/soyuz/scripts/tests/test_gina.py |
582 | +++ b/lib/lp/soyuz/scripts/tests/test_gina.py |
583 | @@ -244,10 +244,13 @@ class TestArchiveFilesystemInfo(TestCase): |
584 | archive_info = ArchiveFilesystemInfo( |
585 | archive_root, "breezy", "main", "i386" |
586 | ) |
587 | - sources = apt_pkg.TagFile(archive_info.srcfile) |
588 | - self.assertEqual("archive-copier", next(sources)["Package"]) |
589 | - binaries = apt_pkg.TagFile(archive_info.binfile) |
590 | - self.assertEqual("python-pam", next(binaries)["Package"]) |
591 | + try: |
592 | + with apt_pkg.TagFile(archive_info.srcfile) as sources: |
593 | + self.assertEqual("archive-copier", next(sources)["Package"]) |
594 | + with apt_pkg.TagFile(archive_info.binfile) as binaries: |
595 | + self.assertEqual("python-pam", next(binaries)["Package"]) |
596 | + finally: |
597 | + archive_info.cleanup() |
598 | |
599 | def test_uncompressed(self): |
600 | self.assertCompressionTypeWorks(lambda path: None) |
601 | diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py |
602 | index eca5535..1e27824 100644 |
603 | --- a/lib/lp/testing/layers.py |
604 | +++ b/lib/lp/testing/layers.py |
605 | @@ -614,7 +614,7 @@ class MemcachedLayer(BaseLayer): |
606 | except OSError: |
607 | pass |
608 | # Clean up the resulting zombie. |
609 | - MemcachedLayer._memcached_process.wait() |
610 | + MemcachedLayer._memcached_process.communicate() |
611 | MemcachedLayer._memcached_process = None |
612 | |
613 | @classmethod |
614 | @@ -1689,6 +1689,7 @@ class LayerProcessController: |
615 | except ProcessLookupError: |
616 | # The child process doesn't exist. Maybe it went away by the |
617 | # time we got here. |
618 | + cls.appserver.communicate() |
619 | cls.appserver = None |
620 | return False |
621 | else: |
622 | @@ -1722,6 +1723,7 @@ class LayerProcessController: |
623 | # The process is already gone. |
624 | return |
625 | until = datetime.datetime.now() + WAIT_INTERVAL |
626 | + cls.appserver.communicate() |
627 | cls.appserver = None |
628 | |
629 | @classmethod |
630 | @@ -1810,6 +1812,7 @@ class LayerProcessController: |
631 | break |
632 | else: |
633 | os.kill(cls.appserver.pid, signal.SIGTERM) |
634 | + cls.appserver.communicate() |
635 | cls.appserver = None |
636 | # Go no further. |
637 | raise AssertionError("App server startup timed out.") |