diff --git a/benchcab/benchcab.py b/benchcab/benchcab.py index 97735cb1..bf9a6330 100644 --- a/benchcab/benchcab.py +++ b/benchcab/benchcab.py @@ -2,7 +2,7 @@ import sys -from benchcab.job_script import create_job_script, submit_job +from benchcab.job_script import 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 default_build, custom_build @@ -144,14 +144,13 @@ def fluxnet(self): if "fluxnet-bitwise-cmp" not in self.args.skip: self.fluxnet_bitwise_cmp() else: - create_job_script( + submit_job( project=self.config["project"], config_path=self.args.config, modules=self.config["modules"], verbose=self.args.verbose, skip_bitwise_cmp="fluxnet-bitwise-cmp" in self.args.skip, ) - submit_job() print( "The CABLE log file for each task is written to " f"{SITE_LOG_DIR}/_log.txt" diff --git a/benchcab/build_cable.py b/benchcab/build_cable.py index 4823c6e0..f912d688 100755 --- a/benchcab/build_cable.py +++ b/benchcab/build_cable.py @@ -3,12 +3,12 @@ import os import contextlib import stat -import subprocess import shlex import shutil import pathlib from benchcab import internal +from benchcab.utils import subprocess from benchcab import environment_modules @@ -23,7 +23,7 @@ def chdir(newdir: pathlib.Path): os.chdir(prevdir) -def patch_build_script(file_path): +def remove_module_lines(file_path): """Remove lines from `file_path` that call the environment modules package.""" with open(file_path, "r", encoding="utf-8") as file: contents = file.read() @@ -58,39 +58,28 @@ def default_build(branch_name: str, modules: list, verbose=False): tmp_script_path = default_script_path.parent / "tmp-build3.sh" if verbose: - print(f" Copying {default_script_path} to {tmp_script_path}") + print(f"Copying {default_script_path} to {tmp_script_path}") shutil.copy(default_script_path, tmp_script_path) + if verbose: - print(f" chmod +x {tmp_script_path}") + print(f"chmod +x {tmp_script_path}") tmp_script_path.chmod(tmp_script_path.stat().st_mode | stat.S_IEXEC) if verbose: print( - f" Patching {tmp_script_path.name}: remove lines that call " + f"Modifying {tmp_script_path.name}: remove lines that call " "environment modules" ) - patch_build_script(tmp_script_path) - - if verbose: - print(" Loading modules: " + " ".join(modules)) - environment_modules.module_load(*modules) - - with chdir(default_script_path.parent): - cmd = f"./{tmp_script_path.name}" + (" mpi" if internal.MPI else "") - if verbose: - print(f" {cmd}") - subprocess.run( - cmd, - shell=True, - check=True, - stdout=None if verbose else subprocess.DEVNULL, - stderr=subprocess.STDOUT, + remove_module_lines(tmp_script_path) + + with chdir(default_script_path.parent), environment_modules.load( + modules, verbose=verbose + ): + subprocess.run_cmd( + f"./{tmp_script_path.name}" + (" mpi" if internal.MPI else ""), + verbose=verbose, ) - if verbose: - print(" Unloading modules: " + " ".join(modules)) - environment_modules.module_unload(*modules) - def custom_build(config_build_script: str, branch_name: str, verbose=False): """Build CABLE with a script provided in configuration file""" @@ -103,13 +92,4 @@ def custom_build(config_build_script: str, branch_name: str, verbose=False): ) with chdir(build_script_path.parent): - cmd = f"./{build_script_path.name}" - if verbose: - print(f" {cmd}") - subprocess.run( - cmd, - shell=True, - check=True, - stdout=None if verbose else subprocess.DEVNULL, - stderr=subprocess.STDOUT, - ) + subprocess.run_cmd(f"./{build_script_path.name}", verbose=verbose) diff --git a/benchcab/environment_modules.py b/benchcab/environment_modules.py index 5d4beb14..d5cb9700 100644 --- a/benchcab/environment_modules.py +++ b/benchcab/environment_modules.py @@ -1,6 +1,7 @@ """Contains a wrapper around the environment modules API.""" import sys +import contextlib sys.path.append("/opt/Modules/v4.3.0/init") try: @@ -18,6 +19,20 @@ class EnvironmentModulesError(Exception): """Custom exception class for environment modules errors.""" +@contextlib.contextmanager +def load(modules, verbose=False): + """Context manager for loading and unloading modules.""" + if verbose: + print("Loading modules: " + " ".join(modules)) + module_load(*modules) + try: + yield + finally: + if verbose: + print("Unloading modules: " + " ".join(modules)) + module_unload(*modules) + + def module_is_avail(*args: str): """Wrapper around `module is-avail modulefile...`""" return module("is-avail", *args) diff --git a/benchcab/get_cable.py b/benchcab/get_cable.py index d9f6a8b2..790b09ee 100755 --- a/benchcab/get_cable.py +++ b/benchcab/get_cable.py @@ -1,10 +1,10 @@ """A module containing functions for checking out CABLE repositories.""" -import subprocess from typing import Union from pathlib import Path from benchcab import internal +from benchcab.utils import subprocess def next_path(path_pattern, sep="-"): @@ -32,9 +32,10 @@ def next_path(path_pattern, sep="-"): def svn_info_show_item(path: Union[Path, str], item: str) -> str: """A wrapper around `svn info --show-item `.""" - cmd = f"svn info --show-item {item} {path}" - out = subprocess.run(cmd, shell=True, capture_output=True, text=True, check=True) - return out.stdout.strip() + proc = subprocess.run_cmd( + f"svn info --show-item {item} {path}", capture_output=True + ) + return proc.stdout.strip() def checkout_cable_auxiliary(verbose=False) -> Path: @@ -42,17 +43,9 @@ def checkout_cable_auxiliary(verbose=False) -> Path: cable_aux_dir = Path(internal.CWD / internal.CABLE_AUX_DIR) - cmd = f"svn checkout {internal.CABLE_SVN_ROOT}/branches/Share/CABLE-AUX {cable_aux_dir}" - - if verbose: - print(cmd) - - subprocess.run( - cmd, - shell=True, - check=True, - stdout=None if verbose else subprocess.DEVNULL, - stderr=subprocess.STDOUT, + subprocess.run_cmd( + f"svn checkout {internal.CABLE_SVN_ROOT}/branches/Share/CABLE-AUX {cable_aux_dir}", + verbose=verbose, ) revision = svn_info_show_item(cable_aux_dir, "revision") @@ -92,16 +85,7 @@ def checkout_cable(branch_config: dict, verbose=False) -> Path: path_to_repo = Path(internal.CWD, internal.SRC_DIR, branch_config["name"]) cmd += f" {internal.CABLE_SVN_ROOT}/{branch_config['path']} {path_to_repo}" - if verbose: - print(cmd) - - subprocess.run( - cmd, - shell=True, - check=True, - stdout=None if verbose else subprocess.DEVNULL, - stderr=subprocess.STDOUT, - ) + subprocess.run_cmd(cmd, verbose=verbose) revision = svn_info_show_item(path_to_repo, "revision") print(f"Successfully checked out {branch_config['name']} at revision {revision}") diff --git a/benchcab/job_script.py b/benchcab/job_script.py index 0ab332a6..3a539c85 100644 --- a/benchcab/job_script.py +++ b/benchcab/job_script.py @@ -1,10 +1,10 @@ """Contains functions for job script creation and submission on Gadi.""" -import os -import subprocess +from subprocess import CalledProcessError from pathlib import Path from benchcab import internal +from benchcab.utils import subprocess def get_local_storage_flag(path: Path) -> str: @@ -16,19 +16,18 @@ def get_local_storage_flag(path: Path) -> str: raise RuntimeError("Current directory structure unknown on Gadi.") -def create_job_script( +def submit_job( project: str, config_path: str, modules: list, verbose=False, skip_bitwise_cmp=False, ): - """Creates a job script that executes all computationally expensive commands. - - Executed commands are: - - benchcab fluxnet-run-tasks - - benchcab fluxnet-bitwise-cmp + """Submits a PBS job that executes all computationally expensive commands. + This includes things such as running CABLE and running bitwise comparison jobs + between model output files. + The PBS job script is written to the current working directory as a side effect. """ job_script_path = internal.CWD / internal.QSUB_FNAME @@ -73,18 +72,12 @@ def create_job_script( """ ) - -def submit_job(): - """Submits the job script specified by `QSUB_FNAME`.""" - - job_script_path = internal.CWD / internal.QSUB_FNAME - cmd = f"qsub {job_script_path}" try: - proc = subprocess.run( - cmd, shell=True, check=True, capture_output=True, text=True + proc = subprocess.run_cmd( + f"qsub {job_script_path}", capture_output=True, verbose=verbose ) print(f"PBS job submitted: {proc.stdout.strip()}") - except subprocess.CalledProcessError as exc: + except CalledProcessError as exc: print("Error when submitting job to NCI queue") print(exc.stderr) raise diff --git a/benchcab/task.py b/benchcab/task.py index 8b2587f1..ddb9e107 100644 --- a/benchcab/task.py +++ b/benchcab/task.py @@ -3,18 +3,19 @@ import os import shutil -import subprocess import multiprocessing import queue import dataclasses from pathlib import Path from typing import TypeVar, Dict, Any +from subprocess import CalledProcessError import flatdict import netCDF4 import f90nml from benchcab import internal +from benchcab.utils import subprocess import benchcab.get_cable @@ -251,12 +252,12 @@ def run_cable(self, verbose=False): exe_path = task_dir / internal.CABLE_EXE nml_path = task_dir / internal.CABLE_NML stdout_path = task_dir / internal.CABLE_STDOUT_FILENAME - cmd = f"{exe_path} {nml_path} > {stdout_path} 2>&1" + try: - if verbose: - print(f" {cmd}") - subprocess.run(cmd, shell=True, check=True) - except subprocess.CalledProcessError as exc: + subprocess.run_cmd( + f"{exe_path} {nml_path}", output_file=stdout_path, verbose=verbose + ) + except CalledProcessError as exc: print(f"Error: CABLE returned an error for task {task_name}") raise CableError from exc @@ -445,16 +446,17 @@ def run_comparison(task_a: Task, task_b: Task, verbose=False): print( f"Comparing files {task_a_output.name} and {task_b_output.name} bitwise..." ) - cmd = f"nccmp -df {task_a_output} {task_b_output} 2>&1" - if verbose: - print(f" {cmd}") - proc = subprocess.run(cmd, shell=True, check=False, capture_output=True, text=True) - if proc.returncode != 0: + try: + subprocess.run_cmd( + f"nccmp -df {task_a_output} {task_b_output}", + capture_output=True, + verbose=verbose, + ) + print(f"Success: files {task_a_output.name} {task_b_output.name} are identical") + except CalledProcessError as exc: with open(output_file, "w", encoding="utf-8") as file: - file.write(proc.stdout) + file.write(exc.stdout) print( f"Failure: files {task_a_output.name} {task_b_output.name} differ. " f"Results of diff have been written to {output_file}" ) - else: - print(f"Success: files {task_a_output.name} {task_b_output.name} are identical") diff --git a/benchcab/utils/__init__.py b/benchcab/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/benchcab/utils/subprocess.py b/benchcab/utils/subprocess.py new file mode 100644 index 00000000..34228e30 --- /dev/null +++ b/benchcab/utils/subprocess.py @@ -0,0 +1,35 @@ +"""A module containing utility functions that wraps around the `subprocess` module.""" + +import subprocess +import contextlib +import pathlib +from typing import Any, Optional + + +def run_cmd( + cmd: str, + capture_output: bool = False, + output_file: Optional[pathlib.Path] = None, + verbose: bool = False, +) -> subprocess.CompletedProcess: + """Helper function that wraps around `subprocess.run()`""" + + kwargs: Any = {} + with contextlib.ExitStack() as stack: + if capture_output: + kwargs["capture_output"] = True + kwargs["text"] = True + else: + if output_file: + kwargs["stdout"] = stack.enter_context( + output_file.open("w", encoding="utf-8") + ) + else: + kwargs["stdout"] = None if verbose else subprocess.DEVNULL + kwargs["stderr"] = subprocess.STDOUT + + if verbose: + print(cmd) + proc = subprocess.run(cmd, shell=True, check=True, **kwargs) + + return proc diff --git a/tests/test_build_cable.py b/tests/test_build_cable.py index 993cd3ed..4f17d7d5 100644 --- a/tests/test_build_cable.py +++ b/tests/test_build_cable.py @@ -1,19 +1,19 @@ """`pytest` tests for build_cable.py""" -import subprocess import unittest.mock import io import contextlib import pytest from benchcab import internal -from benchcab.build_cable import patch_build_script, default_build, custom_build -from benchcab import environment_modules +from benchcab.build_cable import remove_module_lines, default_build, custom_build from .common import MOCK_CWD -def test_patch_build_script(): - """Tests for `patch_build_script()`.""" +def test_remove_module_lines(): + """Tests for `remove_module_lines()`.""" + + # Success case: test 'module' lines are removed from mock shell script file_path = MOCK_CWD / "test-build.sh" with open(file_path, "w", encoding="utf-8") as file: file.write( @@ -40,7 +40,7 @@ def test_patch_build_script(): """ ) - patch_build_script(file_path) + remove_module_lines(file_path) with open(file_path, "r", encoding="utf-8") as file: assert file.read() == ( @@ -60,162 +60,108 @@ def test_patch_build_script(): ) -def test_default_build(): +@unittest.mock.patch("benchcab.environment_modules.module_load") +@unittest.mock.patch("benchcab.environment_modules.module_unload") +def test_default_build( + mock_module_unload, mock_module_load +): # pylint: disable=unused-argument """Tests for `default_build()`.""" - offline_dir = MOCK_CWD / internal.SRC_DIR / "test-branch" / "offline" - offline_dir.mkdir(parents=True) - build_script_path = offline_dir / "build3.sh" + + build_script_path = ( + MOCK_CWD / internal.SRC_DIR / "test-branch" / "offline" / "build3.sh" + ) + build_script_path.parent.mkdir(parents=True) build_script_path.touch() # Success case: execute the default build command with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ), unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ), unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run: - default_build("test-branch", ["foo", "bar"]) - mock_subprocess_run.assert_called_once_with( - "./tmp-build3.sh", - shell=True, - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.STDOUT, - ) - - # Success case: test non-verbose output - with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ), unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ), unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout( - io.StringIO() - ) as buf: + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: default_build("test-branch", ["foo", "bar"]) - assert buf.getvalue() == ( - "Compiling CABLE serially for realisation test-branch...\n" - ) + mock_run_cmd.assert_called_once_with("./tmp-build3.sh", verbose=False) - # Success case: test verbose output + # Success case: execute the default build command with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ), unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ), unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout( - io.StringIO() - ) as buf: + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: default_build("test-branch", ["foo", "bar"], verbose=True) - mock_subprocess_run.assert_called_once_with( - "./tmp-build3.sh", - shell=True, - check=True, - stdout=None, - stderr=subprocess.STDOUT, - ) - assert buf.getvalue() == ( - "Compiling CABLE serially for realisation test-branch...\n" - f" Copying {build_script_path} to {build_script_path.parent}/tmp-build3.sh\n" - f" chmod +x {build_script_path.parent}/tmp-build3.sh\n" - " Patching tmp-build3.sh: remove lines that call environment " - "modules\n" - f" Loading modules: foo bar\n" - f" ./tmp-build3.sh\n" - f" Unloading modules: foo bar\n" - ) + mock_run_cmd.assert_called_once_with("./tmp-build3.sh", verbose=True) - # Failure case: cannot load modules - with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ) as mock_module_load, unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ), unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run: - mock_module_load.side_effect = environment_modules.EnvironmentModulesError - with pytest.raises(environment_modules.EnvironmentModulesError): + # Success case: test non-verbose standard output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: default_build("test-branch", ["foo", "bar"]) + assert buf.getvalue() == ( + "Compiling CABLE serially for realisation test-branch...\n" + ) - # Failure case: cannot unload modules - with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ), unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ) as mock_module_unload, unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run: - mock_module_unload.side_effect = environment_modules.EnvironmentModulesError - with pytest.raises(environment_modules.EnvironmentModulesError): - default_build("test-branch", ["foo", "bar"]) + # Success case: test verbose standard output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + default_build("test-branch", ["foo", "bar"], verbose=True) + assert buf.getvalue() == ( + "Compiling CABLE serially for realisation test-branch...\n" + f"Copying {build_script_path} to {build_script_path.parent}/tmp-build3.sh\n" + f"chmod +x {build_script_path.parent}/tmp-build3.sh\n" + "Modifying tmp-build3.sh: remove lines that call environment " + "modules\n" + "Loading modules: foo bar\n" + "Unloading modules: foo bar\n" + ) # Failure case: cannot find default build script build_script_path.unlink() - with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ), unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ), unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run: + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): with pytest.raises( FileNotFoundError, match=f"The default build script, {MOCK_CWD}/src/test-branch/offline/build3.sh, " "could not be found. Do you need to specify a different build script with the " "'build_script' option in config.yaml?", ): - default_build("test-branch", ["foo", "bar"], verbose=True) + default_build("test-branch", ["foo", "bar"]) def test_custom_build(): """Tests for `custom_build()`.""" - offline_dir = MOCK_CWD / internal.SRC_DIR / "test-branch" / "offline" - offline_dir.mkdir(parents=True) - build_script_path = offline_dir / "custom-build.sh" + + build_script_path = ( + MOCK_CWD / internal.SRC_DIR / "test-branch" / "offline" / "build3.sh" + ) + build_script_path.parent.mkdir(parents=True) build_script_path.touch() # Success case: execute custom build command with unittest.mock.patch( - "benchcab.environment_modules.module_load" - ), unittest.mock.patch( - "benchcab.environment_modules.module_unload" - ), unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run: + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: custom_build(build_script_path, "test-branch") - mock_subprocess_run.assert_called_once_with( - f"./{build_script_path.name}", - shell=True, - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.STDOUT, + mock_run_cmd.assert_called_once_with( + f"./{build_script_path.name}", verbose=False ) - # Success case: test non-verbose output + # Success case: execute custom build command with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - custom_build(build_script_path, "test-branch") - assert buf.getvalue() == ( - "Compiling CABLE using custom build script for realisation test-branch...\n" - ) - - # Success case: test verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: custom_build(build_script_path, "test-branch", verbose=True) - mock_subprocess_run.assert_called_once_with( - f"./{build_script_path.name}", - shell=True, - check=True, - stdout=None, - stderr=subprocess.STDOUT, + mock_run_cmd.assert_called_once_with( + f"./{build_script_path.name}", verbose=True + ) + + # Success case: test non-verbose standard output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + custom_build(build_script_path, "test-branch") + assert buf.getvalue() == ( + "Compiling CABLE using custom build script for realisation test-branch...\n" + ) + + # Success case: test verbose standard output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + custom_build(build_script_path, "test-branch", verbose=True) + assert buf.getvalue() == ( + "Compiling CABLE using custom build script for realisation test-branch...\n" ) - assert buf.getvalue() == ( - "Compiling CABLE using custom build script for realisation test-branch...\n" - f" ./{build_script_path.name}\n" - ) diff --git a/tests/test_get_cable.py b/tests/test_get_cable.py index c435621f..56311686 100644 --- a/tests/test_get_cable.py +++ b/tests/test_get_cable.py @@ -1,6 +1,5 @@ """`pytest` tests for get_cable.py""" -import subprocess import unittest.mock import shutil import io @@ -41,57 +40,56 @@ def test_checkout_cable(): "benchcab.get_cable.svn_info_show_item", mock_svn_info_show_item ): # Success case: checkout mock branch repository from SVN - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + branch_config = setup_mock_branch_config() + assert checkout_cable(branch_config) == MOCK_CWD / "src" / "trunk" + mock_run_cmd.assert_called_once_with( + "svn checkout -r 9000 https://trac.nci.org.au/svn/cable/trunk " + f"{MOCK_CWD}/src/trunk", + verbose=False, + ) + + # Success case: checkout mock branch repository from SVN with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: branch_config = setup_mock_branch_config() - path = checkout_cable(branch_config) - assert path == MOCK_CWD / "src" / "trunk" - mock_subprocess_run.assert_called_once_with( + assert ( + checkout_cable(branch_config, verbose=True) + == MOCK_CWD / "src" / "trunk" + ) + mock_run_cmd.assert_called_once_with( "svn checkout -r 9000 https://trac.nci.org.au/svn/cable/trunk " f"{MOCK_CWD}/src/trunk", - shell=True, - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.STDOUT, + verbose=True, ) # Success case: specify default revision number - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: branch_config = setup_mock_branch_config() branch_config["revision"] = -1 - path = checkout_cable(branch_config) - assert path == MOCK_CWD / "src" / "trunk" - mock_subprocess_run.assert_called_once_with( + assert checkout_cable(branch_config) == MOCK_CWD / "src" / "trunk" + mock_run_cmd.assert_called_once_with( f"svn checkout https://trac.nci.org.au/svn/cable/trunk {MOCK_CWD}/src/trunk", - shell=True, - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.STDOUT, + verbose=False, ) - # Success case: test non-verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - checkout_cable(branch_config) - assert buf.getvalue() == "Successfully checked out trunk at revision 123\n" + # Success case: test non-verbose standard output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + checkout_cable(branch_config) + assert buf.getvalue() == "Successfully checked out trunk at revision 123\n" - # Success case: test verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - checkout_cable(branch_config, verbose=True) - mock_subprocess_run.assert_called_once_with( - "svn checkout https://trac.nci.org.au/svn/cable/trunk " - f"{MOCK_CWD}/src/trunk", - shell=True, - check=True, - stdout=None, - stderr=subprocess.STDOUT, - ) - assert buf.getvalue() == ( - f"svn checkout https://trac.nci.org.au/svn/cable/trunk {MOCK_CWD}/src/trunk\n" - "Successfully checked out trunk at revision 123\n" - ) + # Success case: test verbose standard output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + checkout_cable(branch_config, verbose=True) + assert buf.getvalue() == "Successfully checked out trunk at revision 123\n" def test_checkout_cable_auxiliary(): @@ -114,50 +112,58 @@ def touch_files(*args, **kwargs): # pylint: disable=unused-argument "benchcab.get_cable.svn_info_show_item", mock_svn_info_show_item ): # Success case: checkout CABLE-AUX repository - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: - mock_subprocess_run.side_effect = touch_files + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = touch_files checkout_cable_auxiliary() - mock_subprocess_run.assert_called_once_with( + mock_run_cmd.assert_called_once_with( "svn checkout https://trac.nci.org.au/svn/cable/branches/Share/CABLE-AUX " f"{MOCK_CWD}/{internal.CABLE_AUX_DIR}", - shell=True, - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.STDOUT, + verbose=False, ) - shutil.rmtree(MOCK_CWD / internal.CABLE_AUX_DIR) - - # Success case: test non-verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - mock_subprocess_run.side_effect = touch_files - checkout_cable_auxiliary() - assert buf.getvalue() == "Successfully checked out CABLE-AUX at revision 123\n" shutil.rmtree(MOCK_CWD / internal.CABLE_AUX_DIR) - # Success case: test verbose output + # Success case: checkout CABLE-AUX repository with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - mock_subprocess_run.side_effect = touch_files + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = touch_files checkout_cable_auxiliary(verbose=True) - mock_subprocess_run.assert_called_once_with( + mock_run_cmd.assert_called_once_with( "svn checkout https://trac.nci.org.au/svn/cable/branches/Share/CABLE-AUX " f"{MOCK_CWD}/{internal.CABLE_AUX_DIR}", - shell=True, - check=True, - stdout=None, - stderr=subprocess.STDOUT, + verbose=True, ) - assert buf.getvalue() == ( - "svn checkout https://trac.nci.org.au/svn/cable/branches/Share/CABLE-AUX " - f"{MOCK_CWD}/{internal.CABLE_AUX_DIR}\n" - "Successfully checked out CABLE-AUX at revision 123\n" - ) shutil.rmtree(MOCK_CWD / internal.CABLE_AUX_DIR) - with unittest.mock.patch("subprocess.run"): + # Success case: test non-verbose standard output + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = touch_files + with contextlib.redirect_stdout(io.StringIO()) as buf: + checkout_cable_auxiliary() + assert ( + buf.getvalue() == "Successfully checked out CABLE-AUX at revision 123\n" + ) + shutil.rmtree(MOCK_CWD / internal.CABLE_AUX_DIR) + + # Success case: test verbose standard output + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = touch_files + with contextlib.redirect_stdout(io.StringIO()) as buf: + checkout_cable_auxiliary(verbose=True) + assert ( + buf.getvalue() == "Successfully checked out CABLE-AUX at revision 123\n" + ) + + shutil.rmtree(MOCK_CWD / internal.CABLE_AUX_DIR) + + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): # Failure case: missing grid file in CABLE-AUX repository touch_files() grid_file_path.unlink() @@ -194,42 +200,36 @@ def test_svn_info_show_item(): # Success case: run command for mock item with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd, unittest.mock.patch( + "subprocess.CompletedProcess", autospec=True ) as mock_completed_process: - mock_completed_process.configure_mock( - **{"stdout": "standard output from command"} + mock_completed_process.configure_mock(stdout="standard output from command") + mock_run_cmd.return_value = mock_completed_process + assert ( + svn_info_show_item("foo", "some-mock-item") + == "standard output from command" ) - mock_subprocess_run.return_value = mock_completed_process - ret = svn_info_show_item("foo", "some-mock-item") - assert ret == "standard output from command" - mock_subprocess_run.assert_called_once_with( - "svn info --show-item some-mock-item foo", - shell=True, - capture_output=True, - text=True, - check=True, + mock_run_cmd.assert_called_once_with( + "svn info --show-item some-mock-item foo", capture_output=True ) # Success case: test leading and trailing white space is removed from standard output with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd, unittest.mock.patch( + "subprocess.CompletedProcess", autospec=True ) as mock_completed_process: mock_completed_process.configure_mock( - **{"stdout": " standard output from command\n "} + stdout=" standard output from command\n " + ) + mock_run_cmd.return_value = mock_completed_process + assert ( + svn_info_show_item("foo", "some-mock-item") + == "standard output from command" ) - mock_subprocess_run.return_value = mock_completed_process - ret = svn_info_show_item("foo", "some-mock-item") - assert ret == "standard output from command" - mock_subprocess_run.assert_called_once_with( - "svn info --show-item some-mock-item foo", - shell=True, - capture_output=True, - text=True, - check=True, + mock_run_cmd.assert_called_once_with( + "svn info --show-item some-mock-item foo", capture_output=True ) diff --git a/tests/test_job_script.py b/tests/test_job_script.py index a3f43f64..58a4655d 100644 --- a/tests/test_job_script.py +++ b/tests/test_job_script.py @@ -8,7 +8,7 @@ import pytest from benchcab import internal -from benchcab.job_script import get_local_storage_flag, create_job_script, submit_job +from benchcab.job_script import get_local_storage_flag, submit_job from .common import MOCK_CWD @@ -28,15 +28,51 @@ def test_get_local_storage_flag(): get_local_storage_flag(Path("/home/189/foo")) -def test_create_job_script(): - """Tests for `create_job_script()`.""" +def test_submit_job(): + """Tests for `submit_job()`.""" - # Success case: test default job script creation + # Success case: test qsub command is executed with unittest.mock.patch( - "benchcab.job_script.get_local_storage_flag" - ) as mock_get_local_storage_flag: + "benchcab.job_script.get_local_storage_flag", autospec=True + ) as mock_get_local_storage_flag, unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_get_local_storage_flag.return_value = "storage_flag" + submit_job( + project="tm70", + config_path="/path/to/config.yaml", + modules=["foo", "bar", "baz"], + ) + mock_run_cmd.assert_called_once_with( + f"qsub {MOCK_CWD/internal.QSUB_FNAME}", capture_output=True, verbose=False + ) + + # Success case: test qsub command is executed with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module + with unittest.mock.patch( + "benchcab.job_script.get_local_storage_flag", autospec=True + ) as mock_get_local_storage_flag, unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_get_local_storage_flag.return_value = "storage_flag" + submit_job( + project="tm70", + config_path="/path/to/config.yaml", + modules=["foo", "bar", "baz"], + verbose=True, + ) + mock_run_cmd.assert_called_once_with( + f"qsub {MOCK_CWD/internal.QSUB_FNAME}", capture_output=True, verbose=True + ) + + # Success case: test default job script generated is correct + with unittest.mock.patch( + "benchcab.job_script.get_local_storage_flag", autospec=True + ) as mock_get_local_storage_flag, unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", + ): mock_get_local_storage_flag.return_value = "storage_flag" - create_job_script( + submit_job( project="tm70", config_path="/path/to/config.yaml", modules=["foo", "bar", "baz"], @@ -78,10 +114,12 @@ def test_create_job_script(): # Success case: skip fluxnet-bitwise-cmp step with unittest.mock.patch( - "benchcab.job_script.get_local_storage_flag" - ) as mock_get_local_storage_flag: + "benchcab.job_script.get_local_storage_flag", autospec=True + ) as mock_get_local_storage_flag, unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd" + ): mock_get_local_storage_flag.return_value = "storage_flag" - create_job_script( + submit_job( project="tm70", config_path="/path/to/config.yaml", modules=["foo", "bar", "baz"], @@ -117,13 +155,18 @@ def test_create_job_script(): """ ) - # Success case: test standard output + # Success case: test non-verbose output with unittest.mock.patch( "benchcab.job_script.get_local_storage_flag" - ) as mock_get_local_storage_flag: - mock_get_local_storage_flag.return_value = "storage_flag" + ), unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd, unittest.mock.patch( + "subprocess.CompletedProcess", autospec=True + ) as mock_completed_process: + mock_completed_process.configure_mock(stdout="standard output from qsub") + mock_run_cmd.return_value = mock_completed_process with contextlib.redirect_stdout(io.StringIO()) as buf: - create_job_script( + submit_job( project="tm70", config_path="/path/to/config.yaml", modules=["foo", "bar", "baz"], @@ -131,14 +174,17 @@ def test_create_job_script(): assert buf.getvalue() == ( "Creating PBS job script to run FLUXNET tasks on compute " f"nodes: {internal.QSUB_FNAME}\n" + "PBS job submitted: standard output from qsub\n" ) - # Success case: enable verbose flag + # Success case: add verbose flag to job script with unittest.mock.patch( - "benchcab.job_script.get_local_storage_flag" - ) as mock_get_local_storage_flag: + "benchcab.job_script.get_local_storage_flag", autospec=True + ) as mock_get_local_storage_flag, unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd" + ): mock_get_local_storage_flag.return_value = "storage_flag" - create_job_script( + submit_job( project="tm70", config_path="/path/to/config.yaml", modules=["foo", "bar", "baz"], @@ -179,41 +225,24 @@ def test_create_job_script(): """ ) - -def test_submit_job(): - """Tests for `submit_job()`.""" - - # Success case: submit PBS job - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: - submit_job() - mock_subprocess_run.assert_called_once_with( - f"qsub {MOCK_CWD/internal.QSUB_FNAME}", - shell=True, - check=True, - capture_output=True, - text=True, - ) - - # Success case: test standard output + # Failure case: qsub non-zero exit code prints an error message with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock(stdout="standard output from qsub") - mock_subprocess_run.return_value = mock_completed_process - with contextlib.redirect_stdout(io.StringIO()) as buf: - submit_job() - assert buf.getvalue() == "PBS job submitted: standard output from qsub\n" - - # Failure case: qsub non-zero exit code - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: - mock_subprocess_run.side_effect = subprocess.CalledProcessError( + "benchcab.job_script.get_local_storage_flag" + ), unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = subprocess.CalledProcessError( 1, "dummy-cmd", stderr="standard error from qsub" ) with contextlib.redirect_stdout(io.StringIO()) as buf: with pytest.raises(subprocess.CalledProcessError): - submit_job() + submit_job( + project="tm70", + config_path="/path/to/config.yaml", + modules=["foo", "bar", "baz"], + ) assert buf.getvalue() == ( - "Error when submitting job to NCI queue\n" "standard error from qsub\n" + "Creating PBS job script to run FLUXNET tasks on compute " + f"nodes: {internal.QSUB_FNAME}\n" + "Error when submitting job to NCI queue\nstandard error from qsub\n" ) diff --git a/tests/test_subprocess.py b/tests/test_subprocess.py new file mode 100644 index 00000000..02c68d41 --- /dev/null +++ b/tests/test_subprocess.py @@ -0,0 +1,73 @@ +"""`pytest` tests for utils/subprocess.py""" + +import subprocess +import pytest + +from benchcab.utils.subprocess import run_cmd +from .common import TMP_DIR + + +def test_run_cmd(capfd): + """Tests for `run_cmd()`.""" + + # Success case: test stdout is suppressed in non-verbose mode + run_cmd("echo foo") + captured = capfd.readouterr() + assert captured.out == "" + assert captured.err == "" + + # Success case: test stderr is suppressed in non-verbose mode + run_cmd("echo foo 1>&2") + captured = capfd.readouterr() + assert captured.out == "" + assert captured.err == "" + + # Success case: test stdout is printed in verbose mode + run_cmd("echo foo", verbose=True) + captured = capfd.readouterr() + assert captured.out == "echo foo\nfoo\n" + assert captured.err == "" + + # Success case: test stderr is redirected to stdout in verbose mode + run_cmd("echo foo 1>&2", verbose=True) + captured = capfd.readouterr() + assert captured.out == "echo foo 1>&2\nfoo\n" + assert captured.err == "" + + # Success case: test output is captured with capture_output enabled + proc = run_cmd("echo foo", capture_output=True) + captured = capfd.readouterr() + assert captured.out == "" + assert captured.err == "" + assert proc.stdout == "foo\n" + assert proc.stderr == "" + + # Success case: test command is printed in verbose mode + proc = run_cmd("echo foo", capture_output=True, verbose=True) + captured = capfd.readouterr() + assert captured.out == "echo foo\n" + assert captured.err == "" + assert proc.stdout == "foo\n" + assert proc.stderr == "" + + # Success case: redirect output to file descriptor + file_path = TMP_DIR / "out.txt" + run_cmd("echo foo", output_file=file_path) + with open(file_path, "r", encoding="utf-8") as file: + assert file.read() == "foo\n" + captured = capfd.readouterr() + assert captured.out == "" + assert captured.err == "" + + # Success case: redirect output to file descriptor in verbose mode + file_path = TMP_DIR / "out.txt" + run_cmd("echo foo", output_file=file_path, verbose=True) + with open(file_path, "r", encoding="utf-8") as file: + assert file.read() == "foo\n" + captured = capfd.readouterr() + assert captured.out == "echo foo\n" + assert captured.err == "" + + # Failure case: check non-zero return code throws an exception + with pytest.raises(subprocess.CalledProcessError): + run_cmd("exit 1") diff --git a/tests/test_task.py b/tests/test_task.py index fbdc0112..9a183fe9 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -264,36 +264,48 @@ def test_run_cable(): nml_path = task_dir / internal.CABLE_NML nml_path.touch() + stdout_file = task_dir / internal.CABLE_STDOUT_FILENAME + # Success case: run CABLE executable in subprocess - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: task.run_cable() - mock_subprocess_run.assert_called_once_with( - f"{exe_path} {nml_path} > {task_dir / internal.CABLE_STDOUT_FILENAME} 2>&1", - shell=True, - check=True, + mock_run_cmd.assert_called_once_with( + f"{exe_path} {nml_path}", + output_file=stdout_file, + verbose=False, ) - # Success case: test non-verbose output + # Success case: run CABLE executable in subprocess with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - task.run_cable() - assert not buf.getvalue() + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + task.run_cable(verbose=True) + mock_run_cmd.assert_called_once_with( + f"{exe_path} {nml_path}", + output_file=stdout_file, + verbose=True, + ) + + # Success case: test non-verbose output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + task.run_cable() + assert not buf.getvalue() # Success case: test verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, contextlib.redirect_stdout(io.StringIO()) as buf: - task.run_cable(verbose=True) - assert buf.getvalue() == ( - f" {MOCK_CWD}/runs/site/tasks/forcing-file_R1_S0/cable " - f"{MOCK_CWD}/runs/site/tasks/forcing-file_R1_S0/cable.nml " - f"> {MOCK_CWD}/runs/site/tasks/forcing-file_R1_S0/out.txt 2>&1\n" - ) + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): + with contextlib.redirect_stdout(io.StringIO()) as buf: + task.run_cable(verbose=True) + assert not buf.getvalue() # Failure case: raise CableError on subprocess non-zero exit code - with unittest.mock.patch("subprocess.run") as mock_subprocess_run: - mock_subprocess_run.side_effect = subprocess.CalledProcessError( + with unittest.mock.patch( + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = subprocess.CalledProcessError( returncode=1, cmd="cmd" ) with pytest.raises(CableError): @@ -355,60 +367,6 @@ def mock_svn_info_show_item(*args, **kwargs): # pylint: disable=unused-argument ) -def test_run(): - """Tests for `run()`.""" - - task = setup_mock_task() - - # Success case: run CABLE and add attributes to netcdf output file - with unittest.mock.patch( - "benchcab.task.Task.run_cable" - ) as mock_run_cable, unittest.mock.patch( - "benchcab.task.Task.add_provenance_info" - ) as mock_add_provenance_info: - task.run() - mock_run_cable.assert_called_once() - mock_add_provenance_info.assert_called_once() - - # Success case: do not add attributes to netcdf file on failure - with unittest.mock.patch( - "benchcab.task.Task.run_cable" - ) as mock_run_cable, unittest.mock.patch( - "benchcab.task.Task.add_provenance_info" - ) as mock_add_provenance_info: - mock_run_cable.side_effect = CableError - task.run() - mock_run_cable.assert_called_once() - mock_add_provenance_info.assert_not_called() - - # Success case: test non-verbose output - with unittest.mock.patch( - "benchcab.task.Task.run_cable" - ) as mock_run_cable, unittest.mock.patch( - "benchcab.task.Task.add_provenance_info" - ) as mock_add_provenance_info: - with contextlib.redirect_stdout(io.StringIO()) as buf: - task.run() - mock_run_cable.assert_called_once_with(verbose=False) - mock_add_provenance_info.assert_called_once_with(verbose=False) - assert not buf.getvalue() - - # Success case: test verbose output - with unittest.mock.patch( - "benchcab.task.Task.run_cable" - ) as mock_run_cable, unittest.mock.patch( - "benchcab.task.Task.add_provenance_info" - ) as mock_add_provenance_info: - with contextlib.redirect_stdout(io.StringIO()) as buf: - task.run(verbose=True) - mock_run_cable.assert_called_once_with(verbose=True) - mock_add_provenance_info.assert_called_once_with(verbose=True) - assert buf.getvalue() == ( - "Running task forcing-file_R1_S0... CABLE standard output saved in " - f"{MOCK_CWD}/runs/site/tasks/forcing-file_R1_S0/out.txt\n" - ) - - def test_get_fluxnet_tasks(): """Tests for `get_fluxnet_tasks()`.""" @@ -492,6 +450,7 @@ def test_get_comparison_name(): def test_run_comparison(): """Tests for `run_comparison()`.""" + bitwise_cmp_dir = MOCK_CWD / internal.SITE_BITWISE_CMP_DIR bitwise_cmp_dir.mkdir(parents=True) output_dir = MOCK_CWD / internal.SITE_OUTPUT_DIR @@ -500,40 +459,31 @@ def test_run_comparison(): # Success case: run comparison with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock( - **{ - "returncode": 0, - "stdout": "standard output from subprocess", - } - ) - mock_subprocess_run.return_value = mock_completed_process + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: run_comparison(task_a, task_b) - mock_subprocess_run.assert_called_once_with( + mock_run_cmd.assert_called_once_with( f"nccmp -df {output_dir / task_a.get_output_filename()} " - f"{output_dir / task_b.get_output_filename()} 2>&1", - shell=True, - check=False, + f"{output_dir / task_b.get_output_filename()}", capture_output=True, - text=True, + verbose=False, ) - # Success case: test non-verbose output + # Success case: run comparison with verbose enabled + # TODO(Sean): this test should be removed once we use the logging module with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock( - **{ - "returncode": 0, - "stdout": "standard output from subprocess", - } + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + run_comparison(task_a, task_b, verbose=True) + mock_run_cmd.assert_called_once_with( + f"nccmp -df {output_dir / task_a.get_output_filename()} " + f"{output_dir / task_b.get_output_filename()}", + capture_output=True, + verbose=True, ) - mock_subprocess_run.return_value = mock_completed_process + + # Success case: test non-verbose output + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): with contextlib.redirect_stdout(io.StringIO()) as buf: run_comparison(task_a, task_b) assert buf.getvalue() == ( @@ -542,60 +492,30 @@ def test_run_comparison(): ) # Success case: test verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock( - **{ - "returncode": 0, - "stdout": "standard output from subprocess", - } - ) - mock_subprocess_run.return_value = mock_completed_process + with unittest.mock.patch("benchcab.utils.subprocess.run_cmd"): with contextlib.redirect_stdout(io.StringIO()) as buf: run_comparison(task_a, task_b, verbose=True) assert buf.getvalue() == ( f"Comparing files {task_a.get_output_filename()} and " f"{task_b.get_output_filename()} bitwise...\n" - f" nccmp -df {output_dir / task_a.get_output_filename()} " - f"{output_dir / task_b.get_output_filename()} 2>&1\n" f"Success: files {task_a.get_output_filename()} " f"{task_b.get_output_filename()} are identical\n" ) - # Failure case: run comparison with non-zero exit code with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock( - **{ - "returncode": 1, - "stdout": "standard output from subprocess", - } + "benchcab.utils.subprocess.run_cmd", autospec=True + ) as mock_run_cmd: + mock_run_cmd.side_effect = subprocess.CalledProcessError( + returncode=1, cmd="dummy cmd", output="error message from command" ) - mock_subprocess_run.return_value = mock_completed_process + + # Failure case: test failed comparison check (files differ) run_comparison(task_a, task_b) stdout_file = bitwise_cmp_dir / f"{get_comparison_name(task_a, task_b)}.txt" with open(stdout_file, "r", encoding="utf-8") as file: - assert file.read() == "standard output from subprocess" + assert file.read() == "error message from command" - # Failure case: test non-verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock( - **{ - "returncode": 1, - "stdout": "standard output from subprocess", - } - ) - mock_subprocess_run.return_value = mock_completed_process + # Failure case: test non-verbose standard output on failure with contextlib.redirect_stdout(io.StringIO()) as buf: run_comparison(task_a, task_b) stdout_file = bitwise_cmp_dir / f"{get_comparison_name(task_a, task_b)}.txt" @@ -605,27 +525,13 @@ def test_run_comparison(): f"have been written to {stdout_file}\n" ) - # Failure case: test verbose output - with unittest.mock.patch( - "subprocess.run" - ) as mock_subprocess_run, unittest.mock.patch( - "subprocess.CompletedProcess" - ) as mock_completed_process: - mock_completed_process.configure_mock( - **{ - "returncode": 1, - "stdout": "standard output from subprocess", - } - ) - mock_subprocess_run.return_value = mock_completed_process + # Failure case: test verbose standard output on failure with contextlib.redirect_stdout(io.StringIO()) as buf: run_comparison(task_a, task_b, verbose=True) stdout_file = bitwise_cmp_dir / f"{get_comparison_name(task_a, task_b)}.txt" assert buf.getvalue() == ( f"Comparing files {task_a.get_output_filename()} and " f"{task_b.get_output_filename()} bitwise...\n" - f" nccmp -df {output_dir / task_a.get_output_filename()} " - f"{output_dir / task_b.get_output_filename()} 2>&1\n" f"Failure: files {task_a.get_output_filename()} " f"{task_b.get_output_filename()} differ. Results of diff " f"have been written to {stdout_file}\n"