Adding Several New Unit Tests (#490)

* remove a merge conflict statement that was missed

* add pytest coverage library and add sample_index coverage

* run fix style and add module header

* add tests for encryption modules

* add unit tests for util_sampling

* run fix-style and fix typo

* create directory for context managers and fix issue with an encryption test

* add a context manager for spinning up/down the redis server

* fix issue with path in one test

* rework CONFIG functionality for testing

* refactor config fixture so it doesn't depend on redis server to be started

* split CONFIG fixtures into rabbit and redis configs, run fix-style

* add unit tests for broker.py

* add unit tests for the Config object

* update CHANGELOG

* make CONFIG fixtures more flexible for tests

* add tests for results_backend.py

* fix lint issues for most recent changes

* fix filename issue in setup.cfg and move celeryadapter tests to integration suite

* add ssl filepaths to mysql config object

* add unit tests for configfile.py

* add tests for the utils.py file in config/

* create utilities file and constants file

* move create_dir function to utils.py

* add tests for merlin/examples/generator.py

* run fix-style and update changelog

* add a 'pip freeze' call in github workflow to view reqs versions

* re-delete the old config test files

* fix tests/bugs introduced by merging in develop

* add a unit test file for the dumper module

* begin work on server tests and modular fixtures

* start work on tests for RedisConfig

* add tests for RedisConfig object

* add tests for RedisUsers class

* change server fixtures to use redis config files

* add tests for AppYaml class

* final cleanup of server_utils

* fix lint issues

* parametrize setup examples tests

* sort example output

* ensure directory is changed back on no outdir test

* sort the specs in examples output

* fix lint issues

* start writing tests for server config

* add pytest coverage library and add sample_index coverage

* run fix style and add module header

* add tests for encryption modules

* add unit tests for util_sampling

* run fix-style and fix typo

* create directory for context managers and fix issue with an encryption test

* add a context manager for spinning up/down the redis server

* fix issue with path in one test

* rework CONFIG functionality for testing

* refactor config fixture so it doesn't depend on redis server to be started

* split CONFIG fixtures into rabbit and redis configs, run fix-style

* add unit tests for broker.py

* add unit tests for the Config object

* update CHANGELOG

* make CONFIG fixtures more flexible for tests

* add tests for results_backend.py

* fix lint issues for most recent changes

* fix filename issue in setup.cfg and move celeryadapter tests to integration suite

* add ssl filepaths to mysql config object

* add unit tests for configfile.py

* add tests for the utils.py file in config/

* create utilities file and constants file

* move create_dir function to utils.py

* add tests for merlin/examples/generator.py

* run fix-style and update changelog

* fix tests/bugs introduced by merging in develop

* add a unit test file for the dumper module

* begin work on server tests and modular fixtures

* start work on tests for RedisConfig

* add tests for RedisConfig object

* add tests for RedisUsers class

* change server fixtures to use redis config files

* add tests for AppYaml class

* final cleanup of server_utils

* fix lint issues

* parametrize setup examples tests

* sort example output

* ensure directory is changed back on no outdir test

* sort the specs in examples output

* fix lint issues

* start writing tests for server config

* bake in LC_ALL env variable setting for server cmds

* add tests for parse_redis_output

* fix issue with scope of fixture after rebase

* run fix-style

* split up create_server_config and write tests for it

* add tests for config_merlin_server function

* add tests for pull_server_config

* add tests for pull_server_image

* finish writing tests for server_config.py

* add tests for server_commands.py

* run fix-style

* update README for testing directory

* update the temp_output_directory to include python version

* mock the open.write to try to fix github CI

* ensure config dir is created

* update CHANGELOG

* add print of exception to OSError catch in pull_server_image

* change name of config_file in test that's failing

* update CHANGELOG

* add Ryan and Joe's suggestions

* update tests to use newly named functions

* fix linter issue

.gitignore

@@ -39,8 +39,9 @@ flux.out
 slurm*.out
 docs/build/
 
-# Tox files
+# Test files
 .tox/*
+.coverage
 
 # Jupyter
 jupyter/.ipynb_checkpoints

CHANGELOG.md

@@ -4,6 +4,24 @@ All notable changes to Merlin will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+### Added
+- Several new unit tests for the following subdirectories:
+  - `merlin/common/`
+  - `merlin/config/`
+  - `merlin/examples/`
+  - `merlin/server/`
+- Context managers for the `conftest.py` file to ensure safe spin up and shutdown of fixtures
+  - `RedisServerManager`: context to help with starting/stopping a redis server for tests
+  - `CeleryWorkersManager`: context to help with starting/stopping workers for tests
+- Ability to copy and print the `Config` object from `merlin/config/__init__.py`
+- Equality method to the `ContainerFormatConfig` and `ContainerConfig` objects from `merlin/server/server_util.py`
+
+### Changed
+- Split the `start_server` and `config_server` functions of `merlin/server/server_commands.py` into multiple functions to make testing easier
+- Split the `create_server_config` function of `merlin/server/server_config.py` into two functions to make testing easier
+- Combined `set_snapshot_seconds` and `set_snapshot_changes` methods of `RedisConfig` into one method `set_snapshot`
+
 ## [1.12.2b1]
 ### Added
 - Conflict handler option to the `dict_deep_merge` function in `utils.py`
@@ -95,8 +113,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - this required adding a decent amount of test files to help with the tests; these can be found under the tests/unit/study/status_test_files directory
 - Pytest fixtures in the `conftest.py` file of the integration test suite
   - NOTE: an export command `export LC_ALL='C'` had to be added to fix a bug in the WEAVE CI. This can be removed when we resolve this issue for the `merlin server` command
-- Tests for the `celeryadapter.py` module
-- New CeleryTestWorkersManager context to help with starting/stopping workers for tests
+- Coverage to the test suite. This includes adding tests for:
+  - `merlin/common/`
+  - `merlin/config/`
+  - `merlin/examples/`
+  - `celeryadapter.py`
+- Context managers for the `conftest.py` file to ensure safe spin up and shutdown of fixtures
+  - `RedisServerManager`: context to help with starting/stopping a redis server for tests
+  - `CeleryWorkersManager`: context to help with starting/stopping workers for tests
+- Ability to copy and print the `Config` object from `merlin/config/__init__.py`
 
 ### Changed
 - Reformatted the entire `merlin status` command
@@ -132,7 +157,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The `merlin monitor` command will now keep an allocation up if the queues are empty and workers are still processing tasks
 - Add the restart keyword to the specification docs
 - Cyclical imports and config imports that could easily cause ci issues
-
 ## [1.11.1]
 ### Fixed
 - Typo in `batch.py` that caused lsf launches to fail (`ALL_SGPUS` changed to `ALL_GPUS`)

merlin/common/sample_index.py

@@ -225,8 +225,8 @@ def __setitem__(self, full_address, sub_tree):
 
         # Replace if we already have something at this address.
         if delete_me is not None:
-            self.children.__delitem__(full_address)
             SampleIndex.check_valid_addresses_for_insertion(full_address, sub_tree)
+            self.children.__delitem__(full_address)
             self.children[full_address] = sub_tree
             return
         raise KeyError

merlin/common/security/encrypt.py

@@ -52,11 +52,10 @@ def _get_key_path():
     except AttributeError:
         key_filepath = "~/.merlin/encrypt_data_key"
 
-    try:
-        key_filepath = os.path.abspath(os.path.expanduser(key_filepath))
-    except KeyError as e:
-        raise ValueError("Error! No password provided for RabbitMQ") from e
-    return key_filepath
+    if key_filepath is None:
+        raise ValueError("Error! No password provided for RabbitMQ")
+
+    return os.path.abspath(os.path.expanduser(key_filepath))
 
 
 def _gen_key(key_path):

merlin/common/util_sampling.py

@@ -35,6 +35,7 @@
 import numpy as np
 
 
+# TODO should we move this to merlin-spellbook?
 def scale_samples(samples_norm, limits, limits_norm=(0, 1), do_log=False):
     """Scale samples to new limits, either log10 or linearly.
 

merlin/config/__init__.py

@@ -31,7 +31,7 @@
 """
 Used to store the application configuration.
 """
-
+from copy import copy
 from types import SimpleNamespace
 from typing import Dict, List, Optional
 
@@ -56,6 +56,35 @@ def __init__(self, app_dict):
         self.results_backend: Optional[SimpleNamespace]
         self.load_app_into_namespaces(app_dict)
 
+    def __copy__(self):
+        """
+        A magic method to allow this class to be copied with copy(instance_of_Config).
+        """
+        cls = self.__class__
+        result = cls.__new__(cls)
+        copied_attrs = {
+            "celery": copy(self.__dict__["celery"]),
+            "broker": copy(self.__dict__["broker"]),
+            "results_backend": copy(self.__dict__["results_backend"]),
+        }
+        result.__dict__.update(copied_attrs)
+        return result
+
+    def __str__(self):
+        """
+        A magic method so we can print the CONFIG class.
+        """
+        formatted_str = "config:"
+        attrs = {"celery": self.celery, "broker": self.broker, "results_backend": self.results_backend}
+        for name, attr in attrs.items():
+            if attr is not None:
+                items = (f"    {k}: {v!r}" for k, v in attr.__dict__.items())
+                joined_items = "\n".join(items)
+                formatted_str += f"\n  {name}:\n{joined_items}"
+            else:
+                formatted_str += f"\n  {name}:\n    None"
+        return formatted_str
+
     def load_app_into_namespaces(self, app_dict: Dict) -> None:
         """
         Makes the application dictionary into a namespace, sets the attributes of the Config from the namespace values.

merlin/config/broker.py

@@ -85,13 +85,13 @@ def get_rabbit_connection(include_password, conn="amqps"):
         password_filepath = CONFIG.broker.password
         LOG.debug(f"Broker: password filepath = {password_filepath}")
         password_filepath = os.path.abspath(expanduser(password_filepath))
-    except KeyError as e:  # pylint: disable=C0103
-        raise ValueError("Broker: No password provided for RabbitMQ") from e
+    except (AttributeError, KeyError) as exc:
+        raise ValueError("Broker: No password provided for RabbitMQ") from exc
 
     try:
         password = read_file(password_filepath)
-    except IOError as e:  # pylint: disable=C0103
-        raise ValueError(f"Broker: RabbitMQ password file {password_filepath} does not exist") from e
+    except IOError as exc:
+        raise ValueError(f"Broker: RabbitMQ password file {password_filepath} does not exist") from exc
 
     try:
         port = CONFIG.broker.port
@@ -205,12 +205,6 @@ def get_connection_string(include_password=True):
     except AttributeError:
         broker = ""
 
-    try:
-        config_path = CONFIG.celery.certs
-        config_path = os.path.abspath(os.path.expanduser(config_path))
-    except AttributeError:
-        config_path = None
-
     if broker not in BROKERS:
         raise ValueError(f"Error: {broker} is not a supported broker.")
     return _sort_valid_broker(broker, include_password)

merlin/config/results_backend.py

@@ -236,6 +236,12 @@ def get_mysql(certs_path=None, mysql_certs=None, include_password=True):
         mysql_config["password"] = "******"
     mysql_config["server"] = server
 
+    # Ensure the ssl_key, ssl_ca, and ssl_cert keys are all set
+    if mysql_certs == MYSQL_CONFIG_FILENAMES:
+        for key, cert_file in mysql_certs.items():
+            if key not in mysql_config:
+                mysql_config[key] = os.path.join(certs_path, cert_file)
+
     return MYSQL_CONNECTION_STRING.format(**mysql_config)
 
 

merlin/config/utils.py

@@ -77,8 +77,14 @@ def get_priority(priority: Priority) -> int:
     :param priority: The priority value that we want
     :returns: The priority value as an integer
     """
-    if priority not in Priority:
-        raise ValueError(f"Invalid priority: {priority}")
+    priority_err_msg = f"Invalid priority: {priority}"
+    try:
+        # In python 3.12+ if something is not in the enum it will just return False
+        if priority not in Priority:
+            raise ValueError(priority_err_msg)
+    # In python 3.11 and below, a TypeError is raised when looking for something in an enum that is not there
+    except TypeError:
+        raise ValueError(priority_err_msg)
 
     priority_map = determine_priority_map(CONFIG.broker.name.lower())
     return priority_map.get(priority, priority_map[Priority.MID])  # Default to MID priority for unknown priorities

merlin/examples/generator.py

@@ -48,11 +48,19 @@
 
 EXAMPLES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "workflows")
 
+# TODO modify the example command to eliminate redundancy
+# - e.g. running `merlin example flux_local` will produce the same output
+#   as running `merlin example flux_par` or `merlin example flux_par_restart`.
+#   This should just be `merlin example flux`.
+# - restart and restart delay should be one example
+# - feature demo and remote feature demo should be one example
+# - all openfoam examples should just be under one openfoam label
+
 
 def gather_example_dirs():
     """Get all the example directories"""
     result = {}
-    for directory in os.listdir(EXAMPLES_DIR):
+    for directory in sorted(os.listdir(EXAMPLES_DIR)):
         result[directory] = directory
     return result
 
@@ -82,7 +90,7 @@ def list_examples():
     for example_dir in gather_example_dirs():
         directory = os.path.join(os.path.join(EXAMPLES_DIR, example_dir), "")
         specs = glob.glob(directory + "*.yaml")
-        for spec in specs:
+        for spec in sorted(specs):
             if "template" in spec:
                 continue
             with open(spec) as f:  # pylint: disable=C0103

merlin/main.py

@@ -425,6 +425,14 @@ def process_server(args: Namespace):
     Route to the correct function based on the command
     given via the CLI
     """
+    try:
+        lc_all_val = os.environ["LC_ALL"]
+        if lc_all_val != "C":
+            raise ValueError(f"The 'LC_ALL' environment variable is currently set to {lc_all_val} but it must be set to 'C'.")
+    except KeyError:
+        LOG.debug("The 'LC_ALL' environment variable was not set. Setting this to 'C'.")
+        os.environ["LC_ALL"] = "C"  # Necessary for Redis to configure LOCALE
+
     if args.commands == "init":
         init_server()
     elif args.commands == "start":