From 8420684e7e84205a4c17b4e5b1b4846400e74607 Mon Sep 17 00:00:00 2001
From: Megan Wilhite <mwilhite@vmware.com>
Date: Wed, 11 Oct 2023 14:16:09 -0600
Subject: [PATCH 1/2] Increase code coverage for file module

---
 salt/modules/file.py                          |   5 +
 .../unit/modules/file/test_file_basics.py     |  78 +++++-
 .../unit/modules/file/test_file_chattr.py     |  52 +++-
 .../unit/modules/file/test_file_module.py     | 232 ++++++++++++++++++
 .../unit/modules/file/test_file_user_group.py |  76 ++++++
 5 files changed, 440 insertions(+), 3 deletions(-)
 create mode 100644 tests/pytests/unit/modules/file/test_file_user_group.py

diff --git a/salt/modules/file.py b/salt/modules/file.py
index 34a3ccbd0f45..f225e9e187ae 100644
--- a/salt/modules/file.py
+++ b/salt/modules/file.py
@@ -479,6 +479,9 @@ def lchown(path, user, group):
         else:
             gid = -1
 
+    if err:
+        return err
+
     return os.lchown(path, uid, gid)
 
 
@@ -3604,6 +3607,8 @@ def seek_write(path, data, offset):
 
         salt '*' file.seek_write /path/to/file 'some data' 4096
     """
+    if isinstance(data, str):
+        data = data.encode()
     path = os.path.expanduser(path)
     seek_fh = os.open(path, os.O_WRONLY)
     try:
diff --git a/tests/pytests/unit/modules/file/test_file_basics.py b/tests/pytests/unit/modules/file/test_file_basics.py
index cee60da2fab4..d71a6adc41ee 100644
--- a/tests/pytests/unit/modules/file/test_file_basics.py
+++ b/tests/pytests/unit/modules/file/test_file_basics.py
@@ -1,5 +1,6 @@
 import logging
 import os
+import pathlib
 import shutil
 
 import pytest
@@ -8,18 +9,20 @@
 import salt.loader
 import salt.modules.cmdmod as cmdmod
 import salt.modules.config as configmod
+import salt.modules.cp as cpmod
 import salt.modules.file as filemod
 import salt.utils.data
 import salt.utils.files
 import salt.utils.platform
 import salt.utils.stringutils
+from salt.exceptions import CommandExecutionError
 from tests.support.mock import MagicMock, call, patch
 
 log = logging.getLogger(__name__)
 
 
 @pytest.fixture
-def configure_loader_modules():
+def configure_loader_modules(minion_opts):
     return {
         filemod: {
             "__salt__": {
@@ -35,7 +38,8 @@ def configure_loader_modules():
                 "grains": {},
             },
             "__grains__": {"kernel": "Linux"},
-        }
+        },
+        cpmod: {"__context__": {}, "__opts__": minion_opts},
     }
 
 
@@ -272,3 +276,73 @@ def test_source_list_for_list_returns_local_file_proto_from_dict(myfile):
     ):
         ret = filemod.source_list([{"file://" + myfile: ""}], "filehash", "base")
         assert list(ret) == ["file://" + myfile, "filehash"]
+
+
+def test_set_mode_file_doesnotexist(tmp_path):
+    """
+    Test set_mode when the file does not exist
+    """
+    path = tmp_path / "path"
+    with pytest.raises(CommandExecutionError) as err:
+        filemod.set_mode(path, "0644")
+    assert err.value.message == f"{str(path)}: File not found"
+
+
+def test_get_source_sum_cannot_cache(tmp_path):
+    """
+    test get_source_sum when you cannot cache the source_hash
+    """
+    path = tmp_path / "salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source = "https://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source_hash = "https://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz.sha3_512"
+    with patch.dict(filemod.__salt__, {"cp.cache_file": MagicMock(return_value=False)}):
+        with pytest.raises(CommandExecutionError) as err:
+            ret = filemod.get_source_sum(path, source=source, source_hash=source_hash)
+    assert err.value.message == f"Source hash file {source_hash} not found"
+
+
+def test_get_source_sum_invalid_proto(tmp_path):
+    """
+    test get_source_sum when an invalid proto is being used
+    """
+    path = tmp_path / "salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source = "https://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source_hash = "invalid://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz.sha3_512"
+    with patch.dict(filemod.__salt__, {"cp.cache_file": MagicMock(return_value=False)}):
+        with pytest.raises(CommandExecutionError) as err:
+            ret = filemod.get_source_sum(path, source=source, source_hash=source_hash)
+    assert f"Source hash {source_hash} format is invalid." in err.value.message
+
+
+def test_get_source_sum_invalid_proto_urllib(tmp_path):
+    """
+    test get_source_sum when an invalid proto is being used
+    and urllib cannot parse it
+    """
+    path = tmp_path / "salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source = "https://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source_hash = pathlib.Path("source_hash")
+    with patch.dict(filemod.__salt__, {"cp.cache_file": MagicMock(return_value=False)}):
+        with pytest.raises(CommandExecutionError) as err:
+            ret = filemod.get_source_sum(path, source=source, source_hash=source_hash)
+    assert f"Source hash {source_hash} format is invalid." in err.value.message
+
+
+def test_get_source_sum_cannot_extract_hash(tmp_path):
+    """
+    test get_source_sum when you cannot extract the hash
+    """
+    path = tmp_path / "salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source = "https://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz"
+    source_hash = "https://repo.saltproject.io/salt_rc/salt/py3/onedir/latest/salt-3006.0rc3-onedir-linux-aarch64.tar.xz.sha3_512"
+    mock_extract = MagicMock(return_value=None)
+    patch_extract = patch("salt.modules.file.extract_hash", mock_extract)
+    cache_path = tmp_path / "cache"
+    patch_salt = patch.dict(
+        filemod.__salt__, {"cp.cache_file": MagicMock(return_value=cache_path)}
+    )
+    with patch_extract, patch_salt:
+        with pytest.raises(CommandExecutionError) as err:
+            ret = filemod.get_source_sum(path, source=source, source_hash=source_hash)
+    assert mock_extract.call_args == call(cache_path, "", path, source, None)
+    assert f"Source hash {source_hash} format is invalid." in err.value.message
diff --git a/tests/pytests/unit/modules/file/test_file_chattr.py b/tests/pytests/unit/modules/file/test_file_chattr.py
index edee87b8093e..93a1b841b0a6 100644
--- a/tests/pytests/unit/modules/file/test_file_chattr.py
+++ b/tests/pytests/unit/modules/file/test_file_chattr.py
@@ -5,7 +5,8 @@
 
 import salt.modules.cmdmod as cmdmod
 import salt.modules.file as filemod
-from tests.support.mock import MagicMock, Mock, patch
+from salt.exceptions import SaltInvocationError
+from tests.support.mock import MagicMock, Mock, call, patch
 
 log = logging.getLogger(__name__)
 
@@ -242,3 +243,52 @@ def fake_cmd(cmd, *args, **kwargs):
             follow_symlinks=False,
         )
         assert actual_ret["changes"] == expected
+
+
+@pytest.mark.parametrize(
+    "operator",
+    ["addz", None, "doesnotexist", 1],
+)
+def test_file_chattr_invalid_operator(operator):
+    """
+    Test file.chattr when passing in an invalid operator
+    """
+    with pytest.raises(SaltInvocationError) as exc:
+        filemod.chattr(operator=operator)
+    assert (
+        exc.value.message == "Need an operator: 'add' or 'remove' to modify attributes."
+    )
+
+
+def test_file_chattr_none_attributes():
+    """
+    Test file.chattr when attributes is None
+    """
+    with pytest.raises(SaltInvocationError) as exc:
+        filemod.chattr(operator="add")
+    assert exc.value.message == "Need attributes: [aAcCdDeijPsStTu]"
+
+
+def test_file_chattr_remove():
+    """
+    Test file.chattr when operator is remove
+    """
+    mock_cmd = MagicMock()
+    patch_cmd = patch.dict(filemod.__salt__, {"cmd.run": mock_cmd})
+    with patch_cmd:
+        filemod.chattr(operator="remove", attributes="aCd")
+    assert mock_cmd.call_args_list == [call(["chattr", "-aCd"], python_shell=False)]
+
+
+def test_file_chattr_flags():
+    """
+    Test file.chattr when using flags
+    """
+    mock_cmd = MagicMock(return_value="")
+    patch_cmd = patch.dict(filemod.__salt__, {"cmd.run": mock_cmd})
+    with patch_cmd:
+        ret = filemod.chattr(operator="remove", attributes="aCd", flags="R")
+    assert mock_cmd.call_args_list == [
+        call(["chattr", "-aCd", "-R"], python_shell=False)
+    ]
+    assert ret is True
diff --git a/tests/pytests/unit/modules/file/test_file_module.py b/tests/pytests/unit/modules/file/test_file_module.py
index 09735ad662cb..dfd2fa640107 100644
--- a/tests/pytests/unit/modules/file/test_file_module.py
+++ b/tests/pytests/unit/modules/file/test_file_module.py
@@ -15,6 +15,7 @@
 import salt.utils.files
 import salt.utils.platform
 import salt.utils.stringutils
+from salt.exceptions import CommandExecutionError, SaltInvocationError
 from salt.utils.jinja import SaltCacheLoader
 from tests.support.mock import MagicMock, Mock, patch
 
@@ -607,3 +608,234 @@ def test_file_move_disallow_copy_and_unlink():
         mock_os_rename.assert_called_once()
         mock_shutil_move.assert_not_called()
         assert ret is True
+
+
+def test_file_get_sum(tmp_path):
+    """
+    Test file.get_sum
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    ret = filemod.get_sum(path)
+    assert re.match(r"[a-z0-9]{64}", ret)
+
+
+def test_file_get_sum_no_file(tmp_path):
+    """
+    Test file.get_sum when the file doesn't exist
+    """
+    path = tmp_path / "path"
+    ret = filemod.get_sum(path)
+    assert ret == "File not found"
+
+
+def test_check_hash_file_hash_not_str(tmp_path):
+    """
+    Test file.check_hash when file_hash is not a str
+    """
+    with pytest.raises(SaltInvocationError) as err:
+        filemod.check_hash(tmp_path, 1234)
+    assert err.value.message == "hash must be a string"
+
+
+@pytest.mark.parametrize("hash_sep", ("sha256=", "sha256:"))
+def test_check_hash_file_sep(tmp_path, hash_sep):
+    """
+    Test file.check_hash when file_hash is using a separator
+    for example =
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    _hash = filemod.get_hash(path)
+    ret = filemod.check_hash(path, f"{hash_sep}{_hash}")
+    assert ret is True
+
+
+@pytest.mark.parametrize("hash_sep", ("sha256=", "sha256:"))
+def test_check_hash_file_hash_type_raise(tmp_path, hash_sep):
+    """
+    Test file.check_hash when file_hash is not a valid length
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    with pytest.raises(SaltInvocationError) as err:
+        filemod.check_hash(path, "123")
+    assert "could not be matched to a supported hash type" in err.value.message
+
+
+def test_file_find(tmp_path):
+    """
+    Test file.find
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    ret = filemod.find(tmp_path, name=path.name)
+    assert ret == [str(path)]
+    assert path.is_file()
+
+
+def test_file_find_delete(tmp_path):
+    """
+    Test file.find when adding delete in args
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    ret = filemod.find(tmp_path, "delete", name=path.name)
+    assert ret == [str(path)]
+    assert not path.is_file()
+
+
+def test_seek_read(tmp_path):
+    """
+    test basic functionality for file.seek_read
+    """
+    path = tmp_path / "path"
+    content = "this is a test"
+    path.write_text(f"WARNING: {content}")
+    ret = filemod.seek_read(path, 1028, 9)
+    assert str(ret) == f"b'{content}'"
+
+
+def test_seek_write(tmp_path):
+    """
+    test basic functionality for file.seek_write
+    """
+    path = tmp_path / "path"
+    before_content = "this is a test"
+    after_content = "this is production"
+    path.write_text(f"WARNING: {before_content}")
+    filemod.seek_write(path, after_content, 9)
+    assert path.read_text() == f"WARNING: {after_content}"
+
+
+def test_truncate(tmp_path):
+    """
+    test basic functionality for file.truncate
+    """
+    path = tmp_path / "path"
+    before_content = "this is a test"
+    path.write_text(f"WARNING: {before_content}")
+    filemod.truncate(path, 9)
+    assert path.read_text() == "WARNING: "
+
+
+def test_link(tmp_path):
+    """
+    test basic functionality of file.link
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    symlink = tmp_path / "symlink"
+    ret = filemod.link(path, symlink)
+    assert ret is True
+    assert path.samefile(symlink)
+
+
+@pytest.mark.parametrize("content,exp_ret", [("", 0), ("Test Content", 12)])
+def test_diskusage_file(tmp_path, content, exp_ret):
+    """
+    Test basic functionality of diskusage
+    when targeting a file
+    """
+    path = tmp_path / "path"
+    path.write_text(content)
+    ret = filemod.diskusage(str(path))
+    assert ret == exp_ret
+
+
+@pytest.mark.parametrize("content,exp_ret", [("", 0), ("Test Content", 36)])
+def test_diskusage_directory(tmp_path, content, exp_ret):
+    """
+    Test basic functionality of diskusage
+    when targeting a directory
+    """
+    _dir = tmp_path / "path_dir"
+    _dir.mkdir()
+    _files = ["file1", "file2", "file3"]
+    for _file in _files:
+        path = _dir / _file
+        path.write_text(content)
+    ret = filemod.diskusage(str(_dir))
+    assert ret == exp_ret
+
+
+def test_file_move_not_absolute_path(tmp_path):
+    """
+    test move when file src or dst is not an absolute path
+    """
+    src = tmp_path / "src"
+    dst = tmp_path / "dst"
+    with pytest.raises(SaltInvocationError) as err:
+        ret = filemod.move(src.name, dst.name)
+    assert err.value.message == "Source path must be absolute."
+
+    with pytest.raises(SaltInvocationError) as err:
+        ret = filemod.move(src, dst.name)
+    assert err.value.message == "Destination path must be absolute."
+
+
+def test_file_move_cannot_move(tmp_path):
+    """
+    test move when path cannot be copied.
+    """
+    src = tmp_path / "src"
+    dst = tmp_path / "dst"
+    with pytest.raises(CommandExecutionError) as err:
+        ret = filemod.move(src, dst)
+    assert (
+        err.value.message
+        == f"Unable to move '{str(src)}' to '{str(dst)}': [Errno 2] No such file or directory: '{str(src)}'"
+    )
+
+
+def test_file_join(tmp_path):
+    """
+    test basic functionality of join function
+    """
+    ret = filemod.join("/", "usr", "local", "bin")
+    assert ret == "/usr/local/bin"
+
+
+def test_file_dirname(tmp_path):
+    """
+    test basic functionality of dirname
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    ret = filemod.dirname(path)
+    assert ret == str(path.parent)
+
+
+def test_file_basename(tmp_path):
+    """
+    test basic functionality of basename
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    ret = filemod.basename(path)
+    assert ret == path.name
+
+
+def test_file_normpath():
+    """
+    test basic functionality of normpath
+    """
+    ret = filemod.normpath("/tmp/../test")
+    assert ret == "/test"
+
+
+def test_file_pardir():
+    """
+    test basic functionality of pardir
+    """
+    ret = filemod.pardir()
+    assert ret == ".."
+
+
+@pytest.mark.skip_on_windows(reason="Method not available on windows file module")
+def test_file_open_files():
+    ret = filemod.open_files()
+    assert "/dev/null" in ret
+    values = ret["/dev/null"]
+    for value in values:
+        assert isinstance(value, int)
diff --git a/tests/pytests/unit/modules/file/test_file_user_group.py b/tests/pytests/unit/modules/file/test_file_user_group.py
new file mode 100644
index 000000000000..72934da89f14
--- /dev/null
+++ b/tests/pytests/unit/modules/file/test_file_user_group.py
@@ -0,0 +1,76 @@
+import logging
+import os
+
+import pytest
+
+import salt.modules.file as filemod
+
+log = logging.getLogger(__name__)
+
+
+@pytest.fixture
+def configure_loader_modules():
+    return {filemod: {}}
+
+
+@pytest.fixture
+def user_account():
+    with pytest.helpers.create_account(create_group=True) as _account:
+        yield _account
+
+
+def test_gid_to_group(user_account):
+    """
+    test basic functionality of file.gid_to_group
+    """
+    ret = filemod.gid_to_group(user_account.info.gid)
+    assert ret == user_account.group_name
+
+
+def test_gid_to_group_non_int(user_account):
+    """
+    test file.gid_to_group when group is not an int
+    """
+    ret = filemod.gid_to_group(user_account.group_name)
+    assert ret == user_account.group_name
+
+
+def test_gid_to_group_none(user_account):
+    """
+    test file.gid_to_group when group is nothing
+    """
+    ret = filemod.gid_to_group("")
+    assert ret == ""
+
+
+def test_get_gid(user_account, tmp_path):
+    """
+    test basic functionality of get_gid
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    os.chown(path, user_account.info.uid, user_account.info.gid)
+    ret = filemod.get_gid(path)
+    assert ret == user_account.info.gid
+
+
+def test_get_uid(user_account, tmp_path):
+    """
+    test basic functionality of get_uid
+    """
+    path = tmp_path / "path"
+    path.write_text("")
+    os.chown(path, user_account.info.uid, user_account.info.gid)
+    ret = filemod.get_uid(path)
+    assert ret == user_account.info.uid
+
+
+def test_lchown_user_group_doesnotexist(tmp_path):
+    """
+    test lchown when the user and group does not exist
+    """
+    path = tmp_path / "path"
+    user = "doesnotexist"
+    group = "groupdoesntexist"
+    ret = filemod.lchown(path, user, group)
+    assert ret == "User does not exist\nGroup does not exist\n"

From a5352c3735540b925a475eb608c82ebc176da457 Mon Sep 17 00:00:00 2001
From: Megan Wilhite <mwilhite@vmware.com>
Date: Thu, 16 Nov 2023 12:51:53 -0700
Subject: [PATCH 2/2] Fix file module tests on windows

---
 .../unit/modules/file/test_file_module.py     | 21 ++++++++++++++-----
 .../unit/modules/file/test_file_user_group.py |  4 ++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/tests/pytests/unit/modules/file/test_file_module.py b/tests/pytests/unit/modules/file/test_file_module.py
index dfd2fa640107..910a915bf194 100644
--- a/tests/pytests/unit/modules/file/test_file_module.py
+++ b/tests/pytests/unit/modules/file/test_file_module.py
@@ -780,11 +780,14 @@ def test_file_move_cannot_move(tmp_path):
     """
     src = tmp_path / "src"
     dst = tmp_path / "dst"
+    exp_src = str(src)
+    if salt.utils.platform.is_windows():
+        exp_src = str(src).replace("\\", "\\\\")
     with pytest.raises(CommandExecutionError) as err:
         ret = filemod.move(src, dst)
     assert (
         err.value.message
-        == f"Unable to move '{str(src)}' to '{str(dst)}': [Errno 2] No such file or directory: '{str(src)}'"
+        == f"Unable to move '{str(src)}' to '{str(dst)}': [Errno 2] No such file or directory: '{exp_src}'"
     )
 
 
@@ -792,8 +795,11 @@ def test_file_join(tmp_path):
     """
     test basic functionality of join function
     """
-    ret = filemod.join("/", "usr", "local", "bin")
-    assert ret == "/usr/local/bin"
+    ret = filemod.join(os.sep, "usr", "local", "bin")
+    if salt.utils.platform.is_windows():
+        assert ret == "\\usr\\local\\bin"
+    else:
+        assert ret == "/usr/local/bin"
 
 
 def test_file_dirname(tmp_path):
@@ -820,8 +826,13 @@ def test_file_normpath():
     """
     test basic functionality of normpath
     """
-    ret = filemod.normpath("/tmp/../test")
-    assert ret == "/test"
+    path = "/tmp/../test"
+    exp_path = "/test"
+    if salt.utils.platform.is_windows():
+        path = "C://test"
+        exp_path = "C:\\test"
+    ret = filemod.normpath(path)
+    assert ret == exp_path
 
 
 def test_file_pardir():
diff --git a/tests/pytests/unit/modules/file/test_file_user_group.py b/tests/pytests/unit/modules/file/test_file_user_group.py
index 72934da89f14..5c2490057eb8 100644
--- a/tests/pytests/unit/modules/file/test_file_user_group.py
+++ b/tests/pytests/unit/modules/file/test_file_user_group.py
@@ -7,6 +7,10 @@
 
 log = logging.getLogger(__name__)
 
+pytestmark = [
+    pytest.mark.skip_on_windows,
+]
+
 
 @pytest.fixture
 def configure_loader_modules():