Merge pull request #40451 from isbm/isbm-fileclient-testcase-2016.11

Fileclient testcase (2016.11)

salt/fileclient.py

@@ -6,6 +6,7 @@ from __future__ import absolute_import
 
 # Import python libs
 import contextlib
+import errno
 import logging
 import os
 import string
@@ -142,12 +143,19 @@ class Client(object):
                                     path)
         destdir = os.path.dirname(dest)
         cumask = os.umask(63)
-        if not os.path.isdir(destdir):
-            # remove destdir if it is a regular file to avoid an OSError when
-            # running os.makedirs below
-            if os.path.isfile(destdir):
-                os.remove(destdir)
+
+        # remove destdir if it is a regular file to avoid an OSError when
+        # running os.makedirs below
+        if os.path.isfile(destdir):
+            os.remove(destdir)
+
+        # ensure destdir exists
+        try:
             os.makedirs(destdir)
+        except OSError as exc:
+            if exc.errno != errno.EEXIST:  # ignore if it was there already
+                raise
+
         yield dest
         os.umask(cumask)
 

tests/unit/fileclient_test.py

@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+'''
+    :codeauthor: :email: `Bo Maryniuk <bo@suse.de>`
+'''
+
+# Import Python libs
+from __future__ import absolute_import
+
+# Import Salt Testing libs
+from salttesting import TestCase
+from salttesting.helpers import ensure_in_syspath
+from salttesting.mock import patch
+from mock import Mock
+import errno
+from salt.ext.six.moves import range
+
+ensure_in_syspath('../')
+
+from salt.fileclient import Client
+
+
+class FileclientTestCase(TestCase):
+    '''
+    Fileclient test
+    '''
+    opts = {
+        'extension_modules': '',
+        'cachedir': '/__test__',
+    }
+
+    def _fake_makedir(self, num=errno.EEXIST):
+        def _side_effect(*args, **kwargs):
+            raise OSError(num, 'Errno {0}'.format(num))
+        return Mock(side_effect=_side_effect)
+
+    def test_cache_skips_makedirs_on_race_condition(self):
+        '''
+        If cache contains already a directory, do not raise an exception.
+        '''
+        with patch('os.path.isfile', lambda prm: False):
+            for exists in range(2):
+                with patch('os.makedirs', self._fake_makedir()):
+                    with Client(self.opts)._cache_loc('testfile') as c_ref_itr:
+                        assert c_ref_itr == '/__test__/files/base/testfile'
+
+    def test_cache_raises_exception_on_non_eexist_ioerror(self):
+        '''
+        If makedirs raises other than EEXIST errno, an exception should be raised.
+        '''
+        with patch('os.path.isfile', lambda prm: False):
+            with patch('os.makedirs', self._fake_makedir(num=errno.EREMOTEIO)):
+                with self.assertRaises(OSError):
+                    with Client(self.opts)._cache_loc('testfile') as c_ref_itr:
+                        assert c_ref_itr == '/__test__/files/base/testfile'