On Fri, Sep 26, 2014 at 10:34:11AM +0200, Helga Velroyen wrote:
On Thu, Sep 25, 2014 at 6:02 PM, Petr Pudlak <[email protected]> wrote:
On Tue, Sep 02, 2014 at 04:19:42PM +0200, 'Helga Velroyen' via
ganeti-devel wrote:
This patch adds a unit test for InitSSHSetup before
we start extending it in the next patch.
Signed-off-by: Helga Velroyen <[email protected]>
---
test/py/ganeti.ssh_unittest.py | 41 ++++++++++++++++++++++++++++++
+++++++++++
test/py/testutils.py | 15 +++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/
ganeti.ssh_unittest.py
index b0ba9dd..635426f 100755
--- a/test/py/ganeti.ssh_unittest.py
+++ b/test/py/ganeti.ssh_unittest.py
@@ -407,5 +407,46 @@ class TestPublicSshKeys(testutils.GanetiTestCase):
"789-ABC ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n")
+class TestGetUserFiles(testutils.GanetiTestCase):
+
+ _PRIV_KEY = "my private key"
+ _PUB_KEY = "my public key"
+ _AUTH_KEYS = "a\nb\nc"
+
+ def _setUpFakeKeys(self):
+ os.makedirs(os.path.join(self.tmpdir, ".ssh"))
+
+ self.priv_filename = os.path.join(self.tmpdir, ".ssh", "id_dsa")
+ priv_fd = open(self.priv_filename, 'w')
+ priv_fd.write(self._PRIV_KEY)
+ priv_fd.close()
+
+ self.pub_filename = os.path.join(self.tmpdir, ".ssh", "id_dsa.pub")
+ pub_fd = open(self.pub_filename, 'w')
+ pub_fd.write(self._PUB_KEY)
+ pub_fd.close()
+
+ self.auth_filename = os.path.join(self.tmpdir, ".ssh",
"authorized_keys")
+ auth_fd = open(self.auth_filename, 'w')
+ auth_fd.write(self._AUTH_KEYS)
+ auth_fd.close()
Here the same code is repeated 3 times, I'd suggest to use utils.WriteFile
do write the content of the files.
My original intention was to not use WriteFile because it is a ganeti
library itself, but I checked and we do that at multiple occasions in the
tests, so I guess it is okay here too. See the interdiff at the end of my
mail.
+
+ def setUp(self):
+ testutils.GanetiTestCase.setUp(self)
+ self.tmpdir = tempfile.mkdtemp()
+ self._setUpFakeKeys()
+
+ def tearDown(self):
+ shutil.rmtree(self.tmpdir)
+
+ def _GetTempHomedir(self, _):
+ return self.tmpdir
+
+ def testNewKeys(self):
+ ssh.InitSSHSetup(_homedir_fn=self._GetTempHomedir)
+ self.assertFileContentNotEqual(self.priv_filename, self._PRIV_KEY)
+ self.assertFileContentNotEqual(self.pub_filename, self._PUB_KEY)
+
+
if __name__ == "__main__":
testutils.GanetiTestProgram()
diff --git a/test/py/testutils.py b/test/py/testutils.py
index 033daa8..d73d8b4 100644
--- a/test/py/testutils.py
+++ b/test/py/testutils.py
@@ -105,6 +105,7 @@ class GanetiTestProgram(unittest.TestProgram):
return unittest.TestProgram.runTests(self)
+# pylint: disable=R0904
class GanetiTestCase(unittest.TestCase):
"""Helper class for unittesting.
@@ -134,6 +135,18 @@ class GanetiTestCase(unittest.TestCase):
actual_content = utils.ReadFile(file_name)
self.assertEqual(actual_content, expected_content)
+ def assertFileContentNotEqual(self, file_name, reference_content):
+ """Checks that the content of a file is different to the reference.
+
+ @type file_name: str
+ @param file_name: the file whose contents we should check
+ @type reference_content: str
+ @param reference_content: the content we use as reference
+
+ """
+ actual_content = utils.ReadFile(file_name)
+ self.assertNotEqual(actual_content, reference_content)
+
def assertFileMode(self, file_name, expected_mode):
"""Checks that the mode of a file is what we expect.
@@ -195,6 +208,8 @@ class GanetiTestCase(unittest.TestCase):
self._temp_files.append(fname)
return fname
+# pylint: enable=R0904
+
def patch_object(*args, **kwargs):
"""Unified patch_object for various versions of Python Mock.
-- 2.1.0.rc2.206.gedb03e5
Do I understand it correctly that the point of the test is to verify that
the initialization generates new keys and overwrites the old ones? Perhaps
it'd be helpful to describe it in some comment.
Yes, that was the point. I renamed the test function name to make this more
clear.
Rest LGTM
FYI, Interdiff:
diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/ganeti.ssh_unittest.py
index 043f5f8..1e53658 100755
--- a/test/py/ganeti.ssh_unittest.py
+++ b/test/py/ganeti.ssh_unittest.py
@@ -423,22 +423,17 @@ class TestGetUserFiles(testutils.GanetiTestCase):
_AUTH_KEYS = "a\nb\nc"
def _setUpFakeKeys(self):
- os.makedirs(os.path.join(self.tmpdir, ".ssh"))
+ ssh_tmpdir = os.path.join(self.tmpdir, ".ssh")
+ os.makedirs(ssh_tmpdir)
- self.priv_filename = os.path.join(self.tmpdir, ".ssh", "id_dsa")
- priv_fd = open(self.priv_filename, 'w')
- priv_fd.write(self._PRIV_KEY)
- priv_fd.close()
+ self.priv_filename = os.path.join(ssh_tmpdir, "id_dsa")
+ utils.WriteFile(self.priv_filename, data=self._PRIV_KEY)
- self.pub_filename = os.path.join(self.tmpdir, ".ssh", "id_dsa.pub")
- pub_fd = open(self.pub_filename, 'w')
- pub_fd.write(self._PUB_KEY)
- pub_fd.close()
+ self.pub_filename = os.path.join(ssh_tmpdir, "id_dsa.pub")
+ utils.WriteFile(self.pub_filename, data=self._PUB_KEY)
- self.auth_filename = os.path.join(self.tmpdir, ".ssh",
"authorized_keys")
- auth_fd = open(self.auth_filename, 'w')
- auth_fd.write(self._AUTH_KEYS)
- auth_fd.close()
+ self.auth_filename = os.path.join(ssh_tmpdir, "authorized_keys")
+ utils.WriteFile(self.auth_filename, data=self._AUTH_KEYS)
def setUp(self):
testutils.GanetiTestCase.setUp(self)
@@ -451,7 +446,7 @@ class TestGetUserFiles(testutils.GanetiTestCase):
def _GetTempHomedir(self, _):
return self.tmpdir
- def testNewKeys(self):
+ def testNewKeysOverrideOldKeys(self):
ssh.InitSSHSetup(_homedir_fn=self._GetTempHomedir)
self.assertFileContentNotEqual(self.priv_filename, self._PRIV_KEY)
self.assertFileContentNotEqual(self.pub_filename, self._PUB_KEY)
Cheers,
Helga
--
Helga Velroyen | Software Engineer | [email protected] |
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
LGTM, thanks!