Merge branch '2018.3' into 'fluorine'

Conflicts:
  - .github/CODEOWNERS

.github/CODEOWNERS

@@ -12,9 +12,10 @@
 # This file uses an fnmatch-style matching pattern.
 
 # Team Boto
-salt/**/*boto*                      @saltstack/team-boto
+salt/*/*boto*                       @saltstack/team-boto
 
 # Team Core
+requirements/*                      @saltstack/team-core
 rfcs/*                              @saltstack/team-core
 salt/auth/*                         @saltstack/team-core
 salt/cache/*                        @saltstack/team-core
@@ -25,14 +26,16 @@ salt/daemons/*                      @saltstack/team-core
 salt/pillar/*                       @saltstack/team-core
 salt/loader.py                      @saltstack/team-core
 salt/payload.py                     @saltstack/team-core
-salt/**/master*                     @saltstack/team-core
-salt/**/minion*                     @saltstack/team-core
+salt/master.py                      @saltstack/team-core
+salt/*/master*                      @saltstack/team-core
+salt/minion.py                      @saltstack/team-core
+salt/*/minion*                      @saltstack/team-core
 
 # Team Cloud
 salt/cloud/*                        @saltstack/team-cloud
 salt/utils/openstack/*              @saltstack/team-cloud
 salt/utils/aws.py                   @saltstack/team-cloud
-salt/**/*cloud*                     @saltstack/team-cloud
+salt/*/*cloud*                      @saltstack/team-cloud
 
 # Team NetAPI
 salt/cli/api.py                     @saltstack/team-netapi
@@ -51,18 +54,18 @@ salt/cli/ssh.py                     @saltstack/team-ssh
 salt/client/ssh/*                   @saltstack/team-ssh
 salt/roster/*                       @saltstack/team-ssh
 salt/runners/ssh.py                 @saltstack/team-ssh
-salt/**/thin.py                     @saltstack/team-ssh
+salt/*/thin.py                      @saltstack/team-ssh
 
 # Team State
 salt/state.py                       @saltstack/team-state
 
 # Team SUSE
-salt/**/*btrfs*                     @saltstack/team-suse
-salt/**/*kubernetes*                @saltstack/team-suse
-salt/**/*pkg*                       @saltstack/team-suse
-salt/**/*snapper*                   @saltstack/team-suse
-salt/**/*xfs*                       @saltstack/team-suse
-salt/**/*zypper*                    @saltstack/team-suse
+salt/*/*btrfs*                      @saltstack/team-suse
+salt/*/*kubernetes*                 @saltstack/team-suse
+salt/*/*pkg*                        @saltstack/team-suse
+salt/*/*snapper*                    @saltstack/team-suse
+salt/*/*xfs*                        @saltstack/team-suse
+salt/*/*zypper*                     @saltstack/team-suse
 
 # Team Transport
 salt/transport/*                    @saltstack/team-transport

salt/daemons/__init__.py

@@ -6,7 +6,14 @@ Minion enabling different transports.
 from __future__ import absolute_import, print_function, unicode_literals
 # Import Python Libs
 import sys
-from collections import namedtuple, Iterable, Sequence, Mapping
+
+try:
+    from collections.abc import Iterable, Sequence, Mapping
+except ImportError:
+    from collections import Iterable, Sequence, Mapping
+
+from collections import namedtuple
+
 import logging
 
 # Import Salt Libs

salt/fileserver/__init__.py

@@ -5,12 +5,13 @@ File server pluggable modules and generic backend functions
 
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
-import collections
+
 import errno
 import fnmatch
 import logging
 import os
 import re
+import sys
 import time
 
 # Import salt libs
@@ -23,6 +24,11 @@ import salt.utils.versions
 from salt.utils.args import get_function_argspec as _argspec
 from salt.utils.decorators import ensure_unicode_args
 
+try:
+    from collections.abc import Sequence
+except ImportError:
+    from collections import Sequence
+
 # Import 3rd-party libs
 from salt.ext import six
 
@@ -344,7 +350,7 @@ class Fileserver(object):
                 except AttributeError:
                     back = six.text_type(back).split(',')
 
-        if isinstance(back, collections.Sequence):
+        if isinstance(back, Sequence):
             # The test suite uses an ImmutableList type (based on
             # collections.Sequence) for lists, which breaks this function in
             # the test suite. This normalizes the value from the opts into a
@@ -881,8 +887,6 @@ class FSChan(object):
         cmd = load['cmd'].lstrip('_')
         if cmd in self.cmd_stub:
             return self.cmd_stub[cmd]
-        if cmd == 'file_envs':
-            return self.fs.envs()
         if not hasattr(self.fs, cmd):
             log.error('Malformed request, invalid cmd: %s', load)
             return {}

salt/loader.py

@@ -18,7 +18,6 @@ import functools
 import threading
 import traceback
 import types
-from collections import MutableMapping
 from zipimport import zipimporter
 
 # Import salt libs
@@ -51,6 +50,11 @@ else:
     import imp
     USE_IMPORTLIB = False
 
+try:
+    from collections.abc import MutableMapping
+except ImportError:
+    from collections import MutableMapping
+
 try:
     import pkg_resources
     HAS_PKG_RESOURCES = True

salt/modules/win_service.py

@@ -147,6 +147,30 @@ def _status_wait(service_name, end_time, service_states):
     return info_results
 
 
+def _cmd_quote(cmd):
+    r'''
+    Helper function to properly format the path to the binary for the service
+    Must be wrapped in double quotes to account for paths that have spaces. For
+    example:
+
+    ``"C:\Program Files\Path\to\bin.exe"``
+
+    Args:
+        cmd (str): Full path to the binary
+
+    Returns:
+        str: Properly quoted path to the binary
+    '''
+    # Remove all single and double quotes from the beginning and the end
+    pattern = re.compile('^(\\"|\').*|.*(\\"|\')$')
+    while pattern.match(cmd) is not None:
+        cmd = cmd.strip('"').strip('\'')
+    # Ensure the path to the binary is wrapped in double quotes to account for
+    # spaces in the path
+    cmd = '"{0}"'.format(cmd)
+    return cmd
+
+
 def get_enabled():
     '''
     Return a list of enabled services. Enabled is defined as a service that is
@@ -334,7 +358,7 @@ def info(name):
             None, None, win32service.SC_MANAGER_CONNECT)
     except pywintypes.error as exc:
         raise CommandExecutionError(
-            'Failed to connect to the SCM: {0}'.format(exc[2]))
+            'Failed to connect to the SCM: {0}'.format(exc.strerror))
 
     try:
         handle_svc = win32service.OpenService(
@@ -345,7 +369,7 @@ def info(name):
             win32service.SERVICE_QUERY_STATUS)
     except pywintypes.error as exc:
         raise CommandExecutionError(
-            'Failed To Open {0}: {1}'.format(name, exc[2]))
+            'Failed To Open {0}: {1}'.format(name, exc.strerror))
 
     try:
         config_info = win32service.QueryServiceConfig(handle_svc)
@@ -439,7 +463,8 @@ def start(name, timeout=90):
             .. versionadded:: 2017.7.9, 2018.3.4
 
     Returns:
-        bool: ``True`` if successful, otherwise ``False``
+        bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
+            if the service is already started
 
     CLI Example:
 
@@ -447,9 +472,6 @@ def start(name, timeout=90):
 
         salt '*' service.start <service name>
     '''
-    if status(name):
-        return True
-
     # Set the service to manual if disabled
     if disabled(name):
         modify(name, start_type='Manual')
@@ -457,8 +479,10 @@ def start(name, timeout=90):
     try:
         win32serviceutil.StartService(name)
     except pywintypes.error as exc:
-        raise CommandExecutionError(
-            'Failed To Start {0}: {1}'.format(name, exc[2]))
+        if exc.winerror != 1056:
+            raise CommandExecutionError(
+                'Failed To Start {0}: {1}'.format(name, exc.strerror))
+        log.debug('Service "{0}" is running'.format(name))
 
     srv_status = _status_wait(service_name=name,
                               end_time=time.time() + int(timeout),
@@ -481,7 +505,8 @@ def stop(name, timeout=90):
             .. versionadded:: 2017.7.9, 2018.3.4
 
     Returns:
-        bool: ``True`` if successful, otherwise ``False``
+        bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
+            if the service is already stopped
 
     CLI Example:
 
@@ -492,9 +517,10 @@ def stop(name, timeout=90):
     try:
         win32serviceutil.StopService(name)
     except pywintypes.error as exc:
-        if exc[0] != 1062:
+        if exc.winerror != 1062:
             raise CommandExecutionError(
-                'Failed To Stop {0}: {1}'.format(name, exc[2]))
+                'Failed To Stop {0}: {1}'.format(name, exc.strerror))
+        log.debug('Service "{0}" is not running'.format(name))
 
     srv_status = _status_wait(service_name=name,
                               end_time=time.time() + int(timeout),
@@ -769,7 +795,7 @@ def modify(name,
             win32service.SERVICE_QUERY_CONFIG)
     except pywintypes.error as exc:
         raise CommandExecutionError(
-            'Failed To Open {0}: {1}'.format(name, exc))
+            'Failed To Open {0}: {1}'.format(name, exc.strerror))
 
     config_info = win32service.QueryServiceConfig(handle_svc)
 
@@ -777,7 +803,8 @@ def modify(name,
 
     # Input Validation
     if bin_path is not None:
-        bin_path = bin_path.strip('"')
+        # shlex.quote the path to the binary
+        bin_path = _cmd_quote(bin_path)
         if exe_args is not None:
             bin_path = '{0} {1}'.format(bin_path, exe_args)
         changes['BinaryPath'] = bin_path
@@ -1099,8 +1126,8 @@ def create(name,
     if name in get_all():
         raise CommandExecutionError('Service Already Exists: {0}'.format(name))
 
-    # Input validation
-    bin_path = bin_path.strip('"')
+    # shlex.quote the path to the binary
+    bin_path = _cmd_quote(bin_path)
     if exe_args is not None:
         bin_path = '{0} {1}'.format(bin_path, exe_args)
 
@@ -1188,7 +1215,8 @@ def delete(name, timeout=90):
             .. versionadded:: 2017.7.9, 2018.3.4
 
     Returns:
-        bool: ``True`` if successful, otherwise ``False``
+        bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
+            if the service is not present
 
     CLI Example:
 
@@ -1203,8 +1231,12 @@ def delete(name, timeout=90):
         handle_svc = win32service.OpenService(
             handle_scm, name, win32service.SERVICE_ALL_ACCESS)
     except pywintypes.error as exc:
-        raise CommandExecutionError(
-            'Failed to open {0}. {1}'.format(name, exc.strerror))
+        win32service.CloseServiceHandle(handle_scm)
+        if exc.winerror != 1060:
+            raise CommandExecutionError(
+                'Failed to open {0}. {1}'.format(name, exc.strerror))
+        log.debug('Service "{0}" is not present'.format(name))
+        return True
 
     try:
         win32service.DeleteService(handle_svc)

salt/utils/context.py

@@ -14,7 +14,11 @@ from __future__ import absolute_import, print_function, unicode_literals
 # Import python libs
 import copy
 import threading
-import collections
+try:
+    from collections.abc import MutableMapping
+except ImportError:
+    from collections import MutableMapping
+
 from contextlib import contextmanager
 
 from salt.ext import six
@@ -57,7 +61,7 @@ def func_globals_inject(func, **overrides):
         del func_globals[injected]
 
 
-class ContextDict(collections.MutableMapping):
+class ContextDict(MutableMapping):
     '''
     A context manager that saves some per-thread state globally.
     Intended for use with Tornado's StackContext.
@@ -142,7 +146,7 @@ class ContextDict(collections.MutableMapping):
         return new_obj
 
 
-class ChildContextDict(collections.MutableMapping):
+class ChildContextDict(MutableMapping):
     '''An overrideable child of ContextDict
 
     '''
@@ -190,7 +194,7 @@ class ChildContextDict(collections.MutableMapping):
         self.parent._state.data = self._old_data
 
 
-class NamespacedDictWrapper(collections.MutableMapping, dict):
+class NamespacedDictWrapper(MutableMapping, dict):
     '''
     Create a dict which wraps another dict with a specific prefix of key(s)
 

salt/utils/data.py

@@ -7,12 +7,16 @@ and data structures.
 from __future__ import absolute_import, print_function, unicode_literals
 
 # Import Python libs
-import collections
 import copy
 import fnmatch
 import logging
 import re
 
+try:
+    from collections.abc import Mapping
+except ImportError:
+    from collections import Mapping
+
 # Import Salt libs
 import salt.utils.dictupdate
 import salt.utils.stringutils
@@ -100,7 +104,7 @@ def decode(data, encoding=None, errors='strict', keep=False,
     _decode_func = salt.utils.stringutils.to_unicode \
         if not to_str \
         else salt.utils.stringutils.to_str
-    if isinstance(data, collections.Mapping):
+    if isinstance(data, Mapping):
         return decode_dict(data, encoding, errors, keep, normalize,
                            preserve_dict_class, preserve_tuples, to_str)
     elif isinstance(data, list):
@@ -166,7 +170,7 @@ def decode_dict(data, encoding=None, errors='strict', keep=False,
                 if preserve_tuples \
                 else decode_list(value, encoding, errors, keep, normalize,
                                  preserve_dict_class, preserve_tuples, to_str)
-        elif isinstance(value, collections.Mapping):
+        elif isinstance(value, Mapping):
             value = decode_dict(value, encoding, errors, keep, normalize,
                                 preserve_dict_class, preserve_tuples, to_str)
         else:
@@ -206,7 +210,7 @@ def decode_list(data, encoding=None, errors='strict', keep=False,
                 if preserve_tuples \
                 else decode_list(item, encoding, errors, keep, normalize,
                                  preserve_dict_class, preserve_tuples, to_str)
-        elif isinstance(item, collections.Mapping):
+        elif isinstance(item, Mapping):
             item = decode_dict(item, encoding, errors, keep, normalize,
                                preserve_dict_class, preserve_tuples, to_str)
         else:
@@ -248,7 +252,7 @@ def encode(data, encoding=None, errors='strict', keep=False,
     can be useful for cases where the data passed to this function is likely to
     contain binary blobs.
     '''
-    if isinstance(data, collections.Mapping):
+    if isinstance(data, Mapping):
         return encode_dict(data, encoding, errors, keep,
                            preserve_dict_class, preserve_tuples)
     elif isinstance(data, list):
@@ -307,7 +311,7 @@ def encode_dict(data, encoding=None, errors='strict', keep=False,
                 if preserve_tuples \
                 else encode_list(value, encoding, errors, keep,
                                  preserve_dict_class, preserve_tuples)
-        elif isinstance(value, collections.Mapping):
+        elif isinstance(value, Mapping):
             value = encode_dict(value, encoding, errors, keep,
                                 preserve_dict_class, preserve_tuples)
         else:
@@ -343,7 +347,7 @@ def encode_list(data, encoding=None, errors='strict', keep=False,
                 if preserve_tuples \
                 else encode_list(item, encoding, errors, keep,
                                  preserve_dict_class, preserve_tuples)
-        elif isinstance(item, collections.Mapping):
+        elif isinstance(item, Mapping):
             item = encode_dict(item, encoding, errors, keep,
                                preserve_dict_class, preserve_tuples)
         else:
@@ -424,15 +428,15 @@ def filter_by(lookup_dict,
         if ret is None:
             ret = base_values
 
-        elif isinstance(base_values, collections.Mapping):
-            if not isinstance(ret, collections.Mapping):
+        elif isinstance(base_values, Mapping):
+            if not isinstance(ret, Mapping):
                 raise SaltException(
                     'filter_by default and look-up values must both be '
                     'dictionaries.')
             ret = salt.utils.dictupdate.update(copy.deepcopy(base_values), ret)
 
     if merge:
-        if not isinstance(merge, collections.Mapping):
+        if not isinstance(merge, Mapping):
             raise SaltException(
                 'filter_by merge argument must be a dictionary.')
 

salt/utils/dictupdate.py

@@ -6,7 +6,11 @@ http://stackoverflow.com/a/3233356
 
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
-import collections
+
+try:
+    from collections.abc import Mapping
+except ImportError:
+    from collections import Mapping
 
 # Import 3rd-party libs
 import copy
@@ -35,8 +39,8 @@ def update(dest, upd, recursive_update=True, merge_lists=False):
         When merging lists, duplicate values are removed. Values already
         present in the ``dest`` list are not added from the ``upd`` list.
     '''
-    if (not isinstance(dest, collections.Mapping)) \
-            or (not isinstance(upd, collections.Mapping)):
+    if (not isinstance(dest, Mapping)) \
+            or (not isinstance(upd, Mapping)):
         raise TypeError('Cannot update using non-dict types in dictupdate.update()')
     updkeys = list(upd.keys())
     if not set(list(dest.keys())) & set(updkeys):
@@ -48,8 +52,8 @@ def update(dest, upd, recursive_update=True, merge_lists=False):
                 dest_subkey = dest.get(key, None)
             except AttributeError:
                 dest_subkey = None
-            if isinstance(dest_subkey, collections.Mapping) \
-                    and isinstance(val, collections.Mapping):
+            if isinstance(dest_subkey, Mapping) \
+                    and isinstance(val, Mapping):
                 ret = update(dest_subkey, val, merge_lists=merge_lists)
                 dest[key] = ret
             elif isinstance(dest_subkey, list) \

salt/utils/event.py

@@ -60,7 +60,12 @@ import hashlib
 import logging
 import datetime
 import sys
-from collections import MutableMapping
+
+try:
+    from collections.abc import MutableMapping
+except ImportError:
+    from collections import MutableMapping
+
 from multiprocessing.util import Finalize
 from salt.ext.six.moves import range
 

salt/utils/immutabletypes.py

@@ -11,10 +11,13 @@
 from __future__ import absolute_import, unicode_literals
 
 # Import python libs
-import collections
+try:
+    from collections.abc import Mapping, Sequence, Set
+except ImportError:
+    from collections import Mapping, Sequence, Set
 
 
-class ImmutableDict(collections.Mapping):
+class ImmutableDict(Mapping):
     '''
     An immutable dictionary implementation
     '''
@@ -35,7 +38,7 @@ class ImmutableDict(collections.Mapping):
         return '<{0} {1}>'.format(self.__class__.__name__, repr(self.__obj))
 
 
-class ImmutableList(collections.Sequence):
+class ImmutableList(Sequence):
     '''
     An immutable list implementation
     '''
@@ -62,7 +65,7 @@ class ImmutableList(collections.Sequence):
         return '<{0} {1}>'.format(self.__class__.__name__, repr(self.__obj))
 
 
-class ImmutableSet(collections.Set):
+class ImmutableSet(Set):
     '''
     An immutable set implementation
     '''

salt/utils/lazy.py

@@ -6,9 +6,13 @@ Lazily-evaluated data structures, primarily used by Salt's loader
 # Import Python Libs
 from __future__ import absolute_import, unicode_literals
 import logging
-import collections
 import salt.exceptions
 
+try:
+    from collections.abc import MutableMapping
+except ImportError:
+    from collections import MutableMapping
+
 log = logging.getLogger(__name__)
 
 
@@ -26,7 +30,7 @@ def verify_fun(lazy_obj, fun):
         raise salt.exceptions.CommandExecutionError(lazy_obj.missing_fun_string(fun))
 
 
-class LazyDict(collections.MutableMapping):
+class LazyDict(MutableMapping):
     '''
     A base class of dict which will lazily load keys once they are needed
 

tests/integration/modules/test_service.py

@@ -5,7 +5,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 # Import Salt Testing libs
 from tests.support.case import ModuleCase
-from tests.support.helpers import destructiveTest
+from tests.support.helpers import destructiveTest, flaky
 from tests.support.unit import skipIf
 
 # Import Salt libs
@@ -62,6 +62,7 @@ class ServiceModuleTest(ModuleCase):
                 self.run_function('service.disable', [self.service_name])
         del self.service_name
 
+    @flaky
     def test_service_status_running(self):
         '''
         test service.status execution module

tests/integration/states/test_service.py

@@ -41,6 +41,8 @@ class ServiceTest(ModuleCase, SaltReturnAssertsMixin):
                 self.service_name = 'com.apple.AirPlayXPCHelper'
             self.stopped = ''
             self.running = '[0-9]'
+        elif os_family == 'Windows':
+            self.service_name = 'Spooler'
 
         self.pre_srv_enabled = True if self.service_name in self.run_function('service.get_enabled') else False
         self.post_srv_disable = False
@@ -48,7 +50,7 @@ class ServiceTest(ModuleCase, SaltReturnAssertsMixin):
             self.run_function('service.enable', name=self.service_name)
             self.post_srv_disable = True
 
-        if salt.utils.path.which(cmd_name) is None:
+        if os_family != 'Windows' and salt.utils.path.which(cmd_name) is None:
             self.skipTest('{0} is not installed'.format(cmd_name))
 
     def tearDown(self):

tests/unit/modules/test_win_service.py

@@ -23,6 +23,7 @@ import salt.modules.win_service as win_service
 try:
     WINAPI = True
     import win32serviceutil
+    import pywintypes
 except ImportError:
     WINAPI = False
 
@@ -119,18 +120,38 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
         '''
         mock_true = MagicMock(return_value=True)
         mock_false = MagicMock(return_value=False)
-        mock_info = MagicMock(side_effect=[{'Status': 'Stopped'},
-                                           {'Status': 'Start Pending'},
-                                           {'Status': 'Running'}])
+        mock_info = MagicMock(side_effect=[{'Status': 'Running'}])
 
-        with patch.object(win_service, 'status', mock_true):
+        with patch.object(win32serviceutil, 'StartService', mock_true), \
+                patch.object(win_service, 'disabled', mock_false), \
+                patch.object(win_service, 'info', mock_info):
             self.assertTrue(win_service.start('spongebob'))
 
-        with patch.object(win_service, 'status', mock_false):
-            with patch.object(win32serviceutil, 'StartService', mock_true):
-                with patch.object(win_service, 'info', mock_info):
-                    with patch.object(win_service, 'status', mock_true):
-                        self.assertTrue(win_service.start('spongebob'))
+        mock_info = MagicMock(side_effect=[{'Status': 'Stopped', 'Status_WaitHint': 0},
+                                           {'Status': 'Start Pending', 'Status_WaitHint': 0},
+                                           {'Status': 'Running'}])
+
+        with patch.object(win32serviceutil, 'StartService', mock_true), \
+                patch.object(win_service, 'disabled', mock_false), \
+                patch.object(win_service, 'info', mock_info), \
+                patch.object(win_service, 'status', mock_true):
+            self.assertTrue(win_service.start('spongebob'))
+
+    @skipIf(not WINAPI, 'pywintypes not available')
+    def test_start_already_running(self):
+        '''
+        Test starting a service that is already running
+        '''
+        mock_false = MagicMock(return_value=False)
+        mock_error = MagicMock(
+            side_effect=pywintypes.error(1056,
+                                         'StartService',
+                                         'Service is running'))
+        mock_info = MagicMock(side_effect=[{'Status': 'Running'}])
+        with patch.object(win32serviceutil, 'StartService', mock_error), \
+                 patch.object(win_service, 'disabled', mock_false), \
+                 patch.object(win_service, '_status_wait', mock_info):
+            self.assertTrue(win_service.start('spongebob'))
 
     @skipIf(not WINAPI, 'win32serviceutil not available')
     def test_stop(self):
@@ -141,8 +162,7 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
         mock_false = MagicMock(return_value=False)
         mock_info = MagicMock(side_effect=[{'Status': 'Stopped'}])
 
-        with patch.dict(win_service.__salt__, {'cmd.run': MagicMock(return_value="service was stopped")}), \
-                patch.object(win32serviceutil, 'StopService', mock_true), \
+        with patch.object(win32serviceutil, 'StopService', mock_true), \
                 patch.object(win_service, '_status_wait', mock_info):
             self.assertTrue(win_service.stop('spongebob'))
 
@@ -150,12 +170,25 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
                                            {'Status': 'Stop Pending', 'Status_WaitHint': 0},
                                            {'Status': 'Stopped'}])
 
-        with patch.dict(win_service.__salt__, {'cmd.run': MagicMock(return_value="service was stopped")}), \
-                patch.object(win32serviceutil, 'StopService', mock_true), \
+        with patch.object(win32serviceutil, 'StopService', mock_true), \
                 patch.object(win_service, 'info', mock_info), \
                 patch.object(win_service, 'status', mock_false):
             self.assertTrue(win_service.stop('spongebob'))
 
+    @skipIf(not WINAPI, 'pywintypes not available')
+    def test_stop_not_running(self):
+        '''
+        Test stopping a service that is already stopped
+        '''
+        mock_error = MagicMock(
+            side_effect=pywintypes.error(1062,
+                                         'StopService',
+                                         'Service is not running'))
+        mock_info = MagicMock(side_effect=[{'Status': 'Stopped'}])
+        with patch.object(win32serviceutil, 'StopService', mock_error), \
+                patch.object(win_service, '_status_wait', mock_info):
+            self.assertTrue(win_service.stop('spongebob'))
+
     def test_restart(self):
         '''
             Test to restart the named service
@@ -266,3 +299,26 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
         with patch.object(win_service, 'enabled', mock):
             self.assertTrue(win_service.disabled('spongebob'))
             self.assertFalse(win_service.disabled('squarepants'))
+
+    def test_cmd_quote(self):
+        '''
+        Make sure the command gets quoted correctly
+        '''
+        # Should always return command wrapped in double quotes
+        expected = r'"C:\Program Files\salt\test.exe"'
+
+        # test no quotes
+        bin_path = r'C:\Program Files\salt\test.exe'
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)
+
+        # test single quotes
+        bin_path = r"'C:\Program Files\salt\test.exe'"
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)
+
+        # test double quoted single quotes
+        bin_path = '"\'C:\\Program Files\\salt\\test.exe\'"'
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)
+
+        # test single quoted, double quoted, single quotes
+        bin_path = "\'\"\'C:\\Program Files\\salt\\test.exe\'\"\'"
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)

tests/unit/states/test_boto_vpc.py

@@ -5,6 +5,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 import random
 import string
 import os.path
+import sys
 
 # Import Salt Testing libs
 from tests.support.mixins import LoaderModuleMockMixin
@@ -133,6 +134,7 @@ class BotoVpcTestCase(BotoVpcStateTestCaseBase, BotoVpcTestCaseMixin):
     TestCase for salt.states.boto_vpc state.module
     '''
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_present_when_vpc_does_not_exist(self):
         '''
@@ -144,6 +146,7 @@ class BotoVpcTestCase(BotoVpcStateTestCaseBase, BotoVpcTestCaseMixin):
         self.assertTrue(vpc_present_result['result'])
         self.assertEqual(vpc_present_result['changes']['new']['vpc']['state'], 'available')
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_present_when_vpc_exists(self):
         vpc = self._create_vpc(name='test')
@@ -159,6 +162,7 @@ class BotoVpcTestCase(BotoVpcStateTestCaseBase, BotoVpcTestCaseMixin):
             self.assertFalse(vpc_present_result['result'])
             self.assertTrue('Mocked error' in vpc_present_result['comment'])
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_absent_when_vpc_does_not_exist(self):
         '''
@@ -169,6 +173,7 @@ class BotoVpcTestCase(BotoVpcStateTestCaseBase, BotoVpcTestCaseMixin):
         self.assertTrue(vpc_absent_result['result'])
         self.assertEqual(vpc_absent_result['changes'], {})
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_absent_when_vpc_exists(self):
         vpc = self._create_vpc(name='test')
@@ -197,6 +202,7 @@ class BotoVpcResourceTestCaseMixin(BotoVpcTestCaseMixin):
         _create = getattr(self, '_create_' + self.resource_type)
         _create(vpc_id=vpc_id, name=name, **self.extra_kwargs)
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_present_when_resource_does_not_exist(self):
         '''
@@ -212,6 +218,7 @@ class BotoVpcResourceTestCaseMixin(BotoVpcTestCaseMixin):
         exists = self.funcs['boto_vpc.resource_exists'](self.resource_type, 'test').get('exists')
         self.assertTrue(exists)
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_present_when_resource_exists(self):
         vpc = self._create_vpc(name='test')
@@ -233,6 +240,7 @@ class BotoVpcResourceTestCaseMixin(BotoVpcTestCaseMixin):
             self.assertFalse(resource_present_result['result'])
             self.assertTrue('Mocked error' in resource_present_result['comment'])
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_absent_when_resource_does_not_exist(self):
         '''
@@ -243,6 +251,7 @@ class BotoVpcResourceTestCaseMixin(BotoVpcTestCaseMixin):
         self.assertTrue(resource_absent_result['result'])
         self.assertEqual(resource_absent_result['changes'], {})
 
+    @skipIf(sys.version_info > (3, 6), 'Disabled for 3.7+ pending https://github.com/spulec/moto/issues/1706.')
     @mock_ec2_deprecated
     def test_absent_when_resource_exists(self):
         vpc = self._create_vpc(name='test')

tests/whitelist.txt

@@ -39,7 +39,6 @@ integration.modules.test_sysmod
 integration.modules.test_system
 integration.modules.test_test
 integration.modules.test_useradd
-integration.modules.test_autoruns
 integration.modules.test_win_dns_client
 integration.modules.test_win_firewall
 integration.modules.test_win_pkg
@@ -56,6 +55,7 @@ integration.states.test_pkg
 integration.states.test_reg
 integration.states.test_renderers
 integration.states.test_file
+integration.states.test_service
 integration.states.test_user
 integration.utils.testprogram
 integration.wheel.test_client