Fixed unit tests for archive module

salt/modules/archive.py

@@ -232,7 +232,7 @@ def zip_(archive, sources, template=None, runas=None):
 
 
 @decorators.which('unzip')
-def unzip_cmd(zipfile, dest, excludes=None, template=None, options=None, runas=None):
+def cmd_unzip_(zipfile, dest, excludes=None, template=None, options=None, runas=None):
     '''
     Uses the unzip command to unpack zip files
 
@@ -268,7 +268,7 @@ def unzip_cmd(zipfile, dest, excludes=None, template=None, options=None, runas=N
     return __salt__['cmd.run'](cmd, template=template, runas=runas).splitlines()
 
 
-@decorators.depends('zipfile', fallback_function=unzip_cmd)
+@decorators.depends('zipfile', fallback_function=cmd_unzip_)
 def unzip(archive, dest, excludes=None, template=None, options=None, runas=None):
     '''
     Uses the zipfile module to unpack zip files
@@ -299,7 +299,8 @@ def unzip(archive, dest, excludes=None, template=None, options=None, runas=None)
             zf.extractall(dest)
             return files
 
-        excludes = excludes.split(",")
+        if not isinstance(excludes, list):
+            excludes = excludes.split(",")
         cleaned_files = [x for x in files if x not in excludes]
         for f in cleaned_files:
             if f not in excludes:

tests/unit/modules/archive_test.py

@@ -17,8 +17,20 @@ ensure_in_syspath('../../')
 from salt.modules import archive
 from salt.exceptions import CommandNotFoundError
 
-archive.__salt__ = {}
 
+class ZipFileMock(MagicMock):
+    def __init__(self, files=['salt'], **kwargs):
+        MagicMock.__init__(self, **kwargs)
+        self._files = files
+
+    def namelist(self):
+        return self._files
+
+# Globals
+archive.__salt__ = {}
+archive.__pillar__ = {}
+archive.__grains__ = {"id": "0"}
+archive.__opts__ = {}
 
 @skipIf(NO_MOCK, NO_MOCK_REASON)
 class ArchiveTestCase(TestCase):
@@ -119,10 +131,10 @@ class ArchiveTestCase(TestCase):
             self.assertFalse(mock.called)
 
     @patch('salt.utils.which', lambda exe: exe)
-    def test_zip(self):
+    def test_cmd_zip(self):
         mock = MagicMock(return_value='salt')
         with patch.dict(archive.__salt__, {'cmd.run': mock}):
-            ret = archive.zip_(
+            ret = archive.cmd_zip_(
                 '/tmp/salt.{{grains.id}}.zip',
                 '/tmp/tmpePe8yO,/tmp/tmpLeSw1A',
                 template='jinja'
@@ -137,10 +149,11 @@ class ArchiveTestCase(TestCase):
 
         mock = MagicMock(return_value='salt')
         with patch.dict(archive.__salt__, {'cmd.run': mock}):
-            ret = archive.zip_(
+            ret = archive.cmd_zip_(
                 '/tmp/salt.{{grains.id}}.zip',
                 ['/tmp/tmpePe8yO', '/tmp/tmpLeSw1A'],
-                template='jinja'
+                template='jinja',
+                runas=None
             )
             self.assertEqual(['salt'], ret)
             mock.assert_called_once_with(
@@ -150,13 +163,24 @@ class ArchiveTestCase(TestCase):
                 template='jinja'
             )
 
+    @patch('os.path.exists', MagicMock(return_value=True))
+    @patch('os.path.isdir', MagicMock(return_value=False))
+    @patch('zipfile.ZipFile', MagicMock())
+    def test_zip(self):
+            ret = archive.zip_(
+                '/tmp/salt.{{grains.id}}.zip',
+                '/tmp/tmpePe8yO,/tmp/tmpLeSw1A',
+                template='jinja'
+            )
+            self.assertEqual(['/tmp/tmpePe8yO', '/tmp/tmpLeSw1A'], ret)
+
     @patch('salt.utils.which', lambda exe: None)
     def test_zip_raises_exception_if_not_found(self):
         mock = MagicMock(return_value='salt')
         with patch.dict(archive.__salt__, {'cmd.run': mock}):
             self.assertRaises(
                 CommandNotFoundError,
-                archive.zip_,
+                archive.cmd_zip_,
                 '/tmp/salt.{{grains.id}}.zip',
                 '/tmp/tmpePe8yO,/tmp/tmpLeSw1A',
                 template='jinja',
@@ -164,9 +188,77 @@ class ArchiveTestCase(TestCase):
             self.assertFalse(mock.called)
 
     @patch('salt.utils.which', lambda exe: exe)
+    def test_cmd_unzip(self):
+        mock = MagicMock(return_value='salt')
+        with patch.dict(archive.__salt__, {'cmd.run': mock}):
+            ret = archive.cmd_unzip_(
+                '/tmp/salt.{{grains.id}}.zip',
+                '/tmp/dest',
+                excludes='/tmp/tmpePe8yO,/tmp/tmpLeSw1A',
+                template='jinja',
+                runas=None
+            )
+            self.assertEqual(['salt'], ret)
+            mock.assert_called_once_with(
+                'unzip /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
+                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
+                runas=None,
+                template='jinja'
+            )
+
+        mock = MagicMock(return_value='salt')
+        with patch.dict(archive.__salt__, {'cmd.run': mock}):
+            ret = archive.cmd_unzip_(
+                '/tmp/salt.{{grains.id}}.zip',
+                '/tmp/dest',
+                excludes=['/tmp/tmpePe8yO', '/tmp/tmpLeSw1A'],
+                template='jinja'
+            )
+            self.assertEqual(['salt'], ret)
+            mock.assert_called_once_with(
+                'unzip /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
+                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
+                runas=None,
+                template='jinja'
+            )
+
+        mock = MagicMock(return_value='salt')
+        with patch.dict(archive.__salt__, {'cmd.run': mock}):
+            ret = archive.cmd_unzip_(
+                '/tmp/salt.{{grains.id}}.zip',
+                '/tmp/dest',
+                excludes='/tmp/tmpePe8yO,/tmp/tmpLeSw1A',
+                template='jinja',
+                options='fo'
+            )
+            self.assertEqual(['salt'], ret)
+            mock.assert_called_once_with(
+                'unzip -fo /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
+                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
+                runas=None,
+                template='jinja',
+            )
+
+        mock = MagicMock(return_value='salt')
+        with patch.dict(archive.__salt__, {'cmd.run': mock}):
+            ret = archive.cmd_unzip_(
+                '/tmp/salt.{{grains.id}}.zip',
+                '/tmp/dest',
+                excludes=['/tmp/tmpePe8yO', '/tmp/tmpLeSw1A'],
+                template='jinja',
+                options='fo'
+            )
+            self.assertEqual(['salt'], ret)
+            mock.assert_called_once_with(
+                'unzip -fo /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
+                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
+                runas=None,
+                template='jinja'
+            )
+
     def test_unzip(self):
-        mock = MagicMock(return_value='salt')
-        with patch.dict(archive.__salt__, {'cmd.run': mock}):
+        mock = ZipFileMock()
+        with patch('zipfile.ZipFile', mock):
             ret = archive.unzip(
                 '/tmp/salt.{{grains.id}}.zip',
                 '/tmp/dest',
@@ -174,62 +266,6 @@ class ArchiveTestCase(TestCase):
                 template='jinja'
             )
             self.assertEqual(['salt'], ret)
-            mock.assert_called_once_with(
-                'unzip /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
-                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
-                runas=None,
-                template='jinja'
-            )
-
-        mock = MagicMock(return_value='salt')
-        with patch.dict(archive.__salt__, {'cmd.run': mock}):
-            ret = archive.unzip(
-                '/tmp/salt.{{grains.id}}.zip',
-                '/tmp/dest',
-                excludes=['/tmp/tmpePe8yO', '/tmp/tmpLeSw1A'],
-                template='jinja'
-            )
-            self.assertEqual(['salt'], ret)
-            mock.assert_called_once_with(
-                'unzip /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
-                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
-                runas=None,
-                template='jinja'
-            )
-
-        mock = MagicMock(return_value='salt')
-        with patch.dict(archive.__salt__, {'cmd.run': mock}):
-            ret = archive.unzip(
-                '/tmp/salt.{{grains.id}}.zip',
-                '/tmp/dest',
-                excludes='/tmp/tmpePe8yO,/tmp/tmpLeSw1A',
-                template='jinja',
-                options='fo'
-            )
-            self.assertEqual(['salt'], ret)
-            mock.assert_called_once_with(
-                'unzip -fo /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
-                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
-                runas=None,
-                template='jinja',
-            )
-
-        mock = MagicMock(return_value='salt')
-        with patch.dict(archive.__salt__, {'cmd.run': mock}):
-            ret = archive.unzip(
-                '/tmp/salt.{{grains.id}}.zip',
-                '/tmp/dest',
-                excludes=['/tmp/tmpePe8yO', '/tmp/tmpLeSw1A'],
-                template='jinja',
-                options='fo'
-            )
-            self.assertEqual(['salt'], ret)
-            mock.assert_called_once_with(
-                'unzip -fo /tmp/salt.{{grains.id}}.zip -d /tmp/dest '
-                '-x /tmp/tmpePe8yO /tmp/tmpLeSw1A',
-                runas=None,
-                template='jinja'
-            )
 
     @patch('salt.utils.which', lambda exe: None)
     def test_unzip_raises_exception_if_not_found(self):
@@ -237,7 +273,7 @@ class ArchiveTestCase(TestCase):
         with patch.dict(archive.__salt__, {'cmd.run': mock}):
             self.assertRaises(
                 CommandNotFoundError,
-                archive.unzip,
+                archive.cmd_unzip_,
                 '/tmp/salt.{{grains.id}}.zip',
                 '/tmp/dest',
                 excludes='/tmp/tmpePe8yO,/tmp/tmpLeSw1A',