Incorporate requested changes related to testing/Repo in #255

Author: abhaasgoyal

benchcab/benchcab.py

@@ -230,8 +230,11 @@ def checkout(self, config_path: str):
             try:
                 model.repo.checkout()
             except Exception:
-                self.logger.error("Try using `benchcab clean realisations` first")
-                sys.exit(1)
+                msg = "Try using `benchcab clean realisations` first"
+                self.logger.error(
+                    "Model checkout failed, probably due to existing realisation name"
+                )
+                raise FileExistsError(msg)
             rev_number_log += f"{model.name}: {model.repo.get_revision()}\n"
 
         rev_number_log_path = next_path("rev_number-*.log")

benchcab/data/test/integration.sh

@@ -18,6 +18,7 @@ mkdir -p $TEST_DIR
 # Clone local checkout for CABLE
 git clone $CABLE_REPO $CABLE_DIR
 cd $CABLE_DIR
+# Note: This is temporary, to be removed once #258 is fixed
 git reset --hard 67a52dc5721f0da78ee7d61798c0e8a804dcaaeb
 
 # Clone the example repo
@@ -35,8 +36,7 @@ realisations:
   - repo:
       git:
         branch: main
-        commit: 67a52dc5721f0da78ee7d61798c0e8a804dcaaeb
-
+        commit: 67a52dc5721f0da78ee7d61798c0e8a804dcaaeb  # Note: This is temporary, to be removed once #258 is fixed
 modules: [
   intel-compiler/2021.1.1,
   netcdf/4.7.4,

benchcab/utils/repo.py

@@ -85,7 +85,7 @@ def get_revision(self) -> str:
             Human readable string describing the latest revision.
 
         """
-        return f"Local CABLE build: {self.name}"
+        return f"Local CABLE build: {Path(self.local_path).absolute().as_posix}"
 
     def get_branch_name(self) -> str:
         """Return the branch name of the source code.

tests/test_workdir.py

@@ -6,6 +6,7 @@
 """
 
 from pathlib import Path
+from typing import List
 
 import pytest
 
@@ -60,52 +61,97 @@ def test_directory_structure_generated(self, spatial_directory_list):
 
 
 class TestCleanFiles:
-    """Tests for `clean_directory_tree()` and `clean_cwd_logfiles)."""
+    """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):
+    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.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)
+
     @pytest.fixture()
     def src_path(self) -> Path:
-        return Path("src")
+        """Mock internal.SRC_DIR."""
+        src_path = internal.CWD / Path("src")
+        src_path.mkdir()
+        return src_path
 
     @pytest.fixture()
     def runs_path(self) -> Path:
-        return Path("runs")
+        """Mock internal.RUN_DIR."""
+        runs_path = internal.CWD / Path("runs")
+        runs_path.mkdir()
+        return runs_path
 
-    @pytest.fixture(params=["rev_number-0.log", "rev_number-200.log"])
-    def revision_log_file(self, request) -> Path:
-        return Path(request.param)
+    @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(
-        params=["benchmark_cable_qsub.sh.o21871", "benchmark_cable_qsub.sh"]
-    )
-    def pbs_job_file(self, request) -> Path:
-        return Path(request.param)
+    @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"]
+        self._create_files_in_cwd(pbs_job_files)
+        return pbs_job_files
 
-    def test_clean_realisation_files(
-        self, src_path: Path, tmp_path: Path, revision_log_file: Path
-    ):
-        """Success case: Realisation files created by benchcab are removed after clean."""
-        src_path.mkdir()
-        cable_symlink = src_path / "main"
-        # tmp_path contains the path being symlinked
+    @pytest.fixture()
+    def local_cable_src_path(self, tmp_path) -> Path:
+        """Local sample path for CABLE checkout."""
+        # Temporary directory of CABLE unique to test invocation, independent of CWD
         local_cable_src_path = tmp_path / "CABLE"
         local_cable_src_path.mkdir()
+        return local_cable_src_path
+
+    @pytest.fixture()
+    def src_path_with_local(self, src_path, local_cable_src_path) -> Path:
+        """Local path where CABLE checkout is symlinked."""
+        cable_symlink = src_path / "cable_local"
         cable_symlink.symlink_to(local_cable_src_path)
-        revision_log_file.touch()
-        clean_realisation_files()
+        return src_path
 
+    @pytest.fixture()
+    def src_path_with_git(self, src_path) -> Path:
+        """Local path where CABLE checkout from git is present."""
+        cable_git = src_path / "cable_git"
+        cable_git.mkdir()
+        return src_path
+
+    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.exists()
-        assert not revision_log_file.exists()
+        assert not src_path_with_local.exists()
+        assert not self._check_if_any_files_exist(revision_log_files)
 
-    def test_clean_submission_files(self, runs_path, pbs_job_file):
+    def test_clean_realisation_files_git(
+        self, src_path_with_git: Path, revision_log_files: 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."""
-        runs_path.mkdir()
-        pbs_job_file.touch()
         clean_submission_files()
         assert not runs_path.exists()
-        assert not pbs_job_file.exists()
+        assert not self._check_if_any_files_exist(pbs_job_files)