Improve unit testing

Currently, benchcab does not have extensive coverage in its unit tests.
Parts of the code that have poor test coverage are dependent on system
interaction (e.g. environment modules, running subprocess commands (SVN,
build scripts, executables, PBS).

This change refactors and adds unit testing for these parts of the code
base.

Move the code from the benchcab.run_cable_site module to the
benchcab.task module. This is done so that we reduce coupling between
modules that use fluxnet tasks. This also has the benefit that we can
test the two modules easily by sharing the same test setup functions. We
move the code from the benchcab.run_comparison module to
benchcab.task module for the same reasons.

Unit tests now check exception messages and standard output produced by
benchcab for both non-verbose and verbose modes. **Beware:** standard
output messages have been updated in some areas. These changes will need
to be updated in the documentation.

Refactor the default build so that environment modules are loaded via
python instead of writing module load statements into the temporary
build script. This is done to reduce the complexity of the unit tests.

The stderr output from subprocess commands are now redirected to stdout
so that stderr is suppressed in non-verbose mode.

Fixes #58 #22 #86

.conda/benchcab-dev.yaml

@@ -8,4 +8,6 @@ dependencies:
   - netcdf4
   - numpy
   - pytest
+  - coverage
   - pyyaml
+  - flatdict

.conda/meta.yaml

@@ -25,3 +25,4 @@ requirements:
         - netCDF4
         - PyYAML
         - f90nml
+        - flatdict

benchcab/bench_config.py

@@ -1,12 +1,9 @@
-"""
-A module containing all *_config() functions.
-
-"""
+"""A module containing all *_config() functions."""
 
 from pathlib import Path
 import yaml
 
-from benchcab.internal import MEORG_EXPERIMENTS, DEFAULT_SCIENCE_CONFIGURATIONS
+from benchcab import internal
 
 
 def check_config(config: dict):
@@ -15,12 +12,10 @@ def check_config(config: dict):
     If the config is invalid, an exception is raised. Otherwise, do nothing.
     """
 
-    required_keys = ["realisations", "project", "modules", "experiment"]
-    if any(key not in config for key in required_keys):
+    if any(key not in config for key in internal.CONFIG_REQUIRED_KEYS):
         raise ValueError(
             "The config file does not list all required entries. "
-            "Those are: "
-            ", ".join(required_keys)
+            "Those are: " + ", ".join(internal.CONFIG_REQUIRED_KEYS)
         )
 
     if not isinstance(config["project"], str):
@@ -46,12 +41,13 @@ def check_config(config: dict):
                 "that is compatible with the f90nml python package."
             )
 
-    valid_experiments = list(MEORG_EXPERIMENTS) + MEORG_EXPERIMENTS["five-site-test"]
+    valid_experiments = (
+        list(internal.MEORG_EXPERIMENTS) + internal.MEORG_EXPERIMENTS["five-site-test"]
+    )
     if config["experiment"] not in valid_experiments:
         raise ValueError(
             "The 'experiment' key is invalid.\n"
-            "Valid experiments are: "
-            ", ".join(valid_experiments)
+            "Valid experiments are: " + ", ".join(valid_experiments)
         )
 
     if not isinstance(config["realisations"], list):
@@ -119,6 +115,6 @@ def read_config(config_path: str) -> dict:
         branch.setdefault("build_script", "")
 
     # Add "science_configurations" if not provided and set to default value
-    config.setdefault("science_configurations", DEFAULT_SCIENCE_CONFIGURATIONS)
+    config.setdefault("science_configurations", internal.DEFAULT_SCIENCE_CONFIGURATIONS)
 
     return config

benchcab/benchcab.py

@@ -1,30 +1,36 @@
-#!/usr/bin/env python
-
 """Contains the main program entry point for `benchcab`."""
 
 import sys
 
 from benchcab.job_script import create_job_script, submit_job
 from benchcab.bench_config import read_config
 from benchcab.benchtree import setup_fluxnet_directory_tree, setup_src_dir
-from benchcab.build_cable import build_cable
+from benchcab.build_cable import default_build, custom_build
 from benchcab.get_cable import (
     checkout_cable,
     checkout_cable_auxiliary,
-    archive_rev_number,
+    svn_info_show_item,
+    next_path,
 )
 from benchcab.internal import (
     validate_environment,
     get_met_sites,
+    CWD,
     MULTIPROCESS,
     SITE_LOG_DIR,
     SITE_TASKS_DIR,
     SITE_OUTPUT_DIR,
 )
-from benchcab.task import get_fluxnet_tasks, get_fluxnet_comparisons, Task
+from benchcab.task import (
+    get_fluxnet_tasks,
+    get_fluxnet_comparisons,
+    run_tasks,
+    run_tasks_in_parallel,
+    run_comparisons,
+    run_comparisons_in_parallel,
+    Task,
+)
 from benchcab.cli import generate_parser
-from benchcab.run_cable_site import run_tasks, run_tasks_in_parallel
-from benchcab.run_comparison import run_comparisons, run_comparisons_in_parallel
 from benchcab.environment_modules import module_load, module_is_loaded
 
 
@@ -52,23 +58,42 @@ def _initialise_tasks(self) -> list[Task]:
 
     def checkout(self):
         """Endpoint for `benchcab checkout`."""
+
         setup_src_dir()
+
         print("Checking out repositories...")
+        rev_number_log = ""
         for branch in self.config["realisations"]:
-            checkout_cable(branch, verbose=self.args.verbose)
+            path_to_repo = checkout_cable(branch, verbose=self.args.verbose)
+            rev_number_log += (
+                f"{branch['name']} last changed revision: "
+                f"{svn_info_show_item(path_to_repo, 'last-changed-revision')}\n"
+            )
+
+        # TODO(Sean) we should archive revision numbers for CABLE-AUX
         checkout_cable_auxiliary(self.args.verbose)
-        archive_rev_number()
+
+        rev_number_log_path = CWD / next_path("rev_number-*.log")
+        print(f"Writing revision number info to {rev_number_log_path.relative_to(CWD)}")
+        with open(rev_number_log_path, "w", encoding="utf-8") as file:
+            file.write(rev_number_log)
+
         print("")
 
     def build(self):
         """Endpoint for `benchcab build`."""
         for branch in self.config["realisations"]:
-            build_cable(
-                branch["build_script"],
-                branch["name"],
-                self.config["modules"],
-                verbose=self.args.verbose,
-            )
+            if branch["build_script"]:
+                custom_build(
+                    branch["build_script"], branch["name"], verbose=self.args.verbose
+                )
+            else:
+                default_build(
+                    branch["name"],
+                    self.config["modules"],
+                    verbose=self.args.verbose,
+                )
+            print(f"Successfully compiled CABLE for realisation {branch['name']}")
         print("")
 
     def fluxnet_setup_work_directory(self):

benchcab/benchtree.py

@@ -7,79 +7,77 @@
 from benchcab.task import Task
 
 
-def clean_directory_tree(root_dir=internal.CWD):
-    """Remove pre-existing directories in `root_dir`."""
-    src_dir = Path(root_dir, internal.SRC_DIR)
+def clean_directory_tree():
+    """Remove pre-existing directories in current working directory."""
+    src_dir = Path(internal.CWD, internal.SRC_DIR)
     if src_dir.exists():
         shutil.rmtree(src_dir)
 
-    run_dir = Path(root_dir, internal.RUN_DIR)
+    run_dir = Path(internal.CWD, internal.RUN_DIR)
     if run_dir.exists():
         shutil.rmtree(run_dir)
 
 
-def setup_src_dir(root_dir=internal.CWD):
+def setup_src_dir():
     """Make `src` directory."""
 
-    src_dir = Path(root_dir, internal.SRC_DIR)
+    src_dir = Path(internal.CWD, internal.SRC_DIR)
     if not src_dir.exists():
-        print(f"Creating {src_dir.relative_to(root_dir)} directory: {src_dir}")
+        print(f"Creating {src_dir.relative_to(internal.CWD)} directory: {src_dir}")
         os.makedirs(src_dir)
 
 
-def setup_fluxnet_directory_tree(
-    fluxnet_tasks: list[Task], root_dir=internal.CWD, verbose=False
-):
+def setup_fluxnet_directory_tree(fluxnet_tasks: list[Task], verbose=False):
     """Generate the directory structure used of `benchcab`."""
 
-    run_dir = Path(root_dir, internal.RUN_DIR)
+    run_dir = Path(internal.CWD, internal.RUN_DIR)
     if not run_dir.exists():
         os.makedirs(run_dir)
 
-    site_run_dir = Path(root_dir, internal.SITE_RUN_DIR)
+    site_run_dir = Path(internal.CWD, internal.SITE_RUN_DIR)
     if not site_run_dir.exists():
         os.makedirs(site_run_dir)
 
-    site_log_dir = Path(root_dir, internal.SITE_LOG_DIR)
+    site_log_dir = Path(internal.CWD, internal.SITE_LOG_DIR)
     if not site_log_dir.exists():
         print(
-            f"Creating {site_log_dir.relative_to(root_dir)} directory: {site_log_dir}"
+            f"Creating {site_log_dir.relative_to(internal.CWD)} directory: {site_log_dir}"
         )
         os.makedirs(site_log_dir)
 
-    site_output_dir = Path(root_dir, internal.SITE_OUTPUT_DIR)
+    site_output_dir = Path(internal.CWD, internal.SITE_OUTPUT_DIR)
     if not site_output_dir.exists():
         print(
-            f"Creating {site_output_dir.relative_to(root_dir)} directory: {site_output_dir}"
+            f"Creating {site_output_dir.relative_to(internal.CWD)} directory: {site_output_dir}"
         )
         os.makedirs(site_output_dir)
 
-    site_tasks_dir = Path(root_dir, internal.SITE_TASKS_DIR)
+    site_tasks_dir = Path(internal.CWD, internal.SITE_TASKS_DIR)
     if not site_tasks_dir.exists():
         print(
-            f"Creating {site_tasks_dir.relative_to(root_dir)} directory: {site_tasks_dir}"
+            f"Creating {site_tasks_dir.relative_to(internal.CWD)} directory: {site_tasks_dir}"
         )
         os.makedirs(site_tasks_dir)
 
-    site_analysis_dir = Path(root_dir, internal.SITE_ANALYSIS_DIR)
+    site_analysis_dir = Path(internal.CWD, internal.SITE_ANALYSIS_DIR)
     if not site_analysis_dir.exists():
         print(
-            f"Creating {site_analysis_dir.relative_to(root_dir)} directory: {site_analysis_dir}"
+            f"Creating {site_analysis_dir.relative_to(internal.CWD)} directory: {site_analysis_dir}"
         )
         os.makedirs(site_analysis_dir)
 
-    site_bitwise_cmp_dir = Path(root_dir, internal.SITE_BITWISE_CMP_DIR)
+    site_bitwise_cmp_dir = Path(internal.CWD, internal.SITE_BITWISE_CMP_DIR)
     if not site_bitwise_cmp_dir.exists():
         print(
-            f"Creating {site_bitwise_cmp_dir.relative_to(root_dir)} directory: "
+            f"Creating {site_bitwise_cmp_dir.relative_to(internal.CWD)} directory: "
             f"{site_bitwise_cmp_dir}"
         )
         os.makedirs(site_bitwise_cmp_dir)
 
     print("Creating task directories...")
     for task in fluxnet_tasks:
-        task_dir = Path(root_dir, internal.SITE_TASKS_DIR, task.get_task_name())
+        task_dir = Path(internal.CWD, internal.SITE_TASKS_DIR, task.get_task_name())
         if not task_dir.exists():
             if verbose:
-                print(f"Creating {task_dir.relative_to(root_dir)}: " f"{task_dir}")
+                print(f"Creating {task_dir.relative_to(internal.CWD)}: " f"{task_dir}")
             os.makedirs(task_dir)