Merge pull request #6345 from s0undt3ch/hotfix/pip-tests

PIP module and state test cases, and minor improvments

salt/modules/pip.py

@@ -240,15 +240,25 @@ def install(pkgs=None,
     if env and not bin_env:
         bin_env = env
 
-    # Support deprecated 'runas' arg
-    if not user and runas is not None:
-        user = str(runas)
+    if runas is not None:
+        # The user is using a deprecated argument, warn!
         salt.utils.warn_until(
             (0, 18),
             'The \'runas\' argument to pip.install is deprecated, and will be '
             'removed in 0.18.0. Please use \'user\' instead.'
         )
 
+    # "There can only be one"
+    if runas is not None and user:
+        raise CommandExecutionError(
+            'The \'runas\' and \'user\' arguments are mutually exclusive. '
+            'Please use \'user\' as \'runas\' is being deprecated.'
+        )
+
+    # Support deprecated 'runas' arg
+    elif runas is not None and not user:
+        user = str(runas)
+
     cmd = [_get_pip_bin(bin_env), 'install']
 
     if activate and bin_env:
@@ -519,15 +529,25 @@ def uninstall(pkgs=None,
     '''
     cmd = [_get_pip_bin(bin_env), 'uninstall', '-y']
 
-    # Support deprecated 'runas' arg
-    if not user and runas is not None:
-        user = str(runas)
+    if runas is not None:
+        # The user is using a deprecated argument, warn!
         salt.utils.warn_until(
             (0, 18),
             'The \'runas\' argument to pip.install is deprecated, and will be '
             'removed in 0.18.0. Please use \'user\' instead.'
         )
 
+    # "There can only be one"
+    if runas is not None and user:
+        raise CommandExecutionError(
+            'The \'runas\' and \'user\' arguments are mutually exclusive. '
+            'Please use \'user\' as \'runas\' is being deprecated.'
+        )
+
+    # Support deprecated 'runas' arg
+    elif runas is not None and not user:
+        user = str(runas)
+
     cleanup_requirements = []
     if requirements is not None:
         if isinstance(requirements, string_types):
@@ -629,15 +649,25 @@ def freeze(bin_env=None,
 
         salt '*' pip.freeze /home/code/path/to/virtualenv/
     '''
-    # Support deprecated 'runas' arg
-    if not user and runas is not None:
-        user = str(runas)
+    if runas is not None:
+        # The user is using a deprecated argument, warn!
         salt.utils.warn_until(
             (0, 18),
             'The \'runas\' argument to pip.install is deprecated, and will be '
             'removed in 0.18.0. Please use \'user\' instead.'
         )
 
+    # "There can only be one"
+    if runas is not None and user:
+        raise CommandExecutionError(
+            'The \'runas\' and \'user\' arguments are mutually exclusive. '
+            'Please use \'user\' as \'runas\' is being deprecated.'
+        )
+
+    # Support deprecated 'runas' arg
+    elif runas is not None and not user:
+        user = str(runas)
+
     cmd = [_get_pip_bin(bin_env), 'freeze']
     cmd_kwargs = dict(runas=user, cwd=cwd)
     if bin_env and os.path.isdir(bin_env):
@@ -667,15 +697,25 @@ def list_(prefix=None,
 
     cmd = [_get_pip_bin(bin_env), 'freeze']
 
-    # Support deprecated 'runas' arg
-    if not user and runas is not None:
-        user = str(runas)
+    if runas is not None:
+        # The user is using a deprecated argument, warn!
         salt.utils.warn_until(
             (0, 18),
             'The \'runas\' argument to pip.install is deprecated, and will be '
             'removed in 0.18.0. Please use \'user\' instead.'
         )
 
+    # "There can only be one"
+    if runas is not None and user:
+        raise CommandExecutionError(
+            'The \'runas\' and \'user\' arguments are mutually exclusive. '
+            'Please use \'user\' as \'runas\' is being deprecated.'
+        )
+
+    # Support deprecated 'runas' arg
+    elif runas is not None and not user:
+        user = str(runas)
+
     cmd_kwargs = dict(runas=user, cwd=cwd)
     if bin_env and os.path.isdir(bin_env):
         cmd_kwargs['env'] = {'VIRTUAL_ENV': bin_env}

salt/states/pip.py

@@ -18,9 +18,11 @@ requisite to a pkg.installed state for the package which provides pip
           - pkg: python-pip
 '''
 
+# Import python libs
 import urlparse
 
 # Import salt libs
+import salt.utils
 from salt.exceptions import CommandExecutionError, CommandNotFoundError
 
 
@@ -60,6 +62,7 @@ def installed(name,
               no_download=False,
               install_options=None,
               user=None,
+              runas=None,
               no_chown=False,
               cwd=None,
               pre_releases=False,
@@ -95,6 +98,25 @@ def installed(name,
         prefix = name.split('=')[0].split('<')[0].split('>')[0].strip()
 
     ret = {'name': name, 'result': None, 'comment': '', 'changes': {}}
+    if runas is not None:
+        # The user is using a deprecated argument, warn!
+        msg = (
+            'The \'runas\' argument to pip.installed is deprecated, and will '
+            'be removed in 0.18.0. Please use \'user\' instead.'
+        )
+        salt.utils.warn_until((0, 18), msg)
+        ret.setdefault('warnings', []).append(msg)
+
+    # "There can only be one"
+    if runas is not None and user:
+        raise CommandExecutionError(
+            'The \'runas\' and \'user\' arguments are mutually exclusive. '
+            'Please use \'user\' as \'runas\' is being deprecated.'
+        )
+    # Support deprecated 'runas' arg
+    elif runas is not None and not user:
+        user = runas
+
     try:
         pip_list = __salt__['pip.list'](prefix, bin_env, user=user, cwd=cwd)
     except (CommandNotFoundError, CommandExecutionError) as err:
@@ -199,6 +221,7 @@ def removed(name,
             proxy=None,
             timeout=None,
             user=None,
+            runas=None,
             cwd=None,
             __env__='base'):
     '''
@@ -213,6 +236,25 @@ def removed(name,
     '''
     ret = {'name': name, 'result': None, 'comment': '', 'changes': {}}
 
+    if runas is not None:
+        # The user is using a deprecated argument, warn!
+        msg = (
+            'The \'runas\' argument to pip.installed is deprecated, and will '
+            'be removed in 0.18.0. Please use \'user\' instead.'
+        )
+        salt.utils.warn_until((0, 18), msg)
+        ret.setdefault('warnings', []).append(msg)
+
+    # "There can only be one"
+    if runas is not None and user:
+        raise CommandExecutionError(
+            'The \'runas\' and \'user\' arguments are mutually exclusive. '
+            'Please use \'user\' as \'runas\' is being deprecated.'
+        )
+    # Support deprecated 'runas' arg
+    elif runas is not None and not user:
+        user = runas
+
     try:
         pip_list = __salt__['pip.list'](bin_env=bin_env, user=user, cwd=cwd)
     except (CommandExecutionError, CommandNotFoundError) as err:

tests/integration/__init__.py

@@ -889,6 +889,16 @@ class SaltReturnAssertsMixIn(object):
     def assertSaltCommentRegexpMatches(self, ret, pattern):
         return self.assertInSaltReturnRegexpMatches(ret, pattern, 'comment')
 
+    def assertInSalStatetWarning(self, in_comment, ret):
+        return self.assertIn(
+            in_comment, self.__getWithinSaltReturn(ret, 'warnings')
+        )
+
+    def assertNotInSaltStateWarning(self, not_in_comment, ret):
+        return self.assertNotIn(
+            not_in_comment, self.__getWithinSaltReturn(ret, 'warnings')
+        )
+
     def assertInSaltReturn(self, ret, item_to_check, keys):
         return self.assertIn(
             item_to_check, self.__getWithinSaltReturn(ret, keys)

tests/unit/modules/pip_test.py

@@ -1,3 +1,6 @@
+# Import python libs
+import warnings
+
 # Import Salt Testing libs
 from salttesting import skipIf, TestCase
 from salttesting.helpers import ensure_in_syspath
@@ -855,6 +858,110 @@ class PipTestCase(TestCase):
                 cwd=None
             )
 
+    def test_install_deprecated_runas_triggers_warning(self):
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
+
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            with warnings.catch_warnings(record=True) as w:
+                pip.install('pep8', runas='me!')
+                self.assertEqual(
+                    'The \'runas\' argument to pip.install is deprecated, and '
+                    'will be removed in 0.18.0. Please use \'user\' instead.',
+                    str(w[-1].message)
+                )
+
+    def test_uninstall_deprecated_runas_triggers_warning(self):
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
+
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            with warnings.catch_warnings(record=True) as w:
+                pip.uninstall('pep8', runas='me!')
+                self.assertEqual(
+                    'The \'runas\' argument to pip.install is deprecated, and '
+                    'will be removed in 0.18.0. Please use \'user\' instead.',
+                    str(w[-1].message)
+                )
+
+    def test_freeze_deprecated_runas_triggers_warning(self):
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
+
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            with warnings.catch_warnings(record=True) as w:
+                pip.freeze('/tmp/pip-env', runas='me!')
+                self.assertEqual(
+                    'The \'runas\' argument to pip.install is deprecated, and '
+                    'will be removed in 0.18.0. Please use \'user\' instead.',
+                    str(w[-1].message)
+                )
+
+    def test_list_deprecated_runas_triggers_warning(self):
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
+
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            with warnings.catch_warnings(record=True) as w:
+                pip.list_('blah', runas='me!')
+                self.assertEqual(
+                    'The \'runas\' argument to pip.install is deprecated, and '
+                    'will be removed in 0.18.0. Please use \'user\' instead.',
+                    str(w[-1].message)
+                )
+
+    def test_install_user_and_runas_raises_exception(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            self.assertRaises(
+                CommandExecutionError,
+                pip.install,
+                'pep8',
+                user='Me!',
+                runas='Not Me!'
+            )
+
+    def test_uninstall_user_and_runas_raises_exception(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            self.assertRaises(
+                CommandExecutionError,
+                pip.uninstall,
+                'pep8',
+                user='Me!',
+                runas='Not Me!'
+            )
+
+    def test_freeze_user_and_runas_raises_exception(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            self.assertRaises(
+                CommandExecutionError,
+                pip.freeze,
+                '/tmp/pip-env',
+                user='Me!',
+                runas='Not Me!'
+            )
+
+    def test_list_user_and_runas_raises_exception(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            self.assertRaises(
+                CommandExecutionError,
+                pip.list_,
+                'pep8',
+                user='Me!',
+                runas='Not Me!'
+            )
+
 
 
 if __name__ == '__main__':

tests/unit/modules/virtualenv_test.py

@@ -172,10 +172,9 @@ class VirtualenvTestCase(TestCase):
             )
 
     def test_no_site_packages_deprecation(self):
-        # NOTE: If this test starts failing it might be because the deprecation
-        # warning was removed, or because some other test in this module is
-        # passing 'no_site_packages' to 'virtualenv_mod.create'. The
-        # deprecation warning is shown only once.
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
 
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):

tests/unit/states/pip_test.py

@@ -0,0 +1,117 @@
+# -*- coding: utf-8 -*-
+'''
+    tests.unit.states.pip_test
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)`
+    :copyright: © 2013 by the SaltStack Team, see AUTHORS for more details.
+    :license: Apache 2.0, see LICENSE for more details.
+'''
+
+# Import python libs
+import warnings
+
+# Import Salt Testing libs
+from salttesting import skipIf, TestCase
+from salttesting.helpers import ensure_in_syspath
+ensure_in_syspath('../../')
+
+# Import salt libs
+import integration
+from salt.states import pip
+from salt.exceptions import CommandExecutionError
+
+# Import 3rd-party libs
+try:
+    from mock import MagicMock, patch
+    HAS_MOCK = True
+except ImportError:
+    HAS_MOCK = False
+
+pip.__opts__ = {'test': False}
+pip.__salt__ = {'cmd.which_bin': lambda _: 'pip'}
+
+
+@skipIf(HAS_MOCK is False, 'mock python module is unavailable')
+class PipStateTest(TestCase, integration.SaltReturnAssertsMixIn):
+
+    def test_installed_deprecated_runas(self):
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
+
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        pip_list = MagicMock(return_value=[])
+        pip_install = MagicMock(return_value={'retcode': 0})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock,
+                                       'pip.list': pip_list,
+                                       'pip.install': pip_install}):
+            with warnings.catch_warnings(record=True) as w:
+                ret = pip.installed('pep8', runas='me!')
+                self.assertEqual(
+                    'The \'runas\' argument to pip.installed is deprecated, '
+                    'and will be removed in 0.18.0. Please use \'user\' '
+                    'instead.', str(w[-1].message)
+                )
+                self.assertSaltTrueReturn({'testsuite': ret})
+                # Is the state returning a warnings key with the deprecation
+                # message?
+                self.assertInSalStatetWarning(
+                    'The \'runas\' argument to pip.installed is deprecated, '
+                    'and will be removed in 0.18.0. Please use \'user\' '
+                    'instead.', {'testsuite': ret}
+                )
+
+    def test_installed_runas_and_user_raises_exception(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            self.assertRaises(
+                CommandExecutionError,
+                pip.installed,
+                'pep8',
+                user='Me!',
+                runas='Not Me!'
+            )
+
+    def test_removed_deprecated_runas(self):
+        # We *always* want *all* warnings thrown on this module
+        warnings.resetwarnings()
+        warnings.filterwarnings('always', '', DeprecationWarning, __name__)
+
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        pip_list = MagicMock(return_value=['pep8'])
+        pip_uninstall = MagicMock(return_value=True)
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock,
+                                       'pip.list': pip_list,
+                                       'pip.uninstall': pip_uninstall}):
+            with warnings.catch_warnings(record=True) as w:
+                ret = pip.removed('pep8', runas='me!')
+                self.assertEqual(
+                    'The \'runas\' argument to pip.installed is deprecated, '
+                    'and will be removed in 0.18.0. Please use \'user\' '
+                    'instead.', str(w[-1].message)
+                )
+                self.assertSaltTrueReturn({'testsuite': ret})
+                # Is the state returning a warnings key with the deprecation
+                # message?
+                self.assertInSalStatetWarning(
+                    'The \'runas\' argument to pip.installed is deprecated, '
+                    'and will be removed in 0.18.0. Please use \'user\' '
+                    'instead.', {'testsuite': ret}
+                )
+
+    def test_removed_runas_and_user_raises_exception(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            self.assertRaises(
+                CommandExecutionError,
+                pip.removed,
+                'pep8',
+                user='Me!',
+                runas='Not Me!'
+            )
+
+
+if __name__ == '__main__':
+    from integration import run_tests
+    run_tests(PipStateTest, needs_daemon=False)