From 9997d8e8f442349b4cc94c29a99cec3f4710b3f8 Mon Sep 17 00:00:00 2001 From: Brian Gunnarson Date: Tue, 4 Jun 2024 15:07:05 -0700 Subject: [PATCH] add tests for RedisConfig object --- merlin/server/server_commands.py | 4 +- merlin/server/server_util.py | 79 ++-- tests/unit/server/test_RedisConfig.py | 538 ++++++++++++++++++++++++++ tests/unit/server/test_server_util.py | 64 +-- 4 files changed, 577 insertions(+), 108 deletions(-) create mode 100644 tests/unit/server/test_RedisConfig.py diff --git a/merlin/server/server_commands.py b/merlin/server/server_commands.py index 65d17c42b..40f2689d0 100644 --- a/merlin/server/server_commands.py +++ b/merlin/server/server_commands.py @@ -98,9 +98,7 @@ def config_server(args: Namespace) -> None: # pylint: disable=R0912 redis_config.set_directory(args.directory) - redis_config.set_snapshot_seconds(args.snapshot_seconds) - - redis_config.set_snapshot_changes(args.snapshot_changes) + redis_config.set_snapshot(seconds=args.snapshot_seconds, changes=args.snapshot_changes) redis_config.set_snapshot_file(args.snapshot_file) diff --git a/merlin/server/server_util.py b/merlin/server/server_util.py index aff641d4d..27a83376d 100644 --- a/merlin/server/server_util.py +++ b/merlin/server/server_util.py @@ -304,16 +304,14 @@ class RedisConfig: to write those changes into a redis readable config file. """ - filename = "" - entry_order = [] - entries = {} - comments = {} - trailing_comments = "" - changed = False - def __init__(self, filename) -> None: self.filename = filename self.changed = False + self.entry_order = [] + self.entries = {} + self.comments = {} + self.trailing_comments = "" + self.changed = False self.parse() def parse(self) -> None: @@ -393,7 +391,7 @@ def get_port(self) -> str: """Getter method to get the port from the redis config""" return self.get_config_value("port") - def set_port(self, port: str) -> bool: + def set_port(self, port: int) -> bool: """Validates and sets a given port""" if port is None: return False @@ -428,59 +426,56 @@ def set_directory(self, directory: str) -> bool: """ if directory is None: return False + # Create the directory if it doesn't exist if not os.path.exists(directory): os.mkdir(directory) LOG.info(f"Created directory {directory}") - # Validate the directory input - if os.path.exists(directory): - # Set the save directory to the redis config - if not self.set_config_value("dir", directory): - LOG.error("Unable to set directory for redis config") - return False - else: - LOG.error(f"Directory {directory} given does not exist and could not be created.") + # Set the save directory to the redis config + if not self.set_config_value("dir", directory): + LOG.error("Unable to set directory for redis config") return False LOG.info(f"Directory is set to {directory}") return True - def set_snapshot_seconds(self, seconds: int) -> bool: - """Sets the snapshot wait time""" - if seconds is None: - return False - # Set the snapshot second in the redis config - value = self.get_config_value("save") - if value is None: - LOG.error("Unable to get exisiting parameter values for snapshot") - return False + def set_snapshot(self, seconds: int = None, changes: int = None) -> bool: + """ + Sets the 'seconds' and/or 'changes' values of the snapshot setting, + depending on what the user requests. + + :param seconds: The first value of snapshot to change. If we're leaving it the + same this will be None. + :param changes: The second value of snapshot to change. If we're leaving it the + same this will be None. + :returns: True if successful, False otherwise. + """ - value = value.split() - value[0] = str(seconds) - value = " ".join(value) - if not self.set_config_value("save", value): - LOG.error("Unable to set snapshot value seconds") + # If both values are None, this method is doing nothing + if seconds is None and changes is None: return False - LOG.info(f"Snapshot wait time is set to {seconds} seconds") - return True - - def set_snapshot_changes(self, changes: int) -> bool: - """Sets the snapshot threshold""" - if changes is None: - return False - # Set the snapshot changes into the redis config + # Grab the snapshot value from the redis config value = self.get_config_value("save") if value is None: LOG.error("Unable to get exisiting parameter values for snapshot") return False + # Update the snapshot value value = value.split() - value[1] = str(changes) + log_msg = "" + if seconds is not None: + value[0] = str(seconds) + log_msg += f"Snapshot wait time is set to {seconds} seconds. " + if changes is not None: + value[1] = str(changes) + log_msg += f"Snapshot threshold is set to {changes} changes." value = " ".join(value) + + # Set the new snapshot value if not self.set_config_value("save", value): - LOG.error("Unable to set snapshot value seconds") + LOG.error("Unable to set snapshot value") return False - LOG.info(f"Snapshot threshold is set to {changes} changes") + LOG.info(log_msg) return True def set_snapshot_file(self, file: str) -> bool: @@ -508,7 +503,7 @@ def set_append_mode(self, mode: str) -> bool: LOG.error("Unable to set append_mode in redis config") return False else: - LOG.error("Not a valid append_mode(Only valid modes are always, everysec, no)") + LOG.error("Not a valid append_mode (Only valid modes are always, everysec, no)") return False LOG.info(f"Append mode is set to {mode}") diff --git a/tests/unit/server/test_RedisConfig.py b/tests/unit/server/test_RedisConfig.py new file mode 100644 index 000000000..12880d4d6 --- /dev/null +++ b/tests/unit/server/test_RedisConfig.py @@ -0,0 +1,538 @@ +""" +Tests for the RedisConfig class of the `server_util.py` module. + +This class is especially large so that's why these tests have been +moved to their own file. +""" +import filecmp +import logging +import pytest +from typing import Any + +from merlin.server.server_util import RedisConfig + +class TestRedisConfig: + """Tests for the RedisConfig class.""" + + def test_initialization(self, server_redis_conf_file: str): + """ + Using a dummy redis configuration file, test that the initialization + of the RedisConfig class behaves as expected. + + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + expected_entries = { + "bind": "127.0.0.1", + "port": "6379", + "requirepass": "merlin_password", + "dir": "./", + "save": "300 100", + "dbfilename": "dump.rdb", + "appendfsync": "everysec", + "appendfilename": "appendonly.aof", + } + expected_comments = { + "bind": "# ip address\n", + "port": "\n# port\n", + "requirepass": "\n# password\n", + "dir": "\n# directory\n", + "save": "\n# snapshot\n", + "dbfilename": "\n# db file\n", + "appendfsync": "\n# append mode\n", + "appendfilename": "\n# append file\n", + } + expected_trailing_comment = "\n# dummy trailing comment" + expected_entry_order = list(expected_entries.keys()) + redis_config = RedisConfig(server_redis_conf_file) + assert redis_config.filename == server_redis_conf_file + assert not redis_config.changed + assert redis_config.entries == expected_entries + assert redis_config.entry_order == expected_entry_order + assert redis_config.comments == expected_comments + assert redis_config.trailing_comments == expected_trailing_comment + + def test_write(self, server_redis_conf_file: str, server_testing_dir: str): + """ + Test that the write functionality works by writing the contents of a dummy + configuration file to a blank configuration file. + + :param server_redis_conf_file: The path to a dummy redis configuration file + :param server_testing_dir: The path to the the temp output directory for server tests + """ + copy_redis_conf_file = f"{server_testing_dir}/redis_copy.conf" + + # Create a RedisConf object with the basic redis conf file + redis_config = RedisConfig(server_redis_conf_file) + + # Change the filepath of the redis config file to be the copy that we'll write to + redis_config.set_filename(copy_redis_conf_file) + + # Run the test + redis_config.write() + + # Check that the contents of the copied file match the contents of the basic file + assert filecmp.cmp(server_redis_conf_file, copy_redis_conf_file) + + @pytest.mark.parametrize("key, val, expected_return", [ + ("port", 1234, True), + ("invalid_key", "dummy_val", False) + ]) + def test_set_config_value(self, server_redis_conf_file: str, key: str, val: Any, expected_return: bool): + """ + Test the `set_config_value` method with valid and invalid keys. + + :param server_redis_conf_file: The path to a dummy redis configuration file + :param key: The key value to modify with `set_config_value` + :param val: The value to set `key` to + :param expected_return: The expected return from `set_config_value` + """ + redis_config = RedisConfig(server_redis_conf_file) + actual_return = redis_config.set_config_value(key, val) + assert actual_return == expected_return + if expected_return: + assert redis_config.entries[key] == val + assert redis_config.changes_made() + else: + assert not redis_config.changes_made() + + @pytest.mark.parametrize("key, expected_val", [ + ("bind", "127.0.0.1"), + ("port", "6379"), + ("requirepass", "merlin_password"), + ("dir", "./"), + ("save", "300 100"), + ("dbfilename", "dump.rdb"), + ("appendfsync", "everysec"), + ("appendfilename", "appendonly.aof"), + ("invalid_key", None) + ]) + def test_get_config_value(self, server_redis_conf_file: str, key: str, expected_val: str): + """ + Test the `get_config_value` method with valid and invalid keys. + + :param server_redis_conf_file: The path to a dummy redis configuration file + :param key: The key value to modify with `set_config_value` + :param expected_val: The value we're expecting to get by querying `key` + """ + redis_conf = RedisConfig(server_redis_conf_file) + assert redis_conf.get_config_value(key) == expected_val + + @pytest.mark.parametrize("ip_to_set", [ + "127.0.0.1", # Most common IP + "0.0.0.0", # Edge case (low) + "255.255.255.255", # Edge case (high) + "123.222.199.20", # Random valid IP + ]) + def test_set_ip_address_valid( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + ip_to_set: str + ): + """ + Test the `set_ip_address` method with valid ips. These should all return True + and set the 'bind' value to whatever `ip_to_set` is. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param ip_to_set: The ip address to set + """ + caplog.set_level(logging.INFO) + redis_config = RedisConfig(server_redis_conf_file) + assert redis_config.set_ip_address(ip_to_set) + assert f"Ipaddress is set to {ip_to_set}" in caplog.text, "Missing expected log message" + assert redis_config.get_ip_address() == ip_to_set + + @pytest.mark.parametrize("ip_to_set, expected_log", [ + (None, None), # No IP + ("0.0.0", "Invalid IPv4 address given."), # Invalid IPv4 + ("bind-unset", "Unable to set ip address for redis config"), # Special invalid case where bind doesn't exist + ]) + def test_set_ip_address_invalid( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + ip_to_set: str, + expected_log: str, + ): + """ + Test the `set_ip_address` method with invalid ips. These should all return False. + and not modify the 'bind' setting. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param ip_to_set: The ip address to set + :param expected_log: The string we're expecting the logger to log + """ + redis_config = RedisConfig(server_redis_conf_file) + # For the test where bind is unset, delete bind from dict and set new ip val to a valid value + if ip_to_set == "bind-unset": + del redis_config.entries["bind"] + ip_to_set = "127.0.0.1" + assert not redis_config.set_ip_address(ip_to_set) + assert redis_config.get_ip_address() != ip_to_set + if expected_log is not None: + assert expected_log in caplog.text, "Missing expected log message" + + @pytest.mark.parametrize("port_to_set", [ + 6379, # Most common port + 1, # Edge case (low) + 65535, # Edge case (high) + 12345, # Random valid port + ]) + def test_set_port_valid( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + port_to_set: str, + ): + """ + Test the `set_port` method with valid ports. These should all return True + and set the 'port' value to whatever `port_to_set` is. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param port_to_set: The port to set + """ + caplog.set_level(logging.INFO) + redis_config = RedisConfig(server_redis_conf_file) + assert redis_config.set_port(port_to_set) + assert redis_config.get_port() == port_to_set + assert f"Port is set to {port_to_set}" in caplog.text, "Missing expected log message" + + @pytest.mark.parametrize("port_to_set, expected_log", [ + (None, None), # No port + (0, "Invalid port given."), # Edge case (low) + (65536, "Invalid port given."), # Edge case (high) + ("port-unset", "Unable to set port for redis config"), # Special invalid case where port doesn't exist + ]) + def test_set_port_invalid( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + port_to_set: str, + expected_log: str, + ): + """ + Test the `set_port` method with invalid inputs. These should all return False + and not modify the 'port' setting. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param port_to_set: The port to set + :param expected_log: The string we're expecting the logger to log + """ + redis_config = RedisConfig(server_redis_conf_file) + # For the test where port is unset, delete port from dict and set port val to a valid value + if port_to_set == "port-unset": + del redis_config.entries["port"] + port_to_set = 5 + assert not redis_config.set_port(port_to_set) + assert redis_config.get_port() != port_to_set + if expected_log is not None: + assert expected_log in caplog.text, "Missing expected log message" + + @pytest.mark.parametrize("pass_to_set, expected_return", [ + ("valid_password", True), # Valid password + (None, False), # Invalid password + ]) + def test_set_password( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + pass_to_set: str, + expected_return: bool, + ): + """ + Test the `set_password` method with both valid and invalid input. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param pass_to_set: The password to set + :param expected_return: The expected return value + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + assert redis_conf.set_password(pass_to_set) == expected_return + if expected_return: + assert redis_conf.get_password() == pass_to_set + assert "New password set" in caplog.text, "Missing expected log message" + + def test_set_directory_valid( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + server_testing_dir: str, + ): + """ + Test the `set_directory` method with valid input. This should return True, modify the + 'dir' value, and log some messages about creating/setting the directory. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param server_testing_dir: The path to the the temp output directory for server tests + """ + caplog.set_level(logging.INFO) + redis_config = RedisConfig(server_redis_conf_file) + dir_to_set = f"{server_testing_dir}/dummy_dir" + assert redis_config.set_directory(dir_to_set) + assert redis_config.get_config_value("dir") == dir_to_set + assert f"Created directory {dir_to_set}" in caplog.text, "Missing created log message" + assert f"Directory is set to {dir_to_set}" in caplog.text, "Missing set log message" + + def test_set_directory_none(self, server_redis_conf_file: str): + """ + Test the `set_directory` method with None as the input. This should return False + and not modify the 'dir' setting. + + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_config = RedisConfig(server_redis_conf_file) + assert not redis_config.set_directory(None) + assert redis_config.get_config_value("dir") != None + + def test_set_directory_dir_unset( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + server_testing_dir: str, + ): + """ + Test the `set_directory` method with the 'dir' setting not existing. This should + return False and log an error message. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param server_testing_dir: The path to the the temp output directory for server tests + """ + redis_config = RedisConfig(server_redis_conf_file) + del redis_config.entries["dir"] + dir_to_set = f"{server_testing_dir}/dummy_dir" + assert not redis_config.set_directory(dir_to_set) + assert "Unable to set directory for redis config" in caplog.text, "Missing expected log message" + + def test_set_snapshot_valid(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_snapshot` method with a valid input for 'seconds' and 'changes'. + This should return True and modify both values of 'save'. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + snap_sec_to_set = 20 + snap_changes_to_set = 30 + assert redis_conf.set_snapshot(seconds=snap_sec_to_set, changes=snap_changes_to_set) + save_val = redis_conf.get_config_value("save").split() + assert save_val[0] == str(snap_sec_to_set) + assert save_val[1] == str(snap_changes_to_set) + expected_log = f"Snapshot wait time is set to {snap_sec_to_set} seconds. " \ + f"Snapshot threshold is set to {snap_changes_to_set} changes" + assert expected_log in caplog.text, "Missing expected log message" + + def test_set_snapshot_just_seconds(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_snapshot` method with a valid input for 'seconds'. This should + return True and modify the first value of 'save'. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + orig_save = redis_conf.get_config_value("save").split() + snap_sec_to_set = 20 + assert redis_conf.set_snapshot(seconds=snap_sec_to_set) + save_val = redis_conf.get_config_value("save").split() + assert save_val[0] == str(snap_sec_to_set) + assert save_val[1] == orig_save[1] + expected_log = f"Snapshot wait time is set to {snap_sec_to_set} seconds. " + assert expected_log in caplog.text, "Missing expected log message" + + def test_set_snapshot_just_changes(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_snapshot` method with a valid input for 'changes'. This should + return True and modify the second value of 'save'. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + orig_save = redis_conf.get_config_value("save").split() + snap_changes_to_set = 30 + assert redis_conf.set_snapshot(changes=snap_changes_to_set) + save_val = redis_conf.get_config_value("save").split() + assert save_val[0] == orig_save[0] + assert save_val[1] == str(snap_changes_to_set) + expected_log = f"Snapshot threshold is set to {snap_changes_to_set} changes" + assert expected_log in caplog.text, "Missing expected log message" + + def test_set_snapshot_none(self, server_redis_conf_file: str): + """ + Test the `set_snapshot` method with None as the input for both seconds + and changes. This should return False. + + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + assert not redis_conf.set_snapshot(seconds=None, changes=None) + + def test_set_snapshot_save_unset(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_snapshot` method with the 'save' setting not existing. This should + return False and log an error message. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + del redis_conf.entries["save"] + assert not redis_conf.set_snapshot(seconds=20) + assert "Unable to get exisiting parameter values for snapshot" in caplog.text, "Missing expected log message" + + def test_set_snapshot_file_valid(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_snapshot_file` method with a valid input. This should + return True and modify the value of 'dbfilename'. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + filename = "dummy_file.rdb" + assert redis_conf.set_snapshot_file(filename) + assert redis_conf.get_config_value("dbfilename") == filename + assert f"Snapshot file is set to {filename}" in caplog.text, "Missing expected log message" + + def test_set_snapshot_file_none(self, server_redis_conf_file: str): + """ + Test the `set_snapshot_file` method with None as the input. + This should return False. + + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + assert not redis_conf.set_snapshot_file(None) + + def test_set_snapshot_file_dbfilename_unset(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_snapshot` method with the 'dbfilename' setting not existing. This should + return False and log an error message. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + del redis_conf.entries["dbfilename"] + filename = "dummy_file.rdb" + assert not redis_conf.set_snapshot_file(filename) + assert redis_conf.get_config_value("dbfilename") != filename + assert "Unable to set snapshot_file name" in caplog.text, "Missing expected log message" + + @pytest.mark.parametrize("mode_to_set", [ + "always", + "everysec", + "no", + ]) + def test_set_append_mode_valid( + self, + caplog: "Fixture", # noqa: F821 + server_redis_conf_file: str, + mode_to_set: str, + ): + """ + Test the `set_append_mode` method with valid modes. These should all return True + and modify the value of 'appendfsync'. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + :param mode_to_set: The mode to set + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + assert redis_conf.set_append_mode(mode_to_set) + assert redis_conf.get_config_value("appendfsync") == mode_to_set + assert f"Append mode is set to {mode_to_set}" in caplog.text, "Missing expected log message" + + def test_set_append_mode_invalid(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_append_mode` method with an invalid mode. This should return False + and log an error message. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + invalid_mode = "invalid" + assert not redis_conf.set_append_mode(invalid_mode) + assert redis_conf.get_config_value("appendfsync") != invalid_mode + expected_log = "Not a valid append_mode (Only valid modes are always, everysec, no)" + assert expected_log in caplog.text, "Missing expected log message" + + def test_set_append_mode_none(self, server_redis_conf_file: str): + """ + Test the `set_append_mode` method with None as the input. + This should return False. + + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + assert not redis_conf.set_append_mode(None) + + def test_set_append_mode_appendfsync_unset(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_append_mode` method with the 'appendfsync' setting not existing. This should + return False and log an error message. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + del redis_conf.entries["appendfsync"] + mode = "no" + assert not redis_conf.set_append_mode(mode) + assert redis_conf.get_config_value("appendfsync") != mode + assert "Unable to set append_mode in redis config" in caplog.text, "Missing expected log message" + + def test_set_append_file_valid(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_append_file` method with a valid file. This should return True + and modify the value of 'appendfilename'. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + caplog.set_level(logging.INFO) + redis_conf = RedisConfig(server_redis_conf_file) + valid_file = "valid" + assert redis_conf.set_append_file(valid_file) + assert redis_conf.get_config_value("appendfilename") == f'"{valid_file}"' + assert f"Append file is set to {valid_file}" in caplog.text, "Missing expected log message" + + def test_set_append_file_none(self, server_redis_conf_file: str): + """ + Test the `set_append_file` method with None as the input. + This should return False. + + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + assert not redis_conf.set_append_file(None) + + def test_set_append_file_appendfilename_unset(self, caplog: "Fixture", server_redis_conf_file: str): # noqa: F821 + """ + Test the `set_append_file` method with the 'appendfilename' setting not existing. This should + return False and log an error message. + + :param caplog: A built-in fixture from the pytest library to capture logs + :param server_redis_conf_file: The path to a dummy redis configuration file + """ + redis_conf = RedisConfig(server_redis_conf_file) + del redis_conf.entries["appendfilename"] + filename = "valid_filename" + assert not redis_conf.set_append_file(filename) + assert redis_conf.get_config_value("appendfilename") != filename + assert "Unable to set append filename." in caplog.text, "Missing expected log message" diff --git a/tests/unit/server/test_server_util.py b/tests/unit/server/test_server_util.py index c71e854eb..0332be944 100644 --- a/tests/unit/server/test_server_util.py +++ b/tests/unit/server/test_server_util.py @@ -1,18 +1,15 @@ """ Tests for the `server_util.py` module. """ -import filecmp import os import pytest -import shutil -from typing import Callable, Dict, Union +from typing import Dict, Union from merlin.server.server_util import ( AppYaml, ContainerConfig, ContainerFormatConfig, ProcessConfig, - RedisConfig, RedisUsers, ServerConfig, valid_ipv4, @@ -288,62 +285,3 @@ def test_init_with_missing_data(self, server_process_config_data: Dict[str, str] assert config.process == ProcessConfig(server_process_config_data) assert config.container is None assert config.container_format is None - - -class TestRedisConfig: - """Tests for the RedisConfig class.""" - - def test_initialization(self, server_redis_conf_file: str): - """ - Using a dummy redis configuration file, test that the initialization - of the RedisConfig class behaves as expected. - - :param server_redis_conf_file: The path to a dummy redis configuration file - """ - expected_entries = { - "bind": "127.0.0.1", - "port": "6379", - "requirepass": "merlin_password", - "dir": "./", - "save": "300 100", - "dbfilename": "dump.rdb", - "appendfsync": "everysec", - "appendfilename": "appendonly.aof", - } - expected_comments = { - "bind": "# ip address\n", - "port": "\n# port\n", - "requirepass": "\n# password\n", - "dir": "\n# directory\n", - "save": "\n# snapshot\n", - "dbfilename": "\n# db file\n", - "appendfsync": "\n# append mode\n", - "appendfilename": "\n# append file\n", - } - expected_trailing_comment = "\n# dummy trailing comment" - expected_entry_order = list(expected_entries.keys()) - redis_config = RedisConfig(server_redis_conf_file) - assert redis_config.filename == server_redis_conf_file - assert not redis_config.changed - assert redis_config.entries == expected_entries - assert redis_config.entry_order == expected_entry_order - assert redis_config.comments == expected_comments - assert redis_config.trailing_comments == expected_trailing_comment - - def test_write(self, server_redis_conf_file: str, server_testing_dir: str): - """ - """ - copy_redis_conf_file = f"{server_testing_dir}/redis_copy.conf" - - # Create a RedisConf object with the basic redis conf file - redis_config = RedisConfig(server_redis_conf_file) - - # Change the filepath of the redis config file to be the copy that we'll write to - redis_config.filename = copy_redis_conf_file - - # Run the test - redis_config.write() - - # Check that the contents of the copied file match the contents of the basic file - assert filecmp.cmp(server_redis_conf_file, copy_redis_conf_file) -