Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Rename check_file_size #161

Merged
merged 3 commits into from
Feb 19, 2024
+45 −38
Merged

refactor: Rename check_file_size #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f1ef7b
refactor: Rename check_file_size and switch T/F output
dashmage Feb 13, 2024
48d9db4
Add back doctring with context
dashmage Feb 14, 2024
86a1141
Add more appropriate logging messages
dashmage Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions src/hw_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,19 @@ def symlink(src: Path, dst: Path) -> None:
raise


def check_file_size(path: Path) -> bool:
"""Verify if the file size > 0.
def file_is_empty(path: Path) -> bool:
"""Check whether file size is 0.

Because charm focus us to publish the resources on charmhub,
but most of the hardware related tools have the un-republish
policy. Currently our solution is publish a empty file which
size is 0.
dashmage marked this conversation as resolved.
Show resolved Hide resolved
Returns True if file is empty, otherwise returns False.

Third-party resources are not allowed to be redistributed on Charmhub.
Therefore, an empty file is uploaded as a resource which the user is expected
to replace. This function checks for those empty resource files.
"""
if path.stat().st_size == 0:
logger.info("%s size is 0, skip install", path)
return False
return True
logger.info("%s size is 0", path)
return True
return False


def install_deb(name: str, path: Path) -> None:
Expand Down Expand Up @@ -181,7 +182,8 @@ class StorCLIStrategy(TPRStrategyABC):

def install(self, path: Path) -> None:
"""Install storcli."""
if not check_file_size(path):
if file_is_empty(path):
logger.info("Skipping StorCLI resource install since empty file was detected.")
raise ResourceFileSizeZeroError(tool=self._name, path=path)
if not validate_checksum(STORCLI_VERSION_INFOS, path):
raise ResourceChecksumError
Expand All @@ -208,7 +210,8 @@ class PercCLIStrategy(TPRStrategyABC):

def install(self, path: Path) -> None:
"""Install perccli."""
if not check_file_size(path):
if file_is_empty(path):
logger.info("Skipping PERCCLI resource install since empty file was detected.")
raise ResourceFileSizeZeroError(tool=self._name, path=path)
if not validate_checksum(PERCCLI_VERSION_INFOS, path):
raise ResourceChecksumError
Expand All @@ -234,7 +237,8 @@ class SAS2IRCUStrategy(TPRStrategyABC):

def install(self, path: Path) -> None:
"""Install sas2ircu."""
if not check_file_size(path):
if file_is_empty(path):
logger.info("Skipping SAS2IRCU resource install since empty file was detected.")
raise ResourceFileSizeZeroError(tool=self._name, path=path)
if not validate_checksum(SAS2IRCU_VERSION_INFOS, path):
raise ResourceChecksumError
Expand All @@ -259,7 +263,8 @@ class SAS3IRCUStrategy(SAS2IRCUStrategy):

def install(self, path: Path) -> None:
"""Install sas3ircu."""
if not check_file_size(path):
if file_is_empty(path):
logger.info("Skipping SAS3IRCU resource install since empty file was detected.")
raise ResourceFileSizeZeroError(tool=self._name, path=path)
if not validate_checksum(SAS3IRCU_VERSION_INFOS, path):
raise ResourceChecksumError
Expand Down Expand Up @@ -521,8 +526,10 @@ def check_missing_resources(
missing_resources.append(TPR_RESOURCES[tool])
# Uploaded but file size is zero
path = fetch_tools.get(tool)
if path and not check_file_size(path):
logger.warning("Tool: %s path: %s size is zero", tool, path)
if path and file_is_empty(path):
logger.warning(
"Empty resource file detected for tool %s at path %s", tool, path
)
missing_resources.append(TPR_RESOURCES[tool])
if len(missing_resources) > 0:
return False, f"Missing resources: {missing_resources}"
Expand Down
46 changes: 23 additions & 23 deletions tests/unit/test_hw_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
TPRStrategyABC,
bmc_hw_verifier,
check_deb_pkg_installed,
check_file_size,
copy_to_snap_common_bin,
file_is_empty,
get_hw_tool_white_list,
install_deb,
make_executable,
Expand Down Expand Up @@ -328,8 +328,8 @@ def test_10_install_strategy_errors(self, mock_strategies, mock_hw_white_list):
msg,
)

@mock.patch("hw_tools.check_file_size", return_value=False)
def test_11_check_missing_resources_zero_size_resources(self, check_file_size):
@mock.patch("hw_tools.file_is_empty", return_value=True)
def test_11_check_missing_resources_zero_size_resources(self, file_is_empty):
self.harness.begin()
ok, msg = self.hw_tool_helper.check_missing_resources(
hw_white_list=[HWTool.STORCLI],
Expand Down Expand Up @@ -392,11 +392,11 @@ def test_14_check_installed_not_okay(self, _, mock_os, mock_path):

class TestStorCLIStrategy(unittest.TestCase):
@mock.patch("hw_tools.validate_checksum", return_value=True)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.install_deb")
def test_install(
self, mock_install_deb, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_install_deb, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
strategy = StorCLIStrategy()
strategy.install(path="path-a")
Expand All @@ -419,11 +419,11 @@ def test_install_empty_resource(self, mock_install_deb, mock_symlink, mock_valid
mock_symlink.assert_not_called()

@mock.patch("hw_tools.validate_checksum", return_value=False)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.install_deb")
def test_install_checksum_fail(
self, mock_install_deb, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_install_deb, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
mock_path = mock.Mock()
strategy = StorCLIStrategy()
Expand Down Expand Up @@ -485,20 +485,20 @@ def test_remove_deb_error_handling(self, mock_subprocess_check_outpout):


class TestTPRStrategyABC(unittest.TestCase):
def test_check_file_size_not_zero(self):
self.assertTrue(check_file_size(get_mock_path(size=100)))
def test_file_size_is_non_zero(self):
self.assertFalse(file_is_empty(get_mock_path(size=100)))

def test_check_file_size_zero(self):
self.assertFalse(check_file_size(get_mock_path(size=0)))
def test_file_size_is_zero(self):
self.assertTrue(file_is_empty(get_mock_path(size=0)))


class TestSAS2IRCUStrategy(unittest.TestCase):
@mock.patch("hw_tools.validate_checksum", return_value=True)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.make_executable")
def test_install(
self, mock_make_executable, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_make_executable, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
strategy = SAS2IRCUStrategy()
strategy.install(path="path-a")
Expand All @@ -520,11 +520,11 @@ def test_install_empty_resource(
mock_symlink.assert_not_called()

@mock.patch("hw_tools.validate_checksum", return_value=False)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.install_deb")
def test_install_checksum_fail(
self, mock_install_deb, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_install_deb, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
mock_path = mock.Mock()
strategy = SAS2IRCUStrategy()
Expand All @@ -543,11 +543,11 @@ def test_remove(self):

class TestSAS3IRCUStrategy(unittest.TestCase):
@mock.patch("hw_tools.validate_checksum", return_value=True)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.make_executable")
def test_install(
self, mock_make_executable, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_make_executable, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
strategy = SAS3IRCUStrategy()
strategy.install(path="path-a")
Expand All @@ -569,11 +569,11 @@ def test_install_empty_resource(
mock_symlink.assert_not_called()

@mock.patch("hw_tools.validate_checksum", return_value=False)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.install_deb")
def test_install_checksum_fail(
self, mock_install_deb, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_install_deb, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
mock_path = mock.Mock()
strategy = SAS3IRCUStrategy()
Expand All @@ -592,11 +592,11 @@ def test_remove(self):

class TestPercCLIStrategy(unittest.TestCase):
@mock.patch("hw_tools.validate_checksum", return_value=True)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.install_deb")
def test_install(
self, mock_install_deb, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_install_deb, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
strategy = PercCLIStrategy()
strategy.install(path="path-a")
Expand Down Expand Up @@ -624,11 +624,11 @@ def test_install_empty_resource(self, mock_install_deb, mock_symlink, mock_check
mock_check_sum.assert_not_called()

@mock.patch("hw_tools.validate_checksum", return_value=False)
@mock.patch("hw_tools.check_file_size", return_value=True)
@mock.patch("hw_tools.file_is_empty", return_value=False)
@mock.patch("hw_tools.symlink")
@mock.patch("hw_tools.install_deb")
def test_install_checksum_fail(
self, mock_install_deb, mock_symlink, mock_check_file_size, mock_validate_checksum
self, mock_install_deb, mock_symlink, mock_file_is_empty, mock_validate_checksum
):
mock_path = mock.Mock()
strategy = PercCLIStrategy()
Expand Down
Loading