From e48fe32514ab772466ad5a79ec3c288463eadead Mon Sep 17 00:00:00 2001 From: Brian Gunnarson Date: Tue, 7 May 2024 14:58:41 -0700 Subject: [PATCH] begin work on server tests and modular fixtures --- merlin/server/server_util.py | 39 +++- tests/fixtures/server.py | 84 ++++++++ tests/unit/server/__init__.py | 0 tests/unit/server/test_server_util.py | 295 ++++++++++++++++++++++++++ 4 files changed, 411 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/server.py create mode 100644 tests/unit/server/__init__.py create mode 100644 tests/unit/server/test_server_util.py diff --git a/merlin/server/server_util.py b/merlin/server/server_util.py index c10e0e1d9..bdfd3652e 100644 --- a/merlin/server/server_util.py +++ b/merlin/server/server_util.py @@ -60,7 +60,7 @@ def valid_ipv4(ip: str) -> bool: # pylint: disable=C0103 return False for i in arr: - if int(i) < 0 and int(i) > 255: + if int(i) < 0 or int(i) > 255: return False return True @@ -121,6 +121,15 @@ def __init__(self, data: dict) -> None: self.pass_file = data["pass_file"] if "pass_file" in data else self.PASSWORD_FILE self.user_file = data["user_file"] if "user_file" in data else self.USERS_FILE + def __eq__(self, other: "ContainerFormatConfig"): + """ + Equality magic method used for testing this class + + :param other: Another ContainerFormatConfig object to check if they're the same + """ + variables = ("format", "image_type", "image", "url", "config", "config_dir", "pfile", "pass_file", "user_file") + return all(getattr(self, attr) == getattr(other, attr) for attr in variables) + def get_format(self) -> str: """Getter method to get the container format""" return self.format @@ -208,6 +217,15 @@ def __init__(self, data: dict) -> None: self.stop_command = data["stop_command"] if "stop_command" in data else self.STOP_COMMAND self.pull_command = data["pull_command"] if "pull_command" in data else self.PULL_COMMAND + def __eq__(self, other: "ContainerFormatConfig"): + """ + Equality magic method used for testing this class + + :param other: Another ContainerFormatConfig object to check if they're the same + """ + variables = ("command", "run_command", "stop_command", "pull_command") + return all(getattr(self, attr) == getattr(other, attr) for attr in variables) + def get_command(self) -> str: """Getter method to get the container command""" return self.command @@ -242,6 +260,15 @@ def __init__(self, data: dict) -> None: self.status = data["status"] if "status" in data else self.STATUS_COMMAND self.kill = data["kill"] if "kill" in data else self.KILL_COMMAND + def __eq__(self, other: "ProcessConfig"): + """ + Equality magic method used for testing this class + + :param other: Another ProcessConfig object to check if they're the same + """ + variables = ("status", "kill") + return all(getattr(self, attr) == getattr(other, attr) for attr in variables) + def get_status_command(self) -> str: """Getter method to get the status command""" return self.status @@ -264,12 +291,10 @@ class ServerConfig: # pylint: disable=R0903 container_format: ContainerFormatConfig = None def __init__(self, data: dict) -> None: - if "container" in data: - self.container = ContainerConfig(data["container"]) - if "process" in data: - self.process = ProcessConfig(data["process"]) - if self.container.get_format() in data: - self.container_format = ContainerFormatConfig(data[self.container.get_format()]) + self.container = ContainerConfig(data["container"]) if "container" in data else None + self.process = ProcessConfig(data["process"]) if "process" in data else None + container_format_data = data.get(self.container.get_format() if self.container else None) + self.container_format = ContainerFormatConfig(container_format_data) if container_format_data else None class RedisConfig: diff --git a/tests/fixtures/server.py b/tests/fixtures/server.py new file mode 100644 index 000000000..35efdcd65 --- /dev/null +++ b/tests/fixtures/server.py @@ -0,0 +1,84 @@ +""" +Fixtures specifically for help testing the modules in the server/ directory. +""" +import pytest +import shutil +from typing import Dict + +@pytest.fixture(scope="class") +def server_container_config_data(temp_output_dir: str): + """ + Fixture to provide sample data for ContainerConfig tests + + :param temp_output_dir: The path to the temporary output directory we'll be using for this test run + """ + return { + "format": "docker", + "image_type": "postgres", + "image": "postgres:latest", + "url": "postgres://localhost", + "config": "postgres.conf", + "config_dir": "/path/to/config", + "pfile": "merlin_server_postgres.pf", + "pass_file": f"{temp_output_dir}/postgres.pass", + "user_file": "postgres.users", + } + +@pytest.fixture(scope="class") +def server_container_format_config_data(): + """ + Fixture to provide sample data for ContainerFormatConfig tests + """ + return { + "command": "docker", + "run_command": "{command} run --name {name} -d {image}", + "stop_command": "{command} stop {name}", + "pull_command": "{command} pull {url}", + } + +@pytest.fixture(scope="class") +def server_process_config_data(): + """ + Fixture to provide sample data for ProcessConfig tests + """ + return { + "status": "status {pid}", + "kill": "terminate {pid}", + } + +@pytest.fixture(scope="class") +def server_server_config( + server_container_config_data: Dict[str, str], + server_process_config_data: Dict[str, str], + server_container_format_config_data: Dict[str, str], +): + """ + Fixture to provide sample data for ServerConfig tests + + :param server_container_config_data: A pytest fixture of test data to pass to the ContainerConfig class + :param server_process_config_data: A pytest fixture of test data to pass to the ProcessConfig class + :param server_container_format_config_data: A pytest fixture of test data to pass to the ContainerFormatConfig class + """ + return { + "container": server_container_config_data, + "process": server_process_config_data, + "docker": server_container_format_config_data, + } + + +@pytest.fixture(scope="class") +def server_redis_conf_file(temp_output_dir: str): + """ + Fixture to copy the redis.conf file from the merlin/server/ directory to the + temporary output directory and provide the path to the copied file + + :param temp_output_dir: The path to the temporary output directory we'll be using for this test run + """ + # TODO + # - will probably have to do more than just copying over the conf file + # - likely want to create our own test conf file with the settings that + # can be modified by RedisConf instead + path_to_redis_conf = f"{os.path.dirname(os.path.abspath(__file__))}/../../merlin/server/redis.conf" + path_to_copied_redis = f"{temp_output_dir}/redis.conf" + shutil.copy(path_to_redis_conf, path_to_copied_redis) + return path_to_copied_redis \ No newline at end of file diff --git a/tests/unit/server/__init__.py b/tests/unit/server/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/server/test_server_util.py b/tests/unit/server/test_server_util.py new file mode 100644 index 000000000..384e1ea37 --- /dev/null +++ b/tests/unit/server/test_server_util.py @@ -0,0 +1,295 @@ +""" +Tests for the `server_util.py` module. +""" +import os +import pytest +from typing import Callable, Dict, Union + +from merlin.server.server_util import ( + AppYaml, + ContainerConfig, + ContainerFormatConfig, + ProcessConfig, + RedisConfig, + RedisUsers, + ServerConfig, + valid_ipv4, + valid_port +) + +@pytest.mark.parametrize("valid_ip", [ + "0.0.0.0", + "127.0.0.1", + "14.105.200.58", + "255.255.255.255", +]) +def test_valid_ipv4_valid_ip(valid_ip: str): + """ + Test the `valid_ipv4` function with valid IPs. + This should return True. + + :param valid_ip: A valid port to test. + These are pulled from the parametrized list defined above this test. + """ + assert valid_ipv4(valid_ip) + +@pytest.mark.parametrize("invalid_ip", [ + "256.0.0.1", + "-1.0.0.1", + None, + "127.0.01", +]) +def test_valid_ipv4_invalid_ip(invalid_ip: Union[str, None]): + """ + Test the `valid_ipv4` function with invalid IPs. + An IP is valid if every integer separated by the '.' delimiter are between 0 and 255. + This should return False for both IPs tested here. + + :param invalid_ip: An invalid port to test. + These are pulled from the parametrized list defined above this test. + """ + assert not valid_ipv4(invalid_ip) + +@pytest.mark.parametrize("valid_input", [ + 1, + 433, + 65535, +]) +def test_valid_port_valid_input(valid_input: int): + """ + Test the `valid_port` function with valid port numbers. + Valid ports are ports between 1 and 65535. + This should return True. + + :param valid_input: A valid input value to test. + These are pulled from the parametrized list defined above this test. + """ + assert valid_port(valid_input) + +@pytest.mark.parametrize("invalid_input", [ + -1, + 0, + 65536, +]) +def test_valid_port_invalid_input(invalid_input: int): + """ + Test the `valid_port` function with invalid inputs. + Valid ports are ports between 1 and 65535. + This should return False for each invalid input tested. + + :param invalid_input: An invalid input value to test. + These are pulled from the parametrized list defined above this test. + """ + assert not valid_port(invalid_input) + + +class TestContainerConfig: + """Tests for the ContainerConfig class.""" + + def test_init_with_complete_data(self, server_container_config_data: Dict[str, str]): + """ + Tests that __init__ populates attributes correctly with complete data + + :param server_container_config_data: A pytest fixture of test data to pass to the ContainerConfig class + """ + config = ContainerConfig(server_container_config_data) + assert config.format == server_container_config_data["format"] + assert config.image_type == server_container_config_data["image_type"] + assert config.image == server_container_config_data["image"] + assert config.url == server_container_config_data["url"] + assert config.config == server_container_config_data["config"] + assert config.config_dir == server_container_config_data["config_dir"] + assert config.pfile == server_container_config_data["pfile"] + assert config.pass_file == server_container_config_data["pass_file"] + assert config.user_file == server_container_config_data["user_file"] + + def test_init_with_missing_data(self): + """ + Tests that __init__ uses defaults for missing data + """ + incomplete_data = {"format": "docker"} + config = ContainerConfig(incomplete_data) + assert config.format == incomplete_data["format"] + assert config.image_type == ContainerConfig.IMAGE_TYPE + assert config.image == ContainerConfig.IMAGE_NAME + assert config.url == ContainerConfig.REDIS_URL + assert config.config == ContainerConfig.CONFIG_FILE + assert config.config_dir == ContainerConfig.CONFIG_DIR + assert config.pfile == ContainerConfig.PROCESS_FILE + assert config.pass_file == ContainerConfig.PASSWORD_FILE + assert config.user_file == ContainerConfig.USERS_FILE + + @pytest.mark.parametrize("attr_name", [ + "image", + "config", + "pfile", + "pass_file", + "user_file", + ]) + def test_get_path_methods(self, server_container_config_data: Dict[str, str], attr_name: str): + """ + Tests that get_*_path methods construct the correct path + + :param server_container_config_data: A pytest fixture of test data to pass to the ContainerConfig class + :param attr_name: Name of the attribute to be tested. These are pulled from the parametrized list defined above this test. + """ + config = ContainerConfig(server_container_config_data) + get_path_method = getattr(config, f"get_{attr_name}_path") # Dynamically get the method based on attr_name + expected_path = os.path.join(server_container_config_data["config_dir"], server_container_config_data[attr_name]) + assert get_path_method() == expected_path + + @pytest.mark.parametrize("getter_name, expected_attr", [ + ("get_format", "format"), + ("get_image_type", "image_type"), + ("get_image_name", "image"), + ("get_image_url", "url"), + ("get_config_name", "config"), + ("get_config_dir", "config_dir"), + ("get_pfile_name", "pfile"), + ("get_pass_file_name", "pass_file"), + ("get_user_file_name", "user_file"), + ]) + def test_getter_methods(self, server_container_config_data: Dict[str, str], getter_name: str, expected_attr: str): + """ + Tests that all getter methods return the correct attribute values + + :param server_container_config_data: A pytest fixture of test data to pass to the ContainerConfig class + :param getter_name: Name of the getter method to test. This is pulled from the parametrized list defined above this test. + :param expected_attr: Name of the corresponding attribute. This is pulled from the parametrized list defined above this test. + """ + config = ContainerConfig(server_container_config_data) + getter = getattr(config, getter_name) + assert getter() == server_container_config_data[expected_attr] + + def test_get_container_password(self, server_container_config_data: Dict[str, str]): + """ + Test that the get_container_password is reading the password file properly + + :param server_container_config_data: A pytest fixture of test data to pass to the ContainerConfig class + """ + # Write a fake password to the password file + test_password = "super-secret-password" + with open(server_container_config_data["pass_file"], "w") as pass_file: + pass_file.write(test_password) + + # Run the test + config = ContainerConfig(server_container_config_data) + assert config.get_container_password() == test_password + + +class TestContainerFormatConfig: + """Tests for the ContainerFormatConfig class.""" + + def test_init_with_complete_data(self, server_container_format_config_data: Dict[str, str]): + """ + Tests that __init__ populates attributes correctly with complete data + + :param server_container_format_config_data: A pytest fixture of test data to pass to the ContainerFormatConfig class + """ + config = ContainerFormatConfig(server_container_format_config_data) + assert config.command == server_container_format_config_data["command"] + assert config.run_command == server_container_format_config_data["run_command"] + assert config.stop_command == server_container_format_config_data["stop_command"] + assert config.pull_command == server_container_format_config_data["pull_command"] + + def test_init_with_missing_data(self): + """ + Tests that __init__ uses defaults for missing data + """ + incomplete_data = {"command": "docker"} + config = ContainerFormatConfig(incomplete_data) + assert config.command == incomplete_data["command"] + assert config.run_command == config.RUN_COMMAND + assert config.stop_command == config.STOP_COMMAND + assert config.pull_command == config.PULL_COMMAND + + @pytest.mark.parametrize("getter_name, expected_attr", [ + ("get_command", "command"), + ("get_run_command", "run_command"), + ("get_stop_command", "stop_command"), + ("get_pull_command", "pull_command"), + ]) + def test_getter_methods(self, server_container_format_config_data: Dict[str, str], getter_name: str, expected_attr: str): + """ + Tests that all getter methods return the correct attribute values + + :param server_container_format_config_data: A pytest fixture of test data to pass to the ContainerFormatConfig class + :param getter_name: Name of the getter method to test. This is pulled from the parametrized list defined above this test. + :param expected_attr: Name of the corresponding attribute. This is pulled from the parametrized list defined above this test. + """ + config = ContainerFormatConfig(server_container_format_config_data) + getter = getattr(config, getter_name) + assert getter() == server_container_format_config_data[expected_attr] + + +class TestProcessConfig: + """Tests for the ProcessConfig class.""" + + def test_init_with_complete_data(self, server_process_config_data: Dict[str, str]): + """ + Tests that __init__ populates attributes correctly with complete data + + :param server_process_config_data: A pytest fixture of test data to pass to the ProcessConfig class + """ + config = ProcessConfig(server_process_config_data) + assert config.status == server_process_config_data["status"] + assert config.kill == server_process_config_data["kill"] + + def test_init_with_missing_data(self): + """ + Tests that __init__ uses defaults for missing data + """ + incomplete_data = {"status": "status {pid}"} + config = ProcessConfig(incomplete_data) + assert config.status == incomplete_data["status"] + assert config.kill == config.KILL_COMMAND + + @pytest.mark.parametrize("getter_name, expected_attr", [ + ("get_status_command", "status"), + ("get_kill_command", "kill"), + ]) + def test_getter_methods(self, server_process_config_data: Dict[str, str], getter_name: str, expected_attr: str): + """ + Tests that all getter methods return the correct attribute values + + :param server_process_config_data: A pytest fixture of test data to pass to the ProcessConfig class + :param getter_name: Name of the getter method to test. This is pulled from the parametrized list defined above this test. + :param expected_attr: Name of the corresponding attribute. This is pulled from the parametrized list defined above this test. + """ + config = ProcessConfig(server_process_config_data) + getter = getattr(config, getter_name) + assert getter() == server_process_config_data[expected_attr] + + +class TestServerConfig: + """Tests for the ServerConfig class.""" + + def test_init_with_complete_data(self, server_server_config: Dict[str, str]): + """ + Tests that __init__ populates attributes correctly with complete data + + :param server_server_config: A pytest fixture of test data to pass to the ServerConfig class + """ + config = ServerConfig(server_server_config) + assert config.container == ContainerConfig(server_server_config["container"]) + assert config.process == ProcessConfig(server_server_config["process"]) + assert config.container_format == ContainerFormatConfig(server_server_config["docker"]) + + def test_init_with_missing_data(self, server_process_config_data: Dict[str, str]): + """ + Tests that __init__ uses None for missing data + + :param server_process_config_data: A pytest fixture of test data to pass to the ContainerConfig class + """ + incomplete_data = {"process": server_process_config_data} + config = ServerConfig(incomplete_data) + 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_parse(self, server_redis_conf_file): +# raise ValueError