launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/lpsetup/death-to-doctests-in-subcommands into lp:lpsetup.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~gmb/lpsetup/death-to-doctests-in-subcommands/+merge/114393

This branch removes all the subcommands doctests and turns them into unit tests. I'll tackle other doctests in separate branches.
-- 
https://code.launchpad.net/~gmb/lpsetup/death-to-doctests-in-subcommands/+merge/114393
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/lpsetup/death-to-doctests-in-subcommands into lp:lpsetup.

=== modified file 'lpsetup/subcommands/inithost.py'
--- lpsetup/subcommands/inithost.py	2012-07-10 19:06:12 +0000
+++ lpsetup/subcommands/inithost.py	2012-07-11 11:12:21 +0000
@@ -52,6 +52,7 @@
     )
 
 
+<<<<<<< TREE
 def write_file_contents(filename, contents, mode, header=None):
     """Write `contents` to filename and chmod it to 0644.
 
@@ -86,6 +87,10 @@
         Traceback (most recent call last):
         AssertionError: Mode should be 'w' or 'a' only...
     """
+=======
+def write_file_contents(filename, contents, mode):
+    """Write `contents` to filename and chmod it to 0644."""
+>>>>>>> MERGE-SOURCE
     assert mode in ('w', 'a'), "Mode should be 'w' or 'a' only."
     if mode == 'w' and os.path.exists(filename):
         with open(filename, 'r') as existing_file:
@@ -104,6 +109,7 @@
 
 def ensure_ssh_keys(ssh_dir, private_key, public_key, valid_ssh_keys,
                     ssh_key_path):
+<<<<<<< TREE
     """Ensure that SSH is configured correctly for the user.
 
     If valid SSH keys are passed to the function, they will be written
@@ -153,6 +159,9 @@
         >>> os.remove('/tmp/known_hosts')
 
     """
+=======
+    """Ensure that SSH is configured correctly for the user."""
+>>>>>>> MERGE-SOURCE
     # Set up the user's ssh directory.  The ssh key must be associated
     # with the lpuser's Launchpad account.
     mkdirs(ssh_dir)

=== modified file 'lpsetup/tests/subcommands/test_inithost.py'
--- lpsetup/tests/subcommands/test_inithost.py	2012-07-09 21:18:21 +0000
+++ lpsetup/tests/subcommands/test_inithost.py	2012-07-11 11:12:21 +0000
@@ -4,6 +4,10 @@
 
 """Tests for the inithost sub command."""
 
+import os
+import shutil
+import stat
+import tempfile
 import unittest
 
 from lpsetup import handlers
@@ -36,7 +40,7 @@
         '-v', private_key, '-b', public_key, '-S', ssh_key_name)
 
 
-class InithostTest(StepsBasedSubCommandTestMixin, unittest.TestCase):
+class InithostSmokeTest(StepsBasedSubCommandTestMixin, unittest.TestCase):
 
     sub_command_class = inithost.SubCommand
     expected_arguments = get_arguments()
@@ -51,3 +55,113 @@
         initialize_lxc_step,
         setup_apt_step,)
     needs_root = True
+
+
+class WriteFileContentsTestCase(unittest.TestCase):
+    """Tests for inithost.write_file_contents()."""
+
+    def setUp(self):
+        temp_file = tempfile.NamedTemporaryFile()
+        # Calling close() on the temp_file will delete it, but its name
+        # will live on...
+        temp_file.close()
+        self.temp_filename = temp_file.name
+
+    def test_write_file_contents_writes_file_contents(self):
+        # inithost.write_file_contents writes the supplied file contents
+        # to a given file using the mode provided and chmods the file to
+        # 0644 for the current user.
+        self.addCleanup(os.remove, self.temp_filename)
+        inithost.write_file_contents(self.temp_filename, 'Hello, world!', 'w')
+        with open(self.temp_filename, 'r') as the_file:
+            contents = the_file.read().strip()
+        self.assertEqual('Hello, world!', contents)
+        file_mode = os.stat(self.temp_filename).st_mode
+        # The file permissions should be 0644. Sadly, there seems to be
+        # no way to check this quickly...
+        self.assertTrue(bool(file_mode & stat.S_IWUSR))
+        self.assertTrue(bool(file_mode & stat.S_IRUSR))
+        self.assertTrue(bool(file_mode & stat.S_IRGRP))
+        self.assertTrue(bool(file_mode & stat.S_IROTH))
+        self.assertFalse(bool(file_mode & stat.S_IXUSR))
+        self.assertFalse(bool(file_mode & stat.S_IWGRP))
+        self.assertFalse(bool(file_mode & stat.S_IXGRP))
+        self.assertFalse(bool(file_mode & stat.S_IWOTH))
+        self.assertFalse(bool(file_mode & stat.S_IXOTH))
+
+    def test_write_file_contents_wont_overwrite_existing_files(self):
+        # If the file already exists, `mode` is set to 'w', and the file's
+        # contents differ from `contents`, write_file_contents() raises an
+        # Exception.
+        self.addCleanup(os.remove, self.temp_filename)
+        inithost.write_file_contents(self.temp_filename, 'Hello, world!', 'w')
+        self.assertRaises(
+            Exception, inithost.write_file_contents, self.temp_filename,
+            'Hello again!', 'w')
+
+    def test_write_file_contents_returns_if_file_contents_dont_change(self):
+        # If the existing file's contents don't differ from `contents`,
+        # write_file_contents() will simply return.
+        self.addCleanup(os.remove, self.temp_filename)
+        inithost.write_file_contents(self.temp_filename, 'Hello, world!', 'w')
+        # This second call shouldn't raise an exception.
+        inithost.write_file_contents(self.temp_filename, 'Hello, world!', 'w')
+
+    def test_write_file_contents_accepts_modes_a_or_w(self):
+        # Passing a `mode` other than "a" or "w" to write_file_contents()
+        # will cause an Exception.
+        self.assertRaises(
+            AssertionError, inithost.write_file_contents,
+            '/tmp/foo', 'Hello, world!', 'a+')
+
+class EnsureSSHKeysTestCase(unittest.TestCase):
+    """Tests for inithost.ensure_ssh_keys()."""
+
+    def setUp(self):
+        temp_file = tempfile.NamedTemporaryFile()
+        temp_file.close()
+        self.temp_filename = temp_file.name
+        self.ssh_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.ssh_dir)
+        self.addCleanup(os.remove, self.temp_filename)
+        self.addCleanup(os.remove, self.temp_filename + ".pub")
+
+    def test_ensure_ssh_keys_writes_to_ssh_key_path(self):
+        # If inithost.ensure_ssh_keys() is told that the keys it has
+        # been passed are valid, it will write them out to the provided
+        # ssh_key_path.
+        public_key = "Public"
+        private_key = "Private"
+        inithost.ensure_ssh_keys(
+            self.ssh_dir, private_key, public_key, True, self.temp_filename)
+        with open(self.temp_filename + '.pub', 'r') as pub_file:
+            self.assertIn(public_key, pub_file.read())
+        with open(self.temp_filename, 'r') as priv_file:
+            self.assertIn(private_key, priv_file.read())
+
+    def test_ensure_ssh_keys_generates_keys_if_not_passed(self):
+        # If SSH keys aren't passed to ensure_ssh_keys(), the function
+        # will generate some.
+        inithost.ensure_ssh_keys(
+            self.ssh_dir, None, None, False, self.temp_filename)
+        self.assertTrue(os.path.exists(self.temp_filename))
+        self.assertTrue(os.path.exists(self.temp_filename + ".pub"))
+
+    def test_ensure_ssh_keys_generates_other_files(self):
+        # ensure_ssh_keys() also generates an authorized_keys file and a
+        # known_hosts file in the ssh dir.
+        inithost.ensure_ssh_keys(
+            self.ssh_dir, None, None, False, self.temp_filename)
+        self.assertTrue(
+            os.path.exists(os.path.join(self.ssh_dir, 'authorized_keys')))
+        self.assertTrue(
+            os.path.exists(os.path.join(self.ssh_dir, 'known_hosts')))
+
+    def test_ensure_ssh_keys_creates_ssh_dir(self):
+        # If the ssh_dir passed to ensure_ssh_keys() doesn't exist, it
+        # will be created.
+        shutil.rmtree(self.ssh_dir)
+        inithost.ensure_ssh_keys(
+            self.ssh_dir, None, None, False, self.temp_filename)
+        self.assertTrue(os.path.exists(self.ssh_dir))
+        self.assertTrue(os.path.isdir(self.ssh_dir))