From a33f9d771b9bbf5e6addac2168f5231599689bdb Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 27 Jul 2013 13:04:59 +0100 Subject: [PATCH 1/5] Alaways show triggered deprecation warnings on the unit test. --- tests/unit/modules/virtualenv_test.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/modules/virtualenv_test.py b/tests/unit/modules/virtualenv_test.py index 985f011db4..dbf687d0b6 100644 --- a/tests/unit/modules/virtualenv_test.py +++ b/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}): From 5ee8073979a3e8ed0d753fb21c30586ea32c440e Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 27 Jul 2013 13:06:36 +0100 Subject: [PATCH 2/5] Test the deprecated `runas` argument to `salt.modules.pip`. --- tests/unit/modules/pip_test.py | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/unit/modules/pip_test.py b/tests/unit/modules/pip_test.py index a75ba1c68b..22c242a6cc 100644 --- a/tests/unit/modules/pip_test.py +++ b/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,65 @@ 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) + ) if __name__ == '__main__': From 07e736672842eaa5cc0a1c235318d4226b16307e Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 27 Jul 2013 13:34:58 +0100 Subject: [PATCH 3/5] `user` and `runas` now raise an exception. "There can be only one". * Always show the deprecation message `runas` is not `None`. * If both `user` and `runas` are used, that's an error! Full Stop! * If `runas` is being used and `user` is not, adapt the code and don't fail. --- salt/modules/pip.py | 64 +++++++++++++++++++++++++++------- tests/unit/modules/pip_test.py | 45 ++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/salt/modules/pip.py b/salt/modules/pip.py index fa4a1eb2da..a81f6e6be7 100644 --- a/salt/modules/pip.py +++ b/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} diff --git a/tests/unit/modules/pip_test.py b/tests/unit/modules/pip_test.py index 22c242a6cc..69a47b00a7 100644 --- a/tests/unit/modules/pip_test.py +++ b/tests/unit/modules/pip_test.py @@ -918,6 +918,51 @@ class PipTestCase(TestCase): 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__': from integration import run_tests From c072fa0241964014c82adf5049b8b408b5befdd6 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 27 Jul 2013 15:58:28 +0100 Subject: [PATCH 4/5] Also deprecate `runas` on `salt.states.pip`. * Added mock tests to check if the deprecation is triggered, and to see if the deprecation is also returned to the user in a warnings key. --- salt/states/pip.py | 42 +++++++++++++++ tests/integration/__init__.py | 10 ++++ tests/unit/states/pip_test.py | 96 +++++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 tests/unit/states/pip_test.py diff --git a/salt/states/pip.py b/salt/states/pip.py index 5f50a6ec49..ecffe713b1 100644 --- a/salt/states/pip.py +++ b/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: diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index f1b29f786d..feb2d0f62c 100644 --- a/tests/integration/__init__.py +++ b/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) diff --git a/tests/unit/states/pip_test.py b/tests/unit/states/pip_test.py new file mode 100644 index 0000000000..fa7a8df604 --- /dev/null +++ b/tests/unit/states/pip_test.py @@ -0,0 +1,96 @@ +# -*- 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 + +# 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_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} + ) + + + + +if __name__ == '__main__': + from integration import run_tests + run_tests(PipStateTest, needs_daemon=False) From 42641c6153a39313db9388bf87ab9ca3fa96c837 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 27 Jul 2013 16:02:59 +0100 Subject: [PATCH 5/5] Added test case to see if the usage of `user` and `runas` at the same time raises an exception. --- tests/unit/states/pip_test.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/unit/states/pip_test.py b/tests/unit/states/pip_test.py index fa7a8df604..5fac281b1a 100644 --- a/tests/unit/states/pip_test.py +++ b/tests/unit/states/pip_test.py @@ -19,6 +19,7 @@ ensure_in_syspath('../../') # Import salt libs import integration from salt.states import pip +from salt.exceptions import CommandExecutionError # Import 3rd-party libs try: @@ -61,6 +62,17 @@ class PipStateTest(TestCase, integration.SaltReturnAssertsMixIn): '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() @@ -88,7 +100,16 @@ class PipStateTest(TestCase, integration.SaltReturnAssertsMixIn): '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__':