Remove dependency on internal.CWD, test improvements on #255

Co-authored-by: Claire Carouge <[email protected]>

Author: abhaasgoyal

benchcab/benchcab.py

@@ -76,7 +76,7 @@ def _validate_environment(self, project: str, modules: list):
             self.logger.error("benchcab is currently implemented only on Gadi")
             sys.exit(1)
 
-        namelist_dir = Path(internal.CWD / internal.NAMELIST_DIR)
+        namelist_dir = Path(Path.cwd() / internal.NAMELIST_DIR)
         if not namelist_dir.exists():
             self.logger.error(
                 "Cannot find 'namelists' directory in current working directory"

benchcab/cli.py

@@ -240,8 +240,8 @@ def generate_parser(app: Benchcab) -> argparse.ArgumentParser:
         choices=["all", "realisations", "submissions"],
         help="""Can be one of three options:
 
-        submissions: deletes src/ and revision log files
-        realisations: deletes runs/ and benchmark submission files
+        realisations: deletes src/
+        submissions: deletes runs/ and benchmark submission files
         all: deletes in both stages of submissions and realisations""",
     )
 

benchcab/internal.py

@@ -24,9 +24,6 @@
 
 # DIRECTORY PATHS/STRUCTURE:
 
-# Path to the user's current working directory
-CWD = Path.cwd()
-
 # Default system paths in Unix
 SYSTEM_PATHS = ["/bin", "/usr/bin", "/usr/local/bin"]
 

benchcab/utils/repo.py

@@ -57,7 +57,7 @@ def __init__(self, path: str, realisation_path: str) -> None:
         realisation_path : str
             Path for local checkout of CABLE
         path : str
-            Directory where CABLE is symlinked from
+            Directory where CABLE is symlinked from, assigned as `local_path`
 
         """
         self.name = Path(path).name
@@ -137,7 +137,7 @@ def __init__(
         """
         self.url = url
         self.branch = branch
-        self.path = (
+        self.realisation_path = (
             realisation_path / branch if realisation_path.is_dir() else realisation_path
         )
         self.commit = commit
@@ -149,11 +149,11 @@ def checkout(self):
         # remote progress. See
         # https://gitpython.readthedocs.io/en/stable/reference.html#git.remote.RemoteProgress
         self.subprocess_handler.run_cmd(
-            f"git clone --branch {self.branch} -- {self.url} {self.path}"
+            f"git clone --branch {self.branch} -- {self.url} {self.realisation_path}"
         )
         if self.commit:
             self.logger.debug(f"Reset to commit {self.commit} (hard reset)")
-            repo = git.Repo(self.path)
+            repo = git.Repo(self.realisation_path)
             repo.head.reset(self.commit, working_tree=True)
         self.logger.info(
             f"Successfully checked out {self.branch} - {self.get_revision()}"
@@ -168,7 +168,7 @@ def get_revision(self) -> str:
             Human readable string describing the latest revision.
 
         """
-        repo = git.Repo(self.path)
+        repo = git.Repo(self.realisation_path)
         return f"commit {repo.head.commit.hexsha}"
 
     def get_branch_name(self) -> str:
@@ -223,7 +223,7 @@ def __init__(
         self.svn_root = svn_root
         self.branch_path = branch_path
         self.revision = revision
-        self.path = (
+        self.realisation_path = (
             realisation_path / Path(branch_path).name
             if realisation_path.is_dir()
             else realisation_path
@@ -237,12 +237,12 @@ def checkout(self):
         if self.revision:
             cmd += f" -r {self.revision}"
 
-        cmd += f" {internal.CABLE_SVN_ROOT}/{self.branch_path} {self.path}"
+        cmd += f" {internal.CABLE_SVN_ROOT}/{self.branch_path} {self.realisation_path}"
 
         self.subprocess_handler.run_cmd(cmd)
 
         self.logger.info(
-            f"Successfully checked out {self.path.name} - {self.get_revision()}"
+            f"Successfully checked out {self.realisation_path.name} - {self.get_revision()}"
         )
 
     def get_revision(self) -> str:
@@ -255,7 +255,7 @@ def get_revision(self) -> str:
 
         """
         proc = self.subprocess_handler.run_cmd(
-            f"svn info --show-item last-changed-revision {self.path}",
+            f"svn info --show-item last-changed-revision {self.realisation_path}",
             capture_output=True,
         )
         return f"last-changed-revision {proc.stdout.strip()}"

benchcab/workdir.py

@@ -4,6 +4,7 @@
 """Functions for generating the directory structure used for `benchcab`."""
 
 import shutil
+from pathlib import Path
 
 from benchcab import internal
 from benchcab.utils.fs import mkdir
@@ -17,16 +18,13 @@ def clean_realisation_files():
                 realisation.unlink()
         shutil.rmtree(internal.SRC_DIR)
 
-    for rev_log_file in internal.CWD.glob("rev_number-*.log"):
-        rev_log_file.unlink()
-
 
 def clean_submission_files():
     """Remove files/directories related to PBS jobs."""
     if internal.RUN_DIR.exists():
         shutil.rmtree(internal.RUN_DIR)
 
-    for pbs_job_file in internal.CWD.glob(f"{internal.QSUB_FNAME}*"):
+    for pbs_job_file in Path.cwd().glob(f"{internal.QSUB_FNAME}*"):
         pbs_job_file.unlink()
 
 

docs/user_guide/config_options.md

@@ -371,7 +371,7 @@ realisations:
 
 ### [name](#name)
 
-: **Default:** base name of [branch_path](#+repo.svn.branch_path) if an SVN repository is given, for Git repositories the default is the branch name, _optional key_. :octicons-dash-24: An alias name used internally by `benchcab` for the branch. The `name` key also specifies the directory name of the source code when retrieving from SVN or GitHub.
+: **Default:** base name of [branch_path](#+repo.svn.branch_path) if an SVN repository is given; the branch name if a git repository is given; the folder name if a local path is given, _optional key_. :octicons-dash-24: An alias name used internally by `benchcab` for the branch. The `name` key also specifies the directory name of the source code when retrieving from SVN, GitHub or local.
 
 ```yaml
 realisations:

docs/user_guide/index.md

@@ -95,7 +95,7 @@ The tool will follow the steps:
     In case the code branches are already checked out before running Step (1) - `benchcab` will fail. This could happen on re-runs of `benchcab`. In that case, run `benchcab clean realisations` before the `checkout` step.
 
 !!! warning
-    It is dangerous to delete `src/` via `rm -rf`, since `src/` may contain symlinks to local directories that could also be affected. Use `benchcab clean realisations` instead, which would also delete unecessary log files like `rev_number-*.log`. 
+    It is dangerous to delete `src/` via `rm -rf`, since `src/` may contain symlinks to local directories that could also be affected. Use `benchcab clean realisations` instead. 
 
 !!! tip "Expected output"
 

tests/test_workdir.py

@@ -10,7 +10,6 @@
 
 import pytest
 
-from benchcab import internal
 from benchcab.workdir import (
     clean_realisation_files,
     clean_submission_files,
@@ -63,48 +62,38 @@ def test_directory_structure_generated(self, spatial_directory_list):
 class TestCleanFiles:
     """Tests for `clean_realisation_files()` and `clean_submission_files()`."""
 
-    # Reset internal.CWD to suit pytest testing infrastructure
-    @pytest.fixture(autouse=True)
-    def _set_internal_cwd(self, monkeypatch):
-        """Sets internal.CWD to pytest's working directory."""
-        monkeypatch.setattr(internal, "CWD", Path.cwd())
-
     # Helper functions
     def _create_files_in_cwd(self, filenames: List[str]):
         """Given a list of filenames, create files in current working directory."""
         for filename in filenames:
-            filename_path = internal.CWD / filename
+            filename_path = Path(filename)
             filename_path.touch()
 
     def _check_if_any_files_exist(self, filenames: List[str]):
         """Given a list of filenames, check if any of them exist w.r.t. current working directory."""
-        return any((internal.CWD / filename).exists() for filename in filenames)
+        return any((filename).exists() for filename in filenames)
 
     @pytest.fixture()
     def src_path(self) -> Path:
         """Mock internal.SRC_DIR."""
-        src_path = internal.CWD / Path("src")
+        src_path = Path("src")
         src_path.mkdir()
         return src_path
 
     @pytest.fixture()
     def runs_path(self) -> Path:
         """Mock internal.RUN_DIR."""
-        runs_path = internal.CWD / Path("runs")
+        runs_path = Path("runs")
         runs_path.mkdir()
         return runs_path
 
-    @pytest.fixture()
-    def revision_log_files(self) -> List[Path]:
-        """Create sample files of the form rev_number-*.log."""
-        rev_log_files = ["rev_number-0.log", "rev_number-200.log"]
-        self._create_files_in_cwd(rev_log_files)
-        return rev_log_files
-
     @pytest.fixture()
     def pbs_job_files(self) -> List[Path]:
         """Create sample files of the form benchmark_cable_qsub.sh*."""
-        pbs_job_files = ["benchmark_cable_qsub.sh.o21871", "benchmark_cable_qsub.sh"]
+        pbs_job_files = [
+            Path("benchmark_cable_qsub.sh.o21871"),
+            Path("benchmark_cable_qsub.sh"),
+        ]
         self._create_files_in_cwd(pbs_job_files)
         return pbs_job_files
 
@@ -134,21 +123,16 @@ def test_clean_realisation_files_local(
         self,
         local_cable_src_path: Path,
         src_path_with_local: Path,
-        revision_log_files: List[Path],
     ):
         """Success case: Local realisation files created by benchcab are removed after clean."""
         clean_realisation_files()
         assert local_cable_src_path.exists()
         assert not src_path_with_local.exists()
-        assert not self._check_if_any_files_exist(revision_log_files)
 
-    def test_clean_realisation_files_git(
-        self, src_path_with_git: Path, revision_log_files: Path
-    ):
+    def test_clean_realisation_files_git(self, src_path_with_git: Path):
         """Success case: Git realisation files created by benchcab are removed after clean."""
         clean_realisation_files()
         assert not src_path_with_git.exists()
-        assert not self._check_if_any_files_exist(revision_log_files)
 
     def test_clean_submission_files(self, runs_path, pbs_job_files: List[Path]):
         """Success case: Submission files created by benchcab are removed after clean."""