launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/nest-temp-files into lp:launchpad with lp:~allenap/launchpad/ddebs-n-udebs-go-shopping-bug-886452 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/launchpad/nest-temp-files/+merge/83978

New fixture NestedTempfile that changes tempfile.tempdir to a temporary directory - i.e. from /tmp to /tmp/tmp243fe5 and back - and erases this temporary directory during clean-up. This causes all files and directories created by tempfile (using default options) to be put inside this new directory so that we can catch most things during tear down. It also opens the possibility of easily checking for left-over cruft.

I am tempted to always use this fixture in lp.testing.TestCase.setUp().

-- 
https://code.launchpad.net/~allenap/launchpad/nest-temp-files/+merge/83978
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/nest-temp-files into lp:launchpad.

=== modified file 'lib/lp/poppy/tests/test_twistedsftp.py'
--- lib/lp/poppy/tests/test_twistedsftp.py	2011-11-30 16:58:52 +0000
+++ lib/lp/poppy/tests/test_twistedsftp.py	2011-11-30 16:58:54 +0000
@@ -11,12 +11,16 @@
 
 from lp.poppy.twistedsftp import SFTPServer
 from lp.services.sshserver.sftp import FileIsADirectory
-from lp.testing import TestCase
+from lp.testing import (
+    NestedTempfile,
+    TestCase,
+    )
 
 
 class TestSFTPServer(TestCase):
 
     def setUp(self):
+        self.useFixture(NestedTempfile())
         self.fs_root = self.useFixture(TempDir()).path
         self.sftp_server = SFTPServer(None, self.fs_root)
         super(TestSFTPServer, self).setUp()

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2011-11-30 02:43:41 +0000
+++ lib/lp/testing/__init__.py	2011-11-30 16:58:54 +0000
@@ -1289,6 +1289,17 @@
     return launchpad.load(canonical_url(obj, request=api_request))
 
 
+class NestedTempfile(fixtures.Fixture):
+    """Nest all temporary directories inside a top-level one."""
+
+    def setUp(self):
+        super(NestedTempfile, self).setUp()
+        tempdir = fixtures.TempDir()
+        self.useFixture(tempdir)
+        patch = fixtures.MonkeyPatch("tempfile.tempdir", tempdir.path)
+        self.useFixture(patch)
+
+
 @contextmanager
 def temp_dir():
     """Provide a temporary directory as a ContextManager."""

=== modified file 'lib/lp/testing/tests/test_testing.py'
--- lib/lp/testing/tests/test_testing.py	2011-07-18 14:07:48 +0000
+++ lib/lp/testing/tests/test_testing.py	2011-11-30 16:58:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the testing module."""
@@ -6,12 +6,14 @@
 __metaclass__ = type
 
 import os
+import tempfile
 
 from canonical.config import config
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.services.features import getFeatureFlag
 from lp.testing import (
     feature_flags,
+    NestedTempfile,
     set_feature_flag,
     TestCase,
     YUIUnitTestCase,
@@ -51,3 +53,31 @@
         test_path = os.path.join(config.root, "../bar/baz/../bob.html")
         test.initialize(test_path)
         self.assertEqual("../bar/bob.html", test.id())
+
+
+class NestedTempfileTest(TestCase):
+    """Tests for `NestedTempfile`."""
+
+    def test_normal(self):
+        # The temp directory is removed when the context is exited.
+        starting_tempdir = tempfile.gettempdir()
+        with NestedTempfile():
+            self.assertEqual(tempfile.tempdir, tempfile.gettempdir())
+            self.assertNotEqual(tempfile.tempdir, starting_tempdir)
+            self.assertTrue(os.path.isdir(tempfile.tempdir))
+            nested_tempdir = tempfile.tempdir
+        self.assertEqual(tempfile.tempdir, tempfile.gettempdir())
+        self.assertEqual(starting_tempdir, tempfile.tempdir)
+        self.assertFalse(os.path.isdir(nested_tempdir))
+
+    def test_exception(self):
+        # The temp directory is removed when the context is exited, even if
+        # the code running in context raises an exception.
+        class ContrivedException(Exception):
+            pass
+        try:
+            with NestedTempfile():
+                nested_tempdir = tempfile.tempdir
+                raise ContrivedException
+        except ContrivedException:
+            self.assertFalse(os.path.isdir(nested_tempdir))