split up create_server_config and write tests for it

merlin/server/server_config.py

@@ -136,6 +136,31 @@ def parse_redis_output(redis_stdout: BufferedReader) -> Tuple[bool, str]:
     return False, "Reached end of redis output without seeing 'Ready to accept connections'"
 
 
+def write_container_command_files(config_dir: str) -> bool:
+    """
+    Write the yaml files that contain instructions on how to
+    run certain commands for each container type.
+
+    :param config_dir: The path to the configuration dir where we'll write files
+    :returns: True if successful. False otherwise.
+    """
+    files = [i + ".yaml" for i in CONTAINER_TYPES]
+    for file in files:
+        file_path = os.path.join(config_dir, file)
+        if os.path.exists(file_path):
+            LOG.info(f"{file} already exists.")
+            continue
+        LOG.info(f"Copying file {file} to configuration directory.")
+        try:
+            with resources.path("merlin.server", file) as config_file:
+                with open(file_path, "w") as outfile, open(config_file, "r") as infile:
+                    outfile.write(infile.read())
+        except OSError:
+            LOG.error(f"Destination location {config_dir} is not writable.")
+            return False
+    return True
+
+
 def create_server_config() -> bool:
     """
     Create main configuration file for merlin server in the
@@ -158,20 +183,8 @@ def create_server_config() -> bool:
             LOG.error(err)
             return False
 
-    files = [i + ".yaml" for i in CONTAINER_TYPES]
-    for file in files:
-        file_path = os.path.join(config_dir, file)
-        if os.path.exists(file_path):
-            LOG.info(f"{file} already exists.")
-            continue
-        LOG.info(f"Copying file {file} to configuration directory.")
-        try:
-            with resources.path("merlin.server", file) as config_file:
-                with open(file_path, "w") as outfile, open(config_file, "r") as infile:
-                    outfile.write(infile.read())
-        except OSError:
-            LOG.error(f"Destination location {config_dir} is not writable.")
-            return False
+    if not write_container_command_files(config_dir):
+        return False
 
     # Load Merlin Server Configuration and apply it to app.yaml
     with resources.path("merlin.server", MERLIN_SERVER_CONFIG) as merlin_server_config:
@@ -189,6 +202,7 @@ def create_server_config() -> bool:
         return False
 
     if not os.path.exists(server_config.container.get_config_dir()):
+        print("inside get_config_dir if statement")
         LOG.info("Creating merlin server directory.")
         os.mkdir(server_config.container.get_config_dir())
 

tests/unit/server/test_server_config.py

@@ -3,11 +3,14 @@
 """
 
 import io
+import logging
+import os
 import string
 from typing import Dict, Tuple, Union
 
 import pytest
 
+from merlin.server.server_util import CONTAINER_TYPES, ServerConfig
 from merlin.server.server_config import (
     PASSWORD_LENGTH,
     check_process_file_format,
@@ -20,6 +23,7 @@
     pull_process_file,
     pull_server_config,
     pull_server_image,
+    write_container_command_files,
 )
 
 
@@ -106,3 +110,182 @@ def test_parse_redis_output_with_vars(lines: bytes, expected_config: Tuple[bool,
     reader_input = io.BufferedReader(buffer)
     _, actual_vars = parse_redis_output(reader_input)
     assert expected_config == actual_vars
+
+
+def test_write_container_command_files_with_existing_files(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+):
+    """
+    Test the `write_container_command_files` function with files that already exist.
+    This should skip trying to create the files, log 3 "file already exists" messages,
+    and return True.
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    """
+    caplog.set_level(logging.INFO)
+    mocker.patch('os.path.exists', return_value=True)
+    assert write_container_command_files(server_testing_dir)
+    file_names = [f"{container}.yaml" for container in CONTAINER_TYPES]
+    for file in file_names:
+        assert f"{file} already exists." in caplog.text
+
+
+def test_write_container_command_files_with_nonexisting_files(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+):
+    """
+    Test the `write_container_command_files` function with files that don't already exist.
+    This should create the files, log messages for each file, and return True
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    """
+    caplog.set_level(logging.INFO)
+
+    # Mock the os.path.exists function so it returns False
+    mocker.patch('os.path.exists', return_value=False)
+
+    # Mock the resources.path context manager
+    mock_path = mocker.patch("merlin.server.server_config.resources.path")
+    mock_path.return_value.__enter__.return_value = "mocked_file_path"
+
+    # Mock the open builtin
+    mock_data = mocker.mock_open(read_data="mocked data")
+    mocker.patch("builtins.open", mock_data)
+
+    assert write_container_command_files(server_testing_dir)
+    file_names = [f"{container}.yaml" for container in CONTAINER_TYPES]
+    for file in file_names:
+        assert f"Copying file {file} to configuration directory." in caplog.text
+
+
+def test_write_container_command_files_with_oserror(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+):
+    """
+    Test the `write_container_command_files` function with an OSError being raised.
+    This should log an error message and return False.
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    """
+    # Mock the open function to raise an OSError
+    mocker.patch("builtins.open", side_effect=OSError("File not writeable"))
+
+    assert not write_container_command_files(server_testing_dir)
+    assert f"Destination location {server_testing_dir} is not writable." in caplog.text
+
+
+def test_create_server_config_merlin_config_dir_nonexistent(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+):
+    """
+    Tests the `create_server_config` function with MERLIN_CONFIG_DIR not existing.
+    This should log an error and return False.
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    """
+    nonexistent_dir = f"{server_testing_dir}/merlin_config_dir"
+    mocker.patch('merlin.server.server_config.MERLIN_CONFIG_DIR', nonexistent_dir)
+    assert not create_server_config()
+    assert f"Unable to find main merlin configuration directory at {nonexistent_dir}" in caplog.text
+
+
+def test_create_server_config_server_subdir_nonexistent_oserror(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+):
+    """
+    Tests the `create_server_config` function with MERLIN_CONFIG_DIR/MERLIN_SERVER_SUBDIR
+    not existing and an OSError being raised. This should log an error and return False.
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    """
+
+    # Mock MERLIN_CONFIG_DIR and MERLIN_SERVER_SUBDIR
+    nonexistent_server_subdir = "test_create_server_config_server_subdir_nonexistent"
+    mocker.patch('merlin.server.server_config.MERLIN_CONFIG_DIR', server_testing_dir)
+    mocker.patch('merlin.server.server_config.MERLIN_SERVER_SUBDIR', nonexistent_server_subdir)
+
+    # Mock os.mkdir so it raises an OSError
+    err_msg = "File not writeable"
+    mocker.patch("os.mkdir", side_effect=OSError(err_msg))
+    assert not create_server_config()
+    assert err_msg in caplog.text
+
+
+def test_create_server_config_no_server_config(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+):
+    """
+    Tests the `create_server_config` function with the call to `pull_server_config()`
+    returning None. This should log an error and return False.
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    """
+
+    # Mock the necessary variables/functions to get us to the pull_server_config call
+    mocker.patch("merlin.server.server_config.MERLIN_CONFIG_DIR", server_testing_dir)
+    mocker.patch("merlin.server.server_config.write_container_command_files", return_value=True)
+    mock_open_func = mocker.mock_open(read_data='key: value')
+    mocker.patch("builtins.open", mock_open_func)
+
+    # Mock the pull_server_config call (what we're actually testing) and run the test
+    mocker.patch("merlin.server.server_config.pull_server_config", return_value=None)
+    assert not create_server_config()
+    assert 'Try to run "merlin server init" again to reinitialize values.' in caplog.text
+
+
+def test_create_server_config_no_server_dir(
+    mocker: "Fixture",  # noqa: F821
+    caplog: "Fixture",  # noqa: F821
+    server_testing_dir: str,
+    server_server_config: Dict[str, str],
+):
+    """
+    Tests the `create_server_config` function with the call to
+    `server_config.container.get_config_dir()` returning a non-existent path. This should
+    log a message and create the directory, then return True.
+
+    :param mocker: A built-in fixture from the pytest-mock library to create a Mock object
+    :param caplog: A built-in fixture from the pytest library to capture logs
+    :param server_testing_dir: The path to the the temp output directory for server tests
+    :param server_server_config: A pytest fixture of test data to pass to the ServerConfig class
+    """
+    caplog.set_level(logging.INFO)
+
+    # Mock the necessary variables/functions to get us to the get_config_dir call
+    mocker.patch("merlin.server.server_config.MERLIN_CONFIG_DIR", server_testing_dir)
+    mocker.patch("merlin.server.server_config.write_container_command_files", return_value=True)
+    mock_open_func = mocker.mock_open(read_data='key: value')
+    mocker.patch("builtins.open", mock_open_func)
+    mocker.patch("merlin.server.server_config.pull_server_config", return_value=ServerConfig(server_server_config))
+
+    # Mock the get_config_dir call to return a directory that doesn't exist yet
+    nonexistent_dir = f"{server_testing_dir}/merlin_server"
+    mocker.patch("merlin.server.server_util.ContainerConfig.get_config_dir", return_value=nonexistent_dir)
+
+    assert create_server_config()
+    assert os.path.exists(nonexistent_dir)
+    assert "Creating merlin server directory." in caplog.text