[lib] ensuredir: normalize directory and don't throw error when dir exists

koji/__init__.py

@@ -484,8 +484,18 @@ def safe_xmlrpc_loads(s):
 
 ## BEGIN kojikamid dup
 
+
 def ensuredir(directory):
-    """Create directory, if necessary."""
+    """Create directory, if necessary.
+
+    :param str directory: path of the directory
+
+    :returns: str: normalized directory path
+
+    :raises OSError: If directory is not a dir or it is root(/) or equivalent
+        but doesn't exist(should not happen).
+    """
+    directory = os.path.normpath(directory)
     if os.path.exists(directory):
         if not os.path.isdir(directory):
             raise OSError("Not a directory: %s" % directory)
@@ -500,9 +510,9 @@ def ensuredir(directory):
         # note: if head is blank, then we've reached the top of a relative path
         try:
             os.mkdir(directory)
-        except OSError:
-            #thrown when dir already exists (could happen in a race)
-            if not os.path.isdir(directory):
+        except OSError as e:
+            # do not thrown when dir already exists (could happen in a race)
+            if e.errno == errno.EEXIST and not os.path.isdir(directory):
                 #something else must have gone wrong
                 raise
     return directory

tests/test_lib/test_file_ops.py

@@ -0,0 +1,63 @@
+from __future__ import absolute_import
+
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+
+import mock
+import errno
+
+from koji import ensuredir
+
+
+class TestEnsureDir(unittest.TestCase):
+    @mock.patch('os.mkdir')
+    @mock.patch('os.path.exists')
+    @mock.patch('os.path.isdir')
+    def test_ensuredir_errors(self, mock_isdir, mock_exists, mock_mkdir):
+        mock_exists.return_value = False
+        with self.assertRaises(OSError) as cm:
+            ensuredir('/')
+        self.assertEqual(cm.exception.args[0], 'root directory missing? /')
+        mock_mkdir.assert_not_called()
+
+        mock_exists.return_value = True
+        mock_isdir.return_value = False
+        with self.assertRaises(OSError) as cm:
+            ensuredir('/path/foo/bar')
+        self.assertEqual(cm.exception.args[0],
+                         'Not a directory: /path/foo/bar')
+        mock_mkdir.assert_not_called()
+
+        mock_exists.return_value = False
+        mock_isdir.return_value = False
+        mock_mkdir.side_effect = OSError(errno.EEXIST, 'error msg')
+        with self.assertRaises(OSError) as cm:
+            ensuredir('path')
+        self.assertEqual(cm.exception.args[0], errno.EEXIST)
+        mock_mkdir.assert_called_once_with('path')
+
+        mock_mkdir.reset_mock()
+        mock_mkdir.side_effect = OSError(errno.EEXIST, 'error msg')
+        mock_isdir.return_value = True
+        ensuredir('path')
+        mock_mkdir.assert_called_once_with('path')
+
+        mock_mkdir.reset_mock()
+        mock_mkdir.side_effect = OSError(errno.EEXIST + 1, 'ignored error')
+        mock_isdir.return_value = False
+        ensuredir('path')
+        mock_mkdir.assert_called_once_with('path')
+
+    @mock.patch('os.mkdir')
+    @mock.patch('os.path.exists')
+    @mock.patch('os.path.isdir')
+    def test_ensuredir(self, mock_isdir, mock_exists, mock_mkdir):
+        mock_exists.side_effect = [False, False, True]
+        mock_isdir.return_value = True
+        ensuredir('/path/foo/bar/')
+        self.assertEqual(mock_exists.call_count, 3)
+        self.assertEqual(mock_isdir.call_count, 1)
+        mock_mkdir.assert_has_calls([mock.call('/path/foo'),
+                                     mock.call('/path/foo/bar')])