Improve test for tools.ensure_dirs

- Add more checks, some of them are deliberately redundant
- Descriptive error messages
- Add comment describing order to “tools.ensure_dirs”
- Avoid copying a list in an assertion in “tools.ensure_dirs”

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/tools/ensure_dirs.py b/lib/tools/ensure_dirs.py
index e405260..11dece3 100644 (file)
--- a/lib/tools/ensure_dirs.py
+++ b/lib/tools/ensure_dirs.py
@@ -102,7 +102,7 @@ def ProcessPath(path):
 
   if pathtype in (DIR, QUEUE_DIR):
     # No additional parameters
-    assert len(path[5:]) == 0
+    assert len(path) == 5
     if pathtype == DIR:
       utils.MakeDirWithPerm(pathname, mode, uid, gid)
     elif pathtype == QUEUE_DIR:
@@ -127,6 +127,9 @@ def GetPaths():
   cleaner_log_dir = os.path.join(pathutils.LOG_DIR, "cleaner")
   master_cleaner_log_dir = os.path.join(pathutils.LOG_DIR, "master-cleaner")
 
+  # A note on the ordering: The parent directory (type C{DIR}) must always be
+  # listed before files (type C{FILE}) in that directory. Once the directory is
+  # set, only files directly in that directory can be listed.
   paths = [
     (pathutils.DATA_DIR, DIR, 0755, getent.masterd_uid, getent.masterd_gid),
     (pathutils.CLUSTER_DOMAIN_SECRET_FILE, FILE, 0640,

diff --git a/test/ganeti.tools.ensure_dirs_unittest.py b/test/ganeti.tools.ensure_dirs_unittest.py
index 48700e9..31debc2 100755 (executable)
--- a/test/ganeti.tools.ensure_dirs_unittest.py
+++ b/test/ganeti.tools.ensure_dirs_unittest.py
@@ -24,32 +24,55 @@
 import unittest
 import os.path
 
+from ganeti import utils
 from ganeti.tools import ensure_dirs
 
 import testutils
 
 
-class TestEnsureDirsFunctions(unittest.TestCase):
-  def testPaths(self):
+class TestGetPaths(unittest.TestCase):
+  def testEntryOrder(self):
     paths = [(path[0], path[1]) for path in ensure_dirs.GetPaths()]
 
-    seen = []
-    last_dirname = ""
-    for path, pathtype in paths:
+    # Directories for which permissions have been set
+    seen = set()
+
+    # Current directory (changes when an entry of type C{DIR} or C{QUEUE_DIR}
+    # is encountered)
+    current_dir = None
+
+    for (path, pathtype) in paths:
       self.assertTrue(pathtype in ensure_dirs.ALL_TYPES)
+      self.assertTrue(utils.IsNormAbsPath(path),
+                      msg=("Path '%s' is not absolute and/or normalized" %
+                           path))
+
       dirname = os.path.dirname(path)
-      if dirname != last_dirname or pathtype == ensure_dirs.DIR:
-        if pathtype == ensure_dirs.FILE:
-          self.assertFalse(dirname in seen,
-                           msg="path %s; dirname %s seen in %s" % (path,
-                                                                   dirname,
-                                                                   seen))
-          last_dirname = dirname
-          seen.append(dirname)
-        elif pathtype == ensure_dirs.DIR:
-          self.assertFalse(path in seen)
-          last_dirname = path
-          seen.append(path)
+
+      if pathtype == ensure_dirs.DIR:
+        self.assertFalse(path in seen,
+                         msg=("Directory '%s' was seen before" % path))
+        current_dir = path
+        seen.add(path)
+
+      elif pathtype == ensure_dirs.QUEUE_DIR:
+        self.assertTrue(dirname in seen,
+                        msg=("Queue directory '%s' was not seen before" %
+                             path))
+        current_dir = path
+
+      elif pathtype == ensure_dirs.FILE:
+        self.assertFalse(current_dir is None)
+        self.assertTrue(dirname in seen,
+                        msg=("Directory '%s' of path '%s' has not been seen"
+                             " yet" % (dirname, path)))
+        self.assertTrue((utils.IsBelowDir(current_dir, path) and
+                         current_dir == dirname),
+                        msg=("File '%s' not below current directory '%s'" %
+                             (path, current_dir)))
+
+      else:
+        self.fail("Unknown path type '%s'" % (pathtype, ))
 
 
 if __name__ == "__main__":