Porting #51740 to master

salt/modules/file.py

@@ -4236,7 +4236,7 @@ def rmdir(path, recurse=False, verbose=False, older_than=None):
         return ret
 
 
-def remove(path):
+def remove(path, **kwargs):
     """
     Remove the named file. If a directory is supplied, it will be recursively
     deleted.

salt/states/file.py

@@ -1903,10 +1903,7 @@ def absent(name, **kwargs):
             ret["comment"] = "File {} is set for removal".format(name)
             return ret
         try:
-            if salt.utils.platform.is_windows():
-                __salt__["file.remove"](name, force=True)
-            else:
-                __salt__["file.remove"](name)
+            __salt__["file.remove"](name, force=True)
             ret["comment"] = "Removed file {}".format(name)
             ret["changes"]["removed"] = name
             return ret
@@ -1920,10 +1917,7 @@ def absent(name, **kwargs):
             ret["comment"] = "Directory {} is set for removal".format(name)
             return ret
         try:
-            if salt.utils.platform.is_windows():
-                __salt__["file.remove"](name, force=True)
-            else:
-                __salt__["file.remove"](name)
+            __salt__["file.remove"](name, force=True)
             ret["comment"] = "Removed directory {}".format(name)
             ret["changes"]["removed"] = name
             return ret
@@ -2154,11 +2148,7 @@ def tidied(
         # Iterate over collected items
         try:
             for path in todelete:
-                if salt.utils.platform.is_windows():
-                    __salt__["file.remove"](path, force=True)
-                else:
-                    __salt__["file.remove"](path)
-                # Remember what we've removed, will appear in the summary
+                __salt__["file.remove"](path, force=True)
                 ret["changes"]["removed"].append(path)
         except CommandExecutionError as exc:
             return _error(ret, "{}".format(exc))
@@ -7494,7 +7484,7 @@ def copy_(
         elif not __opts__["test"] and changed:
             # Remove the destination to prevent problems later
             try:
-                __salt__["file.remove"](name)
+                __salt__["file.remove"](name, force=True)
             except OSError:
                 return _error(
                     ret,

tests/pytests/unit/states/test_file.py

@@ -0,0 +1,66 @@
+import pytest
+
+import salt.modules.file as filemod
+import salt.states.file as file
+from tests.support.mock import call, create_autospec, patch
+
+
+@pytest.fixture(autouse=True)
+def setup_loader(request):
+    setup_loader_modules = {file: {"__opts__": {"test": False}}}
+    with pytest.helpers.loader_mock(request, setup_loader_modules) as loader_mock:
+        yield loader_mock
+
+
+@pytest.fixture()
+def fake_remove():
+    fake_remove_mod = create_autospec(filemod.remove)
+    with patch.dict(file.__salt__, {"file.remove": fake_remove_mod}):
+        yield fake_remove_mod
+
+
+# TODO: This file.absent test should be a functional test instead. For now this is probably good enough -W. Werner, 2020-09-15
+@pytest.mark.parametrize("mock_mod", ["os.path.isfile", "os.path.isdir"])
+def test_file_absent_should_use_force_mode_for_file_remove(fake_remove, mock_mod):
+    expected_path = "/some/abspath/foo"
+    with patch(mock_mod, autospec=True, return_value=True):
+        file.absent(expected_path)
+
+    fake_remove.assert_called_with(expected_path, force=True)
+
+
+# TODO: This file.matches test should be a functional test instead. For now this is probably good enough -W. Werner, 2020-09-15
+def test_file_tidied_should_use_force_mode_for_file_remove(fake_remove):
+    patch_is_dir = patch("os.path.isdir", autospec=True, return_value=True)
+    patch_os_walk = patch(
+        "os.walk",
+        autospec=True,
+        return_value=[("some root", ("dirs",), ("file1", "file2"))],
+    )
+    patch_stat = patch("os.stat", autospec=True)
+    with patch_os_walk, patch_is_dir, patch_stat as fake_stat:
+        fake_stat.return_value.st_atime = 1600356711.1166897
+        fake_stat.return_value.st_size = 9001  # It's over 9000!
+
+        file.tidied("/some/directory/tree")
+
+    fake_remove.assert_has_calls(
+        [call("some root/file1", force=True), call("some root/file2", force=True)]
+    )
+
+
+# TODO: This file.copy test should be a functional test instead. For now this is probably good enough -W. Werner, 2020-09-15
+def test_file_copy_should_use_provided_force_mode_for_file_remove(fake_remove):
+
+    with patch("os.path.lexists", autospec=True, return_value=True), patch(
+        "os.path.isfile", autospec=True, return_value=True
+    ), patch("os.path.exists", autospec=True, return_value=True), patch.dict(
+        file.__opts__, {"user": "somefakeouser"}
+    ), patch(
+        "salt.states.file._check_user", autospec=True, return_value=False
+    ), patch(
+        "salt.utils.hashutils.get_hash", autospec=True, return_value=["12345", "54321"]
+    ):
+        file.copy_("/tmp/foo", source="/tmp/bar", group="fnord", force=True, mode=777)
+
+    fake_remove.assert_called_with("/tmp/foo", force=True)