Merge branch '28-declutter-the-top-level-log-file-2' into 'main'

Resolve "declutter the top level log file"

Closes #28

See merge request !115

src/mpsd_software_manager/mpsd_software.py

@@ -648,64 +648,78 @@ def run(*args, counter=[0], **kwargs):
     return process
 
 
+def write_to_file(file_path: Path, content: str) -> None:
+    """Write content to file.
+
+    Parameters
+    ----------
+    file_path : Path
+        The path to the file to write to.
+    content : str
+        The content to write to the file.
+
+    Returns
+    -------
+    None
+
+    """
+    with open(file_path, "a") as f:
+        f.write(content)
+
+
+def write_to_cmd_log(root_dir: Path, msg: str) -> None:
+    """Write message to command log.
+
+    Parameters
+    ----------
+    root_dir : Path
+        The path where the script is initialized.
+    msg : str
+        The message to write to the command log.
+    Returns
+    -------
+    - None
+    """
+    cmd_log_file_path = root_dir / config_vars["cmd_log_file"]
+    write_to_file(cmd_log_file_path, msg)
+
+
 def record_script_execution_summary(
-    root_dir: Path, msg: Union[str, None] = None, **kwargs
+    root_dir: Path, apex_log_file: Union[Path, None] = None
 ) -> None:
     """Log the command used to build the package_set.
 
-    It also logs information about the spack-environments branch and commit hash,
-    as well as the version of the mpsd-software-manager user. It also logs
-    steps taken in the install process using the optional message argument.
+    A date time header is added to the log file each time the script is called,
+    following which the commands executed by the user is logged.
+    The APEX log file is also logged if it is created.
+
+    for example:
+
+    ```
+
+    2023-06-28T11:18:10.718020
+    $ mpsd-software install dev-23a foss2021a-mpi
+    ```
 
     Parameters
     ----------
-    - root_dir : str
-        The path to the directory where the scripts are located.
-    - msg : str, optional
-        An optional message to log in the command log file.
-    - **kwargs : dict
-        A dictionary with values for
-        - spe_branch : str
-            The name of the Spack environments branch.
-        - spe_commit_hash : str
-            The commit hash of the Spack environments branch.
+    - root_dir : Path
+        The path where the script is initialized.
 
     Returns
     -------
     - None
 
     """
-    # Write to the log file with the following format
-    # --------------------------------------------------
-    # 2023-06-20T15:30:37.965370, mpsd_software prepare dev-23a
-    # MPSD Software manager version: 2023.6.16
-    # Spack environments branch: dev-23a (commit hash: abcdefg)
-    # MSGs
-    with os_chdir(root_dir):
-        with open(config_vars["cmd_log_file"], "a") as f:
-            if msg:
-                # Write the message to the log file
-                f.write(msg + "\n")
-            else:
-                # Write the header
-                f.write("-" * 50 + "\n")
-
-                # Gather data to log
-                # call statement:
-                cmd_line = " ".join(sys.argv)
-                # spack-environments branch and commit hash from kwargs
-                spe_branch = kwargs.get("spe_branch", None)
-                spe_commit_hash = kwargs.get("spe_commit_hash", None)
-
-                # Write to log file
-                f.write(f"{datetime.datetime.now().isoformat()}, {cmd_line}\n")
-                # logs script version
-                f.write(f"MPSD Software manager version: {__version__}\n")
-                if spe_branch and spe_commit_hash:
-                    f.write(
-                        f"Spack environments branch: {spe_branch} "
-                        f"(commit hash: {spe_commit_hash})\n"
-                    )
+    cmd_line = command_name + " " + " ".join(sys.argv[1:])
+    date_time = datetime.datetime.now().replace(microsecond=0).isoformat()
+
+    msg = f"\n {date_time}\n"
+    msg += f"$ {cmd_line}\n"
+    if apex_log_file:
+        # apex_log_file is not known when `init` is called
+        msg += f"> logging to {apex_log_file}\n"
+    write_to_cmd_log(root_dir, msg)
 
 
 def clone_repo(
@@ -904,8 +918,12 @@ def prepare_environment(mpsd_release: str, root_dir: Path) -> List[str]:
     spe_branch, spe_commit_hash, available_package_sets = get_release_info(
         mpsd_release, root_dir
     )
-    record_script_execution_summary(
-        root_dir, spe_branch=spe_branch, spe_commit_hash=spe_commit_hash
+    write_to_cmd_log(
+        root_dir=root_dir,
+        msg=(
+            f"Spack environments branch: {spe_branch} "
+            f"(commit hash: {spe_commit_hash})\n"
+        ),
     )
     return available_package_sets
 
@@ -1048,20 +1066,10 @@ def install_environment(
                 mpsd_release, "install", root_dir, package_set
             )
 
+            # Log the build_log_path and the package_set_dir
             logging.info(f"Installing package_set {package_set} to {package_set_dir}")
+            logging.info(f"> Logging installation of {package_set} at {build_log_path}")
 
-            # log the command
-            record_script_execution_summary(
-                root_dir,
-                msg=f"installing {package_set} and logging at {build_log_path}",
-            )
-            record_script_execution_summary(
-                root_dir,
-                msg=(
-                    f"CMD: bash {spack_setup_script} {' '.join(install_flags)} "
-                    f"{package_set}"
-                ),
-            )
             run(
                 f"bash {spack_setup_script} "
                 f"{' '.join(install_flags)} {package_set} 2>&1 "
@@ -1224,9 +1232,11 @@ def initialise_environment(root_dir: Path) -> None:
         # create the log file and fill it with the headers
         record_script_execution_summary(root_dir=root_dir)
         # record the msg in the log file
-        record_script_execution_summary(
+        init_log_msg = f"Initialising MPSD software instance at {root_dir}.\n"
+        init_log_msg += f"MPSD Software manager version: {__version__}\n"
+        write_to_cmd_log(
             root_dir=root_dir,
-            msg=f"Initialising MPSD software instance at {root_dir}.",
+            msg=init_log_msg,
         )
 
 
@@ -1395,23 +1405,24 @@ def main():
 
     # set up logging filename: we record activities that change the installation
     if args.action in ["init", "install", "prepare", "reinstall", "remove"]:
-        log_file = get_log_file_path(
+        apex_log_file = get_log_file_path(
             args.release,
             args.action,
             root_dir,
         )
     # some commands do not write any log_files:
     elif args.action in ["available", "status"]:
-        log_file = None
+        apex_log_file = None
     else:
         # sanity check
         raise NotImplementedError(f"Should never happen: unknown {args.action=}")
 
     set_up_logging(
         args.loglevel,
-        log_file,
+        apex_log_file,
     )
 
+    record_script_execution_summary(root_dir, apex_log_file)
     # Check the command and run related function
     if args.action == "remove":
         remove_environment(args.release, root_dir, args.package_set)

tests/test_mpsd_software.py

@@ -160,24 +160,32 @@ def test_prepare_environment(tmp_path):
         )
 
 
+def test_write_to_cmd_log(tmp_path):
+    """Check that we write to the correct log file"""
+    cmd_log_file = mod.config_vars["cmd_log_file"]
+    mod.write_to_cmd_log(root_dir=tmp_path, msg="test_cmd")
+    assert os.path.exists(tmp_path / cmd_log_file)
+    with open(tmp_path / cmd_log_file, "r") as f:
+        assert "test_cmd" in f.read()
+
+
 def test_record_script_execution_summary(tmp_path):
-    """Check that log is updated.
+    """Check that cmd log is updated with header
 
     Check that logs/install-software-environment.log is updated when the module is run
     """
     cmd_log_file = mod.config_vars["cmd_log_file"]
 
     root_dir = tmp_path / "test_prepare_env"
-    mpsd_release_to_test = "dev-23a"
     script_version = mod.__version__
     if os.path.exists(root_dir / cmd_log_file):
         initial_bytes = os.path.getsize(cmd_log_file)
     else:
         initial_bytes = 0
 
-    # run the prepare_env functionality
+    # run the init functionality to check the creation of log file
     create_mock_git_repository(target_directory=root_dir, create_directory=True)
-    mod.prepare_environment(mpsd_release=mpsd_release_to_test, root_dir=(root_dir))
+    mod.initialise_environment(root_dir=(root_dir))
 
     # check that logs/install-software-environment.log is updated
     assert os.path.exists(root_dir / cmd_log_file)
@@ -186,8 +194,8 @@ def test_record_script_execution_summary(tmp_path):
     # Check that the log file has "Spack environments branch: dev-23a " in the last line
     with open(root_dir / cmd_log_file, "r") as f:
         lines = f.readlines()
-        assert "Spack environments branch: releases/dev-23a " in lines[-1]
-        assert f"MPSD Software manager version: {script_version}" in lines[-2]
+        assert f"Initialising MPSD software instance at {tmp_path}" in lines[-2]
+        assert f"MPSD Software manager version: {script_version}" in lines[-1]
 
 
 def test_install_environment_wrong_package_set(tmp_path):
@@ -294,30 +302,41 @@ def test_install_environment_zlib():
     # print("Debug here ")
     # time.sleep(10)
 
-    build_log = list(
+    log_files = list(
         (release_base_dir / "logs").glob(
             f"{mpsd_release_to_test}_{microarch}_*_install.log"
         )
     )
-    assert len(build_log) == 2
-    # take the most recent build log
-    build_log = sorted(build_log)[1]
+    assert len(log_files) == 2
+    # take the most recent log as build log
+    apex_log = sorted(log_files)[0]
+    build_log = sorted(log_files)[1]
+    assert "APEX" in str(apex_log)
+    assert "BUILD" in str(build_log)
     # check that the build log contains statement ##### Installation finished
     with open(build_log, "r") as f:
         lines = f.read()
         assert "##### Installation finished" in lines
     os.path.basename(build_log)
 
-    # assert that install log files exists
+    # assert that APEX log file points to the build log file
+    with open(apex_log, "r") as f:
+        lines = f.read()
+        assert (
+            f"> Logging installation of {package_set_to_test} at {build_log}" in lines
+        )
+
+    # assert that cmd log files exists
     assert os.path.exists(root_dir / cmd_log_file)
 
-    # assert that the build log is written to the install log file
+    # assert that the mpsd release and hash is written to the cmd log file
     os.path.basename(build_log)
     with open(root_dir / cmd_log_file, "r") as f:
         lines = f.read()
-        assert (
-            f"installing {package_set_to_test} and logging at {str(build_log)}" in lines
-        )
+        assert f"Spack environments branch: releases/{mpsd_release_to_test}" in lines
+        # assert (
+        #     f"> logging to {apex_log}" in lines
+        # ) # TODO this has to be tested when main() called ie via CLI
     # assert that the module files are created correctly
     assert os.path.exists(release_base_dir / microarch)
     assert os.path.exists(release_base_dir / microarch / "lmod")
@@ -665,6 +684,9 @@ def test_interface(tmp_path):
     # check that the help message is printed when -h is provided
     # check that the error messages are also logged to the log file
     # check that `/` in release is handled correctly
+    # check that the cmd_log file contains sys arguments
+    # check that the cmd_log file contains the script version for init
+    # check that the cmd_log file contains the location of APEX log
 
 
 # other tests to add (ideally)