Merge branch '2018.3' into 2018.3-salt-ssh-pillar-propagation-issue-47501

.kitchen.yml

@@ -1,5 +1,5 @@
 ---
-<% vagrant = system('which vagrant 2>/dev/null >/dev/null') %>
+<% vagrant = system('gem list -i kitchen-vagrant 2>/dev/null >/dev/null') %>
 <% version = '2017.7.4' %>
 <% platformsfile = ENV['SALT_KITCHEN_PLATFORMS'] || '.kitchen/platforms.yml' %>
 <% driverfile = ENV['SALT_KITCHEN_DRIVER'] || '.kitchen/driver.yml' %>
@@ -94,12 +94,9 @@ platforms:
         - yum install -y upstart
     provisioner:
       salt_bootstrap_options: -P -p rsync -y -x python2.7 -X git v<%= version %> >/dev/null
-  - name: ubuntu-rolling
+  - name: ubuntu-18.04
     driver_config:
-      image: ubuntu:rolling
       run_command: /lib/systemd/systemd
-    provisioner:
-      salt_bootstrap_url: https://raw.githubusercontent.com/saltstack/salt-bootstrap/develop/bootstrap-salt.sh
   - name: ubuntu-16.04
     driver_config:
       run_command: /lib/systemd/systemd

doc/_ext/youtube.py

@@ -38,7 +38,10 @@ from __future__ import division
 import re
 from docutils import nodes
 from docutils.parsers.rst import directives
-from sphinx.util.compat import Directive
+try:
+    from sphinx.util.compat import Directive
+except ImportError:
+    from docutils.parsers.rst import Directive
 
 CONTROL_HEIGHT = 30
 

doc/man/salt-api.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-API" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-API" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-api \- salt-api Command
 .

doc/man/salt-call.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-CALL" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-CALL" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-call \- salt-call Documentation
 .

doc/man/salt-cloud.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-CLOUD" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-CLOUD" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-cloud \- Salt Cloud Command
 .

doc/man/salt-cp.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-CP" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-CP" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-cp \- salt-cp Documentation
 .

doc/man/salt-key.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-KEY" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-KEY" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-key \- salt-key Documentation
 .

doc/man/salt-master.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-MASTER" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-MASTER" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-master \- salt-master Documentation
 .

doc/man/salt-minion.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-MINION" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-MINION" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-minion \- salt-minion Documentation
 .

doc/man/salt-proxy.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-PROXY" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-PROXY" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-proxy \- salt-proxy Documentation
 .

doc/man/salt-run.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-RUN" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-RUN" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-run \- salt-run Documentation
 .

doc/man/salt-ssh.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-SSH" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-SSH" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-ssh \- salt-ssh Documentation
 .

doc/man/salt-syndic.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-SYNDIC" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-SYNDIC" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-syndic \- salt-syndic Documentation
 .

doc/man/salt-unity.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT-UNITY" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT-UNITY" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt-unity \- salt-unity Command
 .

doc/man/salt.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SALT" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SALT" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 salt \- salt
 .

doc/man/spm.1

@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SPM" "1" "Feb 23, 2018" "2018.3.0" "Salt"
+.TH "SPM" "1" "May 09, 2018" "2018.3.1" "Salt"
 .SH NAME
 spm \- Salt Package Manager Command
 .

doc/ref/states/all/index.rst

@@ -124,7 +124,6 @@ state modules
     influxdb_database
     influxdb_retention_policy
     influxdb_user
-    infoblox
     infoblox_a
     infoblox_cname
     infoblox_host_record

doc/ref/states/all/salt.states.infoblox.rst

@@ -1,5 +0,0 @@
-salt.states.infoblox module
-===========================
-
-.. automodule:: salt.states.infoblox
-    :members:

doc/ref/states/requisites.rst

@@ -410,10 +410,11 @@ exactly like the ``require`` requisite (the watching state will execute if
       service.running:
         - watch_any:
           - file: /etc/apache2/sites-available/site1.conf
-          - file: /etc/apache2/sites-available/site2.conf
+          - file: apache2-site2
       file.managed:
         - name: /etc/apache2/sites-available/site1.conf
         - source: salt://apache2/files/site1.conf
+    apache2-site2:
       file.managed:
         - name: /etc/apache2/sites-available/site2.conf
         - source: salt://apache2/files/site2.conf

doc/topics/cloud/config.rst

@@ -351,6 +351,7 @@ This driver can be configured using the ``/etc/openstack/clouds.yml`` file with
 `os-client-config <https://docs.openstack.org/os-client-config/latest/>`
 
 .. code-block:: yaml
+
     myopenstack:
       driver: openstack
       region_name: RegionOne
@@ -359,6 +360,7 @@ This driver can be configured using the ``/etc/openstack/clouds.yml`` file with
 Or by just configuring the same auth block directly in the cloud provider config.
 
 .. code-block:: yaml
+
     myopenstack:
       driver: openstack
       region_name: RegionOne

doc/topics/releases/2017.7.6.rst

@@ -1,8 +1,9 @@
 ===========================
-Salt 2017.7.6 Release Notes
+In Progress: Salt 2017.7.6 Release Notes
 ===========================
 
-Version 2017.7.6 is a bugfix release for :ref:`2017.7.0 <release-2017-7-0>`.
+Version 2017.7.6 is an **unreleased** bugfix release for :ref:`2017.7.0 <release-2017-7-0>`.
+This release is still in progress and has not been released yet.
 
 Option to Return to Previous Pillar Include Behavior
 ----------------------------------------------------

doc/topics/tutorials/gitfs.rst

@@ -699,15 +699,24 @@ repository to be served up from the Salt fileserver path
 Mountpoints can also be configured on a :ref:`per-remote basis
 <gitfs-per-remote-config>`.
 
+
+Using gitfs in Masterless Mode
+==============================
+
+Since 2014.7.0, gitfs can be used in masterless mode. To do so, simply add the
+gitfs configuration parameters (and set :conf_master:`fileserver_backend`) in
+the _minion_ config file instead of the master config file.
+
+
 Using gitfs Alongside Other Backends
 ====================================
 
 Sometimes it may make sense to use multiple backends; for instance, if ``sls``
 files are stored in git but larger files are stored directly on the master.
 
-The cascading lookup logic used for multiple remotes is also used with
-multiple backends. If the ``fileserver_backend`` option contains
-multiple backends:
+The cascading lookup logic used for multiple remotes is also used with multiple
+backends. If the :conf_master:`fileserver_backend` option contains multiple
+backends:
 
 .. code-block:: yaml
 
@@ -719,7 +728,6 @@ Then the ``roots`` backend (the default backend of files in ``/srv/salt``) will
 be searched first for the requested file; then, if it is not found on the
 master, each configured git remote will be searched.
 
-
 Branches, Environments, and Top Files
 =====================================
 

pkg/windows/modules/get-settings.psm1

@@ -35,7 +35,7 @@ Function Get-Settings {
 
         # Prerequisite software
         $Prerequisites = @{
-            "NSIS"           = "nsis-3.02.1-setup.exe"
+            "NSIS"           = "nsis-3.03-setup.exe"
             "VCforPython"    = "VCForPython27.msi"
             "VCppBuildTools" = "visualcppbuildtools_full.exe"
         }
@@ -59,21 +59,15 @@ Function Get-Settings {
 
         # Filenames for 64 bit Windows
         $64bitPrograms = @{
-            "PyCrypto2" = "pycrypto-2.6.1-cp27-none-win_amd64.whl"
             "Python2"   = "python-2.7.14.amd64.msi"
-            "PyWin322"  = "pywin32-221-cp27-cp27m-win_amd64.whl"
             "Python3"   = "python-3.5.3-amd64.exe"
-            "PyWin323"  = "pywin32-221-cp35-cp35m-win_amd64.whl"
         }
         $ini.Add("64bitPrograms", $64bitPrograms)
 
         # Filenames for 32 bit Windows
         $32bitPrograms = @{
-            "PyCrypto2" = "pycrypto-2.6.1-cp27-none-win32.whl"
             "Python2"   = "python-2.7.14.msi"
-            "PyWin322"  = "pywin32-221-cp27-cp27m-win32.whl"
             "Python3"   = "python-3.5.3.exe"
-            "PyWin323"  = "pywin32-221-cp35-cp35m-win32.whl"
         }
         $ini.Add("32bitPrograms", $32bitPrograms)
 

requirements/base-py2.txt

@@ -0,0 +1,4 @@
+-r base.txt
+
+# Required by Tornado to handle threads stuff.
+futures>=2.0

requirements/base-py3.txt

@@ -0,0 +1 @@
+-r base.txt

salt/cloud/clouds/ec2.py

@@ -3675,6 +3675,25 @@ def enable_term_protect(name, call=None):
     return _toggle_term_protect(name, 'true')
 
 
+def disable_term_protect(name, call=None):
+    '''
+    Disable termination protection on a node
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt-cloud -a disable_term_protect mymachine
+    '''
+    if call != 'action':
+        raise SaltCloudSystemExit(
+            'The enable_term_protect action must be called with '
+            '-a or --action.'
+        )
+
+    return _toggle_term_protect(name, 'false')
+
+
 def disable_detailed_monitoring(name, call=None):
     '''
     Enable/disable detailed monitoring on a node

salt/fileclient.py

@@ -13,6 +13,7 @@ import string
 import shutil
 import ftplib
 from tornado.httputil import parse_response_start_line, HTTPHeaders, HTTPInputError
+import salt.utils.atomicfile
 
 # Import salt libs
 from salt.exceptions import (
@@ -1166,7 +1167,11 @@ class RemoteClient(Client):
             destdir = os.path.dirname(dest)
             if not os.path.isdir(destdir):
                 if makedirs:
-                    os.makedirs(destdir)
+                    try:
+                        os.makedirs(destdir)
+                    except OSError as exc:
+                        if exc.errno != errno.EEXIST:  # ignore if it was there already
+                            raise
                 else:
                     return False
             # We need an open filehandle here, that's why we're not using a
@@ -1221,7 +1226,7 @@ class RemoteClient(Client):
                         # remove it to avoid a traceback trying to write the file
                         if os.path.isdir(dest):
                             salt.utils.files.rm_rf(dest)
-                        fn_ = salt.utils.files.fopen(dest, 'wb+')
+                        fn_ = salt.utils.atomicfile.atomic_open(dest, 'wb+')
                 if data.get('gzip', None):
                     data = salt.utils.gzip_util.uncompress(data['data'])
                 else:

salt/modules/boto3_route53.py

@@ -551,6 +551,10 @@ def associate_vpc_with_hosted_zone(HostedZoneId=None, Name=None, VPCId=None,
             r = conn.associate_vpc_with_hosted_zone(**args)
             return _wait_for_sync(r['ChangeInfo']['Id'], conn)
         except ClientError as e:
+            if e.response.get('Error', {}).get('Code') == 'ConflictingDomainExists':
+                log.debug('VPC Association already exists.')
+                # return True since the current state is the desired one
+                return True
             if tries and e.response.get('Error', {}).get('Code') == 'Throttling':
                 log.debug('Throttled by AWS API.')
                 time.sleep(3)

salt/modules/cmdmod.py

@@ -36,6 +36,7 @@ import salt.utils.timed_subprocess
 import salt.utils.user
 import salt.utils.versions
 import salt.utils.vt
+import salt.utils.win_dacl
 import salt.utils.win_reg
 import salt.grains.extra
 from salt.ext import six
@@ -477,10 +478,18 @@ def _run(cmd,
                 for k, v in six.iteritems(env_runas)
             )
             env_runas.update(env)
+
             # Fix platforms like Solaris that don't set a USER env var in the
             # user's default environment as obtained above.
             if env_runas.get('USER') != runas:
                 env_runas['USER'] = runas
+
+            # Fix some corner cases where shelling out to get the user's
+            # environment returns the wrong home directory.
+            runas_home = os.path.expanduser('~{0}'.format(runas))
+            if env_runas.get('HOME') != runas_home:
+                env_runas['HOME'] = runas_home
+
             env = env_runas
         except ValueError as exc:
             log.exception('Error raised retrieving environment for user %s', runas)
@@ -2200,11 +2209,14 @@ def script(source,
         # "env" is not supported; Use "saltenv".
         kwargs.pop('__env__')
 
+    win_cwd = False
     if salt.utils.platform.is_windows() and runas and cwd is None:
+        # Create a temp working directory
         cwd = tempfile.mkdtemp(dir=__opts__['cachedir'])
-        __salt__['win_dacl.add_ace'](
-            cwd, 'File', runas, 'READ&EXECUTE', 'ALLOW',
-            'FOLDER&SUBFOLDERS&FILES')
+        win_cwd = True
+        salt.utils.win_dacl.set_permissions(obj_name=cwd,
+                                            principal=runas,
+                                            permissions='full_control')
 
     path = salt.utils.files.mkstemp(dir=cwd, suffix=os.path.splitext(source)[1])
 
@@ -2218,10 +2230,10 @@ def script(source,
                                           saltenv,
                                           **kwargs)
         if not fn_:
-            if salt.utils.platform.is_windows() and runas:
+            _cleanup_tempfile(path)
+            # If a temp working directory was created (Windows), let's remove that
+            if win_cwd:
                 _cleanup_tempfile(cwd)
-            else:
-                _cleanup_tempfile(path)
             return {'pid': 0,
                     'retcode': 1,
                     'stdout': '',
@@ -2230,10 +2242,10 @@ def script(source,
     else:
         fn_ = __salt__['cp.cache_file'](source, saltenv)
         if not fn_:
-            if salt.utils.platform.is_windows() and runas:
+            _cleanup_tempfile(path)
+            # If a temp working directory was created (Windows), let's remove that
+            if win_cwd:
                 _cleanup_tempfile(cwd)
-            else:
-                _cleanup_tempfile(path)
             return {'pid': 0,
                     'retcode': 1,
                     'stdout': '',
@@ -2264,10 +2276,10 @@ def script(source,
                bg=bg,
                password=password,
                **kwargs)
-    if salt.utils.platform.is_windows() and runas:
+    _cleanup_tempfile(path)
+    # If a temp working directory was created (Windows), let's remove that
+    if win_cwd:
         _cleanup_tempfile(cwd)
-    else:
-        _cleanup_tempfile(path)
 
     if hide_output:
         ret['stdout'] = ret['stderr'] = ''

salt/modules/dockermod.py

@@ -3210,6 +3210,7 @@ def run_container(image,
     CLI Examples:
 
     .. code-block:: bash
+
         salt myminion docker.run_container myuser/myimage command=/usr/local/bin/myscript.sh
         # Run container in the background
         salt myminion docker.run_container myuser/myimage command=/usr/local/bin/myscript.sh bg=True

salt/modules/glanceng.py

@@ -9,10 +9,12 @@ Glance module for interacting with OpenStack Glance
 Example configuration
 
 .. code-block:: yaml
+
     glance:
       cloud: default
 
 .. code-block:: yaml
+
     glance:
       auth:
         username: admin

salt/modules/keystoneng.py

@@ -9,10 +9,12 @@ Keystone module for interacting with OpenStack Keystone
 Example configuration
 
 .. code-block:: yaml
+
     keystone:
       cloud: default
 
 .. code-block:: yaml
+
     keystone:
       auth:
         username: admin

salt/modules/lxc.py

@@ -867,7 +867,7 @@ def _network_conf(conf_tuples=None, **kwargs):
     # on old versions of lxc, still support the gateway auto mode
     # if we didn't explicitly say no to
     # (lxc.network.ipv4.gateway: auto)
-    if _LooseVersion(version()) <= '1.0.7' and \
+    if _LooseVersion(version()) <= _LooseVersion('1.0.7') and \
             True not in ['lxc.network.ipv4.gateway' in a for a in ret] and \
             True in ['lxc.network.ipv4' in a for a in ret]:
         ret.append({'lxc.network.ipv4.gateway': 'auto'})

salt/modules/neutronng.py

@@ -9,10 +9,12 @@ Neutron module for interacting with OpenStack Neutron
 Example configuration
 
 .. code-block:: yaml
+
     neutron:
       cloud: default
 
 .. code-block:: yaml
+
     neutron:
       auth:
         username: admin

salt/modules/pip.py

@@ -119,40 +119,82 @@ def __virtual__():
     return 'pip'
 
 
+def _clear_context(bin_env=None):
+    '''
+    Remove the cached pip version
+    '''
+    contextkey = 'pip.version'
+    if bin_env is not None:
+        contextkey = '{0}.{1}'.format(contextkey, bin_env)
+    __context__.pop(contextkey, None)
+
+
 def _get_pip_bin(bin_env):
     '''
     Locate the pip binary, either from `bin_env` as a virtualenv, as the
     executable itself, or from searching conventional filesystem locations
     '''
     if not bin_env:
-        which_result = __salt__['cmd.which_bin'](
-            ['pip{0}.{1}'.format(*sys.version_info[:2]),
-             'pip{0}'.format(sys.version_info[0]),
-             'pip', 'pip-python']
-        )
-        if salt.utils.platform.is_windows() and six.PY2 \
-           and isinstance(which_result, str):
-            which_result.encode('string-escape')
-        if which_result is None:
-            raise CommandNotFoundError('Could not find a `pip` binary')
-        return which_result
+        logger.debug('pip: Using pip from currently-running Python')
+        return [os.path.normpath(sys.executable), '-m', 'pip']
 
-    # try to get pip bin from virtualenv, bin_env
+    python_bin = 'python.exe' if salt.utils.platform.is_windows() else 'python'
+
+    def _search_paths(*basedirs):
+        ret = []
+        for path in basedirs:
+            ret.extend([
+                os.path.join(path, python_bin),
+                os.path.join(path, 'bin', python_bin),
+                os.path.join(path, 'Scripts', python_bin)
+            ])
+        return ret
+
+    # try to get python bin from virtualenv (i.e. bin_env)
     if os.path.isdir(bin_env):
-        if salt.utils.platform.is_windows():
-            pip_bin = os.path.join(bin_env, 'Scripts', 'pip.exe')
-        else:
-            pip_bin = os.path.join(bin_env, 'bin', 'pip')
-        if os.path.isfile(pip_bin):
-            return pip_bin
-        msg = 'Could not find a `pip` binary in virtualenv {0}'.format(bin_env)
-        raise CommandNotFoundError(msg)
-    # bin_env is the pip binary
+        for bin_path in _search_paths(bin_env):
+            if os.path.isfile(bin_path):
+                if os.access(bin_path, os.X_OK):
+                    logger.debug('pip: Found python binary: %s', bin_path)
+                    return [os.path.normpath(bin_path), '-m', 'pip']
+                else:
+                    logger.debug(
+                        'pip: Found python binary by name but it is not '
+                        'executable: %s', bin_path
+                    )
+        raise CommandNotFoundError(
+            'Could not find a pip binary in virtualenv {0}'.format(bin_env)
+        )
+
+    # bin_env is the python or pip binary
     elif os.access(bin_env, os.X_OK):
-        if os.path.isfile(bin_env) or os.path.islink(bin_env):
-            return bin_env
+        if os.path.isfile(bin_env):
+            # If the python binary was passed, return it
+            if 'python' in os.path.basename(bin_env):
+                return [os.path.normpath(bin_env), '-m', 'pip']
+            # Try to find the python binary based on the location of pip in a
+            # virtual environment, should be relative
+            if 'pip' in os.path.basename(bin_env):
+                # Look in the same directory as the pip binary, and also its
+                # parent directories.
+                pip_dirname = os.path.dirname(bin_env)
+                pip_parent_dir = os.path.dirname(pip_dirname)
+                for bin_path in _search_paths(pip_dirname, pip_parent_dir):
+                    if os.path.isfile(bin_path):
+                        logger.debug('pip: Found python binary: %s', bin_path)
+                        return [os.path.normpath(bin_path), '-m', 'pip']
+
+            # Couldn't find python, use the passed pip binary
+            # This has the limitation of being unable to update pip itself
+            return [os.path.normpath(bin_env)]
+
+        raise CommandExecutionError(
+            'Could not find a pip binary within {0}'.format(bin_env)
+        )
     else:
-        raise CommandNotFoundError('Could not find a `pip` binary')
+        raise CommandNotFoundError(
+            'Access denied to {0}, could not find a pip binary'.format(bin_env)
+        )
 
 
 def _get_cached_requirements(requirements, saltenv):
@@ -271,15 +313,21 @@ def _process_requirements(requirements, cmd, cwd, saltenv, user):
                     treq = tempfile.mkdtemp()
 
                 __salt__['file.chown'](treq, user, None)
+                # In Windows, just being owner of a file isn't enough. You also
+                # need permissions
+                if salt.utils.platform.is_windows():
+                    __utils__['win_dacl.set_permissions'](
+                        obj_name=treq,
+                        principal=user,
+                        permissions='read_execute')
 
                 current_directory = None
 
                 if not current_directory:
                     current_directory = os.path.abspath(os.curdir)
 
-                logger.info('_process_requirements from directory,' +
-                            '%s -- requirement: %s', cwd, requirement
-                            )
+                logger.info('_process_requirements from directory, '
+                            '%s -- requirement: %s', cwd, requirement)
 
                 if cwd is None:
                     r = requirement
@@ -384,7 +432,6 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
             global_options=None,
             install_options=None,
             user=None,
-            no_chown=False,
             cwd=None,
             pre_releases=False,
             cert=None,
@@ -398,7 +445,8 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
             trusted_host=None,
             no_cache_dir=False,
             cache_dir=None,
-            no_binary=None):
+            no_binary=None,
+            **kwargs):
     '''
     Install packages with pip
 
@@ -412,13 +460,10 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
         Path to requirements
 
     bin_env
-        Path to pip bin or path to virtualenv. If doing a system install,
-        and want to use a specific pip bin (pip-2.7, pip-2.6, etc..) just
-        specify the pip bin you want.
-
-        .. note::
-            If installing into a virtualenv, just use the path to the
-            virtualenv (e.g. ``/home/code/path/to/virtualenv/``)
+        Path to pip (or to a virtualenv). This can be used to specify the path
+        to the pip to use when more than one Python release is installed (e.g.
+        ``/usr/bin/pip-2.7`` or ``/usr/bin/pip-2.6``. If a directory path is
+        specified, it is assumed to be a virtualenv.
 
     use_wheel
         Prefer wheel archives (requires pip>=1.4)
@@ -520,12 +565,8 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
     user
         The user under which to run pip
 
-    no_chown
-        When user is given, do not attempt to copy and chown a requirements
-        file
-
     cwd
-        Current working directory to run pip from
+        Directory from which to run pip
 
     pre_releases
         Include pre-releases in the available versions
@@ -584,9 +625,14 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
                 editable=git+https://github.com/worldcompany/djangoembed.git#egg=djangoembed upgrade=True no_deps=True
 
     '''
-    pip_bin = _get_pip_bin(bin_env)
-
-    cmd = [pip_bin, 'install']
+    if 'no_chown' in kwargs:
+        salt.utils.versions.warn_until(
+            'Flourine',
+            'The no_chown argument has been deprecated and is no longer used. '
+            'Its functionality was removed in Boron.')
+        kwargs.pop('no_chown')
+    cmd = _get_pip_bin(bin_env)
+    cmd.append('install')
 
     cleanup_requirements, error = _process_requirements(
         requirements=requirements,
@@ -599,10 +645,11 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
     if error:
         return error
 
+    cur_version = version(bin_env)
+
     if use_wheel:
         min_version = '1.4'
         max_version = '9.0.3'
-        cur_version = __salt__['pip.version'](bin_env)
         too_low = salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version)
         too_high = salt.utils.versions.compare(ver1=cur_version, oper='>', ver2=max_version)
         if too_low or too_high:
@@ -617,7 +664,6 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
     if no_use_wheel:
         min_version = '1.4'
         max_version = '9.0.3'
-        cur_version = __salt__['pip.version'](bin_env)
         too_low = salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version)
         too_high = salt.utils.versions.compare(ver1=cur_version, oper='>', ver2=max_version)
         if too_low or too_high:
@@ -631,7 +677,6 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
     if no_binary:
         min_version = '7.0.0'
-        cur_version = __salt__['pip.version'](bin_env)
         too_low = salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version)
         if too_low:
             logger.error(
@@ -706,8 +751,7 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
     if mirrors:
         # https://github.com/pypa/pip/pull/2641/files#diff-3ef137fb9ffdd400f117a565cd94c188L216
-        pip_version = version(pip_bin)
-        if salt.utils.versions.compare(ver1=pip_version, oper='>=', ver2='7.0.0'):
+        if salt.utils.versions.compare(ver1=cur_version, oper='>=', ver2='7.0.0'):
             raise CommandExecutionError(
                     'pip >= 7.0.0 does not support mirror argument:'
                     ' use index_url and/or extra_index_url instead'
@@ -735,7 +779,7 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
     if download_cache or cache_dir:
         cmd.extend(['--cache-dir' if salt.utils.versions.compare(
-            ver1=version(bin_env), oper='>=', ver2='6.0'
+            ver1=cur_version, oper='>=', ver2='6.0'
         ) else '--download-cache', download_cache or cache_dir])
 
     if source:
@@ -772,7 +816,7 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
     if pre_releases:
         # Check the locally installed pip version
-        pip_version = version(pip_bin)
+        pip_version = cur_version
 
         # From pip v1.4 the --pre flag is available
         if salt.utils.versions.compare(ver1=pip_version, oper='>=', ver2='1.4'):
@@ -857,6 +901,9 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
     cmd_kwargs = dict(saltenv=saltenv, use_vt=use_vt, runas=user)
 
+    if kwargs:
+        cmd_kwargs.update(kwargs)
+
     if env_vars:
         cmd_kwargs.setdefault('env', {}).update(_format_env_vars(env_vars))
 
@@ -874,6 +921,7 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
         return __salt__['cmd.run_all'](cmd, python_shell=False, **cmd_kwargs)
     finally:
+        _clear_context(bin_env)
         for tempdir in [cr for cr in cleanup_requirements if cr is not None]:
             if os.path.isdir(tempdir):
                 shutil.rmtree(tempdir)
@@ -886,46 +934,42 @@ def uninstall(pkgs=None,
               proxy=None,
               timeout=None,
               user=None,
-              no_chown=False,
               cwd=None,
               saltenv='base',
               use_vt=False):
     '''
-    Uninstall packages with pip
-
-    Uninstall packages individually or from a pip requirements file. Uninstall
-    packages globally or from a virtualenv.
+    Uninstall packages individually or from a pip requirements file
 
     pkgs
         comma separated list of packages to install
+
     requirements
-        path to requirements.
+        Path to requirements file
+
     bin_env
-        path to pip bin or path to virtualenv. If doing an uninstall from
-        the system python and want to use a specific pip bin (pip-2.7,
-        pip-2.6, etc..) just specify the pip bin you want.
-        If uninstalling from a virtualenv, just use the path to the virtualenv
-        (/home/code/path/to/virtualenv/)
+        Path to pip (or to a virtualenv). This can be used to specify the path
+        to the pip to use when more than one Python release is installed (e.g.
+        ``/usr/bin/pip-2.7`` or ``/usr/bin/pip-2.6``. If a directory path is
+        specified, it is assumed to be a virtualenv.
+
     log
         Log file where a complete (maximum verbosity) record will be kept
+
     proxy
-        Specify a proxy in the form
-        user:passwd@proxy.server:port. Note that the
-        user:password@ is optional and required only if you
-        are behind an authenticated proxy.  If you provide
-        user@proxy.server:port then you will be prompted for a
-        password.
+        Specify a proxy in the format ``user:passwd@proxy.server:port``. Note
+        that the ``user:password@`` is optional and required only if you are
+        behind an authenticated proxy.  If you provide
+        ``user@proxy.server:port`` then you will be prompted for a password.
+
     timeout
         Set the socket timeout (default 15 seconds)
+
     user
         The user under which to run pip
-    no_chown
-        When user is given, do not attempt to copy and chown
-        a requirements file (needed if the requirements file refers to other
-        files via relative paths, as the copy-and-chown procedure does not
-        account for such files)
+
     cwd
-        Current working directory to run pip from
+        Directory from which to run pip
+
     use_vt
         Use VT terminal emulation (see output while installing)
 
@@ -937,11 +981,9 @@ def uninstall(pkgs=None,
         salt '*' pip.uninstall requirements=/path/to/requirements.txt
         salt '*' pip.uninstall <package name> bin_env=/path/to/virtualenv
         salt '*' pip.uninstall <package name> bin_env=/path/to/pip_bin
-
     '''
-    pip_bin = _get_pip_bin(bin_env)
-
-    cmd = [pip_bin, 'uninstall', '-y']
+    cmd = _get_pip_bin(bin_env)
+    cmd.extend(['uninstall', '-y'])
 
     cleanup_requirements, error = _process_requirements(
         requirements=requirements, cmd=cmd, saltenv=saltenv, user=user,
@@ -1001,6 +1043,7 @@ def uninstall(pkgs=None,
     try:
         return __salt__['cmd.run_all'](cmd, **cmd_kwargs)
     finally:
+        _clear_context(bin_env)
         for requirement in cleanup_requirements:
             if requirement:
                 try:
@@ -1013,48 +1056,42 @@ def freeze(bin_env=None,
            user=None,
            cwd=None,
            use_vt=False,
-           env_vars=None):
+           env_vars=None,
+           **kwargs):
     '''
     Return a list of installed packages either globally or in the specified
     virtualenv
 
     bin_env
-        path to pip bin or path to virtualenv. If doing an uninstall from
-        the system python and want to use a specific pip bin (pip-2.7,
-        pip-2.6, etc..) just specify the pip bin you want.
-        If uninstalling from a virtualenv, just use the path to the virtualenv
-        (/home/code/path/to/virtualenv/)
+        Path to pip (or to a virtualenv). This can be used to specify the path
+        to the pip to use when more than one Python release is installed (e.g.
+        ``/usr/bin/pip-2.7`` or ``/usr/bin/pip-2.6``. If a directory path is
+        specified, it is assumed to be a virtualenv.
+
     user
         The user under which to run pip
+
     cwd
-        Current working directory to run pip from
+        Directory from which to run pip
 
     .. note::
-
         If the version of pip available is older than 8.0.3, the list will not
-        include the packages pip, wheel, setuptools, or distribute even if they
-        are installed.
+        include the packages ``pip``, ``wheel``, ``setuptools``, or
+        ``distribute`` even if they are installed.
 
     CLI Example:
 
     .. code-block:: bash
 
-        salt '*' pip.freeze /home/code/path/to/virtualenv/
-
-    .. versionchanged:: 2016.11.2
-
-        The packages pip, wheel, setuptools, and distribute are included if the
-        installed pip is new enough.
+        salt '*' pip.freeze bin_env=/home/code/path/to/virtualenv
     '''
-    pip_bin = _get_pip_bin(bin_env)
-
-    cmd = [pip_bin, 'freeze']
+    cmd = _get_pip_bin(bin_env)
+    cmd.append('freeze')
 
     # Include pip, setuptools, distribute, wheel
     min_version = '8.0.3'
     cur_version = version(bin_env)
-    if not salt.utils.versions.compare(ver1=cur_version, oper='>=',
-                                       ver2=min_version):
+    if salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version):
         logger.warning(
             'The version of pip installed is %s, which is older than %s. '
             'The packages pip, wheel, setuptools, and distribute will not be '
@@ -1064,14 +1101,16 @@ def freeze(bin_env=None,
         cmd.append('--all')
 
     cmd_kwargs = dict(runas=user, cwd=cwd, use_vt=use_vt, python_shell=False)
+    if kwargs:
+        cmd_kwargs.update(**kwargs)
     if bin_env and os.path.isdir(bin_env):
         cmd_kwargs['env'] = {'VIRTUAL_ENV': bin_env}
     if env_vars:
         cmd_kwargs.setdefault('env', {}).update(_format_env_vars(env_vars))
     result = __salt__['cmd.run_all'](cmd, **cmd_kwargs)
 
-    if result['retcode'] > 0:
-        raise CommandExecutionError(result['stderr'])
+    if result['retcode']:
+        raise CommandExecutionError(result['stderr'], info=result)
 
     return result['stdout'].splitlines()
 
@@ -1080,7 +1119,8 @@ def list_(prefix=None,
           bin_env=None,
           user=None,
           cwd=None,
-          env_vars=None):
+          env_vars=None,
+          **kwargs):
     '''
     Filter list of installed apps from ``freeze`` and check to see if
     ``prefix`` exists in the list of packages installed.
@@ -1088,28 +1128,27 @@ def list_(prefix=None,
     .. note::
 
         If the version of pip available is older than 8.0.3, the packages
-        wheel, setuptools, and distribute will not be reported by this function
-        even if they are installed. Unlike
-        :py:func:`pip.freeze <salt.modules.pip.freeze>`, this function always
-        reports the version of pip which is installed.
+        ``wheel``, ``setuptools``, and ``distribute`` will not be reported by
+        this function even if they are installed. Unlike :py:func:`pip.freeze
+        <salt.modules.pip.freeze>`, this function always reports the version of
+        pip which is installed.
 
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' pip.list salt
-
-    .. versionchanged:: 2016.11.2
-
-        The packages wheel, setuptools, and distribute are included if the
-        installed pip is new enough.
     '''
     packages = {}
 
     if prefix is None or 'pip'.startswith(prefix):
         packages['pip'] = version(bin_env)
 
-    for line in freeze(bin_env=bin_env, user=user, cwd=cwd, env_vars=env_vars):
+    for line in freeze(bin_env=bin_env,
+                       user=user,
+                       cwd=cwd,
+                       env_vars=env_vars,
+                       **kwargs):
         if line.startswith('-f') or line.startswith('#'):
             # ignore -f line as it contains --find-links directory
             # ignore comment lines
@@ -1119,7 +1158,15 @@ def list_(prefix=None,
             continue
         elif line.startswith('-e'):
             line = line.split('-e ')[1]
-            version_, name = line.split('#egg=')
+            if '#egg=' in line:
+                version_, name = line.split('#egg=')
+            else:
+                if len(line.split('===')) >= 2:
+                    name = line.split('===')[0]
+                    version_ = line.split('===')[1]
+                elif len(line.split('==')) >= 2:
+                    name = line.split('==')[0]
+                    version_ = line.split('==')[1]
         elif len(line.split('===')) >= 2:
             name = line.split('===')[0]
             version_ = line.split('===')[1]
@@ -1154,14 +1201,27 @@ def version(bin_env=None):
 
         salt '*' pip.version
     '''
-    pip_bin = _get_pip_bin(bin_env)
+    contextkey = 'pip.version'
+    if bin_env is not None:
+        contextkey = '{0}.{1}'.format(contextkey, bin_env)
+
+    if contextkey in __context__:
+        return __context__[contextkey]
+
+    cmd = _get_pip_bin(bin_env)[:]
+    cmd.append('--version')
+
+    ret = __salt__['cmd.run_all'](cmd, python_shell=False)
+    if ret['retcode']:
+        raise CommandNotFoundError('Could not find a `pip` binary')
 
-    output = __salt__['cmd.run_stdout'](
-        '{0} --version'.format(pip_bin), python_shell=False)
     try:
-        return re.match(r'^pip (\S+)', output).group(1)
+        pip_version = re.match(r'^pip (\S+)', ret['stdout']).group(1)
     except AttributeError:
-        return None
+        pip_version = None
+
+    __context__[contextkey] = pip_version
+    return pip_version
 
 
 def list_upgrades(bin_env=None,
@@ -1176,15 +1236,15 @@ def list_upgrades(bin_env=None,
 
         salt '*' pip.list_upgrades
     '''
-    pip_bin = _get_pip_bin(bin_env)
+    cmd = _get_pip_bin(bin_env)
+    cmd.extend(['list', '--outdated'])
 
-    cmd = [pip_bin, 'list', '--outdated']
-
-    # If pip >= 9.0 use --format=json
+    pip_version = version(bin_env)
+    # Pip started supporting the ability to output json starting with 9.0.0
     min_version = '9.0'
-    cur_version = version(pip_bin)
-    if salt.utils.versions.compare(ver1=cur_version, oper='>=',
-                                       ver2=min_version):
+    if salt.utils.versions.compare(ver1=pip_version,
+                                   oper='>=',
+                                   ver2=min_version):
         cmd.append('--format=json')
 
     cmd_kwargs = dict(cwd=cwd, runas=user)
@@ -1192,54 +1252,78 @@ def list_upgrades(bin_env=None,
         cmd_kwargs['env'] = {'VIRTUAL_ENV': bin_env}
 
     result = __salt__['cmd.run_all'](cmd, **cmd_kwargs)
-    if result['retcode'] > 0:
-        logger.error(result['stderr'])
-        raise CommandExecutionError(result['stderr'])
+    if result['retcode']:
+        raise CommandExecutionError(result['stderr'], info=result)
 
     packages = {}
-    try:
-        json_results = salt.utils.json.loads(result['stdout'])
-        for json_result in json_results:
-            packages[json_result['name']] = json_result['latest_version']
-    except ValueError:
+    # Pip started supporting the ability to output json starting with 9.0.0
+    # Older versions will have to parse stdout
+    if salt.utils.versions.compare(ver1=pip_version, oper='<', ver2='9.0.0'):
+        # Pip versions < 8.0.0 had a different output format
+        # Sample data:
+        # pip (Current: 7.1.2 Latest: 10.0.1 [wheel])
+        # psutil (Current: 5.2.2 Latest: 5.4.5 [wheel])
+        # pyasn1 (Current: 0.2.3 Latest: 0.4.2 [wheel])
+        # pycparser (Current: 2.17 Latest: 2.18 [sdist])
+        if salt.utils.versions.compare(ver1=pip_version, oper='<', ver2='8.0.0'):
+            logger.debug('pip module: Old output format')
+            pat = re.compile(r'(\S*)\s+\(.*Latest:\s+(.*)\)')
+
+        # New output format for version 8.0.0+
+        # Sample data:
+        # pip (8.0.0) - Latest: 10.0.1 [wheel]
+        # psutil (5.2.2) - Latest: 5.4.5 [wheel]
+        # pyasn1 (0.2.3) - Latest: 0.4.2 [wheel]
+        # pycparser (2.17) - Latest: 2.18 [sdist]
+        else:
+            logger.debug('pip module: New output format')
+            pat = re.compile(r'(\S*)\s+\(.*\)\s+-\s+Latest:\s+(.*)')
+
         for line in result['stdout'].splitlines():
-            match = re.search(r'(\S*)\s+.*Latest:\s+(.*)', line)
+            match = pat.search(line)
             if match:
                 name, version_ = match.groups()
             else:
-                logger.error('Can\'t parse line \'%s\'', line)
+                logger.error('Can\'t parse line \'{0}\''.format(line))
                 continue
             packages[name] = version_
 
+    else:
+        logger.debug('pip module: JSON output format')
+        try:
+            pkgs = salt.utils.json.loads(result['stdout'], strict=False)
+        except ValueError:
+            raise CommandExecutionError('Invalid JSON', info=result)
+
+        for pkg in pkgs:
+            packages[pkg['name']] = '{0} [{1}]'.format(pkg['latest_version'],
+                                                       pkg['latest_filetype'])
+
     return packages
 
 
 def is_installed(pkgname=None,
-          bin_env=None,
-          user=None,
-          cwd=None):
+                 bin_env=None,
+                 user=None,
+                 cwd=None):
     '''
+    .. versionadded:: 2018.3.0
+
     Filter list of installed apps from ``freeze`` and return True or False  if
     ``pkgname`` exists in the list of packages installed.
 
     .. note::
-
         If the version of pip available is older than 8.0.3, the packages
         wheel, setuptools, and distribute will not be reported by this function
-        even if they are installed. Unlike
-        :py:func:`pip.freeze <salt.modules.pip.freeze>`, this function always
-        reports the version of pip which is installed.
+        even if they are installed. Unlike :py:func:`pip.freeze
+        <salt.modules.pip.freeze>`, this function always reports the version of
+        pip which is installed.
 
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' pip.is_installed salt
-
-    .. versionadded:: 2018.3.0
-
-        The packages wheel, setuptools, and distribute are included if the
-        installed pip is new enough.
     '''
     for line in freeze(bin_env=bin_env, user=user, cwd=cwd):
         if line.startswith('-f') or line.startswith('#'):
@@ -1294,7 +1378,11 @@ def upgrade(bin_env=None,
     '''
     .. versionadded:: 2015.5.0
 
-    Upgrades outdated pip packages
+    Upgrades outdated pip packages.
+
+    .. note::
+        On Windows you can't update salt from pip using salt, so salt will be
+        skipped
 
     Returns a dict containing the changes.
 
@@ -1312,16 +1400,19 @@ def upgrade(bin_env=None,
            'result': True,
            'comment': '',
            }
-    pip_bin = _get_pip_bin(bin_env)
+    cmd = _get_pip_bin(bin_env)
+    cmd.extend(['install', '-U'])
 
     old = list_(bin_env=bin_env, user=user, cwd=cwd)
 
-    cmd = [pip_bin, 'install', '-U']
     cmd_kwargs = dict(cwd=cwd, use_vt=use_vt)
     if bin_env and os.path.isdir(bin_env):
         cmd_kwargs['env'] = {'VIRTUAL_ENV': bin_env}
     errors = False
     for pkg in list_upgrades(bin_env=bin_env, user=user, cwd=cwd):
+        if pkg == 'salt':
+            if salt.utils.platform.is_windows():
+                continue
         result = __salt__['cmd.run_all'](cmd + [pkg], **cmd_kwargs)
         if result['retcode'] != 0:
             errors = True
@@ -1330,6 +1421,7 @@ def upgrade(bin_env=None,
     if errors:
         ret['result'] = False
 
+    _clear_context(bin_env)
     new = list_(bin_env=bin_env, user=user, cwd=cwd)
 
     ret['changes'] = salt.utils.data.compare_dicts(old, new)
@@ -1353,9 +1445,10 @@ def list_all_versions(pkg,
         The package to check
 
     bin_env
-        Path to pip bin or path to virtualenv. If doing a system install,
-        and want to use a specific pip bin (pip-2.7, pip-2.6, etc..) just
-        specify the pip bin you want.
+        Path to pip (or to a virtualenv). This can be used to specify the path
+        to the pip to use when more than one Python release is installed (e.g.
+        ``/usr/bin/pip-2.7`` or ``/usr/bin/pip-2.6``. If a directory path is
+        specified, it is assumed to be a virtualenv.
 
     include_alpha
         Include alpha versions in the list
@@ -1370,7 +1463,7 @@ def list_all_versions(pkg,
         The user under which to run pip
 
     cwd
-        Current working directory to run pip from
+        Directory from which to run pip
 
     CLI Example:
 
@@ -1378,9 +1471,8 @@ def list_all_versions(pkg,
 
        salt '*' pip.list_all_versions <package name>
     '''
-    pip_bin = _get_pip_bin(bin_env)
-
-    cmd = [pip_bin, 'install', '{0}==versions'.format(pkg)]
+    cmd = _get_pip_bin(bin_env)
+    cmd.extend(['install', '{0}==versions'.format(pkg)])
 
     cmd_kwargs = dict(cwd=cwd, runas=user, output_loglevel='quiet', redirect_stderr=True)
     if bin_env and os.path.isdir(bin_env):

salt/modules/status.py

@@ -989,7 +989,7 @@ def diskusage(*args):
         elif __grains__['kernel'] in ('FreeBSD', 'SunOS'):
             ifile = __salt__['cmd.run']('mount -p').splitlines()
         else:
-            ifile = []
+            raise CommandExecutionError('status.diskusage not yet supported on this platform')
 
         for line in ifile:
             comps = line.split()

salt/modules/virtualenv_mod.py

@@ -57,7 +57,8 @@ def create(path,
            upgrade=None,
            user=None,
            use_vt=False,
-           saltenv='base'):
+           saltenv='base',
+           **kwargs):
     '''
     Create a virtualenv
 
@@ -103,6 +104,11 @@ def create(path,
     user : None
         Set ownership for the virtualenv
 
+        .. note::
+            On Windows you must also pass a ``password`` parameter. Additionally,
+            the user must have permissions to the location where the virtual
+            environment is being created
+
     runas : None
         Set ownership for the virtualenv
 
@@ -162,7 +168,7 @@ def create(path,
             # Unable to import?? Let's parse the version from the console
             version_cmd = [venv_bin, '--version']
             ret = __salt__['cmd.run_all'](
-                    version_cmd, runas=user, python_shell=False
+                    version_cmd, runas=user, python_shell=False, **kwargs
                 )
             if ret['retcode'] > 0 or not ret['stdout'].strip():
                 raise CommandExecutionError(
@@ -252,7 +258,7 @@ def create(path,
     cmd.append(path)
 
     # Let's create the virtualenv
-    ret = __salt__['cmd.run_all'](cmd, runas=user, python_shell=False)
+    ret = __salt__['cmd.run_all'](cmd, runas=user, python_shell=False, **kwargs)
     if ret['retcode'] != 0:
         # Something went wrong. Let's bail out now!
         return ret

salt/modules/win_lgpo.py

@@ -3691,7 +3691,7 @@ def _checkAllAdmxPolicies(policy_class,
             if ENABLED_VALUE_XPATH(admx_policy) and this_policy_setting == 'Not Configured':
                 # some policies have a disabled list but not an enabled list
                 # added this to address those issues
-                if DISABLED_LIST_XPATH(admx_policy):
+                if DISABLED_LIST_XPATH(admx_policy) or DISABLED_VALUE_XPATH(admx_policy):
                     element_only_enabled_disabled = False
                     explicit_enable_disable_value_setting = True
                 if _checkValueItemParent(admx_policy,
@@ -3701,14 +3701,14 @@ def _checkAllAdmxPolicies(policy_class,
                                          ENABLED_VALUE_XPATH,
                                          policy_filedata):
                     this_policy_setting = 'Enabled'
-                    log.debug('%s is enabled', this_policyname)
+                    log.debug('%s is enabled by detected ENABLED_VALUE_XPATH', this_policyname)
                     if this_policynamespace not in policy_vals:
                         policy_vals[this_policynamespace] = {}
                     policy_vals[this_policynamespace][this_policyname] = this_policy_setting
             if DISABLED_VALUE_XPATH(admx_policy) and this_policy_setting == 'Not Configured':
                 # some policies have a disabled list but not an enabled list
                 # added this to address those issues
-                if ENABLED_LIST_XPATH(admx_policy):
+                if ENABLED_LIST_XPATH(admx_policy) or ENABLED_VALUE_XPATH(admx_policy):
                     element_only_enabled_disabled = False
                     explicit_enable_disable_value_setting = True
                 if _checkValueItemParent(admx_policy,
@@ -3718,25 +3718,27 @@ def _checkAllAdmxPolicies(policy_class,
                                          DISABLED_VALUE_XPATH,
                                          policy_filedata):
                     this_policy_setting = 'Disabled'
-                    log.debug('%s is disabled', this_policyname)
+                    log.debug('%s is disabled by detected DISABLED_VALUE_XPATH', this_policyname)
                     if this_policynamespace not in policy_vals:
                         policy_vals[this_policynamespace] = {}
                     policy_vals[this_policynamespace][this_policyname] = this_policy_setting
             if ENABLED_LIST_XPATH(admx_policy) and this_policy_setting == 'Not Configured':
-                element_only_enabled_disabled = False
-                explicit_enable_disable_value_setting = True
+                if DISABLED_LIST_XPATH(admx_policy) or DISABLED_VALUE_XPATH(admx_policy):
+                    element_only_enabled_disabled = False
+                    explicit_enable_disable_value_setting = True
                 if _checkListItem(admx_policy, this_policyname, this_key, ENABLED_LIST_XPATH, policy_filedata):
                     this_policy_setting = 'Enabled'
-                    log.debug('%s is enabled', this_policyname)
+                    log.debug('%s is enabled by detected ENABLED_LIST_XPATH', this_policyname)
                     if this_policynamespace not in policy_vals:
                         policy_vals[this_policynamespace] = {}
                     policy_vals[this_policynamespace][this_policyname] = this_policy_setting
             if DISABLED_LIST_XPATH(admx_policy) and this_policy_setting == 'Not Configured':
-                element_only_enabled_disabled = False
-                explicit_enable_disable_value_setting = True
+                if ENABLED_LIST_XPATH(admx_policy) or ENABLED_VALUE_XPATH(admx_policy):
+                    element_only_enabled_disabled = False
+                    explicit_enable_disable_value_setting = True
                 if _checkListItem(admx_policy, this_policyname, this_key, DISABLED_LIST_XPATH, policy_filedata):
                     this_policy_setting = 'Disabled'
-                    log.debug('%s is disabled', this_policyname)
+                    log.debug('%s is disabled by detected DISABLED_LIST_XPATH', this_policyname)
                     if this_policynamespace not in policy_vals:
                         policy_vals[this_policynamespace] = {}
                     policy_vals[this_policynamespace][this_policyname] = this_policy_setting
@@ -3751,7 +3753,7 @@ def _checkAllAdmxPolicies(policy_class,
                                                                                 '1')),
                                           policy_filedata):
                     this_policy_setting = 'Enabled'
-                    log.debug('%s is enabled', this_policyname)
+                    log.debug('%s is enabled by no explicit enable/disable list or value', this_policyname)
                     if this_policynamespace not in policy_vals:
                         policy_vals[this_policynamespace] = {}
                     policy_vals[this_policynamespace][this_policyname] = this_policy_setting
@@ -3762,7 +3764,7 @@ def _checkAllAdmxPolicies(policy_class,
                                                                                   check_deleted=True)),
                                             policy_filedata):
                     this_policy_setting = 'Disabled'
-                    log.debug('%s is disabled', this_policyname)
+                    log.debug('%s is disabled by no explicit enable/disable list or value', this_policyname)
                     if this_policynamespace not in policy_vals:
                         policy_vals[this_policynamespace] = {}
                     policy_vals[this_policynamespace][this_policyname] = this_policy_setting

salt/modules/win_pkg.py

@@ -331,6 +331,7 @@ def version(*names, **kwargs):
         dict: The package name(s) with the installed versions.
 
     .. code-block:: cfg
+
         {['<version>', '<version>', ]} OR
         {'<package name>': ['<version>', '<version>', ]}
 

salt/modules/zpool.py

@@ -470,14 +470,13 @@ def get(zpool, prop=None, show_source=False, parsable=True):
 
     '''
     ret = OrderedDict()
-    value_properties = ['property', 'value', 'source']
+    value_properties = ['name', 'property', 'value', 'source']
 
     ## collect get output
     res = __salt__['cmd.run_all'](
         __utils__['zfs.zpool_command'](
             command='get',
             flags=['-H'],
-            opts={'-o': ','.join(value_properties)},
             property_name=prop if prop else 'all',
             target=zpool,
         ),
@@ -503,6 +502,9 @@ def get(zpool, prop=None, show_source=False, parsable=True):
             [x for x in line.strip().split('\t') if x not in ['']],
         )))
 
+        # NOTE: older zfs does not have -o, fall back to manually stipping the name field
+        del prop_data['name']
+
         # NOTE: normalize values
         if parsable:
             # NOTE: raw numbers and pythonic types
@@ -634,6 +636,8 @@ def scrub(zpool, stop=False, pause=False):
 
         .. note::
 
+            Pause is only available on recent versions of ZFS.
+
             If both ``pause`` and ``stop`` are ``True``, then ``stop`` will
             win.
 
@@ -706,6 +710,9 @@ def create(zpool, *vdevs, **kwargs):
 
         .. versionadded:: 2018.3.0
 
+        .. warning:
+          This is only available on illumos and Solaris
+
     CLI Examples:
 
     .. code-block:: bash

salt/serializers/yaml.py

@@ -11,6 +11,7 @@
 
 from __future__ import absolute_import, print_function, unicode_literals
 import datetime
+import logging
 
 import yaml
 from yaml.constructor import ConstructorError
@@ -22,6 +23,8 @@ from salt.utils.odict import OrderedDict
 
 __all__ = ['deserialize', 'serialize', 'available']
 
+log = logging.getLogger(__name__)
+
 available = True
 
 # prefer C bindings over python when available
@@ -46,14 +49,17 @@ def deserialize(stream_or_string, **options):
     try:
         return yaml.load(stream_or_string, **options)
     except ScannerError as error:
+        log.exception('Error encountered while deserializing')
         err_type = ERROR_MAP.get(error.problem, 'Unknown yaml render error')
         line_num = error.problem_mark.line + 1
         raise DeserializationError(err_type,
                                    line_num,
                                    error.problem_mark.buffer)
     except ConstructorError as error:
+        log.exception('Error encountered while deserializing')
         raise DeserializationError(error)
     except Exception as error:
+        log.exception('Error encountered while deserializing')
         raise DeserializationError(error)
 
 
@@ -74,6 +80,7 @@ def serialize(obj, **options):
             return response[:-1]
         return response
     except Exception as error:
+        log.exception('Error encountered while serializing')
         raise SerializationError(error)
 
 
@@ -108,7 +115,6 @@ Loader.add_multi_constructor('tag:yaml.org,2002:set', Loader.construct_yaml_set)
 Loader.add_multi_constructor('tag:yaml.org,2002:str', Loader.construct_yaml_str)
 Loader.add_multi_constructor('tag:yaml.org,2002:seq', Loader.construct_yaml_seq)
 Loader.add_multi_constructor('tag:yaml.org,2002:map', Loader.construct_yaml_map)
-Loader.add_multi_constructor(None, Loader.construct_undefined)
 
 
 class Dumper(BaseDumper):  # pylint: disable=W0232

salt/serializers/yamlex.py

@@ -150,14 +150,17 @@ def deserialize(stream_or_string, **options):
     try:
         return yaml.load(stream_or_string, **options)
     except ScannerError as error:
+        log.exception('Error encountered while deserializing')
         err_type = ERROR_MAP.get(error.problem, 'Unknown yaml render error')
         line_num = error.problem_mark.line + 1
         raise DeserializationError(err_type,
                                    line_num,
                                    error.problem_mark.buffer)
     except ConstructorError as error:
+        log.exception('Error encountered while deserializing')
         raise DeserializationError(error)
     except Exception as error:
+        log.exception('Error encountered while deserializing')
         raise DeserializationError(error)
 
 
@@ -178,6 +181,7 @@ def serialize(obj, **options):
             return response[:-1]
         return response
     except Exception as error:
+        log.exception('Error encountered while serializing')
         raise SerializationError(error)
 
 
@@ -322,7 +326,6 @@ Loader.add_multi_constructor('tag:yaml.org,2002:pairs', Loader.construct_yaml_pa
 Loader.add_multi_constructor('tag:yaml.org,2002:set', Loader.construct_yaml_set)
 Loader.add_multi_constructor('tag:yaml.org,2002:seq', Loader.construct_yaml_seq)
 Loader.add_multi_constructor('tag:yaml.org,2002:map', Loader.construct_yaml_map)
-Loader.add_multi_constructor(None, Loader.construct_undefined)
 
 
 class SLSMap(OrderedDict):

salt/state.py

@@ -3430,43 +3430,45 @@ class BaseHighState(object):
                     'Specified SLS {0} on local filesystem cannot '
                     'be found.'.format(sls)
                 )
+        state = None
         if not fn_:
             errors.append(
                 'Specified SLS {0} in saltenv {1} is not '
                 'available on the salt master or through a configured '
                 'fileserver'.format(sls, saltenv)
             )
-        state = None
-        try:
-            state = compile_template(fn_,
-                                     self.state.rend,
-                                     self.state.opts['renderer'],
-                                     self.state.opts['renderer_blacklist'],
-                                     self.state.opts['renderer_whitelist'],
-                                     saltenv,
-                                     sls,
-                                     rendered_sls=mods
-                                     )
-        except SaltRenderError as exc:
-            msg = 'Rendering SLS \'{0}:{1}\' failed: {2}'.format(
-                saltenv, sls, exc
-            )
-            log.critical(msg)
-            errors.append(msg)
-        except Exception as exc:
-            msg = 'Rendering SLS {0} failed, render error: {1}'.format(
-                sls, exc
-            )
-            log.critical(
-                msg,
-                # Show the traceback if the debug logging level is enabled
-                exc_info_on_loglevel=logging.DEBUG
-            )
-            errors.append('{0}\n{1}'.format(msg, traceback.format_exc()))
-        try:
-            mods.add('{0}:{1}'.format(saltenv, sls))
-        except AttributeError:
-            pass
+        else:
+            try:
+                state = compile_template(fn_,
+                                         self.state.rend,
+                                         self.state.opts['renderer'],
+                                         self.state.opts['renderer_blacklist'],
+                                         self.state.opts['renderer_whitelist'],
+                                         saltenv,
+                                         sls,
+                                         rendered_sls=mods
+                                         )
+            except SaltRenderError as exc:
+                msg = 'Rendering SLS \'{0}:{1}\' failed: {2}'.format(
+                    saltenv, sls, exc
+                )
+                log.critical(msg)
+                errors.append(msg)
+            except Exception as exc:
+                msg = 'Rendering SLS {0} failed, render error: {1}'.format(
+                    sls, exc
+                )
+                log.critical(
+                    msg,
+                    # Show the traceback if the debug logging level is enabled
+                    exc_info_on_loglevel=logging.DEBUG
+                )
+                errors.append('{0}\n{1}'.format(msg, traceback.format_exc()))
+            try:
+                mods.add('{0}:{1}'.format(saltenv, sls))
+            except AttributeError:
+                pass
+
         if state:
             if not isinstance(state, dict):
                 errors.append(

salt/states/firewalld.py

@@ -235,7 +235,7 @@ def present(name,
         salt.utils.versions.warn_until(
             'Neon',
             'The \'prune_services\' argument default is currently True, '
-            'but will be changed to True in future releases.')
+            'but will be changed to False in the Neon release.')
 
     ret = _present(name, block_icmp, prune_block_icmp, default, masquerade, ports, prune_ports,
             port_fwd, prune_port_fwd, services, prune_services, interfaces, prune_interfaces,

salt/states/influxdb_user.py

@@ -48,6 +48,7 @@ def present(name,
     **Example:**
 
     .. code-block:: yaml
+
         example user present in influxdb:
           influxdb_user.present:
             - name: example

salt/states/pip_state.py

@@ -182,24 +182,19 @@ def _check_pkg_version_format(pkg):
     return ret
 
 
-def _check_if_installed(prefix, state_pkg_name, version_spec,
-                        ignore_installed, force_reinstall,
-                        upgrade, user, cwd, bin_env, env_vars):
+def _check_if_installed(prefix, state_pkg_name, version_spec, ignore_installed,
+                        force_reinstall, upgrade, user, cwd, bin_env, env_vars,
+                        **kwargs):
     # result: None means the command failed to run
     # result: True means the package is installed
     # result: False means the package is not installed
     ret = {'result': False, 'comment': None}
 
     # Check if the requested package is already installed.
-    try:
-        pip_list = __salt__['pip.list'](prefix, bin_env=bin_env,
-                                        user=user, cwd=cwd,
-                                        env_vars=env_vars)
-        prefix_realname = _find_key(prefix, pip_list)
-    except (CommandNotFoundError, CommandExecutionError) as err:
-        ret['result'] = None
-        ret['comment'] = 'Error installing \'{0}\': {1}'.format(state_pkg_name, err)
-        return ret
+    pip_list = __salt__['pip.list'](prefix, bin_env=bin_env,
+                                    user=user, cwd=cwd,
+                                    env_vars=env_vars, **kwargs)
+    prefix_realname = _find_key(prefix, pip_list)
 
     # If the package was already installed, check
     # the ignore_installed and force_reinstall flags
@@ -312,7 +307,6 @@ def installed(name,
               install_options=None,
               global_options=None,
               user=None,
-              no_chown=False,
               cwd=None,
               pre_releases=False,
               cert=None,
@@ -325,7 +319,8 @@ def installed(name,
               trusted_host=None,
               no_cache_dir=False,
               cache_dir=None,
-              no_binary=None):
+              no_binary=None,
+              **kwargs):
     '''
     Make sure the package is installed
 
@@ -447,10 +442,6 @@ def installed(name,
     no_install
         Download and unpack all packages, but don't actually install them
 
-    no_chown
-        When user is given, do not attempt to copy and chown
-        a requirements file
-
     no_cache_dir:
         Disable the cache.
 
@@ -593,6 +584,12 @@ def installed(name,
 
     .. _`virtualenv`: http://www.virtualenv.org/en/latest/
     '''
+    if 'no_chown' in kwargs:
+        salt.utils.versions.warn_until(
+            'Flourine',
+            'The no_chown argument has been deprecated and is no longer used. '
+            'Its functionality was removed in Boron.')
+        kwargs.pop('no_chown')
 
     if pip_bin and not bin_env:
         bin_env = pip_bin
@@ -618,11 +615,16 @@ def installed(name,
     ret = {'name': ';'.join(pkgs), 'result': None,
            'comment': '', 'changes': {}}
 
+    try:
+        cur_version = __salt__['pip.version'](bin_env)
+    except (CommandNotFoundError, CommandExecutionError) as err:
+        ret['result'] = None
+        ret['comment'] = 'Error installing \'{0}\': {1}'.format(name, err)
+        return ret
     # Check that the pip binary supports the 'use_wheel' option
     if use_wheel:
         min_version = '1.4'
         max_version = '9.0.3'
-        cur_version = __salt__['pip.version'](bin_env)
         too_low = salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version)
         too_high = salt.utils.versions.compare(ver1=cur_version, oper='>', ver2=max_version)
         if too_low or too_high:
@@ -636,7 +638,6 @@ def installed(name,
     if no_use_wheel:
         min_version = '1.4'
         max_version = '9.0.3'
-        cur_version = __salt__['pip.version'](bin_env)
         too_low = salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version)
         too_high = salt.utils.versions.compare(ver1=cur_version, oper='>', ver2=max_version)
         if too_low or too_high:
@@ -649,7 +650,6 @@ def installed(name,
     # Check that the pip binary supports the 'no_binary' option
     if no_binary:
         min_version = '7.0.0'
-        cur_version = __salt__['pip.version'](bin_env)
         too_low = salt.utils.versions.compare(ver1=cur_version, oper='<', ver2=min_version)
         if too_low:
             ret['result'] = False
@@ -722,7 +722,8 @@ def installed(name,
                 version_spec = version_spec
                 out = _check_if_installed(prefix, state_pkg_name, version_spec,
                                           ignore_installed, force_reinstall,
-                                          upgrade, user, cwd, bin_env, env_vars)
+                                          upgrade, user, cwd, bin_env, env_vars,
+                                          **kwargs)
                 # If _check_if_installed result is None, something went wrong with
                 # the command running. This way we keep stateful output.
                 if out['result'] is None:
@@ -802,7 +803,6 @@ def installed(name,
         install_options=install_options,
         global_options=global_options,
         user=user,
-        no_chown=no_chown,
         cwd=cwd,
         pre_releases=pre_releases,
         cert=cert,
@@ -814,7 +814,8 @@ def installed(name,
         env_vars=env_vars,
         use_vt=use_vt,
         trusted_host=trusted_host,
-        no_cache_dir=no_cache_dir
+        no_cache_dir=no_cache_dir,
+        **kwargs
     )
 
     if pip_install_call and pip_install_call.get('retcode', 1) == 0:
@@ -870,7 +871,8 @@ def installed(name,
                 if prefix:
                     pipsearch = __salt__['pip.list'](prefix, bin_env,
                                                      user=user, cwd=cwd,
-                                                     env_vars=env_vars)
+                                                     env_vars=env_vars,
+                                                     **kwargs)
 
                     # If we didn't find the package in the system after
                     # installing it report it

salt/states/virtualenv_mod.py

@@ -42,7 +42,6 @@ def managed(name,
             never_download=None,
             prompt=None,
             user=None,
-            no_chown=False,
             cwd=None,
             index_url=None,
             extra_index_url=None,
@@ -61,7 +60,8 @@ def managed(name,
             pip_no_cache_dir=False,
             pip_cache_dir=None,
             process_dependency_links=False,
-            no_binary=None):
+            no_binary=None,
+            **kwargs):
     '''
     Create a virtualenv and optionally manage it with pip
 
@@ -85,11 +85,6 @@ def managed(name,
     user: None
         The user under which to run virtualenv and pip.
 
-    no_chown: False
-        When user is given, do not attempt to copy and chown a requirements file
-        (needed if the requirements file refers to other files via relative
-        paths, as the copy-and-chown procedure does not account for such files)
-
     cwd: None
         Path to the working directory where `pip install` is executed.
 
@@ -140,6 +135,13 @@ def managed(name,
             - env_vars:
                 PATH_VAR: '/usr/local/bin/'
     '''
+    if 'no_chown' in kwargs:
+        salt.utils.versions.warn_until(
+            'Flourine',
+            'The no_chown argument has been deprecated and is no longer used. '
+            'Its functionality was removed in Boron.')
+        kwargs.pop('no_chown')
+
     ret = {'name': name, 'result': True, 'comment': '', 'changes': {}}
 
     if 'virtualenv.create' not in __salt__:
@@ -211,6 +213,7 @@ def managed(name,
                 prompt=prompt,
                 user=user,
                 use_vt=use_vt,
+                **kwargs
             )
         except CommandNotFoundError as err:
             ret['result'] = False
@@ -311,7 +314,6 @@ def managed(name,
             extra_index_url=extra_index_url,
             download=pip_download,
             download_cache=pip_download_cache,
-            no_chown=no_chown,
             pre_releases=pre_releases,
             exists_action=pip_exists_action,
             ignore_installed=pip_ignore_installed,
@@ -321,7 +323,8 @@ def managed(name,
             use_vt=use_vt,
             env_vars=env_vars,
             no_cache_dir=pip_no_cache_dir,
-            cache_dir=pip_cache_dir
+            cache_dir=pip_cache_dir,
+            **kwargs
         )
         ret['result'] &= pip_ret['retcode'] == 0
         if pip_ret['retcode'] > 0:

salt/utils/stringutils.py

@@ -361,23 +361,46 @@ def check_whitelist_blacklist(value, whitelist=None, blacklist=None):
     in the blacklist, then the whitelist is checked. If the value isn't
     found in the whitelist, the function returns ``False``.
     '''
-    if blacklist is not None:
-        if not isinstance(blacklist, list):
+    # Normalize the input so that we have a list
+    if blacklist:
+        if isinstance(blacklist, six.string_types):
             blacklist = [blacklist]
-        for expr in blacklist:
-            if expr_match(value, expr):
-                return False
+        if not hasattr(blacklist, '__iter__'):
+            raise TypeError(
+                'Expecting iterable blacklist, but got {0} ({1})'.format(
+                    type(blacklist).__name__, blacklist
+                )
+            )
+    else:
+        blacklist = []
 
     if whitelist:
-        if not isinstance(whitelist, list):
+        if isinstance(whitelist, six.string_types):
             whitelist = [whitelist]
-        for expr in whitelist:
-            if expr_match(value, expr):
-                return True
+        if not hasattr(whitelist, '__iter__'):
+            raise TypeError(
+                'Expecting iterable whitelist, but got {0} ({1})'.format(
+                    type(whitelist).__name__, whitelist
+                )
+            )
     else:
-        return True
+        whitelist = []
 
-    return False
+    _blacklist_match = any(expr_match(value, expr) for expr in blacklist)
+    _whitelist_match = any(expr_match(value, expr) for expr in whitelist)
+
+    if blacklist and not whitelist:
+        # Blacklist but no whitelist
+        return not _blacklist_match
+    elif whitelist and not blacklist:
+        # Whitelist but no blacklist
+        return _whitelist_match
+    elif blacklist and whitelist:
+        # Both whitelist and blacklist
+        return not _blacklist_match and _whitelist_match
+    else:
+        # No blacklist or whitelist passed
+        return True
 
 
 def check_include_exclude(path_str, include_pat=None, exclude_pat=None):

salt/utils/thin.py

@@ -464,6 +464,7 @@ def gen_min(cachedir, extra_mods='', overwrite=False, so_mods='',
         'salt/__init__.py',
         'salt/utils',
         'salt/utils/__init__.py',
+        'salt/utils/atomicfile.py',
         'salt/utils/validate',
         'salt/utils/validate/__init__.py',
         'salt/utils/validate/path.py',

salt/utils/zfs.py

@@ -279,7 +279,6 @@ def _command(source, command, flags=None, opts=None,
     return ' '.join(cmd)
 
 
-@real_memoize
 def is_supported():
     '''
     Check the system for ZFS support
@@ -302,7 +301,7 @@ def is_supported():
         on_supported_platform = True
 
     # Additional check for the zpool command
-    return (_zpool_cmd() and on_supported_platform) is True
+    return (salt.utils.path.which('zpool') and on_supported_platform) is True
 
 
 @real_memoize

tests/integration/files/file/base/issue-1959-virtualenv-runas/init.sls

@@ -6,3 +6,5 @@
     {#- wheels are disabled because the pip cache dir will not be owned by the above issue-1959 user. Need to check this ASAP #}
     - no_binary: ':all:'
     {%- endif %}
+    - env:
+        XDG_CACHE_HOME: /tmp

tests/integration/modules/test_pip.py

@@ -9,9 +9,9 @@
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
 import os
-import pwd
-import shutil
 import re
+import shutil
+import sys
 import tempfile
 
 # Import Salt Testing libs
@@ -23,6 +23,7 @@ from tests.support.helpers import skip_if_not_root
 # Import salt libs
 import salt.utils.files
 import salt.utils.path
+import salt.utils.platform
 from salt.modules.virtualenv_mod import KNOWN_BINARY_NAMES
 
 
@@ -74,23 +75,34 @@ class PipModuleTest(ModuleCase):
 
         # Let's remove the pip binary
         pip_bin = os.path.join(self.venv_dir, 'bin', 'pip')
+        py_dir = 'python{0}.{1}'.format(*sys.version_info[:2])
+        site_dir = os.path.join(self.venv_dir, 'lib', py_dir, 'site-packages')
+        if salt.utils.platform.is_windows():
+            pip_bin = os.path.join(self.venv_dir, 'Scripts', 'pip.exe')
+            site_dir = os.path.join(self.venv_dir, 'lib', 'site-packages')
         if not os.path.isfile(pip_bin):
             self.skipTest(
                 'Failed to find the pip binary to the test virtualenv'
             )
         os.remove(pip_bin)
 
+        # Also remove the pip dir from site-packages
+        # This is needed now that we're using python -m pip instead of the
+        # pip binary directly. python -m pip will still work even if the
+        # pip binary is missing
+        shutil.rmtree(os.path.join(site_dir, 'pip'))
+
         # Let's run a pip depending functions
         for func in ('pip.freeze', 'pip.list'):
             ret = self.run_function(func, bin_env=self.venv_dir)
             self.assertIn(
                 'Command required for \'{0}\' not found: '
-                'Could not find a `pip` binary in virtualenv'.format(func),
+                'Could not find a `pip` binary'.format(func),
                 ret
             )
 
     @skip_if_not_root
-    def test_requirements_as_list_of_chains__sans_no_chown__cwd_set__absolute_file_path(self):
+    def test_requirements_as_list_of_chains__cwd_set__absolute_file_path(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         # Create a requirements file that depends on another one.
@@ -109,11 +121,10 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2b_filename, 'w') as f:
             f.write('pep8\n')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         requirements_list = [req1_filename, req2_filename]
 
         ret = self.run_function(
-            'pip.install', requirements=requirements_list, user=this_user,
+            'pip.install', requirements=requirements_list,
             bin_env=self.venv_dir, cwd=self.venv_dir
         )
         try:
@@ -128,7 +139,7 @@ class PipModuleTest(ModuleCase):
             raise
 
     @skip_if_not_root
-    def test_requirements_as_list_of_chains__sans_no_chown__cwd_not_set__absolute_file_path(self):
+    def test_requirements_as_list_of_chains__cwd_not_set__absolute_file_path(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         # Create a requirements file that depends on another one.
@@ -147,12 +158,10 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2b_filename, 'w') as f:
             f.write('pep8\n')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         requirements_list = [req1_filename, req2_filename]
 
         ret = self.run_function(
-            'pip.install', requirements=requirements_list, user=this_user,
-            bin_env=self.venv_dir
+            'pip.install', requirements=requirements_list, bin_env=self.venv_dir
         )
         try:
             self.assertEqual(ret['retcode'], 0)
@@ -167,7 +176,7 @@ class PipModuleTest(ModuleCase):
             raise
 
     @skip_if_not_root
-    def test_requirements_as_list__sans_no_chown__absolute_file_path(self):
+    def test_requirements_as_list__absolute_file_path(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         req1_filename = os.path.join(self.venv_dir, 'requirements.txt')
@@ -178,12 +187,10 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2_filename, 'w') as f:
             f.write('pep8\n')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         requirements_list = [req1_filename, req2_filename]
 
         ret = self.run_function(
-            'pip.install', requirements=requirements_list, user=this_user,
-            bin_env=self.venv_dir
+            'pip.install', requirements=requirements_list, bin_env=self.venv_dir
         )
 
         found = self.pip_successful_install(ret['stdout'])
@@ -198,7 +205,7 @@ class PipModuleTest(ModuleCase):
             raise
 
     @skip_if_not_root
-    def test_requirements_as_list__sans_no_chown__non_absolute_file_path(self):
+    def test_requirements_as_list__non_absolute_file_path(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         # Create a requirements file that depends on another one.
@@ -215,11 +222,10 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2_filepath, 'w') as f:
             f.write('pep8\n')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         requirements_list = [req1_filename, req2_filename]
 
         ret = self.run_function(
-            'pip.install', requirements=requirements_list, user=this_user,
+            'pip.install', requirements=requirements_list,
             bin_env=self.venv_dir, cwd=req_cwd
         )
         try:
@@ -234,7 +240,7 @@ class PipModuleTest(ModuleCase):
             raise
 
     @skip_if_not_root
-    def test_chained_requirements__sans_no_chown__absolute_file_path(self):
+    def test_chained_requirements__absolute_file_path(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         # Create a requirements file that depends on another one.
@@ -247,10 +253,8 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2_filename, 'w') as f:
             f.write('pep8')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         ret = self.run_function(
-            'pip.install', requirements=req1_filename, user=this_user,
-            bin_env=self.venv_dir
+            'pip.install', requirements=req1_filename, bin_env=self.venv_dir
         )
         try:
             self.assertEqual(ret['retcode'], 0)
@@ -261,7 +265,7 @@ class PipModuleTest(ModuleCase):
             raise
 
     @skip_if_not_root
-    def test_chained_requirements__sans_no_chown__non_absolute_file_path(self):
+    def test_chained_requirements__non_absolute_file_path(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         # Create a requirements file that depends on another one.
@@ -278,10 +282,9 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2_file, 'w') as f:
             f.write('pep8')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         ret = self.run_function(
-            'pip.install', requirements=req1_filename, user=this_user,
-            no_chown=False, cwd=req_basepath, bin_env=self.venv_dir
+            'pip.install', requirements=req1_filename, cwd=req_basepath,
+            bin_env=self.venv_dir
         )
         try:
             self.assertEqual(ret['retcode'], 0)
@@ -292,7 +295,7 @@ class PipModuleTest(ModuleCase):
             raise
 
     @skip_if_not_root
-    def test_issue_4805_nested_requirements_user_no_chown(self):
+    def test_issue_4805_nested_requirements(self):
         self.run_function('virtualenv.create', [self.venv_dir])
 
         # Create a requirements file that depends on another one.
@@ -303,11 +306,8 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req2_filename, 'w') as f:
             f.write('pep8')
 
-        this_user = pwd.getpwuid(os.getuid())[0]
         ret = self.run_function(
-            'pip.install', requirements=req1_filename, user=this_user,
-            no_chown=True, bin_env=self.venv_dir
-        )
+            'pip.install', requirements=req1_filename, bin_env=self.venv_dir)
         if self._check_download_error(ret['stdout']):
             self.skipTest('Test skipped due to pip download error')
         try:
@@ -436,9 +436,9 @@ class PipModuleTest(ModuleCase):
     def tearDown(self):
         super(PipModuleTest, self).tearDown()
         if os.path.isdir(self.venv_test_dir):
-            shutil.rmtree(self.venv_test_dir)
+            shutil.rmtree(self.venv_test_dir, ignore_errors=True)
         if os.path.isdir(self.pip_temp):
-            shutil.rmtree(self.pip_temp)
+            shutil.rmtree(self.pip_temp, ignore_errors=True)
         del self.venv_dir
         del self.venv_test_dir
         del self.pip_temp

tests/integration/modules/test_status.py

@@ -49,7 +49,9 @@ class StatusModuleTest(ModuleCase):
         status.diskusage
         '''
         ret = self.run_function('status.diskusage')
-        if salt.utils.platform.is_windows():
+        if salt.utils.platform.is_darwin():
+            self.assertIn('not yet supported on this platform', ret)
+        elif salt.utils.platform.is_windows():
             self.assertTrue(isinstance(ret['percent'], float))
         else:
             self.assertIn('total', str(ret))

tests/integration/states/test_pip_state.py

@@ -3,7 +3,7 @@
     :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)`
 
 
-    tests.integration.states.pip
+    tests.integration.states.pip_state
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 '''
 
@@ -11,28 +11,38 @@
 from __future__ import absolute_import, print_function, unicode_literals
 import errno
 import os
-import pwd
 import glob
 import shutil
 import sys
 
+try:
+    import pwd
+    HAS_PWD = True
+except ImportError:
+    HAS_PWD = False
+
 # Import Salt Testing libs
-from tests.support.mixins import SaltReturnAssertsMixin
-from tests.support.unit import skipIf
-from tests.support.runtests import RUNTIME_VARS
+from tests.support.case import ModuleCase
 from tests.support.helpers import (
     destructiveTest,
     requires_system_grains,
     with_system_user,
-    skip_if_not_root
+    skip_if_not_root,
+    with_tempdir
 )
+from tests.support.mixins import SaltReturnAssertsMixin
+from tests.support.runtests import RUNTIME_VARS
+from tests.support.unit import skipIf
+
 # Import salt libs
 import salt.utils.files
 import salt.utils.path
+import salt.utils.platform
 import salt.utils.versions
+import salt.utils.win_dacl
+import salt.utils.win_functions
 from salt.modules.virtualenv_mod import KNOWN_BINARY_NAMES
 from salt.exceptions import CommandExecutionError
-from tests.support.case import ModuleCase
 
 # Import 3rd-party libs
 from salt.ext import six
@@ -49,7 +59,7 @@ class VirtualEnv(object):
 
     def __exit__(self, exc_type, exc_value, traceback):
         if os.path.isdir(self.venv_dir):
-            shutil.rmtree(self.venv_dir)
+            shutil.rmtree(self.venv_dir, ignore_errors=True)
 
 
 @skipIf(salt.utils.path.which_bin(KNOWN_BINARY_NAMES) is None, 'virtualenv not installed')
@@ -115,9 +125,10 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             else:
                 os.environ['SHELL'] = orig_shell
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
 
     @skipIf(six.PY3, 'Issue is specific to carbon module, which is PY2-only')
+    @skipIf(salt.utils.platform.is_windows(), "Carbon does not install in Windows")
     @requires_system_grains
     def test_pip_installed_weird_install(self, grains=None):
         # First, check to see if this is running on CentOS 5 or MacOS.
@@ -143,7 +154,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
                 )
         finally:
             if os.path.isdir(ographite):
-                shutil.rmtree(ographite)
+                shutil.rmtree(ographite, ignore_errors=True)
 
         venv_dir = os.path.join(RUNTIME_VARS.TMP, 'pip-installed-weird-install')
         try:
@@ -152,7 +163,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             # context when running the call to state.sls that comes after.
             self.run_function('saltutil.sync_modules')
             # Since we don't have the virtualenv created, pip.installed will
-            # thrown and error.
+            # throw an error.
             ret = self.run_function(
                 'state.sls', mods='pip-installed-weird-install'
             )
@@ -174,7 +185,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
                 raise Exception('Expected state did not run')
         finally:
             if os.path.isdir(ographite):
-                shutil.rmtree(ographite)
+                shutil.rmtree(ographite, ignore_errors=True)
 
     def test_issue_2028_pip_installed_state(self):
         ret = self.run_function('state.sls', mods='issue-2028-pip-installed')
@@ -183,14 +194,18 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             RUNTIME_VARS.TMP, 'issue-2028-pip-installed'
         )
 
+        pep8_bin = os.path.join(venv_dir, 'bin', 'pep8')
+        if salt.utils.platform.is_windows():
+            pep8_bin = os.path.join(venv_dir, 'Scripts', 'pep8.exe')
+
         try:
             self.assertSaltTrueReturn(ret)
             self.assertTrue(
-                os.path.isfile(os.path.join(venv_dir, 'bin', 'pep8'))
+                os.path.isfile(pep8_bin)
             )
         finally:
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
 
     def test_issue_2087_missing_pip(self):
         venv_dir = os.path.join(
@@ -204,12 +219,23 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
 
             # Let's remove the pip binary
             pip_bin = os.path.join(venv_dir, 'bin', 'pip')
+            py_dir = 'python{0}.{1}'.format(*sys.version_info[:2])
+            site_dir = os.path.join(venv_dir, 'lib', py_dir, 'site-packages')
+            if salt.utils.platform.is_windows():
+                pip_bin = os.path.join(venv_dir, 'Scripts', 'pip.exe')
+                site_dir = os.path.join(venv_dir, 'lib', 'site-packages')
             if not os.path.isfile(pip_bin):
                 self.skipTest(
                     'Failed to find the pip binary to the test virtualenv'
                 )
             os.remove(pip_bin)
 
+            # Also remove the pip dir from site-packages
+            # This is needed now that we're using python -m pip instead of the
+            # pip binary directly. python -m pip will still work even if the
+            # pip binary is missing
+            shutil.rmtree(os.path.join(site_dir, 'pip'))
+
             # Let's run the state which should fail because pip is missing
             ret = self.run_function('state.sls', mods='issue-2087-missing-pip')
             self.assertSaltFalseReturn(ret)
@@ -219,7 +245,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             )
         finally:
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
 
     def test_issue_5940_multiple_pip_mirrors(self):
         '''
@@ -244,148 +270,100 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
                 self.skipTest('the --mirrors arg has been deprecated and removed in pip==7.0.0')
         finally:
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
 
     @destructiveTest
     @skip_if_not_root
-    @with_system_user('issue-6912', on_existing='delete', delete=True)
-    def test_issue_6912_wrong_owner(self, username):
-        venv_dir = os.path.join(
-            RUNTIME_VARS.TMP, '6912-wrong-owner'
-        )
-        # ----- Using runas ------------------------------------------------->
-        venv_create = self.run_function(
-            'virtualenv.create', [venv_dir], user=username
-        )
-        if venv_create['retcode'] > 0:
-            self.skipTest(
-                'Failed to create testcase virtual environment: {0}'.format(
-                    venv_create
-                )
-            )
+    @with_system_user('issue-6912', on_existing='delete', delete=True,
+                      password='PassWord1!')
+    @with_tempdir()
+    def test_issue_6912_wrong_owner(self, temp_dir, username):
+        # Setup virtual environment directory to be used throughout the test
+        venv_dir = os.path.join(temp_dir, '6912-wrong-owner')
 
-        # Using the package name.
-        try:
-            ret = self.run_state(
-                'pip.installed', name='pep8', user=username, bin_env=venv_dir
-            )
-            self.assertSaltTrueReturn(ret)
-            uinfo = pwd.getpwnam(username)
-            for globmatch in (os.path.join(venv_dir, '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '*', '**', 'pep8*')):
-                for path in glob.glob(globmatch):
+        # The virtual environment needs to be in a location that is accessible
+        # by both the user running the test and the runas user
+        if salt.utils.platform.is_windows():
+            salt.utils.win_dacl.set_permissions(temp_dir, username, 'full_control')
+        else:
+            uid = self.run_function('file.user_to_uid', [username])
+            os.chown(temp_dir, uid, -1)
+
+        # Create the virtual environment
+        venv_create = self.run_function(
+            'virtualenv.create', [venv_dir], user=username,
+            password='PassWord1!')
+        if venv_create['retcode'] > 0:
+            self.skipTest('Failed to create testcase virtual environment: {0}'
+                          ''.format(venv_create))
+
+        # pip install passing the package name in `name`
+        ret = self.run_state(
+            'pip.installed', name='pep8', user=username, bin_env=venv_dir,
+            password='PassWord1!')
+        self.assertSaltTrueReturn(ret)
+
+        if HAS_PWD:
+            uid = pwd.getpwnam(username).pw_uid
+        for globmatch in (os.path.join(venv_dir, '**', 'pep8*'),
+                          os.path.join(venv_dir, '*', '**', 'pep8*'),
+                          os.path.join(venv_dir, '*', '*', '**', 'pep8*')):
+            for path in glob.glob(globmatch):
+                if HAS_PWD:
+                    self.assertEqual(uid, os.stat(path).st_uid)
+                elif salt.utils.platform.is_windows():
                     self.assertEqual(
-                        uinfo.pw_uid, os.stat(path).st_uid
-                    )
+                        salt.utils.win_dacl.get_owner(path), username)
 
-        finally:
-            if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+    @destructiveTest
+    @skip_if_not_root
+    @with_system_user('issue-6912', on_existing='delete', delete=True,
+                      password='PassWord1!')
+    @with_tempdir()
+    def test_issue_6912_wrong_owner_requirements_file(self, temp_dir, username):
+        # Setup virtual environment directory to be used throughout the test
+        venv_dir = os.path.join(temp_dir, '6912-wrong-owner')
 
-        # Using a requirements file
+        # The virtual environment needs to be in a location that is accessible
+        # by both the user running the test and the runas user
+        if salt.utils.platform.is_windows():
+            salt.utils.win_dacl.set_permissions(temp_dir, username, 'full_control')
+        else:
+            uid = self.run_function('file.user_to_uid', [username])
+            os.chown(temp_dir, uid, -1)
+
+        # Create the virtual environment again as it should have been removed
         venv_create = self.run_function(
-            'virtualenv.create', [venv_dir], user=username
-        )
+            'virtualenv.create', [venv_dir], user=username,
+            password='PassWord1!')
         if venv_create['retcode'] > 0:
-            self.skipTest(
-                'Failed to create testcase virtual environment: {0}'.format(
-                    ret
-                )
-            )
+            self.skipTest('failed to create testcase virtual environment: {0}'
+                          ''.format(venv_create))
+
+        # pip install using a requirements file
         req_filename = os.path.join(
             RUNTIME_VARS.TMP_STATE_TREE, 'issue-6912-requirements.txt'
         )
         with salt.utils.files.fopen(req_filename, 'wb') as reqf:
             reqf.write(b'pep8\n')
 
-        try:
-            ret = self.run_state(
-                'pip.installed', name='', user=username, bin_env=venv_dir,
-                requirements='salt://issue-6912-requirements.txt'
-            )
-            self.assertSaltTrueReturn(ret)
-            uinfo = pwd.getpwnam(username)
-            for globmatch in (os.path.join(venv_dir, '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '*', '**', 'pep8*')):
-                for path in glob.glob(globmatch):
+        ret = self.run_state(
+            'pip.installed', name='', user=username, bin_env=venv_dir,
+            requirements='salt://issue-6912-requirements.txt',
+            password='PassWord1!')
+        self.assertSaltTrueReturn(ret)
+
+        if HAS_PWD:
+            uid = pwd.getpwnam(username).pw_uid
+        for globmatch in (os.path.join(venv_dir, '**', 'pep8*'),
+                          os.path.join(venv_dir, '*', '**', 'pep8*'),
+                          os.path.join(venv_dir, '*', '*', '**', 'pep8*')):
+            for path in glob.glob(globmatch):
+                if HAS_PWD:
+                    self.assertEqual(uid, os.stat(path).st_uid)
+                elif salt.utils.platform.is_windows():
                     self.assertEqual(
-                        uinfo.pw_uid, os.stat(path).st_uid
-                    )
-
-        finally:
-            if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
-            os.unlink(req_filename)
-        # <---- Using runas --------------------------------------------------
-
-        # ----- Using user -------------------------------------------------->
-        venv_create = self.run_function(
-            'virtualenv.create', [venv_dir], user=username
-        )
-        if venv_create['retcode'] > 0:
-            self.skipTest(
-                'Failed to create testcase virtual environment: {0}'.format(
-                    ret
-                )
-            )
-
-        # Using the package name
-        try:
-            ret = self.run_state(
-                'pip.installed', name='pep8', user=username, bin_env=venv_dir
-            )
-            self.assertSaltTrueReturn(ret)
-            uinfo = pwd.getpwnam(username)
-            for globmatch in (os.path.join(venv_dir, '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '*', '**', 'pep8*')):
-                for path in glob.glob(globmatch):
-                    self.assertEqual(
-                        uinfo.pw_uid, os.stat(path).st_uid
-                    )
-
-        finally:
-            if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
-
-        # Using a requirements file
-        venv_create = self.run_function(
-            'virtualenv.create', [venv_dir], user=username
-        )
-        if venv_create['retcode'] > 0:
-            self.skipTest(
-                'Failed to create testcase virtual environment: {0}'.format(
-                    ret
-                )
-            )
-        req_filename = os.path.join(
-            RUNTIME_VARS.TMP_STATE_TREE, 'issue-6912-requirements.txt'
-        )
-        with salt.utils.files.fopen(req_filename, 'wb') as reqf:
-            reqf.write(b'pep8\n')
-
-        try:
-            ret = self.run_state(
-                'pip.installed', name='', user=username, bin_env=venv_dir,
-                requirements='salt://issue-6912-requirements.txt'
-            )
-            self.assertSaltTrueReturn(ret)
-            uinfo = pwd.getpwnam(username)
-            for globmatch in (os.path.join(venv_dir, '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '**', 'pep8*'),
-                              os.path.join(venv_dir, '*', '*', '**', 'pep8*')):
-                for path in glob.glob(globmatch):
-                    self.assertEqual(
-                        uinfo.pw_uid, os.stat(path).st_uid
-                    )
-
-        finally:
-            if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
-            os.unlink(req_filename)
-        # <---- Using user ---------------------------------------------------
+                        salt.utils.win_dacl.get_owner(path), username)
 
     def test_issue_6833_pip_upgrade_pip(self):
         # Create the testing virtualenv
@@ -393,6 +371,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             RUNTIME_VARS.TMP, '6833-pip-upgrade-pip'
         )
         ret = self.run_function('virtualenv.create', [venv_dir])
+
         try:
             try:
                 self.assertEqual(ret['retcode'], 0)
@@ -435,18 +414,15 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             )
             try:
                 self.assertSaltTrueReturn(ret)
-                self.assertInSaltReturn(
-                    'Installed',
-                    ret,
-                    ['changes', 'pip==8.0.1']
-                )
+                self.assertSaltStateChangesEqual(
+                    ret, {'pip==8.0.1': 'Installed'})
             except AssertionError:
                 import pprint
                 pprint.pprint(ret)
                 raise
         finally:
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
 
     def test_pip_installed_specific_env(self):
         # Create the testing virtualenv
@@ -498,16 +474,14 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             )
         finally:
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
             if os.path.isfile(requirements_file):
                 os.unlink(requirements_file)
 
     def test_22359_pip_installed_unless_does_not_trigger_warnings(self):
         # This test case should be moved to a format_call unit test specific to
         # the state internal keywords
-        venv_dir = venv_dir = os.path.join(
-            RUNTIME_VARS.TMP, 'pip-installed-unless'
-        )
+        venv_dir = os.path.join(RUNTIME_VARS.TMP, 'pip-installed-unless')
         venv_create = self.run_function('virtualenv.create', [venv_dir])
         if venv_create['retcode'] > 0:
             self.skipTest(
@@ -516,17 +490,21 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
                 )
             )
 
+        false_cmd = '/bin/false'
+        if salt.utils.platform.is_windows():
+            false_cmd = 'exit 1 >nul'
         try:
             ret = self.run_state(
-                'pip.installed', name='pep8', bin_env=venv_dir, unless='/bin/false'
+                'pip.installed', name='pep8', bin_env=venv_dir, unless=false_cmd
             )
             self.assertSaltTrueReturn(ret)
             self.assertNotIn('warnings', next(six.itervalues(ret)))
         finally:
             if os.path.isdir(venv_dir):
-                shutil.rmtree(venv_dir)
+                shutil.rmtree(venv_dir, ignore_errors=True)
 
     @skipIf(sys.version_info[:2] >= (3, 6), 'Old version of virtualenv too old for python3.6')
+    @skipIf(salt.utils.platform.is_windows(), "Carbon does not install in Windows")
     def test_46127_pip_env_vars(self):
         '''
         Test that checks if env_vars passed to pip.installed are also passed
@@ -550,7 +528,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
                 )
         finally:
             if os.path.isdir(ographite):
-                shutil.rmtree(ographite)
+                shutil.rmtree(ographite, ignore_errors=True)
 
         venv_dir = os.path.join(RUNTIME_VARS.TMP, 'issue-46127-pip-env-vars')
         try:
@@ -559,7 +537,7 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
             # context when running the call to state.sls that comes after.
             self.run_function('saltutil.sync_modules')
             # Since we don't have the virtualenv created, pip.installed will
-            # thrown and error.
+            # throw an error.
             ret = self.run_function(
                 'state.sls', mods='issue-46127-pip-env-vars'
             )
@@ -598,6 +576,6 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
                 raise Exception('Expected state did not run')
         finally:
             if os.path.isdir(ographite):
-                shutil.rmtree(ographite)
+                shutil.rmtree(ographite, ignore_errors=True)
             if os.path.isdir(venv_dir):
                 shutil.rmtree(venv_dir)

tests/integration/utils/testprogram.py

@@ -20,6 +20,7 @@ import tempfile
 import time
 
 import salt.utils.files
+import salt.utils.platform
 import salt.utils.process
 import salt.utils.psutil_compat as psutils
 import salt.utils.yaml
@@ -28,6 +29,7 @@ from salt.ext import six
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
 
 from tests.support.unit import TestCase
+from tests.support.helpers import win32_kill_process_tree
 from tests.support.paths import CODE_DIR
 from tests.support.processes import terminate_process, terminate_process_list
 
@@ -413,9 +415,6 @@ class TestProgram(six.with_metaclass(TestProgramMeta, object)):
 
             popen_kwargs['preexec_fn'] = detach_from_parent_group
 
-        elif sys.platform.lower().startswith('win') and timeout is not None:
-            raise RuntimeError('Timeout is not supported under windows')
-
         self.argv = [self.program]
         self.argv.extend(args)
         log.debug('TestProgram.run: %s Environment %s', self.argv, env_delta)
@@ -430,16 +429,26 @@ class TestProgram(six.with_metaclass(TestProgramMeta, object)):
 
                 if datetime.now() > stop_at:
                     if term_sent is False:
-                        # Kill the process group since sending the term signal
-                        # would only terminate the shell, not the command
-                        # executed in the shell
-                        os.killpg(os.getpgid(process.pid), signal.SIGINT)
-                        term_sent = True
-                        continue
+                        if salt.utils.platform.is_windows():
+                            _, alive = win32_kill_process_tree(process.pid)
+                            if alive:
+                                log.error("Child processes still alive: %s", alive)
+                        else:
+                            # Kill the process group since sending the term signal
+                            # would only terminate the shell, not the command
+                            # executed in the shell
+                            os.killpg(os.getpgid(process.pid), signal.SIGINT)
+                            term_sent = True
+                            continue
 
                     try:
-                        # As a last resort, kill the process group
-                        os.killpg(os.getpgid(process.pid), signal.SIGKILL)
+                        if salt.utils.platform.is_windows():
+                            _, alive = win32_kill_process_tree(process.pid)
+                            if alive:
+                                log.error("Child processes still alive: %s", alive)
+                        else:
+                            # As a last resort, kill the process group
+                            os.killpg(os.getpgid(process.pid), signal.SIGKILL)
                         process.wait()
                     except OSError as exc:
                         if exc.errno != errno.ESRCH:

tests/runtests.py

@@ -740,6 +740,9 @@ class SaltTestsuiteParser(SaltCoverageTestingParser):
         with TestDaemon(self):
             if self.options.name:
                 for name in self.options.name:
+                    name = name.strip()
+                    if not name:
+                        continue
                     if os.path.isfile(name):
                         if not name.endswith('.py'):
                             continue

tests/support/case.py

@@ -29,13 +29,14 @@ from datetime import datetime, timedelta
 
 # Import salt testing libs
 from tests.support.unit import TestCase
-from tests.support.helpers import RedirectStdStreams, requires_sshd_server
+from tests.support.helpers import (
+    RedirectStdStreams, requires_sshd_server, win32_kill_process_tree
+)
 from tests.support.runtests import RUNTIME_VARS
 from tests.support.mixins import AdaptedConfigurationTestCaseMixin, SaltClientTestCaseMixin
 from tests.support.paths import ScriptPathMixin, INTEGRATION_TEST_DIR, CODE_DIR, PYEXEC, SCRIPT_DIR
 
 # Import 3rd-party libs
-import salt.utils.json
 from salt.ext import six
 from salt.ext.six.moves import cStringIO  # pylint: disable=import-error
 
@@ -287,11 +288,11 @@ class ShellTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
 
             popen_kwargs['preexec_fn'] = detach_from_parent_group
 
-        elif sys.platform.lower().startswith('win') and timeout is not None:
-            raise RuntimeError('Timeout is not supported under windows')
-
         process = subprocess.Popen(cmd, **popen_kwargs)
 
+        # Late import
+        import salt.utils.platform
+
         if timeout is not None:
             stop_at = datetime.now() + timedelta(seconds=timeout)
             term_sent = False
@@ -303,13 +304,23 @@ class ShellTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
                         # Kill the process group since sending the term signal
                         # would only terminate the shell, not the command
                         # executed in the shell
-                        os.killpg(os.getpgid(process.pid), signal.SIGINT)
+                        if salt.utils.platform.is_windows():
+                            _, alive = win32_kill_process_tree(process.pid)
+                            if alive:
+                                log.error("Child processes still alive: %s", alive)
+                        else:
+                            os.killpg(os.getpgid(process.pid), signal.SIGINT)
                         term_sent = True
                         continue
 
                     try:
                         # As a last resort, kill the process group
-                        os.killpg(os.getpgid(process.pid), signal.SIGKILL)
+                        if salt.utils.platform.is_windows():
+                            _, alive = win32_kill_process_tree(process.pid)
+                            if alive:
+                                log.error("Child processes still alive: %s", alive)
+                        else:
+                            os.killpg(os.getpgid(process.pid), signal.SIGINT)
                     except OSError as exc:
                         if exc.errno != errno.ESRCH:
                             # If errno is not "no such process", raise
@@ -855,6 +866,10 @@ class SSHCase(ShellCase):
                            wipe=wipe, raw=raw)
         log.debug('SSHCase run_function executed %s with arg %s', function, arg)
         log.debug('SSHCase JSON return: %s', ret)
+
+        # Late import
+        import salt.utils.json
+
         try:
             return salt.utils.json.loads(ret)['localhost']
         except Exception:

tests/support/helpers.py

@@ -57,6 +57,7 @@ from tests.support.paths import FILES, TMP
 
 # Import Salt libs
 import salt.utils.files
+import salt.utils.platform
 
 log = logging.getLogger(__name__)
 
@@ -613,10 +614,10 @@ def requires_network(only_local_network=False):
     return decorator
 
 
-def with_system_user(username, on_existing='delete', delete=True):
+def with_system_user(username, on_existing='delete', delete=True, password=None):
     '''
     Create and optionally destroy a system user to be used within a test
-    case. The system user is crated using the ``user`` salt module.
+    case. The system user is created using the ``user`` salt module.
 
     The decorated testcase function must accept 'username' as an argument.
 
@@ -646,7 +647,10 @@ def with_system_user(username, on_existing='delete', delete=True):
 
             # Let's add the user to the system.
             log.debug('Creating system user {0!r}'.format(username))
-            create_user = cls.run_function('user.add', [username])
+            kwargs = {'timeout': 60}
+            if salt.utils.platform.is_windows():
+                kwargs.update({'password': password})
+            create_user = cls.run_function('user.add', [username], **kwargs)
             if not create_user:
                 log.debug('Failed to create system user')
                 # The user was not created
@@ -702,7 +706,7 @@ def with_system_user(username, on_existing='delete', delete=True):
             finally:
                 if delete:
                     delete_user = cls.run_function(
-                        'user.delete', [username, True, True]
+                        'user.delete', [username, True, True], timeout=60
                     )
                     if not delete_user:
                         if failure is None:
@@ -1117,7 +1121,6 @@ def requires_salt_modules(*names):
 
 
 def skip_if_binaries_missing(*binaries, **kwargs):
-    import salt.utils
     import salt.utils.path
     if len(binaries) == 1:
         if isinstance(binaries[0], (list, tuple, set, frozenset)):
@@ -1569,3 +1572,23 @@ class Webserver(object):
         '''
         self.ioloop.add_callback(self.ioloop.stop)
         self.server_thread.join()
+
+
+def win32_kill_process_tree(pid, sig=signal.SIGTERM, include_parent=True,
+        timeout=None, on_terminate=None):
+    '''
+    Kill a process tree (including grandchildren) with signal "sig" and return
+    a (gone, still_alive) tuple.  "on_terminate", if specified, is a callabck
+    function which is called as soon as a child terminates.
+    '''
+    if pid == os.getpid():
+        raise RuntimeError("I refuse to kill myself")
+    parent = psutil.Process(pid)
+    children = parent.children(recursive=True)
+    if include_parent:
+        children.append(parent)
+    for p in children:
+        p.send_signal(sig)
+    gone, alive = psutil.wait_procs(children, timeout=timeout,
+                                    callback=on_terminate)
+    return (gone, alive)

tests/unit/cloud/test_libcloudfuncs.py

@@ -1,8 +1,6 @@
 # -*- coding: utf-8 -*-
 '''
-    :codeauthor: `Anthony Shaw <anthonyshaw@apache.org>`
-
-    tests.unit.cloud.clouds.dimensiondata_test
+    tests.unit.cloud.test_libcloudfuncs
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 '''
 

tests/unit/grains/test_core.py

@@ -864,9 +864,9 @@ SwapTotal:       4789244 kB'''
         test virtual grain with cmd virt-what
         '''
         virt = 'kvm'
-        with patch.object(salt.utils, 'is_windows',
+        with patch.object(salt.utils.platform, 'is_windows',
                           MagicMock(return_value=False)):
-            with patch.object(salt.utils, 'which',
+            with patch.object(salt.utils.path, 'which',
                               MagicMock(return_value=True)):
                 with patch.dict(core.__salt__, {'cmd.run_all':
                                                 MagicMock(return_value={'pid': 78,

tests/unit/modules/test_pip.py

@@ -3,6 +3,7 @@
 # Import python libs
 from __future__ import absolute_import
 import os
+import sys
 
 # Import Salt Testing libs
 from tests.support.mixins import LoaderModuleMockMixin
@@ -10,7 +11,6 @@ from tests.support.unit import skipIf, TestCase
 from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch
 
 # Import salt libs
-from salt.ext import six
 import salt.utils.platform
 import salt.modules.pip as pip
 from salt.exceptions import CommandExecutionError
@@ -25,9 +25,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(requirements='requirements.txt')
-            expected_cmd = ['pip', 'install', '--requirement',
-                            'requirements.txt']
-            mock.assert_called_once_with(
+            expected_cmd = [sys.executable, '-m', 'pip', 'install', '--requirement', 'requirements.txt']
+            mock.assert_called_with(
                 expected_cmd,
                 saltenv='base',
                 runas=None,
@@ -50,7 +49,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting'
         ]
 
-        expected = ['pip', 'install']
+        expected = [sys.executable, '-m', 'pip', 'install']
         for item in editables:
             expected.extend(['--editable', item])
 
@@ -58,7 +57,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(editable=editables)
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -70,7 +69,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(editable=','.join(editables))
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -85,7 +84,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting'
         ]
 
-        expected = ['pip', 'install'] + pkgs
+        expected = [sys.executable, '-m', 'pip', 'install']
+        expected.extend(pkgs)
         for item in editables:
             expected.extend(['--editable', item])
 
@@ -93,7 +93,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkgs=pkgs, editable=editables)
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -105,7 +105,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkgs=','.join(pkgs), editable=','.join(editables))
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -117,8 +117,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkgs=pkgs[0], editable=editables[0])
-            mock.assert_called_once_with(
-                ['pip', 'install', pkgs[0], '--editable', editables[0]],
+            expected = [sys.executable, '-m', 'pip', 'install', pkgs[0], '--editable', editables[0]]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -136,7 +137,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                 'http://pypi.crate.io'
             ]
 
-            expected = ['pip', 'install', '--use-mirrors']
+            expected = [sys.executable, '-m', 'pip', 'install', '--use-mirrors']
             for item in mirrors:
                 expected.extend(['--mirrors', item])
             expected.append('pep8')
@@ -145,7 +146,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(pkgs=['pep8'], mirrors=mirrors)
-                mock.assert_called_once_with(
+                mock.assert_called_with(
                     expected,
                     saltenv='base',
                     runas=None,
@@ -157,7 +158,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(pkgs=['pep8'], mirrors=','.join(mirrors))
-                mock.assert_called_once_with(
+                mock.assert_called_with(
                     expected,
                     saltenv='base',
                     runas=None,
@@ -165,12 +166,14 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                     python_shell=False,
                 )
 
+            expected = [sys.executable, '-m', 'pip', 'install', '--use-mirrors', '--mirrors', mirrors[0], 'pep8']
+
             # As single string (just use the first element from mirrors)
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(pkgs=['pep8'], mirrors=mirrors[0])
-                mock.assert_called_once_with(
-                    ['pip', 'install', '--use-mirrors', '--mirrors', mirrors[0], 'pep8'],
+                mock.assert_called_with(
+                    expected,
                     saltenv='base',
                     runas=None,
                     use_vt=False,
@@ -185,7 +188,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         ]
         pkg = 'pep8'
 
-        expected = ['pip', 'install']
+        expected = [sys.executable, '-m', 'pip', 'install']
         for item in find_links:
             expected.extend(['--find-links', item])
         expected.append(pkg)
@@ -194,7 +197,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, find_links=find_links)
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -206,7 +209,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, find_links=','.join(find_links))
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -214,12 +217,26 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                 python_shell=False,
             )
 
+        # Valid protos work?
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            pip.install(pkg, find_links=find_links)
+            mock.assert_called_with(
+                expected,
+                saltenv='base',
+                runas=None,
+                use_vt=False,
+                python_shell=False,
+            )
+
+        expected = [sys.executable, '-m', 'pip', 'install', '--find-links', find_links[0], pkg]
+
         # As single string (just use the first element from find_links)
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, find_links=find_links[0])
-            mock.assert_called_once_with(
-                ['pip', 'install', '--find-links', find_links[0], pkg],
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -236,18 +253,6 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                 find_links='sftp://pypi.crate.io'
             )
 
-        # Valid protos work?
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
-            pip.install(pkg, find_links=find_links)
-            mock.assert_called_once_with(
-                expected,
-                saltenv='base',
-                runas=None,
-                use_vt=False,
-                python_shell=False,
-            )
-
     def test_install_no_index_with_index_url_or_extra_index_url_raises(self):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
@@ -276,8 +281,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(requirements='salt://requirements.txt')
-                expected = ['pip', 'install', '--requirement', 'my_cached_reqs']
-                mock.assert_called_once_with(
+                expected = [sys.executable, '-m', 'pip', 'install', '--requirement', 'my_cached_reqs']
+                mock.assert_called_with(
                     expected,
                     saltenv='base',
                     runas=None,
@@ -287,28 +292,29 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
 
     def test_install_venv(self):
         with patch('os.path') as mock_path:
-            mock_path.is_file.return_value = True
-            mock_path.isdir.return_value = True
-
-            pkg = 'mock'
 
             def join(*args):
-                return os.sep.join(args)
+                return os.path.normpath(os.sep.join(args))
 
+            mock_path.is_file.return_value = True
+            mock_path.isdir.return_value = True
             mock_path.join = join
+
+            if salt.utils.platform.is_windows():
+                venv_path = 'C:\\test_env'
+                bin_path = os.path.join(venv_path, 'python.exe')
+            else:
+                venv_path = '/test_env'
+                bin_path = os.path.join(venv_path, 'python')
+
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-            with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
-                if salt.utils.platform.is_windows():
-                    venv_path = 'c:\\test_env'
-                    bin_path = os.path.join(venv_path, 'Scripts', 'pip.exe')
-                    if six.PY2:
-                        bin_path = bin_path.encode('string-escape')
-                else:
-                    venv_path = '/test_env'
-                    bin_path = os.path.join(venv_path, 'bin', 'pip')
-                pip.install(pkg, bin_env=venv_path)
-                mock.assert_called_once_with(
-                    [bin_path, 'install', pkg],
+            pip_bin = MagicMock(return_value=[bin_path, '-m', 'pip'])
+
+            with patch.dict(pip.__salt__, {'cmd.run_all': mock}), \
+                    patch.object(pip, '_get_pip_bin', pip_bin):
+                pip.install('mock', bin_env=venv_path)
+                mock.assert_called_with(
+                    [bin_path, '-m', 'pip', 'install', 'mock'],
                     env={'VIRTUAL_ENV': venv_path},
                     saltenv='base',
                     runas=None,
@@ -323,8 +329,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(pkg, log=log_path)
-                mock.assert_called_once_with(
-                    ['pip', 'install', '--log', log_path, pkg],
+                expected = [sys.executable, '-m', 'pip', 'install', '--log', log_path, pkg]
+                mock.assert_called_with(
+                    expected,
                     saltenv='base',
                     runas=None,
                     use_vt=False,
@@ -349,12 +356,12 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
     def test_install_timeout_argument_in_resulting_command(self):
         # Passing an int
         pkg = 'pep8'
-        expected_prefix = ['pip', 'install', '--timeout']
+        expected = [sys.executable, '-m', 'pip', 'install', '--timeout']
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, timeout=10)
-            mock.assert_called_once_with(
-                expected_prefix + [10, pkg],
+            mock.assert_called_with(
+                expected + [10, pkg],
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -365,8 +372,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, timeout='10')
-            mock.assert_called_once_with(
-                expected_prefix + ['10', pkg],
+            mock.assert_called_with(
+                expected + ['10', pkg],
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -389,8 +396,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, index_url=index_url)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--index-url', index_url, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--index-url', index_url, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -403,8 +411,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, extra_index_url=extra_index_url)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--extra-index-url', extra_index_url, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--extra-index-url', extra_index_url, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -416,8 +425,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, no_index=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--no-index', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--no-index', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -430,8 +440,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, build=build)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--build', build, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--build', build, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -444,8 +455,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, target=target)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--target', target, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--target', target, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -458,8 +470,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, download=download)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--download', download, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--download', download, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -471,8 +484,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, no_download=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--no-download', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--no-download', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -494,8 +508,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                            MagicMock(return_value=pip_version)):
                     # test `download_cache` kwarg
                     pip.install(pkg, download_cache='/tmp/foo')
+                    expected = [sys.executable, '-m', 'pip', 'install', cmd_arg, download_cache, pkg]
                     mock.assert_called_with(
-                        ['pip', 'install', cmd_arg, download_cache, pkg],
+                        expected,
                         saltenv='base',
                         runas=None,
                         use_vt=False,
@@ -505,7 +520,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                     # test `cache_dir` kwarg
                     pip.install(pkg, cache_dir='/tmp/foo')
                     mock.assert_called_with(
-                        ['pip', 'install', cmd_arg, download_cache, pkg],
+                        expected,
                         saltenv='base',
                         runas=None,
                         use_vt=False,
@@ -518,8 +533,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, source=source)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--source', source, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--source', source, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -531,9 +547,10 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         for action in ('s', 'i', 'w', 'b'):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
-                pip.install('pep8', exists_action=action)
-                mock.assert_called_once_with(
-                    ['pip', 'install', '--exists-action', action, pkg],
+                pip.install(pkg, exists_action=action)
+                expected = [sys.executable, '-m', 'pip', 'install', '--exists-action', action, pkg]
+                mock.assert_called_with(
+                    expected,
                     saltenv='base',
                     runas=None,
                     use_vt=False,
@@ -557,7 +574,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         ]
         pkg = 'pep8'
 
-        expected = ['pip', 'install']
+        expected = [sys.executable, '-m', 'pip', 'install']
         for item in install_options:
             expected.extend(['--install-option', item])
         expected.append(pkg)
@@ -566,7 +583,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, install_options=install_options)
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -578,7 +595,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, install_options=','.join(install_options))
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -590,9 +607,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, install_options=install_options[0])
-            mock.assert_called_once_with(
-                ['pip', 'install', '--install-option',
-                 install_options[0], pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--install-option', install_options[0], pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -606,7 +623,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         ]
         pkg = 'pep8'
 
-        expected = ['pip', 'install']
+        expected = [sys.executable, '-m', 'pip', 'install']
         for item in global_options:
             expected.extend(['--global-option', item])
         expected.append(pkg)
@@ -615,7 +632,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, global_options=global_options)
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -627,7 +644,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, global_options=','.join(global_options))
-            mock.assert_called_once_with(
+            mock.assert_called_with(
                 expected,
                 saltenv='base',
                 runas=None,
@@ -639,8 +656,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, global_options=global_options[0])
-            mock.assert_called_once_with(
-                ['pip', 'install', '--global-option', global_options[0], pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--global-option', global_options[0], pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -652,8 +670,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, upgrade=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--upgrade', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--upgrade', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -665,8 +684,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, force_reinstall=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--force-reinstall', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--force-reinstall', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -678,8 +698,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, ignore_installed=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--ignore-installed', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--ignore-installed', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -691,8 +712,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, no_deps=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--no-deps', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--no-deps', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -704,8 +726,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, no_install=True)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--no-install', pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--no-install', pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -718,8 +741,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.install(pkg, proxy=proxy)
-            mock.assert_called_once_with(
-                ['pip', 'install', '--proxy', proxy, pkg],
+            expected = [sys.executable, '-m', 'pip', 'install', '--proxy', proxy, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 runas=None,
                 use_vt=False,
@@ -736,7 +760,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                 'salt://requirements-1.txt', 'salt://requirements-2.txt'
             ]
 
-            expected = ['pip', 'install']
+            expected = [sys.executable, '-m', 'pip', 'install']
             for item in cached_reqs:
                 expected.extend(['--requirement', item])
 
@@ -744,7 +768,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(requirements=requirements)
-                mock.assert_called_once_with(
+                mock.assert_called_with(
                     expected,
                     saltenv='base',
                     runas=None,
@@ -757,7 +781,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(requirements=','.join(requirements))
-                mock.assert_called_once_with(
+                mock.assert_called_with(
                     expected,
                     saltenv='base',
                     runas=None,
@@ -770,8 +794,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.install(requirements=requirements[0])
-                mock.assert_called_once_with(
-                    ['pip', 'install', '--requirement', cached_reqs[0]],
+                expected = [sys.executable, '-m', 'pip', 'install', '--requirement', cached_reqs[0]]
+                mock.assert_called_with(
+                    expected,
                     saltenv='base',
                     runas=None,
                     use_vt=False,
@@ -788,7 +813,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                 'salt://requirements-1.txt', 'salt://requirements-2.txt'
             ]
 
-            expected = ['pip', 'uninstall', '-y']
+            expected = [sys.executable, '-m', 'pip', 'uninstall', '-y']
             for item in cached_reqs:
                 expected.extend(['--requirement', item])
 
@@ -796,7 +821,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.uninstall(requirements=requirements)
-                mock.assert_called_once_with(
+                mock.assert_called_with(
                     expected,
                     cwd=None,
                     saltenv='base',
@@ -810,7 +835,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.uninstall(requirements=','.join(requirements))
-                mock.assert_called_once_with(
+                mock.assert_called_with(
                     expected,
                     cwd=None,
                     saltenv='base',
@@ -824,8 +849,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
                 pip.uninstall(requirements=requirements[0])
-                mock.assert_called_once_with(
-                    ['pip', 'uninstall', '-y', '--requirement', cached_reqs[0]],
+                expected = [sys.executable, '-m', 'pip', 'uninstall', '-y', '--requirement', cached_reqs[0]]
+                mock.assert_called_with(
+                    expected,
                     cwd=None,
                     saltenv='base',
                     runas=None,
@@ -839,8 +865,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.uninstall(pkg, proxy=proxy)
-            mock.assert_called_once_with(
-                ['pip', 'uninstall', '-y', '--proxy', proxy, pkg],
+            expected = [sys.executable, '-m', 'pip', 'uninstall', '-y', '--proxy', proxy, pkg]
+            mock.assert_called_with(
+                expected,
                 saltenv='base',
                 cwd=None,
                 runas=None,
@@ -849,22 +876,24 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             )
 
     def test_uninstall_log_argument_in_resulting_command(self):
-        with patch('os.path') as mock_path:
-            pkg = 'pep8'
-            log_path = '/tmp/pip-install.log'
-            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-            with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
-                pip.uninstall(pkg, log=log_path)
-                mock.assert_called_once_with(
-                    ['pip', 'uninstall', '-y', '--log', log_path, pkg],
-                    saltenv='base',
-                    cwd=None,
-                    runas=None,
-                    use_vt=False,
-                    python_shell=False,
-                )
+        pkg = 'pep8'
+        log_path = '/tmp/pip-install.log'
 
-            # Let's fake a non-writable log file
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
+            pip.uninstall(pkg, log=log_path)
+            expected = [sys.executable, '-m', 'pip', 'uninstall', '-y', '--log', log_path, pkg]
+            mock.assert_called_with(
+                expected,
+                saltenv='base',
+                cwd=None,
+                runas=None,
+                use_vt=False,
+                python_shell=False,
+            )
+
+        # Let's fake a non-writable log file
+        with patch('os.path') as mock_path:
             mock_path.exists.side_effect = IOError('Fooo!')
             mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
             with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
@@ -877,13 +906,13 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
 
     def test_uninstall_timeout_argument_in_resulting_command(self):
         pkg = 'pep8'
-        expected_prefix = ['pip', 'uninstall', '-y', '--timeout']
+        expected = [sys.executable, '-m', 'pip', 'uninstall', '-y', '--timeout']
         # Passing an int
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.uninstall(pkg, timeout=10)
-            mock.assert_called_once_with(
-                expected_prefix + [10, pkg],
+            mock.assert_called_with(
+                expected + [10, pkg],
                 cwd=None,
                 saltenv='base',
                 runas=None,
@@ -895,8 +924,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
             pip.uninstall(pkg, timeout='10')
-            mock.assert_called_once_with(
-                expected_prefix + ['10', pkg],
+            mock.assert_called_with(
+                expected + ['10', pkg],
                 cwd=None,
                 saltenv='base',
                 runas=None,
@@ -915,6 +944,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             )
 
     def test_freeze_command(self):
+        expected = [sys.executable, '-m', 'pip', 'freeze']
         eggs = [
             'M2Crypto==0.21.1',
             '-e git+git@github.com:s0undt3ch/salt-testing.git@9ed81aa2f918d59d3706e56b18f0782d1ea43bf8#egg=SaltTesting-dev',
@@ -932,8 +962,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value='6.1.1')):
                 ret = pip.freeze()
-                mock.assert_called_once_with(
-                    ['pip', 'freeze'],
+                mock.assert_called_with(
+                    expected,
                     cwd=None,
                     runas=None,
                     use_vt=False,
@@ -952,8 +982,8 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value='6.1.1')):
                 ret = pip.freeze(env_vars={"foo": "bar"})
-                mock.assert_called_once_with(
-                    ['pip', 'freeze'],
+                mock.assert_called_with(
+                    expected,
                     cwd=None,
                     runas=None,
                     use_vt=False,
@@ -992,8 +1022,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value='9.0.1')):
                 ret = pip.freeze()
-                mock.assert_called_once_with(
-                    ['pip', 'freeze', '--all'],
+                expected = [sys.executable, '-m', 'pip', 'freeze', '--all']
+                mock.assert_called_with(
+                    expected,
                     cwd=None,
                     runas=None,
                     use_vt=False,
@@ -1025,8 +1056,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value=mock_version)):
                 ret = pip.list_()
+                expected = [sys.executable, '-m', 'pip', 'freeze']
                 mock.assert_called_with(
-                    ['pip', 'freeze'],
+                    expected,
                     cwd=None,
                     runas=None,
                     python_shell=False,
@@ -1072,8 +1104,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value=mock_version)):
                 ret = pip.list_()
+                expected = [sys.executable, '-m', 'pip', 'freeze', '--all']
                 mock.assert_called_with(
-                    ['pip', 'freeze', '--all'],
+                    expected,
                     cwd=None,
                     runas=None,
                     python_shell=False,
@@ -1119,8 +1152,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value='6.1.1')):
                 ret = pip.list_(prefix='bb')
+                expected = [sys.executable, '-m', 'pip', 'freeze']
                 mock.assert_called_with(
-                    ['pip', 'freeze'],
+                    expected,
                     cwd=None,
                     runas=None,
                     python_shell=False,
@@ -1135,9 +1169,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
 
     def test_list_upgrades_legacy(self):
         eggs = [
-            'apache-libcloud (1.1.0) - Latest: 2.2.1 [wheel]',
-            'appdirs (1.4.1) - Latest: 1.4.3 [wheel]',
-            'awscli (1.11.63) - Latest: 1.12.1 [sdist]'
+            'apache-libcloud (Current: 1.1.0 Latest: 2.2.1 [wheel])',
+            'appdirs (Current: 1.4.1 Latest: 1.4.3 [wheel])',
+            'awscli (Current: 1.11.63 Latest: 1.12.1 [sdist])'
         ]
         mock = MagicMock(
             return_value={
@@ -1150,7 +1184,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                        MagicMock(return_value='6.1.1')):
                 ret = pip.list_upgrades()
                 mock.assert_called_with(
-                    ['pip', 'list', '--outdated'],
+                    [sys.executable, '-m', 'pip', 'list', '--outdated'],
                     cwd=None,
                     runas=None,
                 )
@@ -1178,15 +1212,15 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                        MagicMock(return_value='9.1.1')):
                 ret = pip.list_upgrades()
                 mock.assert_called_with(
-                    ['pip', 'list', '--outdated', '--format=json'],
+                    [sys.executable, '-m', 'pip', 'list', '--outdated', '--format=json'],
                     cwd=None,
                     runas=None,
                 )
                 self.assertEqual(
                     ret, {
-                        'apache-libcloud': '2.2.1',
-                        'appdirs': '1.4.3',
-                        'awscli': '1.12.1'
+                        'apache-libcloud': '2.2.1 [wheel]',
+                        'appdirs': '1.4.3 [wheel]',
+                        'awscli': '1.12.1 [sdist]'
                     }
                 )
 
@@ -1209,7 +1243,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                        MagicMock(return_value='6.1.1')):
                 ret = pip.is_installed(pkgname='bbfreeze')
                 mock.assert_called_with(
-                    ['pip', 'freeze'],
+                    [sys.executable, '-m', 'pip', 'freeze'],
                     cwd=None,
                     runas=None,
                     python_shell=False,
@@ -1236,7 +1270,7 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
                        MagicMock(return_value='6.1.1')):
                 ret = pip.is_installed(pkgname='notexist')
                 mock.assert_called_with(
-                    ['pip', 'freeze'],
+                    [sys.executable, '-m', 'pip', 'freeze'],
                     cwd=None,
                     runas=None,
                     python_shell=False,
@@ -1256,8 +1290,9 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
             with patch('salt.modules.pip.version',
                        MagicMock(return_value='1.3')):
                 pip.install(pkg, pre_releases=True)
+                expected = [sys.executable, '-m', 'pip', 'install', pkg]
                 mock.assert_called_with(
-                    ['pip', 'install', pkg],
+                    expected,
                     saltenv='base',
                     runas=None,
                     use_vt=False,
@@ -1269,10 +1304,11 @@ class PipTestCase(TestCase, LoaderModuleMockMixin):
         with patch.dict(pip.__salt__, {'cmd.run_stdout': mock_run,
                                        'cmd.run_all': mock_run_all}):
             with patch('salt.modules.pip._get_pip_bin',
-                       MagicMock(return_value='pip')):
+                       MagicMock(return_value=['pip'])):
                 pip.install(pkg, pre_releases=True)
+                expected = ['pip', 'install', '--pre', pkg]
                 mock_run_all.assert_called_with(
-                    ['pip', 'install', '--pre', pkg],
+                    expected,
                     saltenv='base',
                     runas=None,
                     use_vt=False,

tests/unit/modules/test_zpool.py

@@ -223,7 +223,7 @@ class ZpoolTestCase(TestCase, LoaderModuleMockMixin):
         Tests successful return of get function
         '''
         ret = {}
-        ret['stdout'] = "size\t1.81T\t-\n"
+        ret['stdout'] = "mypool\tsize\t1.81T\t-\n"
         ret['stderr'] = ""
         ret['retcode'] = 0
         mock_cmd = MagicMock(return_value=ret)
@@ -238,7 +238,7 @@ class ZpoolTestCase(TestCase, LoaderModuleMockMixin):
         Tests successful return of get function with parsable output
         '''
         ret = {}
-        ret['stdout'] = "size\t1.81T\t-\n"
+        ret['stdout'] = "mypool\tsize\t1.81T\t-\n"
         ret['stderr'] = ""
         ret['retcode'] = 0
         mock_cmd = MagicMock(return_value=ret)
@@ -253,7 +253,7 @@ class ZpoolTestCase(TestCase, LoaderModuleMockMixin):
         Tests successful return of get function with a string with whitespaces
         '''
         ret = {}
-        ret['stdout'] = "comment\tmy testing pool\t-\n"
+        ret['stdout'] = "mypool\tcomment\tmy testing pool\t-\n"
         ret['stderr'] = ""
         ret['retcode'] = 0
         mock_cmd = MagicMock(return_value=ret)

tests/unit/states/test_pip.py

@@ -1,294 +0,0 @@
-# -*- coding: utf-8 -*-
-'''
-    :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)`
-
-
-    tests.unit.states.pip_test
-    ~~~~~~~~~~~~~~~~~~~~~~~~~~
-'''
-
-# Import python libs
-from __future__ import absolute_import, print_function, unicode_literals
-
-# Import Salt Testing libs
-from tests.support.mixins import LoaderModuleMockMixin, SaltReturnAssertsMixin
-from tests.support.unit import skipIf, TestCase
-from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch
-
-# Import salt libs
-import salt.states.pip_state as pip_state
-
-# Import 3rd-party libs
-try:
-    import pip
-    HAS_PIP = True
-except ImportError:
-    HAS_PIP = False
-
-
-@skipIf(NO_MOCK, NO_MOCK_REASON)
-@skipIf(not HAS_PIP,
-        'The \'pip\' library is not importable(installed system-wide)')
-class PipStateTest(TestCase, SaltReturnAssertsMixin, LoaderModuleMockMixin):
-
-    def setup_loader_modules(self):
-        return {
-            pip_state: {
-                '__env__': 'base',
-                '__opts__': {'test': False},
-                '__salt__': {'cmd.which_bin': lambda _: 'pip'}
-            }
-        }
-
-    def test_install_requirements_parsing(self):
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.3'})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed('pep8=1.3.2')
-                self.assertSaltFalseReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Invalid version specification in package pep8=1.3.2. '
-                    '\'=\' is not supported, use \'==\' instead.',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.3'})
-        pip_install = MagicMock(return_value={'retcode': 0})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed('pep8>=1.3.2')
-                self.assertSaltTrueReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package pep8>=1.3.2 was already installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.3'})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed('pep8<1.3.2')
-                self.assertSaltNoneReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package pep8<1.3.2 is set to be installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.2'})
-        pip_install = MagicMock(return_value={'retcode': 0})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed('pep8>1.3.1,<1.3.3')
-                self.assertSaltTrueReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package pep8>1.3.1,<1.3.3 was already installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.1'})
-        pip_install = MagicMock(return_value={'retcode': 0})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed('pep8>1.3.1,<1.3.3')
-                self.assertSaltNoneReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package pep8>1.3.1,<1.3.3 is set to be installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.1'})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed(
-                    'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting>=0.5.1'
-                )
-                self.assertSaltNoneReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package git+https://github.com/saltstack/'
-                    'salt-testing.git#egg=SaltTesting>=0.5.1 is set to be '
-                    'installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.1'})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed(
-                    'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting'
-                )
-                self.assertSaltNoneReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package git+https://github.com/saltstack/'
-                    'salt-testing.git#egg=SaltTesting is set to be '
-                    'installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.1'})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list}):
-            with patch.dict(pip_state.__opts__, {'test': True}):
-                ret = pip_state.installed(
-                    'https://pypi.python.org/packages/source/S/SaltTesting/'
-                    'SaltTesting-0.5.0.tar.gz'
-                    '#md5=e6760af92b7165f8be53b5763e40bc24'
-                )
-                self.assertSaltNoneReturn({'test': ret})
-                self.assertInSaltComment(
-                    'Python package https://pypi.python.org/packages/source/'
-                    'S/SaltTesting/SaltTesting-0.5.0.tar.gz'
-                    '#md5=e6760af92b7165f8be53b5763e40bc24 is set to be '
-                    'installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
-        pip_install = MagicMock(return_value={
-            'retcode': 0,
-            'stderr': '',
-            'stdout': 'Downloading/unpacking https://pypi.python.org/packages'
-                      '/source/S/SaltTesting/SaltTesting-0.5.0.tar.gz\n  '
-                      'Downloading SaltTesting-0.5.0.tar.gz\n  Running '
-                      'setup.py egg_info for package from '
-                      'https://pypi.python.org/packages/source/S/SaltTesting/'
-                      'SaltTesting-0.5.0.tar.gz\n    \nCleaning up...'
-        })
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            ret = pip_state.installed(
-                'https://pypi.python.org/packages/source/S/SaltTesting/'
-                'SaltTesting-0.5.0.tar.gz'
-                '#md5=e6760af92b7165f8be53b5763e40bc24'
-            )
-            self.assertSaltTrueReturn({'test': ret})
-            self.assertInSaltComment('All packages were successfully installed',
-                {'test': ret}
-            )
-            self.assertInSaltReturn(
-                'Installed',
-                {'test': ret},
-                ('changes', 'https://pypi.python.org/packages/source/S/'
-                            'SaltTesting/SaltTesting-0.5.0.tar.gz'
-                            '#md5=e6760af92b7165f8be53b5763e40bc24==???')
-            )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
-        pip_install = MagicMock(return_value={
-            'retcode': 0,
-            'stderr': '',
-            'stdout': 'Cloned!'
-        })
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            with patch.dict(pip_state.__opts__, {'test': False}):
-                ret = pip_state.installed(
-                    'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting'
-                )
-                self.assertSaltTrueReturn({'test': ret})
-                self.assertInSaltComment(
-                    'packages are already installed',
-                    {'test': ret}
-                )
-
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'pep8': '1.3.1'})
-        pip_install = MagicMock(return_value={'retcode': 0})
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            with patch.dict(pip_state.__opts__, {'test': False}):
-                ret = pip_state.installed(
-                    'arbitrary ID that should be ignored due to requirements specified',
-                    requirements='/tmp/non-existing-requirements.txt'
-                )
-                self.assertSaltTrueReturn({'test': ret})
-
-        # Test VCS installations using git+git://
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
-        pip_install = MagicMock(return_value={
-            'retcode': 0,
-            'stderr': '',
-            'stdout': 'Cloned!'
-        })
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            with patch.dict(pip_state.__opts__, {'test': False}):
-                ret = pip_state.installed(
-                    'git+git://github.com/saltstack/salt-testing.git#egg=SaltTesting'
-                )
-                self.assertSaltTrueReturn({'test': ret})
-                self.assertInSaltComment(
-                    'packages are already installed',
-                    {'test': ret}
-                )
-
-        # Test VCS installations with version info like >= 0.1
-        with patch.object(pip, '__version__', MagicMock(side_effect=AttributeError(
-                                                    'Faked missing __version__ attribute'))):
-            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-            pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
-            pip_install = MagicMock(return_value={
-                'retcode': 0,
-                'stderr': '',
-                'stdout': 'Cloned!'
-            })
-            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                                 'pip.list': pip_list,
-                                                 'pip.install': pip_install}):
-                with patch.dict(pip_state.__opts__, {'test': False}):
-                    ret = pip_state.installed(
-                        'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting>=0.5.0'
-                    )
-                    self.assertSaltTrueReturn({'test': ret})
-                    self.assertInSaltComment(
-                        'packages are already installed',
-                        {'test': ret}
-                    )
-
-    def test_install_in_editable_mode(self):
-        '''
-        Check that `name` parameter containing bad characters is not parsed by
-        pip when package is being installed in editable mode.
-        For more information, see issue #21890.
-        '''
-        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
-        pip_list = MagicMock(return_value={})
-        pip_install = MagicMock(return_value={
-            'retcode': 0,
-            'stderr': '',
-            'stdout': 'Cloned!'
-        })
-        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
-                                             'pip.list': pip_list,
-                                             'pip.install': pip_install}):
-            ret = pip_state.installed('state@name',
-                                      cwd='/path/to/project',
-                                      editable=['.'])
-            self.assertSaltTrueReturn({'test': ret})
-            self.assertInSaltComment(
-                'successfully installed',
-                {'test': ret}
-            )

tests/unit/states/test_pip_state.py

@@ -0,0 +1,306 @@
+# -*- coding: utf-8 -*-
+'''
+    :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)`
+
+
+    tests.unit.states.pip_test
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~
+'''
+
+# Import python libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+# Import Salt Testing libs
+from tests.support.mixins import LoaderModuleMockMixin, SaltReturnAssertsMixin
+from tests.support.unit import skipIf, TestCase
+from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch
+
+# Import salt libs
+import salt.states.pip_state as pip_state
+import salt.utils.versions
+
+# Import 3rd-party libs
+try:
+    import pip
+    HAS_PIP = True
+except ImportError:
+    HAS_PIP = False
+
+
+@skipIf(NO_MOCK, NO_MOCK_REASON)
+@skipIf(not HAS_PIP,
+        'The \'pip\' library is not importable(installed system-wide)')
+class PipStateTest(TestCase, SaltReturnAssertsMixin, LoaderModuleMockMixin):
+
+    def setup_loader_modules(self):
+        return {
+            pip_state: {
+                '__env__': 'base',
+                '__opts__': {'test': False},
+                '__salt__': {'cmd.which_bin': lambda _: 'pip'}
+            }
+        }
+
+    def test_install_requirements_parsing(self):
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        pip_list = MagicMock(return_value={'pep8': '1.3.3'})
+        pip_version = pip.__version__
+        mock_pip_version = MagicMock(return_value=pip_version)
+        with patch.dict(pip_state.__salt__, {'pip.version': mock_pip_version}):
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    if salt.utils.versions.compare(ver1=pip_version,
+                                                   oper='<',
+                                                   ver2='10.0'):
+                        ret = pip_state.installed('pep8=1.3.2')
+                        self.assertSaltFalseReturn({'test': ret})
+                        self.assertInSaltComment(
+                            'Invalid version specification in package pep8=1.3.2. '
+                            '\'=\' is not supported, use \'==\' instead.',
+                            {'test': ret})
+                    else:
+                        self.assertRaises(
+                            pip._internal.exceptions.InstallationError,
+                            pip_state.installed, 'pep=1.3.2')
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.3'})
+            pip_install = MagicMock(return_value={'retcode': 0})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed('pep8>=1.3.2')
+                    self.assertSaltTrueReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package pep8>=1.3.2 was already installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.3'})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed('pep8<1.3.2')
+                    self.assertSaltNoneReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package pep8<1.3.2 is set to be installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.2'})
+            pip_install = MagicMock(return_value={'retcode': 0})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed('pep8>1.3.1,<1.3.3')
+                    self.assertSaltTrueReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package pep8>1.3.1,<1.3.3 was already installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.1'})
+            pip_install = MagicMock(return_value={'retcode': 0})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed('pep8>1.3.1,<1.3.3')
+                    self.assertSaltNoneReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package pep8>1.3.1,<1.3.3 is set to be installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.1'})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed(
+                        'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting>=0.5.1'
+                    )
+                    self.assertSaltNoneReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package git+https://github.com/saltstack/'
+                        'salt-testing.git#egg=SaltTesting>=0.5.1 is set to be '
+                        'installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.1'})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed(
+                        'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting'
+                    )
+                    self.assertSaltNoneReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package git+https://github.com/saltstack/'
+                        'salt-testing.git#egg=SaltTesting is set to be '
+                        'installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.1'})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list}):
+                with patch.dict(pip_state.__opts__, {'test': True}):
+                    ret = pip_state.installed(
+                        'https://pypi.python.org/packages/source/S/SaltTesting/'
+                        'SaltTesting-0.5.0.tar.gz'
+                        '#md5=e6760af92b7165f8be53b5763e40bc24'
+                    )
+                    self.assertSaltNoneReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'Python package https://pypi.python.org/packages/source/'
+                        'S/SaltTesting/SaltTesting-0.5.0.tar.gz'
+                        '#md5=e6760af92b7165f8be53b5763e40bc24 is set to be '
+                        'installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
+            pip_install = MagicMock(return_value={
+                'retcode': 0,
+                'stderr': '',
+                'stdout': 'Downloading/unpacking https://pypi.python.org/packages'
+                          '/source/S/SaltTesting/SaltTesting-0.5.0.tar.gz\n  '
+                          'Downloading SaltTesting-0.5.0.tar.gz\n  Running '
+                          'setup.py egg_info for package from '
+                          'https://pypi.python.org/packages/source/S/SaltTesting/'
+                          'SaltTesting-0.5.0.tar.gz\n    \nCleaning up...'
+            })
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                ret = pip_state.installed(
+                    'https://pypi.python.org/packages/source/S/SaltTesting/'
+                    'SaltTesting-0.5.0.tar.gz'
+                    '#md5=e6760af92b7165f8be53b5763e40bc24'
+                )
+                self.assertSaltTrueReturn({'test': ret})
+                self.assertInSaltComment('All packages were successfully installed',
+                    {'test': ret}
+                )
+                self.assertInSaltReturn(
+                    'Installed',
+                    {'test': ret},
+                    ('changes', 'https://pypi.python.org/packages/source/S/'
+                                'SaltTesting/SaltTesting-0.5.0.tar.gz'
+                                '#md5=e6760af92b7165f8be53b5763e40bc24==???')
+                )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
+            pip_install = MagicMock(return_value={
+                'retcode': 0,
+                'stderr': '',
+                'stdout': 'Cloned!'
+            })
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                with patch.dict(pip_state.__opts__, {'test': False}):
+                    ret = pip_state.installed(
+                        'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting'
+                    )
+                    self.assertSaltTrueReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'packages are already installed',
+                        {'test': ret}
+                    )
+
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'pep8': '1.3.1'})
+            pip_install = MagicMock(return_value={'retcode': 0})
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                with patch.dict(pip_state.__opts__, {'test': False}):
+                    ret = pip_state.installed(
+                        'arbitrary ID that should be ignored due to requirements specified',
+                        requirements='/tmp/non-existing-requirements.txt'
+                    )
+                    self.assertSaltTrueReturn({'test': ret})
+
+            # Test VCS installations using git+git://
+            mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+            pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
+            pip_install = MagicMock(return_value={
+                'retcode': 0,
+                'stderr': '',
+                'stdout': 'Cloned!'
+            })
+            with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                 'pip.list': pip_list,
+                                                 'pip.install': pip_install}):
+                with patch.dict(pip_state.__opts__, {'test': False}):
+                    ret = pip_state.installed(
+                        'git+git://github.com/saltstack/salt-testing.git#egg=SaltTesting'
+                    )
+                    self.assertSaltTrueReturn({'test': ret})
+                    self.assertInSaltComment(
+                        'packages are already installed',
+                        {'test': ret}
+                    )
+
+            # Test VCS installations with version info like >= 0.1
+            with patch.object(pip, '__version__', MagicMock(side_effect=AttributeError(
+                                                        'Faked missing __version__ attribute'))):
+                mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+                pip_list = MagicMock(return_value={'SaltTesting': '0.5.0'})
+                pip_install = MagicMock(return_value={
+                    'retcode': 0,
+                    'stderr': '',
+                    'stdout': 'Cloned!'
+                })
+                with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                                     'pip.list': pip_list,
+                                                     'pip.install': pip_install}):
+                    with patch.dict(pip_state.__opts__, {'test': False}):
+                        ret = pip_state.installed(
+                            'git+https://github.com/saltstack/salt-testing.git#egg=SaltTesting>=0.5.0'
+                        )
+                        self.assertSaltTrueReturn({'test': ret})
+                        self.assertInSaltComment(
+                            'packages are already installed',
+                            {'test': ret}
+                        )
+
+    def test_install_in_editable_mode(self):
+        '''
+        Check that `name` parameter containing bad characters is not parsed by
+        pip when package is being installed in editable mode.
+        For more information, see issue #21890.
+        '''
+        mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
+        pip_list = MagicMock(return_value={})
+        pip_install = MagicMock(return_value={
+            'retcode': 0,
+            'stderr': '',
+            'stdout': 'Cloned!'
+        })
+        pip_version = MagicMock(return_value='10.0.1')
+        with patch.dict(pip_state.__salt__, {'cmd.run_all': mock,
+                                             'pip.list': pip_list,
+                                             'pip.install': pip_install,
+                                             'pip.version': pip_version}):
+            ret = pip_state.installed('state@name',
+                                      cwd='/path/to/project',
+                                      editable=['.'])
+            self.assertSaltTrueReturn({'test': ret})
+            self.assertInSaltComment(
+                'successfully installed',
+                {'test': ret}
+            )

tests/unit/test_files.py

@@ -1,79 +0,0 @@
-# -*- coding: utf-8 -*-
-'''
-    tests.unit.file_test
-    ~~~~~~~~~~~~~~~~~~~~
-'''
-# Import pytohn libs
-from __future__ import absolute_import
-import os
-import copy
-import shutil
-import tempfile
-
-# Import Salt Testing libs
-from tests.support.unit import TestCase
-
-# Import Salt libs
-from salt.ext import six
-import salt.utils.files
-
-
-class FilesTestCase(TestCase):
-
-    STRUCTURE = {
-        'foo': {
-            'foofile.txt': 'fooSTRUCTURE'
-        },
-        'bar': {
-            'barfile.txt': 'barSTRUCTURE'
-        }
-    }
-
-    def _create_temp_structure(self, temp_directory, structure):
-        for folder, files in six.iteritems(structure):
-            current_directory = os.path.join(temp_directory, folder)
-            os.makedirs(current_directory)
-            for name, content in six.iteritems(files):
-                path = os.path.join(temp_directory, folder, name)
-                with salt.utils.files.fopen(path, 'w+') as fh:
-                    fh.write(content)
-
-    def _validate_folder_structure_and_contents(self, target_directory,
-                                                desired_structure):
-        for folder, files in six.iteritems(desired_structure):
-            for name, content in six.iteritems(files):
-                path = os.path.join(target_directory, folder, name)
-                with salt.utils.files.fopen(path) as fh:
-                    assert fh.read().strip() == content
-
-    def setUp(self):
-        super(FilesTestCase, self).setUp()
-        self.temp_dir = tempfile.mkdtemp()
-        self._create_temp_structure(self.temp_dir,
-                                    self.STRUCTURE)
-
-    def tearDown(self):
-        super(FilesTestCase, self).tearDown()
-        shutil.rmtree(self.temp_dir)
-
-    def test_recursive_copy(self):
-        test_target_directory = tempfile.mkdtemp()
-        TARGET_STRUCTURE = {
-            'foo': {
-                'foo.txt': 'fooTARGET_STRUCTURE'
-            },
-            'baz': {
-                'baz.txt': 'bazTARGET_STRUCTURE'
-            }
-        }
-        self._create_temp_structure(test_target_directory, TARGET_STRUCTURE)
-        try:
-            salt.utils.files.recursive_copy(self.temp_dir, test_target_directory)
-            DESIRED_STRUCTURE = copy.copy(TARGET_STRUCTURE)
-            DESIRED_STRUCTURE.update(self.STRUCTURE)
-            self._validate_folder_structure_and_contents(
-                test_target_directory,
-                DESIRED_STRUCTURE
-            )
-        finally:
-            shutil.rmtree(test_target_directory)

tests/unit/test_target.py

@@ -1,175 +0,0 @@
-# -*- coding: utf-8 -*-
-'''
-    :codeauthor: :email: `Mike Place <mp@saltstack.com>`
-
-    tests.unit.target_test
-    ~~~~~~~~~~~~~~~~~~~~~~
-'''
-
-# Import Python libs
-from __future__ import absolute_import
-import sys
-
-# Import Salt libs
-import salt.utils.minions
-import salt.config
-
-# Import Salt Testing libs
-from tests.support.unit import TestCase, skipIf
-
-import logging
-
-log = logging.getLogger(__name__)
-
-
-class CkMinionTestCase(TestCase):
-
-    def setUp(self):
-        self.ck_ = salt.utils.minions.CkMinions(salt.config.DEFAULT_MASTER_OPTS)
-
-    def tearDown(self):
-        self.ck_ = None
-
-    #TODO This is just a stub for upcoming tests
-
-
-@skipIf(sys.version_info < (2, 7), 'Python 2.7 needed for dictionary equality assertions')
-class TargetParseTestCase(TestCase):
-
-    def test_parse_grains_target(self):
-        '''
-        Ensure proper parsing for grains
-        '''
-        g_tgt = 'G@a:b'
-        ret = salt.utils.minions.parse_target(g_tgt)
-        self.assertDictEqual(ret, {'engine': 'G', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_grains_pcre_target(self):
-        '''
-        Ensure proper parsing for grains PCRE matching
-        '''
-        p_tgt = 'P@a:b'
-        ret = salt.utils.minions.parse_target(p_tgt)
-        self.assertDictEqual(ret, {'engine': 'P', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_pillar_pcre_target(self):
-        '''
-        Ensure proper parsing for pillar PCRE matching
-        '''
-        j_tgt = 'J@a:b'
-        ret = salt.utils.minions.parse_target(j_tgt)
-        self.assertDictEqual(ret, {'engine': 'J', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_list_target(self):
-        '''
-        Ensure proper parsing for list matching
-        '''
-        l_tgt = 'L@a:b'
-        ret = salt.utils.minions.parse_target(l_tgt)
-        self.assertDictEqual(ret, {'engine': 'L', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_nodegroup_target(self):
-        '''
-        Ensure proper parsing for pillar matching
-        '''
-        n_tgt = 'N@a:b'
-        ret = salt.utils.minions.parse_target(n_tgt)
-        self.assertDictEqual(ret, {'engine': 'N', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_subnet_target(self):
-        '''
-        Ensure proper parsing for subnet matching
-        '''
-        s_tgt = 'S@a:b'
-        ret = salt.utils.minions.parse_target(s_tgt)
-        self.assertDictEqual(ret, {'engine': 'S', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_minion_pcre_target(self):
-        '''
-        Ensure proper parsing for minion PCRE matching
-        '''
-        e_tgt = 'E@a:b'
-        ret = salt.utils.minions.parse_target(e_tgt)
-        self.assertDictEqual(ret, {'engine': 'E', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_range_target(self):
-        '''
-        Ensure proper parsing for range matching
-        '''
-        r_tgt = 'R@a:b'
-        ret = salt.utils.minions.parse_target(r_tgt)
-        self.assertDictEqual(ret, {'engine': 'R', 'pattern': 'a:b', 'delimiter': None})
-
-    def test_parse_multiword_target(self):
-        '''
-        Ensure proper parsing for multi-word targets
-
-        Refs https://github.com/saltstack/salt/issues/37231
-        '''
-        mw_tgt = 'G@a:b c'
-        ret = salt.utils.minions.parse_target(mw_tgt)
-        self.assertEqual(ret['pattern'], 'a:b c')
-
-
-class NodegroupCompTest(TestCase):
-    '''
-    Test nodegroup comparisons found in
-    salt.utils.minions.nodgroup_comp()
-    '''
-
-    def test_simple_nodegroup(self):
-        '''
-        Smoke test a very simple nodegroup. No recursion.
-        '''
-        simple_nodegroup = {'group1': 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com'}
-
-        ret = salt.utils.minions.nodegroup_comp('group1', simple_nodegroup)
-        expected_ret = ['L@foo.domain.com,bar.domain.com,baz.domain.com', 'or', 'bl*.domain.com']
-        self.assertListEqual(ret, expected_ret)
-
-    def test_simple_expression_nodegroup(self):
-        '''
-        Smoke test a nodegroup with a simple expression. No recursion.
-        '''
-        simple_nodegroup = {'group1': '[foo,bar,baz].domain.com'}
-
-        ret = salt.utils.minions.nodegroup_comp('group1', simple_nodegroup)
-        expected_ret = ['E@[foo,bar,baz].domain.com']
-        self.assertListEqual(ret, expected_ret)
-
-    def test_simple_recurse(self):
-        '''
-        Test a case where one nodegroup contains a second nodegroup
-        '''
-        referenced_nodegroups = {
-                'group1': 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com',
-                'group2': 'G@os:Debian and N@group1'
-                }
-
-        ret = salt.utils.minions.nodegroup_comp('group2', referenced_nodegroups)
-        expected_ret = [
-                '(',
-                'G@os:Debian',
-                'and',
-                '(',
-                'L@foo.domain.com,bar.domain.com,baz.domain.com',
-                'or',
-                'bl*.domain.com',
-                ')',
-                ')'
-                ]
-        self.assertListEqual(ret, expected_ret)
-
-    def test_circular_nodegroup_reference(self):
-        '''
-        Test to see what happens if A refers to B
-        and B in turn refers back to A
-        '''
-        referenced_nodegroups = {
-                'group1': 'N@group2',
-                'group2': 'N@group1'
-                }
-
-        # If this works, it should also print an error to the console
-        ret = salt.utils.minions.nodegroup_comp('group1', referenced_nodegroups)
-        self.assertEqual(ret, [])

tests/unit/utils/test_files.py

@@ -1,10 +1,11 @@
 # -*- coding: utf-8 -*-
 '''
-Unit Tests for functions located in salt.utils.files.py.
+Unit Tests for functions located in salt/utils/files.py
 '''
 
 # Import python libs
 from __future__ import absolute_import, unicode_literals, print_function
+import copy
 import os
 
 # Import Salt libs
@@ -21,7 +22,7 @@ from tests.support.mock import (
 )
 
 
-class FilesUtilTestCase(TestCase):
+class FilesTestCase(TestCase):
     '''
     Test case for files util.
     '''
@@ -94,3 +95,54 @@ class FilesUtilTestCase(TestCase):
                     'fopen() should have been prevented from opening a file '
                     'using {0} as the filename'.format(invalid_fn)
                 )
+
+    def _create_temp_structure(self, temp_directory, structure):
+        for folder, files in six.iteritems(structure):
+            current_directory = os.path.join(temp_directory, folder)
+            os.makedirs(current_directory)
+            for name, content in six.iteritems(files):
+                path = os.path.join(temp_directory, folder, name)
+                with salt.utils.files.fopen(path, 'w+') as fh:
+                    fh.write(content)
+
+    def _validate_folder_structure_and_contents(self, target_directory,
+                                                desired_structure):
+        for folder, files in six.iteritems(desired_structure):
+            for name, content in six.iteritems(files):
+                path = os.path.join(target_directory, folder, name)
+                with salt.utils.files.fopen(path) as fh:
+                    assert fh.read().strip() == content
+
+    @with_tempdir()
+    @with_tempdir()
+    def test_recursive_copy(self, src, dest):
+        src_structure = {
+            'foo': {
+                'foofile.txt': 'fooSTRUCTURE'
+            },
+            'bar': {
+                'barfile.txt': 'barSTRUCTURE'
+            }
+        }
+        dest_structure = {
+            'foo': {
+                'foo.txt': 'fooTARGET_STRUCTURE'
+            },
+            'baz': {
+                'baz.txt': 'bazTARGET_STRUCTURE'
+            }
+        }
+
+        # Create the file structures in both src and dest dirs
+        self._create_temp_structure(src, src_structure)
+        self._create_temp_structure(dest, dest_structure)
+
+        # Perform the recursive copy
+        salt.utils.files.recursive_copy(src, dest)
+
+        # Confirm results match expected results
+        desired_structure = copy.copy(dest_structure)
+        desired_structure.update(src_structure)
+        self._validate_folder_structure_and_contents(
+            dest,
+            desired_structure)

tests/unit/utils/test_minions.py

@@ -2,12 +2,13 @@
 
 # Import python libs
 from __future__ import absolute_import, unicode_literals
+import sys
 
 # Import Salt Libs
-import salt.utils.minions as minions
+import salt.utils.minions
 
 # Import Salt Testing Libs
-from tests.support.unit import TestCase
+from tests.support.unit import TestCase, skipIf
 from tests.support.mock import (
     patch,
     MagicMock,
@@ -38,7 +39,7 @@ class MinionsTestCase(TestCase):
         '''
         for nodegroup in NODEGROUPS:
             expected = EXPECTED[nodegroup]
-            ret = minions.nodegroup_comp(nodegroup, NODEGROUPS)
+            ret = salt.utils.minions.nodegroup_comp(nodegroup, NODEGROUPS)
             self.assertEqual(ret, expected)
 
 
@@ -47,7 +48,7 @@ class CkMinionsTestCase(TestCase):
     TestCase for salt.utils.minions.CkMinions class
     '''
     def setUp(self):
-        self.ckminions = minions.CkMinions({})
+        self.ckminions = salt.utils.minions.CkMinions({})
 
     def test_spec_check(self):
         # Test spec-only rule
@@ -366,3 +367,145 @@ class CkMinionsTestCase(TestCase):
         args = ['1', '2']
         ret = self.ckminions.auth_check(auth_list, 'test.arg', args, 'runner')
         self.assertTrue(ret)
+
+
+@skipIf(sys.version_info < (2, 7), 'Python 2.7 needed for dictionary equality assertions')
+class TargetParseTestCase(TestCase):
+
+    def test_parse_grains_target(self):
+        '''
+        Ensure proper parsing for grains
+        '''
+        g_tgt = 'G@a:b'
+        ret = salt.utils.minions.parse_target(g_tgt)
+        self.assertDictEqual(ret, {'engine': 'G', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_grains_pcre_target(self):
+        '''
+        Ensure proper parsing for grains PCRE matching
+        '''
+        p_tgt = 'P@a:b'
+        ret = salt.utils.minions.parse_target(p_tgt)
+        self.assertDictEqual(ret, {'engine': 'P', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_pillar_pcre_target(self):
+        '''
+        Ensure proper parsing for pillar PCRE matching
+        '''
+        j_tgt = 'J@a:b'
+        ret = salt.utils.minions.parse_target(j_tgt)
+        self.assertDictEqual(ret, {'engine': 'J', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_list_target(self):
+        '''
+        Ensure proper parsing for list matching
+        '''
+        l_tgt = 'L@a:b'
+        ret = salt.utils.minions.parse_target(l_tgt)
+        self.assertDictEqual(ret, {'engine': 'L', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_nodegroup_target(self):
+        '''
+        Ensure proper parsing for pillar matching
+        '''
+        n_tgt = 'N@a:b'
+        ret = salt.utils.minions.parse_target(n_tgt)
+        self.assertDictEqual(ret, {'engine': 'N', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_subnet_target(self):
+        '''
+        Ensure proper parsing for subnet matching
+        '''
+        s_tgt = 'S@a:b'
+        ret = salt.utils.minions.parse_target(s_tgt)
+        self.assertDictEqual(ret, {'engine': 'S', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_minion_pcre_target(self):
+        '''
+        Ensure proper parsing for minion PCRE matching
+        '''
+        e_tgt = 'E@a:b'
+        ret = salt.utils.minions.parse_target(e_tgt)
+        self.assertDictEqual(ret, {'engine': 'E', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_range_target(self):
+        '''
+        Ensure proper parsing for range matching
+        '''
+        r_tgt = 'R@a:b'
+        ret = salt.utils.minions.parse_target(r_tgt)
+        self.assertDictEqual(ret, {'engine': 'R', 'pattern': 'a:b', 'delimiter': None})
+
+    def test_parse_multiword_target(self):
+        '''
+        Ensure proper parsing for multi-word targets
+
+        Refs https://github.com/saltstack/salt/issues/37231
+        '''
+        mw_tgt = 'G@a:b c'
+        ret = salt.utils.minions.parse_target(mw_tgt)
+        self.assertEqual(ret['pattern'], 'a:b c')
+
+
+class NodegroupCompTest(TestCase):
+    '''
+    Test nodegroup comparisons found in
+    salt.utils.minions.nodgroup_comp()
+    '''
+
+    def test_simple_nodegroup(self):
+        '''
+        Smoke test a very simple nodegroup. No recursion.
+        '''
+        simple_nodegroup = {'group1': 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com'}
+
+        ret = salt.utils.minions.nodegroup_comp('group1', simple_nodegroup)
+        expected_ret = ['L@foo.domain.com,bar.domain.com,baz.domain.com', 'or', 'bl*.domain.com']
+        self.assertListEqual(ret, expected_ret)
+
+    def test_simple_expression_nodegroup(self):
+        '''
+        Smoke test a nodegroup with a simple expression. No recursion.
+        '''
+        simple_nodegroup = {'group1': '[foo,bar,baz].domain.com'}
+
+        ret = salt.utils.minions.nodegroup_comp('group1', simple_nodegroup)
+        expected_ret = ['E@[foo,bar,baz].domain.com']
+        self.assertListEqual(ret, expected_ret)
+
+    def test_simple_recurse(self):
+        '''
+        Test a case where one nodegroup contains a second nodegroup
+        '''
+        referenced_nodegroups = {
+                'group1': 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com',
+                'group2': 'G@os:Debian and N@group1'
+                }
+
+        ret = salt.utils.minions.nodegroup_comp('group2', referenced_nodegroups)
+        expected_ret = [
+                '(',
+                'G@os:Debian',
+                'and',
+                '(',
+                'L@foo.domain.com,bar.domain.com,baz.domain.com',
+                'or',
+                'bl*.domain.com',
+                ')',
+                ')'
+                ]
+        self.assertListEqual(ret, expected_ret)
+
+    def test_circular_nodegroup_reference(self):
+        '''
+        Test to see what happens if A refers to B
+        and B in turn refers back to A
+        '''
+        referenced_nodegroups = {
+                'group1': 'N@group2',
+                'group2': 'N@group1'
+                }
+
+        # If this works, it should also print an error to the console
+        ret = salt.utils.minions.nodegroup_comp('group1', referenced_nodegroups)
+        self.assertEqual(ret, [])

tests/unit/utils/test_stringutils.py

@@ -204,3 +204,238 @@ class StringutilsTestCase(TestCase):
         self.assertTrue(salt.utils.stringutils.expr_match(val, 'foo/*/baz'))
         # Glob non-match
         self.assertFalse(salt.utils.stringutils.expr_match(val, 'foo/*/bar'))
+
+    def test_check_whitelist_blacklist(self):
+        '''
+        Ensure that whitelist matching works on both PY2 and PY3
+        '''
+        whitelist = ['one/two/three', r'web[0-9]']
+        blacklist = ['four/five/six', r'web[5-9]']
+
+        # Tests with string whitelist/blacklist
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_one',
+                whitelist=whitelist[1],
+                blacklist=None,
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_one',
+                whitelist=whitelist[1],
+                blacklist=[],
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web1',
+                whitelist=whitelist[1],
+                blacklist=None,
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web1',
+                whitelist=whitelist[1],
+                blacklist=[],
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=None,
+                blacklist=blacklist[1],
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=[],
+                blacklist=blacklist[1],
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_five',
+                whitelist=None,
+                blacklist=blacklist[1],
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_five',
+                whitelist=[],
+                blacklist=blacklist[1],
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=whitelist[1],
+                blacklist=blacklist[1],
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web4',
+                whitelist=whitelist[1],
+                blacklist=blacklist[1],
+            )
+        )
+
+        # Tests with list whitelist/blacklist
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_one',
+                whitelist=whitelist,
+                blacklist=None,
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_one',
+                whitelist=whitelist,
+                blacklist=[],
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web1',
+                whitelist=whitelist,
+                blacklist=None,
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web1',
+                whitelist=whitelist,
+                blacklist=[],
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=None,
+                blacklist=blacklist,
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=[],
+                blacklist=blacklist,
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_five',
+                whitelist=None,
+                blacklist=blacklist,
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_five',
+                whitelist=[],
+                blacklist=blacklist,
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=whitelist,
+                blacklist=blacklist,
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web4',
+                whitelist=whitelist,
+                blacklist=blacklist,
+            )
+        )
+
+        # Tests with set whitelist/blacklist
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_one',
+                whitelist=set(whitelist),
+                blacklist=None,
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_one',
+                whitelist=set(whitelist),
+                blacklist=set(),
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web1',
+                whitelist=set(whitelist),
+                blacklist=None,
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web1',
+                whitelist=set(whitelist),
+                blacklist=set(),
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=None,
+                blacklist=set(blacklist),
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=set(),
+                blacklist=set(blacklist),
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_five',
+                whitelist=None,
+                blacklist=set(blacklist),
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web_five',
+                whitelist=set(),
+                blacklist=set(blacklist),
+            )
+        )
+        self.assertFalse(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web5',
+                whitelist=set(whitelist),
+                blacklist=set(blacklist),
+            )
+        )
+        self.assertTrue(
+            salt.utils.stringutils.check_whitelist_blacklist(
+                'web4',
+                whitelist=set(whitelist),
+                blacklist=set(blacklist),
+            )
+        )
+
+        # Test with invalid type for whitelist/blacklist
+        self.assertRaises(
+            TypeError,
+            salt.utils.stringutils.check_whitelist_blacklist,
+            'foo', whitelist=123
+        )
+        self.assertRaises(
+            TypeError,
+            salt.utils.stringutils.check_whitelist_blacklist,
+            'foo', blacklist=123
+        )

tests/unit/utils/test_zfs.py

@@ -891,6 +891,17 @@ class ZfsUtilsTestCase(TestCase):
     This class contains a set of functions that test salt.utils.zfs utils
     '''
     ## NOTE: test parameter parsing
+    def test_is_supported(self):
+        '''
+        Test zfs.is_supported method
+        '''
+        for value in [False, True]:
+            with patch('salt.utils.path.which',
+                       MagicMock(return_value=value)):
+                with patch('salt.utils.platform.is_linux',
+                                  MagicMock(return_value=value)):
+                    self.assertEqual(value, zfs.is_supported())
+
     def test_property_data_zpool(self):
         '''
         Test parsing of zpool get output

tests/whitelist.txt

@@ -19,6 +19,7 @@ integration.modules.test_mine
 integration.modules.test_network
 integration.modules.test_ntp
 integration.modules.test_pillar
+integration.modules.test_pip
 integration.modules.test_pkg
 integration.modules.test_publish
 integration.modules.test_service
@@ -36,6 +37,7 @@ integration.runners.test_jobs
 integration.runners.test_salt
 integration.sdb.test_env
 integration.states.test_host
+integration.states.test_pip_state
 integration.states.test_reg
 integration.states.test_renderers
 integration.utils.testprogram