From f893bf61e918670ab764013f8a01b2e0762600d7 Mon Sep 17 00:00:00 2001 From: Yu Ming Zhu Date: Jan 16 2019 06:37:20 +0000 Subject: [lib] ensuredir: normalize directory and don't throw error when dir exists --- diff --git a/koji/__init__.py b/koji/__init__.py index 05c8e5f..e6acbf9 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -482,8 +482,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) @@ -498,9 +508,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 diff --git a/tests/test_lib/test_file_ops.py b/tests/test_lib/test_file_ops.py new file mode 100644 index 0000000..ceb2a9a --- /dev/null +++ b/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')])