Merge pull request #47010 from terminalmage/issue40146

Rewrite file.patch state

doc/topics/releases/fluorine.rst

@@ -48,6 +48,20 @@ serialized. See the documentation for the new ``serializer_opts`` option in the
 information.
 
 
+:py:func:`file.patch <salt.sates.file.patch>` State Rewritten
+-------------------------------------------------------------
+
+The :py:func:`file.patch <salt.sates.file.patch>` state has been rewritten with
+several new features:
+
+- Patch sources can now be remote files instead of only ``salt://`` URLs
+- Multi-file patches are now supported
+- Patch files can be templated
+
+In addition, it is no longer necessary to specify what the hash of the patched
+file should be.
+
+
 Deprecations
 ------------
 

salt/modules/file.py

@@ -4080,7 +4080,7 @@ def get_managed(
         if parsed_scheme == 'salt':
             source_sum = __salt__['cp.hash_file'](source, saltenv)
             if not source_sum:
-                return '', {}, 'Source file {0} not found'.format(source)
+                return '', {}, 'Source file {0} not found in saltenv \'{1}\''.format(source, saltenv)
         elif not source_hash and unix_local_source:
             source_sum = _get_local_file_source_sum(parsed_path)
         elif not source_hash and source.startswith(os.sep):

salt/states/file.py

@@ -266,6 +266,7 @@ For example:
 
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
+import copy
 import difflib
 import itertools
 import logging
@@ -294,7 +295,7 @@ import salt.utils.templates
 import salt.utils.url
 import salt.utils.versions
 from salt.utils.locales import sdecode
-from salt.exceptions import CommandExecutionError, SaltInvocationError
+from salt.exceptions import CommandExecutionError
 from salt.state import get_accumulator_dir as _get_accumulator_dir
 
 if salt.utils.platform.is_windows():
@@ -314,6 +315,10 @@ log = logging.getLogger(__name__)
 COMMENT_REGEX = r'^([[:space:]]*){0}[[:space:]]?'
 __NOT_FOUND = object()
 
+__func_alias__ = {
+    'copy_': 'copy',
+}
+
 
 def _get_accumulator_filepath():
     '''
@@ -5155,120 +5160,426 @@ def prepend(name,
 
 def patch(name,
           source=None,
+          source_hash=None,
+          source_hash_name=None,
+          skip_verify=False,
+          template=None,
+          context=None,
+          defaults=None,
           options='',
-          dry_run_first=True,
+          reject_file=None,
+          strip=None,
+          saltenv=None,
           **kwargs):
     '''
-    Ensure that a patch has been applied to the specified file
+    Ensure that a patch has been applied to the specified file or directory
+
+    .. versionchanged:: Fluorine
+        The ``hash`` and ``dry_run_first`` options are now ignored, as the
+        logic which determines whether or not the patch has already been
+        applied no longer requires them. Additionally, this state now supports
+        patch files that modify more than one file. To use these sort of
+        patches, specify a directory (and, if necessary, the ``strip`` option)
+        instead of a file.
 
     .. note::
-        A suitable ``patch`` executable must be available on the minion
+        A suitable ``patch`` executable must be available on the minion. Also,
+        keep in mind that the pre-check this state does to determine whether or
+        not changes need to be made will create a temp file and send all patch
+        output to that file. This means that, in the event that the patch would
+        not have applied cleanly, the comment included in the state results will
+        reference a temp file that will no longer exist once the state finishes
+        running.
 
     name
-        The file to which the patch should be applied
+        The file or directory to which the patch should be applied
 
     source
-        The source patch to download to the minion, this source file must be
+        The patch file to apply
-        hosted on the salt master server. If the file is located in the
-        directory named spam, and is called eggs, the source string is
-        salt://spam/eggs. A source is required.
 
-    hash
+        .. versionchanged:: Fluorine
-        The hash of the patched file. If the hash of the target file matches
+            The source can now be from any file source supported by Salt
-        this value then the patch is assumed to have been applied. For versions
+            (``salt://``, ``http://``, ``https://``, ``ftp://``, etc.).
-        2016.11.4 and newer, the hash can be specified without an accompanying
+            Templating is also now supported.
-        hash type (e.g. ``e138491e9d5b97023cea823fe17bac22``), but for earlier
+
-        releases it is necessary to also specify the hash type in the format
+    source_hash
-        ``<hash_type>:<hash_value>`` (e.g.
+        Works the same way as in :py:func:`file.managed
-        ``md5:e138491e9d5b97023cea823fe17bac22``).
+        <salt.states.file.managed>`.
+
+        .. versionadded:: Fluorine
+
+    source_hash_name
+        Works the same way as in :py:func:`file.managed
+        <salt.states.file.managed>`
+
+        .. versionadded:: Fluorine
+
+    skip_verify
+        Works the same way as in :py:func:`file.managed
+        <salt.states.file.managed>`
+
+        .. versionadded:: Fluorine
+
+    template
+        Works the same way as in :py:func:`file.managed
+        <salt.states.file.managed>`
+
+        .. versionadded:: Fluorine
+
+    context
+        Works the same way as in :py:func:`file.managed
+        <salt.states.file.managed>`
+
+        .. versionadded:: Fluorine
+
+    defaults
+        Works the same way as in :py:func:`file.managed
+        <salt.states.file.managed>`
+
+        .. versionadded:: Fluorine
 
     options
-        Extra options to pass to patch.
+        Extra options to pass to patch. This should not be necessary in most
+        cases.
 
-    dry_run_first : ``True``
+        .. note::
-        Run patch with ``--dry-run`` first to check if it will apply cleanly.
+            For best results, short opts should be separate from one another.
+            The ``-N`` and ``-r``, and ``-o`` options are used internally by
+            this state and cannot be used here. Additionally, instead of using
+            ``-pN`` or ``--strip=N``, use the ``strip`` option documented
+            below.
+
+    reject_file
+        If specified, any rejected hunks will be written to this file. If not
+        specified, then they will be written to a temp file which will be
+        deleted when the state finishes running.
+
+        .. important::
+            The parent directory must exist. Also, this will overwrite the file
+            if it is already present.
+
+        .. versionadded:: Fluorine
+
+    strip
+        Number of directories to strip from paths in the patch file. For
+        example, using the below SLS would instruct Salt to use ``-p1`` when
+        applying the patch:
+
+        .. code-block:: yaml
+
+            /etc/myfile.conf:
+              file.patch:
+                - source: salt://myfile.patch
+                - strip: 1
+
+        .. versionadded:: Fluorine
+            In previous versions, ``-p1`` would need to be passed as part of
+            the ``options`` value.
 
     saltenv
         Specify the environment from which to retrieve the patch file indicated
         by the ``source`` parameter. If not provided, this defaults to the
         environment from which the state is being executed.
 
+        .. note::
+            Ignored when the patch file is from a non-``salt://`` source.
+
     **Usage:**
 
     .. code-block:: yaml
 
-        # Equivalent to ``patch --forward /opt/file.txt file.patch``
+        # Equivalent to ``patch --forward /opt/myfile.txt myfile.patch``
-        /opt/file.txt:
+        /opt/myfile.txt:
           file.patch:
-            - source: salt://file.patch
+            - source: salt://myfile.patch
-            - hash: e138491e9d5b97023cea823fe17bac22
-
-    .. note::
-        For minions running version 2016.11.3 or older, the hash in the example
-        above would need to be specified with the hash type (i.e.
-        ``md5:e138491e9d5b97023cea823fe17bac22``).
     '''
-    hash_ = kwargs.pop('hash', None)
-
-    if 'env' in kwargs:
-        # "env" is not supported; Use "saltenv".
-        kwargs.pop('env')
-
-    name = os.path.expanduser(name)
-
     ret = {'name': name, 'changes': {}, 'result': False, 'comment': ''}
+
+    if not salt.utils.path.which('patch'):
+        ret['comment'] = 'patch executable not found on minion'
+        return ret
+
+    # is_dir should be defined if we proceed past the if/else block below, but
+    # just in case, avoid a NameError.
+    is_dir = False
+
     if not name:
-        return _error(ret, 'Must provide name to file.patch')
+        ret['comment'] = 'A file/directory to be patched is required'
-    check_res, check_msg = _check_file(name)
+        return ret
-    if not check_res:
+    else:
-        return _error(ret, check_msg)
+        try:
-    if not source:
+            name = os.path.expanduser(name)
-        return _error(ret, 'Source is required')
+        except Exception:
-    if hash_ is None:
+            ret['comment'] = 'Invalid path \'{0}\''.format(name)
-        return _error(ret, 'Hash is required')
+            return ret
+        else:
+            if not os.path.isabs(name):
+                ret['comment'] = '{0} is not an absolute path'.format(name)
+                return ret
+            elif not os.path.exists(name):
+                ret['comment'] = '{0} does not exist'.format(name)
+                return ret
+            else:
+                is_dir = os.path.isdir(name)
+
+    for deprecated_arg in ('hash', 'dry_run_first'):
+        if deprecated_arg in kwargs:
+            ret.setdefault('warnings', []).append(
+                'The \'{0}\' argument is no longer used and has been '
+                'ignored.'.format(deprecated_arg)
+            )
+
+    if reject_file is not None:
+        try:
+            reject_file_parent = os.path.dirname(reject_file)
+        except Exception:
+            ret['comment'] = 'Invalid path \'{0}\' for reject_file'.format(
+                reject_file
+            )
+            return ret
+        else:
+            if not os.path.isabs(reject_file_parent):
+                ret['comment'] = '\'{0}\' is not an absolute path'.format(
+                    reject_file
+                )
+                return ret
+            elif not os.path.isdir(reject_file_parent):
+                ret['comment'] = (
+                    'Parent directory for reject_file \'{0}\' either does '
+                    'not exist, or is not a directory'.format(reject_file)
+                )
+                return ret
+
+    sanitized_options = []
+    options = salt.utils.args.shlex_split(options)
+    index = 0
+    max_index = len(options) - 1
+    # Not using enumerate here because we may need to consume more than one
+    # option if --strip is used.
+    blacklisted_options = []
+    while index <= max_index:
+        option = options[index]
+        if not isinstance(option, six.string_types):
+            option = six.text_type(option)
+
+        for item in ('-N', '--forward', '-r', '--reject-file', '-o', '--output'):
+            if option.startswith(item):
+                blacklisted = option
+                break
+        else:
+            blacklisted = None
+
+        if blacklisted is not None:
+            blacklisted_options.append(blacklisted)
+
+        if option.startswith('-p'):
+            try:
+                strip = int(option[2:])
+            except Exception:
+                ret['comment'] = (
+                    'Invalid format for \'-p\' CLI option. Consider using '
+                    'the \'strip\' option for this state.'
+                )
+                return ret
+        elif option.startswith('--strip'):
+            if '=' in option:
+                # Assume --strip=N
+                try:
+                    strip = int(option.rsplit('=', 1)[-1])
+                except Exception:
+                    ret['comment'] = (
+                        'Invalid format for \'-strip\' CLI option. Consider '
+                        'using the \'strip\' option for this state.'
+                    )
+                    return ret
+            else:
+                # Assume --strip N and grab the next option in the list
+                try:
+                    strip = int(options[index + 1])
+                except Exception:
+                    ret['comment'] = (
+                        'Invalid format for \'-strip\' CLI option. Consider '
+                        'using the \'strip\' option for this state.'
+                    )
+                    return ret
+                else:
+                    # We need to increment again because we grabbed the next
+                    # option in the list.
+                    index += 1
+        else:
+            sanitized_options.append(option)
+
+        # Increment the index
+        index += 1
+
+    if blacklisted_options:
+        ret['comment'] = (
+            'The following CLI options are not allowed: {0}'.format(
+                ', '.join(blacklisted_options)
+            )
+        )
+        return ret
+
+    options = sanitized_options
 
     try:
-        if hash_ and __salt__['file.check_hash'](name, hash_):
+        source_match = __salt__['file.source_list'](source,
-            ret['result'] = True
+                                                    source_hash,
-            ret['comment'] = 'Patch is already applied'
+                                                    __env__)[0]
-            return ret
+    except CommandExecutionError as exc:
-    except (SaltInvocationError, ValueError) as exc:
-        ret['comment'] = exc.__str__()
-        return ret
-
-    # get cached file or copy it to cache
-    cached_source_path = __salt__['cp.cache_file'](source, __env__)
-    if not cached_source_path:
-        ret['comment'] = ('Unable to cache {0} from saltenv \'{1}\''
-                          .format(source, __env__))
-        return ret
-
-    log.debug(
-        'State patch.applied cached source %s -> %s',
-        source, cached_source_path
-    )
-
-    if dry_run_first or __opts__['test']:
-        ret['changes'] = __salt__['file.patch'](
-            name, cached_source_path, options=options, dry_run=True
-        )
-        if __opts__['test']:
-            ret['comment'] = 'File {0} will be patched'.format(name)
-            ret['result'] = None
-            return ret
-        if ret['changes']['retcode'] != 0:
-            return ret
-
-    ret['changes'] = __salt__['file.patch'](
-        name, cached_source_path, options=options
-    )
-    ret['result'] = ret['changes']['retcode'] == 0
-    # No need to check for SaltInvocationError or ValueError this time, since
-    # these exceptions would have been caught above.
-    if ret['result'] and hash_ and not __salt__['file.check_hash'](name, hash_):
         ret['result'] = False
-        ret['comment'] = 'Hash mismatch after patch was applied'
+        ret['comment'] = exc.strerror
-    return ret
+        return ret
+    else:
+        # Passing the saltenv to file.managed to pull down the patch file is
+        # not supported, because the saltenv is already being passed via the
+        # state compiler and this would result in two values for that argument
+        # (and a traceback). Therefore, we will add the saltenv to the source
+        # URL to ensure we pull the file from the correct environment.
+        if saltenv is not None:
+            source_match_url, source_match_saltenv = \
+                salt.utils.url.parse(source_match)
+            if source_match_url.startswith('salt://'):
+                if source_match_saltenv is not None \
+                        and source_match_saltenv != saltenv:
+                    ret.setdefault('warnings', []).append(
+                        'Ignoring \'saltenv\' option in favor of saltenv '
+                        'included in the source URL.'
+                    )
+                else:
+                    source_match += '?saltenv={0}'.format(saltenv)
+
+    cleanup = []
+
+    try:
+        patch_file = salt.utils.files.mkstemp()
+        cleanup.append(patch_file)
+
+        try:
+            orig_test = __opts__['test']
+            __opts__['test'] = False
+            sys.modules[__salt__['test.ping'].__module__].__opts__['test'] = False
+            result = managed(patch_file,
+                             source=source_match,
+                             source_hash=source_hash,
+                             source_hash_name=source_hash_name,
+                             skip_verify=skip_verify,
+                             template=template,
+                             context=context,
+                             defaults=defaults)
+        except Exception as exc:
+            msg = 'Failed to cache patch file {0}: {1}'.format(
+                salt.utils.url.redact_http_basic_auth(source_match),
+                exc
+            )
+            log.exception(msg)
+            ret['comment'] = msg
+            return ret
+        else:
+            log.debug('file.managed: %s', result)
+        finally:
+            __opts__['test'] = orig_test
+            sys.modules[__salt__['test.ping'].__module__].__opts__['test'] = orig_test
+
+        if not result['result']:
+            log.debug(
+                'failed to download %s',
+                salt.utils.url.redact_http_basic_auth(source_match)
+            )
+            return result
+
+        def _patch(patch_file, options=None, dry_run=False):
+            patch_opts = copy.copy(sanitized_options)
+            if options is not None:
+                patch_opts.extend(options)
+            return __salt__['file.patch'](
+                name,
+                patch_file,
+                options=patch_opts,
+                dry_run=dry_run)
+
+        if reject_file is not None:
+            patch_rejects = reject_file
+        else:
+            # No rejects file specified, create a temp file
+            patch_rejects = salt.utils.files.mkstemp()
+            cleanup.append(patch_rejects)
+
+        patch_output = salt.utils.files.mkstemp()
+        cleanup.append(patch_output)
+
+        # Older patch releases can only write patch output to regular files,
+        # meaning that /dev/null can't be relied on. Also, if we ever want this
+        # to work on Windows with patch.exe, /dev/null is a non-starter.
+        # Therefore, redirect all patch output to a temp file, which we will
+        # then remove.
+        patch_opts = ['-N', '-r', patch_rejects, '-o', patch_output]
+        if is_dir and strip is not None:
+            patch_opts.append('-p{0}'.format(strip))
+
+        pre_check = _patch(patch_file, patch_opts)
+        if pre_check['retcode'] != 0:
+            # Try to reverse-apply hunks from rejects file using a dry-run.
+            # If this returns a retcode of 0, we know that the patch was
+            # already applied. Rejects are written from the base of the
+            # directory, so the strip option doesn't apply here.
+            reverse_pass = _patch(patch_rejects, ['-R', '-f'], dry_run=True)
+            already_applied = reverse_pass['retcode'] == 0
+
+            if already_applied:
+                ret['comment'] = 'Patch was already applied'
+                ret['result'] = True
+                return ret
+            else:
+                ret['comment'] = (
+                    'Patch would not apply cleanly, no changes made. Results '
+                    'of dry-run are below.'
+                )
+                if reject_file is None:
+                    ret['comment'] += (
+                        ' Run state again using the reject_file option to '
+                        'save rejects to a persistent file.'
+                    )
+                opts = copy.copy(__opts__)
+                opts['color'] = False
+                ret['comment'] += '\n\n' + salt.output.out_format(
+                    pre_check,
+                    'nested',
+                    opts,
+                    nested_indent=14)
+                return ret
+
+        if __opts__['test']:
+            ret['result'] = None
+            ret['comment'] = 'The patch would be applied'
+            ret['changes'] = pre_check
+            return ret
+
+        # If we've made it here, the patch should apply cleanly
+        patch_opts = []
+        if is_dir and strip is not None:
+            patch_opts.append('-p{0}'.format(strip))
+        ret['changes'] = _patch(patch_file, patch_opts)
+
+        if ret['changes']['retcode'] == 0:
+            ret['comment'] = 'Patch successfully applied'
+            ret['result'] = True
+        else:
+            ret['comment'] = 'Failed to apply patch'
+
+        return ret
+
+    finally:
+        # Clean up any temp files
+        for path in cleanup:
+            try:
+                os.remove(path)
+            except OSError as exc:
+                if exc.errno != os.errno.ENOENT:
+                    log.error(
+                        'file.patch: Failed to remove temp file %s: %s',
+                        path, exc
+                    )
 
 
 def touch(name, atime=None, mtime=None, makedirs=False):
@@ -5343,17 +5654,16 @@ def touch(name, atime=None, mtime=None, makedirs=False):
     return ret
 
 
-def copy(
+def copy_(name,
-        name,
+          source,
-        source,
+          force=False,
-        force=False,
+          makedirs=False,
-        makedirs=False,
+          preserve=False,
-        preserve=False,
+          user=None,
-        user=None,
+          group=None,
-        group=None,
+          mode=None,
-        mode=None,
+          subdir=False,
-        subdir=False,
+          **kwargs):
-        **kwargs):
     '''
     If the file defined by the ``source`` option exists on the minion, copy it
     to the named path. The file will not be overwritten if it already exists,

tests/integration/files/file/base/patches/all.patch

@@ -0,0 +1,24 @@
+diff -ur a/foo/bar/math.txt b/foo/bar/math.txt
+--- a/foo/bar/math.txt	2018-04-09 18:43:52.883205365 -0500
++++ b/foo/bar/math.txt	2018-04-09 18:44:58.525061654 -0500
+@@ -1,3 +1,3 @@
+-Five plus five is ten
++5 + 5 = 10
+
+-Four squared is sixteen
++4² = 16
+diff -ur a/foo/numbers.txt b/foo/numbers.txt
+--- a/foo/numbers.txt	2018-04-09 18:43:58.014272504 -0500
++++ b/foo/numbers.txt	2018-04-09 18:44:46.487905044 -0500
+@@ -1,7 +1,7 @@
+-one
+-two
+ three
++two
++one
+
+-1
+-2
+ 3
++2
++1

tests/integration/files/file/base/patches/all.patch.jinja

@@ -0,0 +1,24 @@
+diff -ur a/foo/bar/math.txt b/foo/bar/math.txt
+--- a/foo/bar/math.txt	2018-04-09 18:43:52.883205365 -0500
++++ b/foo/bar/math.txt	2018-04-09 18:44:58.525061654 -0500
+@@ -1,3 +1,3 @@
+-Five plus five is ten
++5 + 5 = {{ ten }}
+
+-Four squared is sixteen
++4² = 16
+diff -ur a/foo/numbers.txt b/foo/numbers.txt
+--- a/foo/numbers.txt	2018-04-09 18:43:58.014272504 -0500
++++ b/foo/numbers.txt	2018-04-09 18:44:46.487905044 -0500
+@@ -1,7 +1,7 @@
+-one
+-two
+ three
++{{ two }}
++one
+
+-1
+-2
+ 3
++2
++1

tests/integration/files/file/base/patches/math.patch

@@ -0,0 +1,8 @@
+--- a/foo/bar/math.txt	2018-04-09 18:43:52.883205365 -0500
++++ b/foo/bar/math.txt	2018-04-09 18:44:58.525061654 -0500
+@@ -1,3 +1,3 @@
+-Five plus five is ten
++5 + 5 = 10
+
+-Four squared is sixteen
++4² = 16

tests/integration/files/file/base/patches/math.patch.jinja

@@ -0,0 +1,8 @@
+--- a/foo/bar/math.txt	2018-04-09 18:43:52.883205365 -0500
++++ b/foo/bar/math.txt	2018-04-09 18:44:58.525061654 -0500
+@@ -1,3 +1,3 @@
+-Five plus five is ten
++5 + 5 = {{ ten }}
+
+-Four squared is sixteen
++4² = 16

tests/integration/files/file/base/patches/numbers.patch

@@ -0,0 +1,14 @@
+--- a/foo/numbers.txt	2018-04-09 18:43:58.014272504 -0500
++++ b/foo/numbers.txt	2018-04-09 18:44:46.487905044 -0500
+@@ -1,7 +1,7 @@
+-one
+-two
+ three
++two
++one
+
+-1
+-2
+ 3
++2
++1

tests/integration/files/file/base/patches/numbers.patch.jinja

@@ -0,0 +1,14 @@
+--- a/foo/numbers.txt	2018-04-09 18:43:58.014272504 -0500
++++ b/foo/numbers.txt	2018-04-09 18:44:46.487905044 -0500
+@@ -1,7 +1,7 @@
+-one
+-two
+ three
++{{ two }}
++one
+
+-1
+-2
+ 3
++2
++1

tests/integration/states/test_file.py

@@ -1883,39 +1883,6 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin):
             if os.path.isfile(tmp_file_append):
                 os.remove(tmp_file_append)
 
-    def do_patch(self, patch_name='hello', src='Hello\n'):
-        if not self.run_function('cmd.has_exec', ['patch']):
-            self.skipTest('patch is not installed')
-        src_file = os.path.join(TMP, 'src.txt')
-        with salt.utils.files.fopen(src_file, 'w+') as fp:
-            fp.write(src)
-        ret = self.run_state(
-            'file.patch',
-            name=src_file,
-            source='salt://{0}.patch'.format(patch_name),
-            hash='md5=f0ef7081e1539ac00ef5b761b4fb01b3',
-        )
-        return src_file, ret
-
-    def test_patch(self):
-        src_file, ret = self.do_patch()
-        self.assertSaltTrueReturn(ret)
-        with salt.utils.files.fopen(src_file) as fp:
-            self.assertEqual(fp.read(), 'Hello world\n')
-
-    def test_patch_hash_mismatch(self):
-        src_file, ret = self.do_patch('hello_dolly')
-        self.assertSaltFalseReturn(ret)
-        self.assertInSaltComment(
-            'Hash mismatch after patch was applied',
-            ret
-        )
-
-    def test_patch_already_applied(self):
-        src_file, ret = self.do_patch(src='Hello world\n')
-        self.assertSaltTrueReturn(ret)
-        self.assertInSaltComment('Patch is already applied', ret)
-
     def test_issue_2401_file_comment(self):
         # Get a path to the temporary file
         tmp_file = os.path.join(TMP, 'issue-2041-comment.txt')
@@ -3815,3 +3782,591 @@ class RemoteFileTest(ModuleCase, SaltReturnAssertsMixin):
                              skip_verify=True)
         log.debug('ret = %s', ret)
         self.assertSaltTrueReturn(ret)
+
+
+@skipIf(not salt.utils.path.which('patch'), 'patch is not installed')
+class PatchTest(ModuleCase, SaltReturnAssertsMixin):
+
+    @classmethod
+    def setUpClass(cls):
+        cls.webserver = Webserver()
+        cls.webserver.start()
+
+        cls.numbers_patch_name = 'numbers.patch'
+        cls.math_patch_name = 'math.patch'
+        cls.all_patch_name = 'all.patch'
+        cls.numbers_patch_template_name = cls.numbers_patch_name + '.jinja'
+        cls.math_patch_template_name = cls.math_patch_name + '.jinja'
+        cls.all_patch_template_name = cls.all_patch_name + '.jinja'
+
+        cls.numbers_patch_path = 'patches/' + cls.numbers_patch_name
+        cls.math_patch_path = 'patches/' + cls.math_patch_name
+        cls.all_patch_path = 'patches/' + cls.all_patch_name
+        cls.numbers_patch_template_path = \
+            'patches/' + cls.numbers_patch_template_name
+        cls.math_patch_template_path = \
+            'patches/' + cls.math_patch_template_name
+        cls.all_patch_template_path = \
+            'patches/' + cls.all_patch_template_name
+
+        cls.numbers_patch = 'salt://' + cls.numbers_patch_path
+        cls.math_patch = 'salt://' + cls.math_patch_path
+        cls.all_patch = 'salt://' + cls.all_patch_path
+        cls.numbers_patch_template = 'salt://' + cls.numbers_patch_template_path
+        cls.math_patch_template = 'salt://' + cls.math_patch_template_path
+        cls.all_patch_template = 'salt://' + cls.all_patch_template_path
+
+        cls.numbers_patch_http = cls.webserver.url(cls.numbers_patch_path)
+        cls.math_patch_http = cls.webserver.url(cls.math_patch_path)
+        cls.all_patch_http = cls.webserver.url(cls.all_patch_path)
+        cls.numbers_patch_template_http = \
+            cls.webserver.url(cls.numbers_patch_template_path)
+        cls.math_patch_template_http = \
+            cls.webserver.url(cls.math_patch_template_path)
+        cls.all_patch_template_http = \
+            cls.webserver.url(cls.all_patch_template_path)
+
+        patches_dir = os.path.join(FILES, 'file', 'base', 'patches')
+        cls.numbers_patch_hash = salt.utils.hashutils.get_hash(
+            os.path.join(patches_dir, cls.numbers_patch_name)
+        )
+        cls.math_patch_hash = salt.utils.hashutils.get_hash(
+            os.path.join(patches_dir, cls.math_patch_name)
+        )
+        cls.all_patch_hash = salt.utils.hashutils.get_hash(
+            os.path.join(patches_dir, cls.all_patch_name)
+        )
+        cls.numbers_patch_template_hash = salt.utils.hashutils.get_hash(
+            os.path.join(patches_dir, cls.numbers_patch_template_name)
+        )
+        cls.math_patch_template_hash = salt.utils.hashutils.get_hash(
+            os.path.join(patches_dir, cls.math_patch_template_name)
+        )
+        cls.all_patch_template_hash = salt.utils.hashutils.get_hash(
+            os.path.join(patches_dir, cls.all_patch_template_name)
+        )
+
+        cls.context = {'two': 'two', 'ten': 10}
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.webserver.stop()
+
+    def setUp(self):
+        '''
+        Create a new unpatched set of files
+        '''
+        self.base_dir = tempfile.mkdtemp(dir=TMP)
+        os.makedirs(os.path.join(self.base_dir, 'foo', 'bar'))
+        self.numbers_file = os.path.join(self.base_dir, 'foo', 'numbers.txt')
+        self.math_file = os.path.join(self.base_dir, 'foo', 'bar', 'math.txt')
+        with salt.utils.files.fopen(self.numbers_file, 'w') as fp_:
+            fp_.write(textwrap.dedent('''\
+                one
+                two
+                three
+
+                1
+                2
+                3
+                '''))
+        with salt.utils.files.fopen(self.math_file, 'w') as fp_:
+            fp_.write(textwrap.dedent('''\
+                Five plus five is ten
+
+                Four squared is sixteen
+                '''))
+
+        self.addCleanup(shutil.rmtree, self.base_dir, ignore_errors=True)
+
+    def test_patch_single_file(self):
+        '''
+        Test file.patch using a patch applied to a single file
+        '''
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run the state, should succeed and there should be a message about
+        # a partially-applied hunk.
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_directory(self):
+        '''
+        Test file.patch using a patch applied to a directory, with changes
+        spanning multiple files.
+        '''
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            strip=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run the state, should succeed and there should be a message about
+        # a partially-applied hunk.
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            strip=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_strip_parsing(self):
+        '''
+        Test that we successfuly parse -p/--strip when included in the options
+        '''
+        # Run the state using -p1
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            options='-p1',
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run the state using --strip=1
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            options='--strip=1',
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+        # Re-run the state using --strip 1
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            options='--strip 1',
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_saltenv(self):
+        '''
+        Test that we attempt to download the patch from a non-base saltenv
+        '''
+        # This state will fail because we don't have a patch file in that
+        # environment, but that is OK, we just want to test that we're looking
+        # in an environment other than base.
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch,
+            saltenv='prod',
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(
+            ret['comment'],
+            "Source file {0} not found in saltenv 'prod'".format(self.math_patch)
+        )
+
+    def test_patch_single_file_failure(self):
+        '''
+        Test file.patch using a patch applied to a single file. This tests a
+        failed patch.
+        '''
+        # Empty the file to ensure that the patch doesn't apply cleanly
+        with salt.utils.files.fopen(self.numbers_file, 'w'):
+            pass
+
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Patch would not apply cleanly', ret['comment'])
+
+        # Test the reject_file option and ensure that the rejects are written
+        # to the path specified.
+        reject_file = salt.utils.files.mkstemp()
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+            reject_file=reject_file,
+            strip=1,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Patch would not apply cleanly', ret['comment'])
+        self.assertIn(
+            'saving rejects to file {0}'.format(reject_file),
+            ret['comment']
+        )
+
+    def test_patch_directory_failure(self):
+        '''
+        Test file.patch using a patch applied to a directory, with changes
+        spanning multiple files.
+        '''
+        # Empty the file to ensure that the patch doesn't apply
+        with salt.utils.files.fopen(self.math_file, 'w'):
+            pass
+
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            strip=1,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Patch would not apply cleanly', ret['comment'])
+
+        # Test the reject_file option and ensure that the rejects are written
+        # to the path specified.
+        reject_file = salt.utils.files.mkstemp()
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch,
+            reject_file=reject_file,
+            strip=1,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Patch would not apply cleanly', ret['comment'])
+        self.assertIn(
+            'saving rejects to file {0}'.format(reject_file),
+            ret['comment']
+        )
+
+    def test_patch_single_file_remote_source(self):
+        '''
+        Test file.patch using a patch applied to a single file, with the patch
+        coming from a remote source.
+        '''
+        # Try without a source_hash and without skip_verify=True, this should
+        # fail with a message about the source_hash
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch_http,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Unable to verify upstream hash', ret['comment'])
+
+        # Re-run the state with a source hash, it should now succeed
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch_http,
+            source_hash=self.math_patch_hash,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run again, this time with no hash and skip_verify=True to test
+        # skipping hash verification
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch_http,
+            skip_verify=True,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_directory_remote_source(self):
+        '''
+        Test file.patch using a patch applied to a directory, with changes
+        spanning multiple files, and the patch file coming from a remote
+        source.
+        '''
+        # Try without a source_hash and without skip_verify=True, this should
+        # fail with a message about the source_hash
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_http,
+            strip=1,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Unable to verify upstream hash', ret['comment'])
+
+        # Re-run the state with a source hash, it should now succeed
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_http,
+            source_hash=self.all_patch_hash,
+            strip=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run again, this time with no hash and skip_verify=True to test
+        # skipping hash verification
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_http,
+            strip=1,
+            skip_verify=True,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_single_file_template(self):
+        '''
+        Test file.patch using a patch applied to a single file, with jinja
+        templating applied to the patch file.
+        '''
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch_template,
+            template='jinja',
+            context=self.context,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run the state, should succeed and there should be a message about
+        # a partially-applied hunk.
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch_template,
+            template='jinja',
+            context=self.context,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_directory_template(self):
+        '''
+        Test file.patch using a patch applied to a directory, with changes
+        spanning multiple files, and with jinja templating applied to the patch
+        file.
+        '''
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_template,
+            template='jinja',
+            context=self.context,
+            strip=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run the state, should succeed and there should be a message about
+        # a partially-applied hunk.
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_template,
+            template='jinja',
+            context=self.context,
+            strip=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_single_file_remote_source_template(self):
+        '''
+        Test file.patch using a patch applied to a single file, with the patch
+        coming from a remote source.
+        '''
+        # Try without a source_hash and without skip_verify=True, this should
+        # fail with a message about the source_hash
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch_template_http,
+            template='jinja',
+            context=self.context,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Unable to verify upstream hash', ret['comment'])
+
+        # Re-run the state with a source hash, it should now succeed
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch_template_http,
+            source_hash=self.math_patch_template_hash,
+            template='jinja',
+            context=self.context,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run again, this time with no hash and skip_verify=True to test
+        # skipping hash verification
+        ret = self.run_state(
+            'file.patch',
+            name=self.math_file,
+            source=self.math_patch_template_http,
+            template='jinja',
+            context=self.context,
+            skip_verify=True,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_directory_remote_source_template(self):
+        '''
+        Test file.patch using a patch applied to a directory, with changes
+        spanning multiple files, and the patch file coming from a remote
+        source.
+        '''
+        # Try without a source_hash and without skip_verify=True, this should
+        # fail with a message about the source_hash
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_template_http,
+            template='jinja',
+            context=self.context,
+            strip=1,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Unable to verify upstream hash', ret['comment'])
+
+        # Re-run the state with a source hash, it should now succeed
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_template_http,
+            source_hash=self.all_patch_template_hash,
+            template='jinja',
+            context=self.context,
+            strip=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+
+        # Re-run again, this time with no hash and skip_verify=True to test
+        # skipping hash verification
+        ret = self.run_state(
+            'file.patch',
+            name=self.base_dir,
+            source=self.all_patch_template_http,
+            template='jinja',
+            context=self.context,
+            strip=1,
+            skip_verify=True,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+    def test_patch_test_mode(self):
+        '''
+        Test file.patch using test=True
+        '''
+        # Try without a source_hash and without skip_verify=True, this should
+        # fail with a message about the source_hash
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+            test=True,
+        )
+        self.assertSaltNoneReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'The patch would be applied')
+        self.assertTrue(ret['changes'])
+
+        # Apply the patch for real. We'll then be able to test below that we
+        # exit with a True rather than a None result if test=True is used on an
+        # already-applied patch.
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch successfully applied')
+        self.assertTrue(ret['changes'])
+
+        # Run again with test=True. Since the pre-check happens before we do
+        # the __opts__['test'] check, we should exit with a True result just
+        # the same as if we try to run this state on an already-patched file
+        # *without* test=True.
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+            test=True,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertEqual(ret['comment'], 'Patch was already applied')
+        self.assertEqual(ret['changes'], {})
+
+        # Empty the file to ensure that the patch doesn't apply cleanly
+        with salt.utils.files.fopen(self.numbers_file, 'w'):
+            pass
+
+        # Run again with test=True. Similar to the above run, we are testing
+        # that we return before we reach the __opts__['test'] check. In this
+        # case we should return a False result because we should already know
+        # by this point that the patch will not apply cleanly.
+        ret = self.run_state(
+            'file.patch',
+            name=self.numbers_file,
+            source=self.numbers_patch,
+            test=True,
+        )
+        self.assertSaltFalseReturn(ret)
+        ret = ret[next(iter(ret))]
+        self.assertIn('Patch would not apply cleanly', ret['comment'])
+        self.assertEqual(ret['changes'], {})

tests/unit/states/test_file.py

@@ -1217,76 +1217,6 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                 self.assertDictEqual(filestate.prepend
                                                      (name, text=text), ret)
 
-    # 'patch' function tests: 1
-
-    def test_patch(self):
-        '''
-        Test to apply a patch to a file.
-        '''
-        name = '/opt/file.txt'
-        source = 'salt://file.patch'
-        ha_sh = 'md5=e138491e9d5b97023cea823fe17bac22'
-
-        ret = {'name': name,
-               'result': False,
-               'comment': '',
-               'changes': {}}
-
-        comt = ('Must provide name to file.patch')
-        ret.update({'comment': comt, 'name': ''})
-        self.assertDictEqual(filestate.patch(''), ret)
-
-        comt = ('{0}: file not found'.format(name))
-        ret.update({'comment': comt, 'name': name})
-        self.assertDictEqual(filestate.patch(name), ret)
-
-        mock_t = MagicMock(return_value=True)
-        mock_true = MagicMock(side_effect=[True, False, False, False, False])
-        mock_false = MagicMock(side_effect=[False, True, True, True])
-        mock_ret = MagicMock(return_value={'retcode': True})
-        with patch.object(os.path, 'isabs', mock_t):
-            with patch.object(os.path, 'exists', mock_t):
-                comt = ('Source is required')
-                ret.update({'comment': comt})
-                self.assertDictEqual(filestate.patch(name), ret)
-
-                comt = ('Hash is required')
-                ret.update({'comment': comt})
-                self.assertDictEqual(filestate.patch(name, source=source), ret)
-
-                with patch.dict(filestate.__salt__,
-                                {'file.check_hash': mock_true,
-                                 'cp.cache_file': mock_false,
-                                 'file.patch': mock_ret}):
-                    comt = ('Patch is already applied')
-                    ret.update({'comment': comt, 'result': True})
-                    self.assertDictEqual(filestate.patch(name, source=source,
-                                                         hash=ha_sh), ret)
-
-                    comt = ("Unable to cache salt://file.patch"
-                            " from saltenv 'base'")
-                    ret.update({'comment': comt, 'result': False})
-                    self.assertDictEqual(filestate.patch(name, source=source,
-                                                         hash=ha_sh), ret)
-
-                    with patch.dict(filestate.__opts__, {'test': True}):
-                        comt = ('File /opt/file.txt will be patched')
-                        ret.update({'comment': comt, 'result': None,
-                                    'changes': {'retcode': True}})
-                        self.assertDictEqual(filestate.patch(name,
-                                                             source=source,
-                                                             hash=ha_sh), ret)
-
-                    with patch.dict(filestate.__opts__, {'test': False}):
-                        ret.update({'comment': '', 'result': False})
-                        self.assertDictEqual(filestate.patch(name,
-                                                             source=source,
-                                                             hash=ha_sh), ret)
-
-                        self.assertDictEqual(filestate.patch
-                                             (name, source=source, hash=ha_sh,
-                                              dry_run_first=False), ret)
-
     # 'touch' function tests: 1
 
     def test_touch(self):
@@ -1351,7 +1281,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
 
         comt = ('Must provide name to file.copy')
         ret.update({'comment': comt, 'name': ''})
-        self.assertDictEqual(filestate.copy('', source), ret)
+        self.assertDictEqual(filestate.copy_('', source), ret)
 
         mock_t = MagicMock(return_value=True)
         mock_f = MagicMock(return_value=False)
@@ -1363,13 +1293,13 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
         with patch.object(os.path, 'isabs', mock_f):
             comt = ('Specified file {0} is not an absolute path'.format(name))
             ret.update({'comment': comt, 'name': name})
-            self.assertDictEqual(filestate.copy(name, source), ret)
+            self.assertDictEqual(filestate.copy_(name, source), ret)
 
         with patch.object(os.path, 'isabs', mock_t):
             with patch.object(os.path, 'exists', mock_f):
                 comt = ('Source file "{0}" is not present'.format(source))
                 ret.update({'comment': comt, 'result': False})
-                self.assertDictEqual(filestate.copy(name, source), ret)
+                self.assertDictEqual(filestate.copy_(name, source), ret)
 
             with patch.object(os.path, 'exists', mock_t):
                 with patch.dict(filestate.__salt__,
@@ -1389,8 +1319,8 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                         comt = ('User salt is not available Group saltstack'
                                 ' is not available')
                     ret.update({'comment': comt, 'result': False})
-                    self.assertDictEqual(filestate.copy(name, source, user=user,
+                    self.assertDictEqual(filestate.copy_(name, source, user=user,
-                                                        group=group), ret)
+                                                         group=group), ret)
 
                     comt1 = ('Failed to delete "{0}" in preparation for'
                              ' forced move'.format(name))
@@ -1406,7 +1336,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                                 {'file.remove': mock_io}):
                                     ret.update({'comment': comt1,
                                                 'result': False})
-                                    self.assertDictEqual(filestate.copy
+                                    self.assertDictEqual(filestate.copy_
                                                          (name, source,
                                                           preserve=True,
                                                           force=True), ret)
@@ -1414,14 +1344,14 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                 with patch.object(os.path, 'isfile', mock_t):
                                     ret.update({'comment': comt2,
                                                 'result': True})
-                                    self.assertDictEqual(filestate.copy
+                                    self.assertDictEqual(filestate.copy_
                                                          (name, source,
                                                           preserve=True), ret)
 
                         with patch.object(os.path, 'lexists', mock_f):
                             with patch.dict(filestate.__opts__, {'test': True}):
                                 ret.update({'comment': comt3, 'result': None})
-                                self.assertDictEqual(filestate.copy
+                                self.assertDictEqual(filestate.copy_
                                                      (name, source,
                                                       preserve=True), ret)
 
@@ -1429,7 +1359,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                 comt = ('The target directory /tmp is'
                                         ' not present')
                                 ret.update({'comment': comt, 'result': False})
-                                self.assertDictEqual(filestate.copy
+                                self.assertDictEqual(filestate.copy_
                                                      (name, source,
                                                       preserve=True), ret)