Launchpad itself

Merge lp:~jml/launchpad/sftp-poppy into lp:launchpad

  1. sftp-poppy
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Jonathan Lange
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jml/launchpad/sftp-poppy
Merge into: lp:launchpad
Diff against target: 485 lines (+125/-146)
8 files modified
lib/canonical/launchpad/daemons/tachandler.py (+83/-47)
lib/lp/poppy/filesystem.py (+0/-1)
lib/lp/poppy/tests/__init__.py (+5/-0)
lib/lp/poppy/tests/helpers.py (+8/-16)
lib/lp/poppy/tests/test_poppy.py (+17/-23)
lib/lp/services/osutils.py (+8/-54)
lib/lp/soyuz/doc/soyuz-upload.txt.disabled (+1/-1)
lib/lp/testing/__init__.py (+3/-4)
To merge this branch: bzr merge lp:~jml/launchpad/sftp-poppy
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Michael Nelson Pending
Review via email: mp+21627@code.launchpad.net

Commit message

Add two_stage_kill, speed up tachandler and kill by pidfile. Clean up poppy tests.

Description of the change

Contrary to what you might expect, this branch actually has nothing to do with SFTP. Instead, it does a bunch of cleanups and fixes to testing code that I had to touch in order to start thinking about with Poppy.

 canonical/launchpad/daemons/tachandler.py | 36 -----------------

Use osutils.kill_by_pidfile, which appeared to a direct copy and paste from this file.

 lp/services/osutils.py | 61 +++++++++++++++++++-----------

Extract the two-stage kill part of osutils.kill_by_pidfile into a separate function called two_stage_kill.

Fix a bug in the two-stage kill algorithm. If you terminate a process and don't give it the chance to report its results back to the parent process, the terminated process remains as a zombie. Without the waitpid, the polling loop will *always* run to completion and then do the SIGKILL, thus making the call take over 5s guaranteed.

Fixing this may well make some layers tear down faster.

 lp/poppy/filesystem.py | 1

Remove flakes.

 lp/poppy/tests/__init__.py | 5 ++
 lp/poppy/tests/helpers.py | 24 +++--------

Move all of the helper code out of __init__ and into helpers.py. Add a stub __init__.py.

Fix up some of the formatting in helpers, and re-use two_stage_kill, getting rid of the slow (wait for 2s) two-stage kill implementation here.

 lp/poppy/tests/test_poppy.py | 40 ++++++++-----------

Bits and pieces of cleanup:
 * Use lp.testing.TestCase
 * Fix some flakes
 * Rename tests to test_foo from testFoo, according to the standard
 * Delete the useless layer
 * Fix spelling mistakes
 * Use addCleanup instead of tearDown
 * Use makeTemporaryDirectory
 * Whitespace cleanups

 lp/soyuz/doc/soyuz-upload.txt.disabled | 2

The return value of killPoppy isn't useful and isn't used.

 lp/testing/__init__.py | 7 +--

Avoid using lambdas in addCleanup -- this is a throwback to an old addCleanup API. Re-use makeTemporaryDirectory in useTempDir.

Thanks,
jml

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Looks good. Two comments:

* It's not necessary to do waitpid(pid, os.WNOHANG) followed by
  kill(pid, 0); the return value from waitpid() will be (0, 0) when
  the process has not exited yet, and it will error the same as kill()
  if the process does not exist.

* It might be nice to be able to pass a subprocess.Popen into
  two_stage_kill, so that its poll() method is used, and so the
  returncode attribute is set correctly to avoid confusion. But,
  really, this is probably not worth the effort.

review: Approve
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (8.7 KiB)

Review: Approve
Nothing I can add to Gavin's (which you're investigating).

On Thu, Mar 18, 2010 at 11:35 AM, Jonathan Lange <email address hidden> wrote:
> Jonathan Lange has proposed merging lp:~jml/launchpad/sftp-poppy into lp:launchpad/devel.
>
> Requested reviews:
>  Canonical Launchpad Engineering (launchpad)
>
>
> Contrary to what you might expect, this branch actually has nothing to do with SFTP. Instead, it does a bunch of cleanups and fixes to testing code that I had to touch in order to start thinking about with Poppy.
>
>  canonical/launchpad/daemons/tachandler.py |   36 -----------------
>
> Use osutils.kill_by_pidfile, which appeared to a direct copy and paste from this file.
>
>  lp/services/osutils.py                    |   61 +++++++++++++++++++-----------
>
> Extract the two-stage kill part of osutils.kill_by_pidfile into a separate function called two_stage_kill.
>
> Fix a bug in the two-stage kill algorithm. If you terminate a process and don't give it the chance to report its results back to the parent process, the terminated process remains as a zombie. Without the waitpid, the polling loop will *always* run to completion and then do the SIGKILL, thus making the call take over 5s guaranteed.

Nice.

>
> Fixing this may well make some layers tear down faster.
>
>  lp/poppy/filesystem.py                    |    1
>
> Remove flakes.
>
>  lp/poppy/tests/__init__.py                |    5 ++
>  lp/poppy/tests/helpers.py                 |   24 +++--------
>
> Move all of the helper code out of __init__ and into helpers.py. Add a stub __init__.py.
>
> Fix up some of the formatting in helpers, and re-use two_stage_kill, getting rid of the slow (wait for 2s) two-stage kill implementation here.
>
>  lp/poppy/tests/test_poppy.py              |   40 ++++++++-----------
>
> Bits and pieces of cleanup:
>  * Use lp.testing.TestCase
>  * Fix some flakes
>  * Rename tests to test_foo from testFoo, according to the standard
>  * Delete the useless layer
>  * Fix spelling mistakes
>  * Use addCleanup instead of tearDown
>  * Use makeTemporaryDirectory
>  * Whitespace cleanups
>
>  lp/soyuz/doc/soyuz-upload.txt.disabled    |    2
>
> The return value of killPoppy isn't useful and isn't used.
>
>  lp/testing/__init__.py                    |    7 +--
>
> Avoid using lambdas in addCleanup -- this is a throwback to an old addCleanup API. Re-use makeTemporaryDirectory in useTempDir.
>
> Thanks,
> jml
> --
> https://code.launchpad.net/~jml/launchpad/sftp-poppy/+merge/21627
> Your team Launchpad code reviewers from Canonical is subscribed to branch lp:launchpad/devel.
>
> === modified file 'lib/canonical/launchpad/daemons/tachandler.py'
> --- lib/canonical/launchpad/daemons/tachandler.py       2010-02-19 13:27:29 +0000
> +++ lib/canonical/launchpad/daemons/tachandler.py       2010-03-18 11:35:30 +0000
> @@ -16,12 +16,12 @@
>  import sys
>  import os
>  import time
> -from signal import SIGTERM, SIGKILL
>  import subprocess
>
>  from twisted.application import service
>  from twisted.python import log
>
> +from lp.services.osutils import kill_by_pidfile
>
>  twistd_script = os.path.abspath(os.path.join(
>     os.path.dirname(__file__),
> @@ -118,39 +11...

Read more...

Revision history for this message
Jonathan Lange (jml) wrote :

On Thu, Mar 18, 2010 at 11:57 AM, Gavin Panella
<email address hidden> wrote:
> Review: Approve
> Looks good. Two comments:
>
> * It's not necessary to do waitpid(pid, os.WNOHANG) followed by
>  kill(pid, 0); the return value from waitpid() will be (0, 0) when
>  the process has not exited yet, and it will error the same as kill()
>  if the process does not exist.
>

Hmm. I see. I wasn't quite sure what the behaviour actually is, so I
did Science.

$ ps ax | grep 3711
 7702 pts/2 R+ 0:00 grep 3711
$ python
Python 2.6.4 (r264:75706, Dec 7 2009, 18:43:55)
[GCC 4.4.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import os

Spawn a long-running process

>>> pid = os.spawnv(os.P_NOWAIT, '/bin/sleep', ['/bin/sleep', '10'])
>>> pid
7820

waitpid on-existent process raises errors

>>> os.waitpid(3711, os.WNOHANG)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 10] No child processes

waitpid returns (0, 0) while it's running for (pid, result)
>>> os.waitpid(7820, os.WNOHANG)
(0, 0)

Once it's terminated, returns (pid, result) with a non-zero pid.
>>> os.waitpid(7820, os.WNOHANG)
(7820, 0)

Now that it's terminated and we've reaped it, it'll raise an exception
>>> os.waitpid(7820, os.WNOHANG)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 10] No child processes

I'll fix the code accordingly, making sure that we don't force a 0.1s
sleep if the SIGTERM was successful.

> * It might be nice to be able to pass a subprocess.Popen into
>  two_stage_kill, so that its poll() method is used, and so the
>  returncode attribute is set correctly to avoid confusion. But,
>  really, this is probably not worth the effort.

It's valuable having a low-level function that takes a pid. I agree
that what you describe would also be useful, but maybe not right now.
I wonder what we have to do to encourage others who might need this to
stick code into this module and re-use two-stage kill.

Thanks!
jml

Revision history for this message
Gavin Panella (allenap) wrote :

jml wrote:
> I wonder what we have to do to encourage others who might need this to
> stick code into this module and re-use two-stage kill.

Get it into the standard library! :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
2--- lib/canonical/launchpad/daemons/tachandler.py 2010-02-19 13:27:29 +0000
3+++ lib/canonical/launchpad/daemons/tachandler.py 2010-03-18 19:22:29 +0000
4@@ -7,11 +7,16 @@
5
6 __metaclass__ = type
7
8-__all__ = ['TacTestSetup', 'ReadyService', 'TacException']
9-
10-
11-# This file is used by launchpad-buildd, so it cannot import any
12-# Launchpad code!
13+__all__ = [
14+ 'TacTestSetup',
15+ 'ReadyService',
16+ 'TacException',
17+ 'kill_by_pidfile',
18+ 'remove_if_exists',
19+ 'two_stage_kill',
20+ ]
21+
22+
23 import errno
24 import sys
25 import os
26@@ -22,6 +27,77 @@
27 from twisted.application import service
28 from twisted.python import log
29
30+# This file is used by launchpad-buildd, so it cannot import any
31+# Launchpad code!
32+
33+
34+def two_stage_kill(pid, poll_interval=0.1, num_polls=50):
35+ """Kill process 'pid' with SIGTERM. If it doesn't die, SIGKILL it.
36+
37+ :param pid: The pid of the process to kill.
38+ :param poll_interval: The polling interval used to check if the
39+ process is still around.
40+ :param num_polls: The number of polls to do before doing a SIGKILL.
41+ """
42+ # Kill the process.
43+ try:
44+ os.kill(pid, SIGTERM)
45+ except OSError, e:
46+ if e.errno in (errno.ESRCH, errno.ECHILD):
47+ # Process has already been killed.
48+ return
49+
50+ # Poll until the process has ended.
51+ for i in range(num_polls):
52+ try:
53+ # Reap the child process and get its return value. If it's not
54+ # gone yet, continue.
55+ new_pid, result = os.waitpid(pid, os.WNOHANG)
56+ if new_pid:
57+ return result
58+ time.sleep(poll_interval)
59+ except OSError, e:
60+ # Raised if the process is gone by the time we try to get the
61+ # return value.
62+ return
63+
64+ # The process is still around, so terminate it violently.
65+ try:
66+ os.kill(pid, SIGKILL)
67+ except OSError:
68+ # Already terminated
69+ pass
70+
71+
72+def kill_by_pidfile(pidfile_path):
73+ """Kill a process identified by the pid stored in a file.
74+
75+ The pid file is removed from disk.
76+ """
77+ if not os.path.exists(pidfile_path):
78+ return
79+ try:
80+ # Get the pid.
81+ pid = open(pidfile_path, 'r').read().split()[0]
82+ try:
83+ pid = int(pid)
84+ except ValueError:
85+ # pidfile contains rubbish
86+ return
87+
88+ two_stage_kill(pid)
89+ finally:
90+ remove_if_exists(pidfile_path)
91+
92+
93+def remove_if_exists(path):
94+ """Remove the given file if it exists."""
95+ try:
96+ os.remove(path)
97+ except OSError, e:
98+ if e.errno != errno.ENOENT:
99+ raise
100+
101
102 twistd_script = os.path.abspath(os.path.join(
103 os.path.dirname(__file__),
104@@ -48,7 +124,7 @@
105 # started. If it sees an old logfile, then it will find the LOG_MAGIC
106 # string and return immediately, provoking hard-to-diagnose race
107 # conditions. Delete the logfile to make sure this does not happen.
108- self._removeFile(self.logfile)
109+ remove_if_exists(self.logfile)
110
111 self.setUpRoot()
112 args = [sys.executable, twistd_script, '-o', '-y', self.tacfile,
113@@ -107,50 +183,10 @@
114 def tearDown(self):
115 self.killTac()
116
117- def _removeFile(self, filename):
118- """Remove the given file if it exists."""
119- try:
120- os.remove(filename)
121- except OSError, e:
122- if e.errno != errno.ENOENT:
123- raise
124-
125 def killTac(self):
126 """Kill the TAC file if it is running."""
127 pidfile = self.pidfile
128- if not os.path.exists(pidfile):
129- return
130-
131- # Get the pid.
132- pid = open(pidfile, 'r').read().strip()
133- try:
134- pid = int(pid)
135- except ValueError:
136- # pidfile contains rubbish
137- return
138-
139- # Kill the process.
140- try:
141- os.kill(pid, SIGTERM)
142- except OSError, e:
143- if e.errno in (errno.ESRCH, errno.ECHILD):
144- # Process has already been killed.
145- return
146-
147- # Poll until the process has ended.
148- for i in range(50):
149- try:
150- os.kill(pid, 0)
151- time.sleep(0.1)
152- except OSError, e:
153- break
154- else:
155- # The process is still around, so terminate it violently.
156- try:
157- os.kill(pid, SIGKILL)
158- except OSError:
159- # Already terminated
160- pass
161+ kill_by_pidfile(pidfile)
162
163 def setUpRoot(self):
164 """Override this.
165
166=== modified file 'lib/lp/poppy/filesystem.py'
167--- lib/lp/poppy/filesystem.py 2009-06-25 05:30:52 +0000
168+++ lib/lp/poppy/filesystem.py 2010-03-18 19:22:29 +0000
169@@ -6,7 +6,6 @@
170 import datetime
171 import os
172 import stat
173-import shutil
174 from zope.security.interfaces import Unauthorized
175 from zope.server.interfaces.ftp import IFileSystem
176 from zope.interface import implements
177
178=== added file 'lib/lp/poppy/tests/__init__.py'
179--- lib/lp/poppy/tests/__init__.py 1970-01-01 00:00:00 +0000
180+++ lib/lp/poppy/tests/__init__.py 2010-03-18 19:22:29 +0000
181@@ -0,0 +1,5 @@
182+# Copyright 2010 Canonical Ltd. This software is licensed under the
183+# GNU Affero General Public License version 3 (see the file LICENSE).
184+
185+"""Tests for Poppy."""
186+
187
188=== renamed file 'lib/lp/poppy/tests/__init__.py' => 'lib/lp/poppy/tests/helpers.py'
189--- lib/lp/poppy/tests/__init__.py 2009-06-25 05:39:50 +0000
190+++ lib/lp/poppy/tests/helpers.py 2010-03-18 19:22:29 +0000
191@@ -5,16 +5,18 @@
192
193 import os
194 import select
195-import signal
196 import subprocess
197 import sys
198 import time
199
200 from canonical.config import config
201+from lp.services.osutils import two_stage_kill
202+
203
204 class SoyuzUploadError(Exception):
205 """Used in the soyuz-upload test."""
206
207+
208 class PoppyTestSetup:
209 """Provides a setup and teardown mechanism for a poppy subprocess instance.
210
211@@ -34,28 +36,18 @@
212 def startPoppy(self):
213 """Start the poppy instance."""
214 script = os.path.join(config.root, "daemons/poppy-upload.py")
215- self.process = subprocess.Popen([sys.executable, script,
216- "--allow-user", self.user,
217- "--cmd", self.cmd,
218- self.fsroot, self.port],
219- stdout=subprocess.PIPE)
220+ self.process = subprocess.Popen(
221+ [sys.executable, script, "--allow-user", self.user,
222+ "--cmd", self.cmd, self.fsroot, self.port],
223+ stdout=subprocess.PIPE)
224 self.running = True
225
226 def killPoppy(self):
227 """Kill the poppy instance dead."""
228 if not self.alive:
229 return
230- os.kill(self.process.pid, signal.SIGTERM)
231- # Give poppy a chance to die
232- time.sleep(2)
233- # Look to see if it has died yet
234- ret = self.process.poll()
235- if ret is None:
236- # Poppy had not died, so send it SIGKILL
237- os.kill(self.process.pid, signal.SIGKILL)
238- # Finally return the result.
239+ two_stage_kill(self.process.pid)
240 self.running = False
241- return self.process.wait()
242
243 @property
244 def alive(self):
245
246=== modified file 'lib/lp/poppy/tests/test_poppy.py'
247--- lib/lp/poppy/tests/test_poppy.py 2010-03-17 11:04:20 +0000
248+++ lib/lp/poppy/tests/test_poppy.py 2010-03-18 19:22:29 +0000
249@@ -7,45 +7,38 @@
250
251 import ftplib
252 import os
253-import shutil
254 import socket
255-import tempfile
256 import unittest
257 import StringIO
258
259-from canonical.testing import LaunchpadZopelessLayer
260-from lp.poppy.tests import PoppyTestSetup
261-
262-
263-class TestPoppy(unittest.TestCase):
264+from lp.poppy.tests.helpers import PoppyTestSetup
265+from lp.testing import TestCase
266+
267+
268+class TestPoppy(TestCase):
269 """Test if poppy.py daemon works properly."""
270
271- layer = LaunchpadZopelessLayer
272-
273 def setUp(self):
274 """Set up poppy in a temp dir."""
275- self.root_dir = tempfile.mkdtemp()
276+ super(TestPoppy, self).setUp()
277+ self.root_dir = self.makeTemporaryDirectory()
278 self.port = 3421
279 self.poppy = PoppyTestSetup(self.root_dir, port=self.port,
280 cmd='echo CLOSED')
281 self.poppy.startPoppy()
282-
283- def tearDown(self):
284- """Purge poppy root directory."""
285- self.poppy.killPoppy()
286- shutil.rmtree(self.root_dir)
287+ self.addCleanup(self.poppy.killPoppy)
288
289 def getFTPConnection(self, login=True, user="ubuntu", password=""):
290 """Build and return a FTP connection to the current poppy.
291
292- Optionally log in with as 'annonymous' & empty password, or passed
293+ Optionally log in with as 'anonymous' & empty password, or passed
294 user/password.
295 """
296 conn = ftplib.FTP()
297 # poppy usually takes sometime to come up, we need to wait, or insist.
298 while True:
299 try:
300- reply = conn.connect("localhost", self.port)
301+ conn.connect("localhost", self.port)
302 except socket.error:
303 if not self.poppy.alive:
304 raise
305@@ -75,7 +68,7 @@
306 upload_dir = contents[1]
307 return os.path.join(self.root_dir, upload_dir, path)
308
309- def testLOGIN(self):
310+ def test_LOGIN(self):
311 """Check login procedure and the state of the toplevel lockfile.
312
313 The toplevel lockfile should allow the process-upload runner (lp_queue,
314@@ -90,7 +83,7 @@
315 lockfile_path = os.path.join(self.root_dir, '.lock')
316 self.assertEqual(os.stat(lockfile_path).st_mode, 0100664)
317
318- def testCWD(self):
319+ def test_CWD(self):
320 """Check automatic creation of directories 'cwd'ed in.
321
322 Also ensure they are created with proper permission (g+rwxs)
323@@ -107,7 +100,7 @@
324 self.assertTrue(os.path.exists(wanted_path))
325 self.assertEqual(os.stat(wanted_path).st_mode, 042775)
326
327- def testMKD(self):
328+ def test_MKD(self):
329 """Check recursive MKD (aka mkdir -p).
330
331 Also ensure they are created with proper permission (g+rwxs)
332@@ -128,7 +121,7 @@
333 self.assertTrue(os.path.exists(wanted_path))
334 self.assertEqual(os.stat(wanted_path).st_mode, 042775)
335
336- def testRMD(self):
337+ def test_RMD(self):
338 """Check recursive RMD (aka rmdir)"""
339 conn = self.getFTPConnection()
340 self.assertEqual(
341@@ -143,7 +136,7 @@
342 wanted_path = self._uploadPath('foo')
343 self.assertFalse(os.path.exists(wanted_path))
344
345- def testSTOR(self):
346+ def test_STOR(self):
347 """Check if the parent directories are created during file upload.
348
349 The uploaded file permissions are also special (g+rwxs).
350@@ -160,7 +153,7 @@
351 self.assertEqual(fs_content, "fake contents")
352 self.assertEqual(os.stat(wanted_path).st_mode, 0102674)
353
354- def testUploadIsolation(self):
355+ def test_uploadIsolation(self):
356 """Check if poppy isolates the uploads properly.
357
358 Upload should be done atomically, i.e., poppy should isolate the
359@@ -216,5 +209,6 @@
360 self.root_dir, upload_dirs[index], "test")).read()
361 self.assertEqual(content, expected_contents[index])
362
363+
364 def test_suite():
365 return unittest.TestLoader().loadTestsFromName(__name__)
366
367=== modified file 'lib/lp/services/osutils.py'
368--- lib/lp/services/osutils.py 2009-12-21 02:50:08 +0000
369+++ lib/lp/services/osutils.py 2010-03-18 19:22:29 +0000
370@@ -8,67 +8,21 @@
371 'remove_tree',
372 'kill_by_pidfile',
373 'remove_if_exists',
374+ 'two_stage_kill',
375 ]
376
377-import errno
378 import os.path
379-from signal import SIGTERM, SIGKILL
380 import shutil
381-import time
382+
383+
384+from canonical.launchpad.daemons.tachandler import (
385+ kill_by_pidfile,
386+ remove_if_exists,
387+ two_stage_kill,
388+ )
389
390
391 def remove_tree(path):
392 """Remove the tree at 'path' from disk."""
393 if os.path.exists(path):
394 shutil.rmtree(path)
395-
396-
397-def kill_by_pidfile(pidfile_path):
398- """Kill a process identified by the pid stored in a file.
399-
400- The pid file is removed from disk.
401- """
402- if not os.path.exists(pidfile_path):
403- return
404- try:
405- # Get the pid.
406- pid = open(pidfile_path, 'r').read().split()[0]
407- try:
408- pid = int(pid)
409- except ValueError:
410- # pidfile contains rubbish
411- return
412-
413- # Kill the process.
414- try:
415- os.kill(pid, SIGTERM)
416- except OSError, e:
417- if e.errno in (errno.ESRCH, errno.ECHILD):
418- # Process has already been killed.
419- return
420-
421- # Poll until the process has ended.
422- for i in range(50):
423- try:
424- os.kill(pid, 0)
425- time.sleep(0.1)
426- except OSError, e:
427- break
428- else:
429- # The process is still around, so terminate it violently.
430- try:
431- os.kill(pid, SIGKILL)
432- except OSError:
433- # Already terminated
434- pass
435- finally:
436- remove_if_exists(pidfile_path)
437-
438-
439-def remove_if_exists(path):
440- """Remove the given file if it exists."""
441- try:
442- os.remove(path)
443- except OSError, e:
444- if e.errno != errno.ENOENT:
445- raise
446
447=== modified file 'lib/lp/soyuz/doc/soyuz-upload.txt.disabled'
448--- lib/lp/soyuz/doc/soyuz-upload.txt.disabled 2010-03-17 11:04:20 +0000
449+++ lib/lp/soyuz/doc/soyuz-upload.txt.disabled 2010-03-18 19:22:29 +0000
450@@ -186,7 +186,7 @@
451 Right, that's all we need from the FTP server. We don't need it anymore,
452 so we'll just kill the process.
453
454- >>> status = poppy.killPoppy()
455+ >>> poppy.killPoppy()
456
457 Finally, we'll just create an entirely empty upload folder. We rely for
458 our tests on a poppy-like naming system, ie. that the upload folder
459
460=== modified file 'lib/lp/testing/__init__.py'
461--- lib/lp/testing/__init__.py 2010-03-16 18:03:34 +0000
462+++ lib/lp/testing/__init__.py 2010-03-18 19:22:29 +0000
463@@ -215,7 +215,7 @@
464 def makeTemporaryDirectory(self):
465 """Create a temporary directory, and return its path."""
466 tempdir = tempfile.mkdtemp()
467- self.addCleanup(lambda: shutil.rmtree(tempdir))
468+ self.addCleanup(shutil.rmtree, tempdir)
469 return tempdir
470
471 def assertProvides(self, obj, interface):
472@@ -380,11 +380,10 @@
473
474 def useTempDir(self):
475 """Use a temporary directory for this test."""
476- tempdir = tempfile.mkdtemp()
477- self.addCleanup(lambda: shutil.rmtree(tempdir))
478+ tempdir = self.makeTemporaryDirectory()
479 cwd = os.getcwd()
480 os.chdir(tempdir)
481- self.addCleanup(lambda: os.chdir(cwd))
482+ self.addCleanup(os.chdir, cwd)
483
484
485 class TestCaseWithFactory(TestCase):