launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/testcase-helpers into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/testcase-helpers/+merge/102835

I raised this for discussion on IRC, but without success; the best way forward at this point is probably just to put it up for review.

This adds factory methods to create temporary files and directories.  It's something I need for another branch I'm getting ready for review.

A temporary file is always created inside a temporary directory, so that there's just a single mechanism for cleanup.  It also avoids the (almost entirely theoretical, but hey we're talking about deleting files here) hazard of a test deleting the file, and then somebody else creating another file with the same name just before the test goes through cleanup and accidentally deletes that new file.

More interestingly, some of the work is done in TestCase.  The sole reason to do it that way is that it avoids the distraction of “self.useFixture(…),” and the extraction of the directory's path from the fixture, in tests.  But I kept as much as I thought possible in the factory.
-- 
https://code.launchpad.net/~jtv/maas/testcase-helpers/+merge/102835
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/testcase-helpers into lp:maas.

=== modified file 'src/maastesting/factory.py'
--- src/maastesting/factory.py	2012-04-16 10:00:51 +0000
+++ src/maastesting/factory.py	2012-04-20 11:40:29 +0000
@@ -21,6 +21,7 @@
     islice,
     repeat,
     )
+import os.path
 import random
 import string
 
@@ -63,6 +64,26 @@
         octets = islice(self.random_octets, 6)
         return b":".join(format(octet, b"02x") for octet in octets)
 
+    def make_file(self, location, name=None, contents=None):
+        """Create a file, and write data to it.
+
+        Prefer the eponymous convenience wrapper in
+        :class:`maastesting.testcase.TestCase`.  It creates a temporary
+        directory and arranges for its eventual cleanup.
+
+        :param location: Directory.  Use a temporary directory for this, and
+            make sure it gets cleaned up after the test!
+        :return: Path to the file.
+        """
+        if name is None:
+            name = self.getRandomString()
+        if contents is None:
+            contents = self.getRandomString().encode('ascii')
+        path = os.path.join(location, name)
+        with open(path, 'w') as f:
+            f.write(contents)
+        return path
+
 
 # Create factory singleton.
 factory = Factory()

=== modified file 'src/maastesting/testcase.py'
--- src/maastesting/testcase.py	2012-04-19 15:48:46 +0000
+++ src/maastesting/testcase.py	2012-04-20 11:40:29 +0000
@@ -16,6 +16,8 @@
 
 import unittest
 
+from fixtures import TempDir
+from maastesting.factory import factory
 import testresources
 import testtools
 
@@ -46,6 +48,23 @@
         testresources.tearDownResources(
             self, self.resources, testresources._get_result())
 
+    def make_dir(self):
+        """Create a temporary directory.
+
+        This is a convenience wrapper around a fixture incantation.  That's
+        the only reason why it's on the test case and not in a factory.
+        """
+        return self.useFixture(TempDir()).path
+
+    def make_file(self, name=None, contents=None):
+        """Create, and write to, a file.
+
+        This is a convenience wrapper around `make_dir` and a factory
+        call.  It ensures that the file is in a directory that will be
+        cleaned up at the end of the test.
+        """
+        return factory.make_file(self.make_dir(), name, contents)
+
     # Django's implementation for this seems to be broken and was
     # probably only added to support compatibility with python 2.6.
     assertItemsEqual = unittest.TestCase.assertItemsEqual

=== modified file 'src/maastesting/tests/test_factory.py'
--- src/maastesting/tests/test_factory.py	2012-04-16 10:00:51 +0000
+++ src/maastesting/tests/test_factory.py	2012-04-20 11:40:29 +0000
@@ -12,8 +12,14 @@
 __metaclass__ = type
 __all__ = []
 
+import os.path
+
 from maastesting.factory import factory
 from maastesting.testcase import TestCase
+from testtools.matchers import (
+    FileContains,
+    FileExists,
+    )
 
 
 class TestFactory(TestCase):
@@ -35,3 +41,25 @@
         self.assertEqual(17, len(mac_address))
         for hex_octet in mac_address.split(":"):
             self.assertTrue(0 <= int(hex_octet, 16) <= 255)
+
+    def test_make_file_creates_file(self):
+        self.assertThat(factory.make_file(self.make_dir()), FileExists())
+
+    def test_make_file_writes_contents(self):
+        contents = factory.getRandomString().encode('ascii')
+        self.assertThat(
+            factory.make_file(self.make_dir(), contents=contents),
+            FileContains(contents))
+
+    def test_make_file_uses_given_name(self):
+        name = factory.getRandomString()
+        self.assertEqual(
+            name,
+            os.path.basename(factory.make_file(self.make_dir(), name=name)))
+
+    def test_make_file_uses_given_dir(self):
+        directory = self.make_dir()
+        name = factory.getRandomString()
+        self.assertEqual(
+            (directory, name),
+            os.path.split(factory.make_file(directory, name=name)))

=== added file 'src/maastesting/tests/test_testcase.py'
--- src/maastesting/tests/test_testcase.py	1970-01-01 00:00:00 +0000
+++ src/maastesting/tests/test_testcase.py	2012-04-20 11:40:29 +0000
@@ -0,0 +1,50 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `TestCase`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+import os.path
+from shutil import rmtree
+from tempfile import mkdtemp
+
+from maastesting.testcase import TestCase
+from testtools.matchers import (
+    DirExists,
+    FileExists,
+    )
+
+
+class TestTestCase(TestCase):
+    """Tests the base `TestCase` facilities."""
+
+    def test_make_dir_creates_directory(self):
+        self.assertThat(self.make_dir(), DirExists())
+
+    def test_make_dir_creates_temporary_directory(self):
+        other_temp_dir = mkdtemp()
+        self.addCleanup(rmtree, other_temp_dir)
+        other_temp_root, other_subdir = os.path.split(other_temp_dir)
+        temp_root, subdir = os.path.split(self.make_dir())
+        self.assertEqual(other_temp_root, temp_root)
+        self.assertNotIn(subdir, [b'', u'', None])
+
+    def test_make_dir_creates_one_directory_per_call(self):
+        self.assertNotEqual(self.make_dir(), self.make_dir())
+
+    def test_make_file_creates_file(self):
+        self.assertThat(self.make_file(), FileExists())
+
+    def test_make_file_uses_temporary_directory(self):
+        directory = self.make_dir()
+        self.patch(self, 'make_dir', lambda: directory)
+        dir_part, file_part = os.path.split(self.make_file())
+        self.assertEqual(directory, dir_part)