From d37b36b87b0f896913311776e30c5ef1683e8112 Mon Sep 17 00:00:00 2001 From: Sean Bryan Date: Fri, 1 Dec 2023 14:16:56 +1100 Subject: [PATCH] Remove dependence on root directory This change forces paths to be relative to the CWD throughout the code base unless absolute paths are required. Improves code readability and the logging to standard output. Fixes #162 --- benchcab/benchcab.py | 13 +++---- benchcab/comparison.py | 5 +-- benchcab/fluxsite.py | 73 +++++++++++++--------------------------- benchcab/internal.py | 3 +- benchcab/model.py | 51 +++++++++------------------- benchcab/utils/fs.py | 4 +-- tests/test_comparison.py | 14 +++----- tests/test_fluxsite.py | 72 ++++++++++++--------------------------- tests/test_fs.py | 16 ++++----- tests/test_model.py | 30 ++++++----------- 10 files changed, 94 insertions(+), 187 deletions(-) diff --git a/benchcab/benchcab.py b/benchcab/benchcab.py index fab16f4c..6b2a7ead 100644 --- a/benchcab/benchcab.py +++ b/benchcab/benchcab.py @@ -33,7 +33,6 @@ class Benchcab: """A class that represents the `benchcab` application.""" - root_dir: Path = internal.CWD subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper() modules_handler: EnvironmentModulesInterface = EnvironmentModules() @@ -143,10 +142,10 @@ def fluxsite_submit_job( msg = "Path to benchcab executable is undefined." raise RuntimeError(msg) - job_script_path = self.root_dir / internal.QSUB_FNAME + job_script_path = Path(internal.QSUB_FNAME) print( "Creating PBS job script to run fluxsite tasks on compute " - f"nodes: {job_script_path.relative_to(self.root_dir)}" + f"nodes: {job_script_path}" ) with job_script_path.open("w", encoding="utf-8") as file: contents = render_job_script( @@ -202,12 +201,8 @@ def checkout(self, config_path: str, verbose: bool): ) cable_aux_repo.checkout(verbose=verbose) - rev_number_log_path = self.root_dir / next_path( - self.root_dir, "rev_number-*.log" - ) - print( - f"Writing revision number info to {rev_number_log_path.relative_to(self.root_dir)}" - ) + rev_number_log_path = next_path("rev_number-*.log") + print(f"Writing revision number info to {rev_number_log_path}") with rev_number_log_path.open("w", encoding="utf-8") as file: file.write(rev_number_log) diff --git a/benchcab/comparison.py b/benchcab/comparison.py index 32bbd79c..e3f1200d 100644 --- a/benchcab/comparison.py +++ b/benchcab/comparison.py @@ -16,7 +16,6 @@ class ComparisonTask: """A class used to represent a single bitwise comparison task.""" - root_dir: Path = internal.CWD subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper() def __init__( @@ -42,9 +41,7 @@ def run(self, verbose=False) -> None: print(f"Success: files {file_a.name} {file_b.name} are identical") except CalledProcessError as exc: output_file = ( - self.root_dir - / internal.FLUXSITE_DIRS["BITWISE_CMP"] - / f"{self.task_name}.txt" + internal.FLUXSITE_DIRS["BITWISE_CMP"] / f"{self.task_name}.txt" ) with output_file.open("w", encoding="utf-8") as file: file.write(exc.stdout) diff --git a/benchcab/fluxsite.py b/benchcab/fluxsite.py index 3797c1cd..fb6a09de 100644 --- a/benchcab/fluxsite.py +++ b/benchcab/fluxsite.py @@ -93,7 +93,6 @@ class CableError(Exception): class Task: """A class used to represent a single fluxsite task.""" - root_dir: Path = internal.CWD subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper() def __init__( @@ -147,10 +146,7 @@ def setup_task(self, verbose=False): self.fetch_files(verbose=verbose) nml_path = ( - self.root_dir - / internal.FLUXSITE_DIRS["TASKS"] - / self.get_task_name() - / internal.CABLE_NML + internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() / internal.CABLE_NML ) if verbose: @@ -162,25 +158,26 @@ def setup_task(self, verbose=False): "filename": { "met": str(internal.MET_DIR / self.met_forcing_file), "out": str( - self.root_dir - / internal.FLUXSITE_DIRS["OUTPUT"] - / self.get_output_filename() + ( + internal.FLUXSITE_DIRS["OUTPUT"] + / self.get_output_filename() + ).resolve() ), "log": str( - self.root_dir - / internal.FLUXSITE_DIRS["LOG"] - / self.get_log_filename() + ( + internal.FLUXSITE_DIRS["LOG"] / self.get_log_filename() + ).resolve() ), "restart_out": " ", - "type": str(self.root_dir / internal.GRID_FILE), + "type": str(internal.GRID_FILE.resolve()), }, "output": { "restart": False, }, "fixedCO2": internal.CABLE_FIXED_CO2_CONC, "casafile": { - "phen": str(self.root_dir / internal.PHEN_FILE), - "cnpbiome": str(self.root_dir / internal.CNPBIOME_FILE), + "phen": str(internal.PHEN_FILE.resolve()), + "cnpbiome": str(internal.CNPBIOME_FILE.resolve()), }, "spinup": False, } @@ -210,9 +207,7 @@ def clean_task(self, verbose=False): if verbose: print(" Cleaning task") - task_dir = ( - self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() - ) + task_dir = internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() cable_exe = task_dir / internal.CABLE_EXE if cable_exe.exists(): @@ -230,17 +225,11 @@ def clean_task(self, verbose=False): if cable_soil_nml.exists(): cable_soil_nml.unlink() - output_file = ( - self.root_dir - / internal.FLUXSITE_DIRS["OUTPUT"] - / self.get_output_filename() - ) + output_file = internal.FLUXSITE_DIRS["OUTPUT"] / self.get_output_filename() if output_file.exists(): output_file.unlink() - log_file = ( - self.root_dir / internal.FLUXSITE_DIRS["LOG"] / self.get_log_filename() - ) + log_file = internal.FLUXSITE_DIRS["LOG"] / self.get_log_filename() if log_file.exists(): log_file.unlink() @@ -253,19 +242,14 @@ def fetch_files(self, verbose=False): - copies contents of 'namelists' directory to 'runs/fluxsite/tasks/' directory. - copies cable executable from source to 'runs/fluxsite/tasks/' directory. """ - task_dir = ( - self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() - ) + task_dir = internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() if verbose: print( - f" Copying namelist files from {self.root_dir / internal.NAMELIST_DIR} " - f"to {task_dir}" + f" Copying namelist files from {internal.NAMELIST_DIR} to {task_dir}" ) - shutil.copytree( - self.root_dir / internal.NAMELIST_DIR, task_dir, dirs_exist_ok=True - ) + shutil.copytree(internal.NAMELIST_DIR, task_dir, dirs_exist_ok=True) exe_src = self.model.get_exe_path() exe_dest = task_dir / internal.CABLE_EXE @@ -280,7 +264,7 @@ def fetch_files(self, verbose=False): def run(self, verbose=False): """Runs a single fluxsite task.""" task_name = self.get_task_name() - task_dir = self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / task_name + task_dir = internal.FLUXSITE_DIRS["TASKS"] / task_name if verbose: print( f"Running task {task_name}... CABLE standard output " @@ -303,14 +287,14 @@ def run_cable(self, verbose=False): Raises `CableError` when CABLE returns a non-zero exit code. """ task_name = self.get_task_name() - task_dir = self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / task_name + task_dir = internal.FLUXSITE_DIRS["TASKS"] / task_name stdout_path = task_dir / internal.CABLE_STDOUT_FILENAME try: with chdir(task_dir): self.subprocess_handler.run_cmd( f"./{internal.CABLE_EXE} {internal.CABLE_NML}", - output_file=stdout_path, + output_file=stdout_path.relative_to(task_dir), verbose=verbose, ) except CalledProcessError as exc: @@ -323,16 +307,9 @@ def add_provenance_info(self, verbose=False): Attributes include branch url, branch revision number and key value pairs in the namelist file used to run cable. """ - nc_output_path = ( - self.root_dir - / internal.FLUXSITE_DIRS["OUTPUT"] - / self.get_output_filename() - ) + nc_output_path = internal.FLUXSITE_DIRS["OUTPUT"] / self.get_output_filename() nml = f90nml.read( - self.root_dir - / internal.FLUXSITE_DIRS["TASKS"] - / self.get_task_name() - / internal.CABLE_NML + internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() / internal.CABLE_NML ) if verbose: print(f"Adding attributes to output file: {nc_output_path}") @@ -389,16 +366,14 @@ def run_tasks_in_parallel( pool.map(run_task, tasks, chunksize=1) -def get_fluxsite_comparisons( - tasks: list[Task], root_dir=internal.CWD -) -> list[ComparisonTask]: +def get_fluxsite_comparisons(tasks: list[Task]) -> list[ComparisonTask]: """Returns a list of `ComparisonTask` objects to run comparisons with. Pairs should be matching in science configurations and meteorological forcing, but differ in realisations. When multiple realisations are specified, return all pair wise combinations between all realisations. """ - output_dir = root_dir / internal.FLUXSITE_DIRS["OUTPUT"] + output_dir = internal.FLUXSITE_DIRS["OUTPUT"] return [ ComparisonTask( files=( diff --git a/benchcab/internal.py b/benchcab/internal.py index bf6b6799..365f4481 100644 --- a/benchcab/internal.py +++ b/benchcab/internal.py @@ -61,7 +61,8 @@ ) # Fluxsite directory tree -FLUXSITE_DIRS = {} +FLUXSITE_DIRS: dict[str, Path] = {} + # Relative path to root directory for CABLE fluxsite runs FLUXSITE_DIRS["RUN"] = RUN_DIR / "fluxsite" diff --git a/benchcab/model.py b/benchcab/model.py index 269f5b70..dafd21e3 100644 --- a/benchcab/model.py +++ b/benchcab/model.py @@ -20,7 +20,6 @@ class Model: """A class used to represent a CABLE model version.""" - root_dir: Path = internal.CWD subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper() modules_handler: EnvironmentModulesInterface = EnvironmentModules() @@ -61,19 +60,12 @@ def model_id(self, value: int): def get_exe_path(self) -> Path: """Return the path to the built executable.""" return ( - self.root_dir - / internal.SRC_DIR - / self.name - / self.src_dir - / "offline" - / internal.CABLE_EXE + internal.SRC_DIR / self.name / self.src_dir / "offline" / internal.CABLE_EXE ) def custom_build(self, modules: list[str], verbose=False): """Build CABLE using a custom build script.""" - build_script_path = ( - self.root_dir / internal.SRC_DIR / self.name / self.build_script - ) + build_script_path = internal.SRC_DIR / self.name / self.build_script if not build_script_path.is_file(): msg = ( @@ -85,6 +77,7 @@ def custom_build(self, modules: list[str], verbose=False): tmp_script_path = build_script_path.parent / "tmp-build.sh" + # TODO(Sean) should this be a utility function? if verbose: print(f"Copying {build_script_path} to {tmp_script_path}") shutil.copy(build_script_path, tmp_script_path) @@ -110,50 +103,40 @@ def custom_build(self, modules: list[str], verbose=False): def pre_build(self, verbose=False): """Runs CABLE pre-build steps.""" - path_to_repo = self.root_dir / internal.SRC_DIR / self.name + path_to_repo = internal.SRC_DIR / self.name tmp_dir = path_to_repo / self.src_dir / "offline" / ".tmp" if not tmp_dir.exists(): if verbose: - print(f"mkdir {tmp_dir.relative_to(self.root_dir)}") + print(f"mkdir {tmp_dir}") tmp_dir.mkdir() for pattern in internal.OFFLINE_SOURCE_FILES: for path in (path_to_repo / self.src_dir).glob(pattern): if not path.is_file(): continue - copy2( - path.relative_to(self.root_dir), - tmp_dir.relative_to(self.root_dir), - verbose=verbose, - ) + copy2(path, tmp_dir, verbose=verbose) copy2( - (path_to_repo / self.src_dir / "offline" / "Makefile").relative_to( - self.root_dir - ), - tmp_dir.relative_to(self.root_dir), + (path_to_repo / self.src_dir / "offline" / "Makefile"), + tmp_dir, verbose=verbose, ) copy2( - (path_to_repo / self.src_dir / "offline" / "parallel_cable").relative_to( - self.root_dir - ), - tmp_dir.relative_to(self.root_dir), + (path_to_repo / self.src_dir / "offline" / "parallel_cable"), + tmp_dir, verbose=verbose, ) copy2( - (path_to_repo / self.src_dir / "offline" / "serial_cable").relative_to( - self.root_dir - ), - tmp_dir.relative_to(self.root_dir), + (path_to_repo / self.src_dir / "offline" / "serial_cable"), + tmp_dir, verbose=verbose, ) def run_build(self, modules: list[str], verbose=False): """Runs CABLE build scripts.""" - path_to_repo = self.root_dir / internal.SRC_DIR / self.name + path_to_repo = internal.SRC_DIR / self.name tmp_dir = path_to_repo / self.src_dir / "offline" / ".tmp" with chdir(tmp_dir), self.modules_handler.load(modules, verbose=verbose): @@ -177,14 +160,12 @@ def run_build(self, modules: list[str], verbose=False): def post_build(self, verbose=False): """Runs CABLE post-build steps.""" - path_to_repo = self.root_dir / internal.SRC_DIR / self.name + path_to_repo = internal.SRC_DIR / self.name tmp_dir = path_to_repo / self.src_dir / "offline" / ".tmp" rename( - (tmp_dir / internal.CABLE_EXE).relative_to(self.root_dir), - (path_to_repo / self.src_dir / "offline" / internal.CABLE_EXE).relative_to( - self.root_dir - ), + (tmp_dir / internal.CABLE_EXE), + (path_to_repo / self.src_dir / "offline" / internal.CABLE_EXE), verbose=verbose, ) diff --git a/benchcab/utils/fs.py b/benchcab/utils/fs.py index a53f46d2..13726835 100644 --- a/benchcab/utils/fs.py +++ b/benchcab/utils/fs.py @@ -34,7 +34,7 @@ def copy2(src: Path, dest: Path, verbose=False): shutil.copy2(src, dest) -def next_path(path: Path, path_pattern: str, sep: str = "-"): +def next_path(path_pattern: str, path: Path = Path(), sep: str = "-") -> Path: """Finds the next free path in a sequentially named list of files with the following pattern in the `path` directory: @@ -54,7 +54,7 @@ def next_path(path: Path, path_pattern: str, sep: str = "-"): common_filename, last_file_index = pattern_files_sorted[-1].stem.split(sep) new_file_index = int(last_file_index) + 1 - return f"{common_filename}{sep}{new_file_index}{loc_pattern.suffix}" + return Path(f"{common_filename}{sep}{new_file_index}{loc_pattern.suffix}") def mkdir(new_path: Path, verbose=False, **kwargs): diff --git a/tests/test_comparison.py b/tests/test_comparison.py index d5ebba47..6dd212e9 100644 --- a/tests/test_comparison.py +++ b/tests/test_comparison.py @@ -25,18 +25,17 @@ def files(): @pytest.fixture() -def comparison_task(files, mock_cwd, mock_subprocess_handler): +def comparison_task(files, mock_subprocess_handler): """Returns a mock `ComparisonTask` instance for testing against.""" _comparison_task = ComparisonTask(files=files, task_name=TASK_NAME) _comparison_task.subprocess_handler = mock_subprocess_handler - _comparison_task.root_dir = mock_cwd return _comparison_task class TestRun: """Tests for `ComparisonTask.run()`.""" - @pytest.fixture() + @pytest.fixture(autouse=True) def bitwise_cmp_dir(self): """Create and return the fluxsite bitwise comparison directory.""" internal.FLUXSITE_DIRS["BITWISE_CMP"].mkdir(parents=True) @@ -78,11 +77,6 @@ def test_failed_comparison_check( with stdout_file.open("r", encoding="utf-8") as file: assert file.read() == mock_subprocess_handler.stdout - # TODO(Sean) fix for issue https://github.com/CABLE-LSM/benchcab/issues/162 - @pytest.mark.skip( - reason="""This will always fail since `parametrize()` parameters are - dependent on the `mock_cwd` fixture.""" - ) @pytest.mark.parametrize( ("verbosity", "expected"), [ @@ -90,14 +84,14 @@ def test_failed_comparison_check( False, f"Failure: files {FILE_NAME_A} {FILE_NAME_B} differ. Results of " "diff have been written to " - f"{internal.FLUXSITE_DIRS['BITWISE_CMP']}/{TASK_NAME}\n", + f"{internal.FLUXSITE_DIRS['BITWISE_CMP']}/{TASK_NAME}.txt\n", ), ( True, f"Comparing files {FILE_NAME_A} and {FILE_NAME_B} bitwise...\n" f"Failure: files {FILE_NAME_A} {FILE_NAME_B} differ. Results of " "diff have been written to " - f"{internal.FLUXSITE_DIRS['BITWISE_CMP']}/{TASK_NAME}\n", + f"{internal.FLUXSITE_DIRS['BITWISE_CMP']}/{TASK_NAME}.txt\n", ), ], ) diff --git a/tests/test_fluxsite.py b/tests/test_fluxsite.py index 8f97f0c4..9042cd3e 100644 --- a/tests/test_fluxsite.py +++ b/tests/test_fluxsite.py @@ -48,7 +48,7 @@ def get_revision(self) -> str: @pytest.fixture() -def model(mock_cwd, mock_subprocess_handler, mock_repo): +def model(mock_subprocess_handler, mock_repo): """Returns a `Model` instance.""" _model = Model( model_id=1, @@ -56,12 +56,11 @@ def model(mock_cwd, mock_subprocess_handler, mock_repo): patch={"cable": {"some_branch_specific_setting": True}}, ) _model.subprocess_handler = mock_subprocess_handler - _model.root_dir = mock_cwd return _model @pytest.fixture() -def task(model, mock_cwd, mock_subprocess_handler): +def task(model, mock_subprocess_handler): """Returns a mock `Task` instance.""" _task = Task( model=model, @@ -70,7 +69,6 @@ def task(model, mock_cwd, mock_subprocess_handler): sci_config={"cable": {"some_setting": True}}, ) _task.subprocess_handler = mock_subprocess_handler - _task.root_dir = mock_cwd return _task @@ -260,7 +258,7 @@ def _setup(self, task): exe_build_dir.mkdir(parents=True) (exe_build_dir / internal.CABLE_EXE).touch() - def test_all_settings_are_patched_into_namelist_file(self, task, mock_cwd): + def test_all_settings_are_patched_into_namelist_file(self, task): """Success case: test all settings are patched into task namelist file.""" task.setup_task() task_dir = internal.FLUXSITE_DIRS["TASKS"] / task.get_task_name() @@ -269,32 +267,27 @@ def test_all_settings_are_patched_into_namelist_file(self, task, mock_cwd): "filename": { "met": str(internal.MET_DIR / "forcing-file.nc"), "out": str( - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / task.get_output_filename() + ( + internal.FLUXSITE_DIRS["OUTPUT"] / task.get_output_filename() + ).resolve() ), "log": str( - mock_cwd / internal.FLUXSITE_DIRS["LOG"] / task.get_log_filename() + (internal.FLUXSITE_DIRS["LOG"] / task.get_log_filename()).resolve() ), "restart_out": " ", - "type": str(mock_cwd / internal.GRID_FILE), + "type": str(internal.GRID_FILE.resolve()), }, "output": {"restart": False}, "fixedco2": internal.CABLE_FIXED_CO2_CONC, "casafile": { - "phen": str(mock_cwd / internal.PHEN_FILE), - "cnpbiome": str(mock_cwd / internal.CNPBIOME_FILE), + "phen": str(internal.PHEN_FILE.resolve()), + "cnpbiome": str(internal.CNPBIOME_FILE.resolve()), }, "spinup": False, "some_setting": True, "some_branch_specific_setting": True, } - # TODO(Sean) fix for issue https://github.com/CABLE-LSM/benchcab/issues/162 - @pytest.mark.skip( - reason="""This will always fail since `parametrize()` parameters are - dependent on the `mock_cwd` fixture.""" - ) @pytest.mark.parametrize( ("verbosity", "expected"), [ @@ -406,11 +399,6 @@ def test_netcdf_global_attributes(self, task, nc_output_path, mock_repo, nml): assert atts[r"filename%foo"] == nml["cable"]["filename"]["foo"] assert atts[r"bar"] == ".true." - # TODO(Sean) fix for issue https://github.com/CABLE-LSM/benchcab/issues/162 - @pytest.mark.skip( - reason="""This will always fail since `parametrize()` parameters are - dependent on the `mock_cwd` fixture.""" - ) @pytest.mark.parametrize( ("verbosity", "expected"), [ @@ -476,7 +464,7 @@ def test_task_product_across_branches_forcings_and_configurations( class TestGetFluxsiteComparisons: """Tests for `get_fluxsite_comparisons()`.""" - def test_comparisons_for_two_branches_with_two_tasks(self, mock_cwd, mock_repo): + def test_comparisons_for_two_branches_with_two_tasks(self, mock_repo): """Success case: comparisons for two branches with two tasks.""" tasks = [ Task( @@ -487,23 +475,19 @@ def test_comparisons_for_two_branches_with_two_tasks(self, mock_cwd, mock_repo): ) for model_id in range(2) ] - comparisons = get_fluxsite_comparisons(tasks, root_dir=mock_cwd) + comparisons = get_fluxsite_comparisons(tasks) n_models, n_science_configurations, n_met_forcings = 2, 1, 1 assert ( len(comparisons) == math.comb(n_models, 2) * n_science_configurations * n_met_forcings ) assert comparisons[0].files == ( - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[0].get_output_filename(), - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[1].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[0].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[1].get_output_filename(), ) assert comparisons[0].task_name == "foo_S0_R0_R1" - def test_comparisons_for_three_branches_with_three_tasks(self, mock_cwd, mock_repo): + def test_comparisons_for_three_branches_with_three_tasks(self, mock_repo): """Success case: comparisons for three branches with three tasks.""" tasks = [ Task( @@ -514,35 +498,23 @@ def test_comparisons_for_three_branches_with_three_tasks(self, mock_cwd, mock_re ) for model_id in range(3) ] - comparisons = get_fluxsite_comparisons(tasks, root_dir=mock_cwd) + comparisons = get_fluxsite_comparisons(tasks) n_models, n_science_configurations, n_met_forcings = 3, 1, 1 assert ( len(comparisons) == math.comb(n_models, 2) * n_science_configurations * n_met_forcings ) assert comparisons[0].files == ( - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[0].get_output_filename(), - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[1].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[0].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[1].get_output_filename(), ) assert comparisons[1].files == ( - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[0].get_output_filename(), - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[2].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[0].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[2].get_output_filename(), ) assert comparisons[2].files == ( - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[1].get_output_filename(), - mock_cwd - / internal.FLUXSITE_DIRS["OUTPUT"] - / tasks[2].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[1].get_output_filename(), + internal.FLUXSITE_DIRS["OUTPUT"] / tasks[2].get_output_filename(), ) assert comparisons[0].task_name == "foo_S0_R0_R1" assert comparisons[1].task_name == "foo_S0_R0_R2" diff --git a/tests/test_fs.py b/tests/test_fs.py index be55cf19..c84cc4e2 100644 --- a/tests/test_fs.py +++ b/tests/test_fs.py @@ -22,14 +22,14 @@ def pattern(self): """Return a file pattern for testing against.""" return "rev_number-*.log" - def test_next_path_in_empty_cwd(self, pattern, mock_cwd): - """Success case: get next path in 'empty' CWD.""" - assert next_path(mock_cwd, pattern) == "rev_number-1.log" - - def test_next_path_in_non_empty_cwd(self, pattern, mock_cwd): - """Success case: get next path in 'non-empty' CWD.""" - (mock_cwd / next_path(mock_cwd, pattern)).touch() - assert next_path(mock_cwd, pattern) == "rev_number-2.log" + def test_next_path_in_empty_cwd(self, pattern): + """Success case: get next path in 'empty' directory.""" + assert next_path(pattern) == Path("rev_number-1.log") + + def test_next_path_in_non_empty_cwd(self, pattern): + """Success case: get next path in 'non-empty' directory.""" + Path(next_path(pattern)).touch() + assert next_path(pattern) == Path("rev_number-2.log") class TestMkdir: diff --git a/tests/test_model.py b/tests/test_model.py index b35ca3c4..86153478 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -42,12 +42,9 @@ def get_revision(self) -> str: @pytest.fixture() -def model( - mock_repo, mock_cwd, mock_subprocess_handler, mock_environment_modules_handler -): +def model(mock_repo, mock_subprocess_handler, mock_environment_modules_handler): """Return a mock `Model` instance for testing against.""" _model = Model(repo=mock_repo) - _model.root_dir = mock_cwd _model.subprocess_handler = mock_subprocess_handler _model.modules_handler = mock_environment_modules_handler return _model @@ -74,11 +71,11 @@ def test_undefined_model_id(self, model): class TestGetExePath: """Tests for `Model.get_exe_path()`.""" - def test_serial_exe_path(self, model, mock_cwd): + def test_serial_exe_path(self, model): """Success case: get path to serial executable.""" assert ( model.get_exe_path() - == mock_cwd / internal.SRC_DIR / model.name / "offline" / internal.CABLE_EXE + == internal.SRC_DIR / model.name / "offline" / internal.CABLE_EXE ) @@ -90,22 +87,22 @@ def test_serial_exe_path(self, model, mock_cwd): class TestCheckout: """Tests for `Model.checkout()`.""" - def test_checkout_command_execution(self, model, mock_cwd, mock_subprocess_handler): + def test_checkout_command_execution(self, model, mock_subprocess_handler): """Success case: `svn checkout` command is executed.""" model.checkout() assert ( - f"svn checkout https://trac.nci.org.au/svn/cable/trunk {mock_cwd}/src/trunk" + "svn checkout https://trac.nci.org.au/svn/cable/trunk src/trunk" in mock_subprocess_handler.commands ) def test_checkout_command_execution_with_revision_number( - self, model, mock_cwd, mock_subprocess_handler + self, model, mock_subprocess_handler ): """Success case: `svn checkout` command is executed with specified revision number.""" model.revision = 9000 model.checkout() assert ( - f"svn checkout -r 9000 https://trac.nci.org.au/svn/cable/trunk {mock_cwd}/src/trunk" + "svn checkout -r 9000 https://trac.nci.org.au/svn/cable/trunk src/trunk" in mock_subprocess_handler.commands ) @@ -131,13 +128,13 @@ def test_standard_output(self, model, verbosity, expected): class TestSVNInfoShowItem: """Tests for `Model.svn_info_show_item()`.""" - def test_svn_info_command_execution(self, model, mock_subprocess_handler, mock_cwd): + def test_svn_info_command_execution(self, model, mock_subprocess_handler): """Success case: call svn info command and get result.""" assert ( model.svn_info_show_item("some-mock-item") == mock_subprocess_handler.stdout ) assert ( - f"svn info --show-item some-mock-item {mock_cwd}/src/trunk" + "svn info --show-item some-mock-item src/trunk" in mock_subprocess_handler.commands ) @@ -350,11 +347,6 @@ def test_modules_loaded_at_runtime( "module unload " + " ".join(modules) ) in mock_environment_modules_handler.commands - # TODO(Sean) fix for issue https://github.com/CABLE-LSM/benchcab/issues/162 - @pytest.mark.skip( - reason="""This will always fail since `parametrize()` parameters are - dependent on the `mock_cwd` fixture.""" - ) @pytest.mark.parametrize( ("verbosity", "expected"), [ @@ -380,9 +372,9 @@ def test_standard_output(self, model, build_script, modules, verbosity, expected model.custom_build(modules, verbose=verbosity) assert buf.getvalue() == expected - def test_file_not_found_exception(self, model, build_script, modules, mock_cwd): + def test_file_not_found_exception(self, model, build_script, modules): """Failure case: cannot find custom build script.""" - build_script_path = mock_cwd / internal.SRC_DIR / model.name / build_script + build_script_path = internal.SRC_DIR / model.name / build_script build_script_path.unlink() model.build_script = str(build_script) with pytest.raises(