Merge pull request SciLifeLab#413 from kedhammar/pytest-ci

Implement CI testing and increase testing coverage

.devcontainer/devcontainer.json

@@ -6,13 +6,13 @@
     // Sets the run context to one level up instead of the .devcontainer folder.
     "context": "..",
     // Update the 'dockerFile' property if you aren't using the standard 'Dockerfile' filename.
-    "dockerfile": "../Dockerfile",
+    "dockerfile": "../Dockerfile"
   },
   "features": {},
   "customizations": {
     "vscode": {
-      "extensions": ["ms-python.python"],
-    },
+      "extensions": ["ms-python.python"]
+    }
   },
   // Features to add to the dev container. More info: https://containers.dev/features.
   // "features": {},
@@ -24,6 +24,6 @@
   // Uncomment to connect as an existing user other than the container default. More info: https://aka.ms/dev-containers-non-root.
   // "remoteUser": "devcontainer"
   "mounts": [
-    "source=${localEnv:HOME}/repos/flowcell_parser,target=/workspaces/flowcell_parser,type=bind,consistency=cached",
-  ],
+    "source=${localEnv:HOME}/repos/flowcell_parser,target=/workspaces/flowcell_parser,type=bind,consistency=cached"
+  ]
 }

.github/workflows/test-code.yml

@@ -0,0 +1,23 @@
+name: Test code
+on: [push, pull_request]
+
+jobs:
+  pytest:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@v4
+      - name: Set up Python
+        uses: actions/setup-python@v4
+        with:
+          python-version: "3.10"
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+          pip install -r requirements-dev.txt
+      - name: Install TACA
+        run: pip install -e .
+      - name: pytest
+        # Options are configured in pyproject.toml
+        run: pytest

.gitignore

@@ -13,3 +13,4 @@ __pycache__
 .vscode
 .ruff_cache
 .mypy_cache
+node_modules

.pre-commit-config.yaml

@@ -9,6 +9,7 @@ repos:
     rev: "v1.7.1"
     hooks:
       - id: mypy
+        additional_dependencies: [types-PyYAML]
   - repo: https://github.com/pre-commit/mirrors-prettier
     rev: "v4.0.0-alpha.8"
     hooks:

VERSIONLOG.md

@@ -1,5 +1,9 @@
 # TACA Version Log
 
+## 20240208.2
+
+Implement CI testing and increase testing coverage.
+
 ## 20240208.1
 
 Fix bug with isinstance clause

pyproject.toml

@@ -1,27 +1,42 @@
 title = "taca"
 
+# === LINTING ================================================================
 
 [tool.ruff.lint]
-select =[
+select = [
     #         Ruff default rules
     # ------------------------------
-    "E4",   # pycodestyle Imports
-    "E7",   # pycodestyle Statements
-    "E9",   # pycodestyle Runtime
-    "F",    # Pyflakes
+    "E4", # pycodestyle Imports
+    "E7", # pycodestyle Statements
+    "E9", # pycodestyle Runtime
+    "F",  # Pyflakes
 
     #         Additional    Comment
     # ------------------------------------------------------
-    "I",    # isort         Best-practice sorting of imports
-    "UP",   # pyupgrade     Make sure syntax is up-to-date
+    "I",  # isort         Best-practice sorting of imports
+    "UP", # pyupgrade     Make sure syntax is up-to-date
 ]
 ignore = [
     "E402", # Module level import not at top of file
     "E722", # Do not use bare 'except'
     "E741", # Ambiguous variable name
 ]
 
-
 [tool.mypy]
 ignore_missing_imports = true
 follow_imports = 'skip'
+
+# === Testing ================================================================
+
+[tool.pytest.ini_options]
+# Omit deprecation warnings from 3rd party packages
+filterwarnings = [
+    'ignore::DeprecationWarning:couchdb.*',
+    'ignore::DeprecationWarning:pkg_resources.*',
+]
+addopts = "--cov=./taca --cov-report term-missing -vv --cache-clear tests/pytest/"
+
+[tool.coverage.run]
+# The comment "# pragma: no cover" can be used to exclude a line from coverage
+source = ["taca"]
+omit = ["**/__init__.py"]

requirements-dev.txt

@@ -5,6 +5,7 @@ mock
 sphinx
 sphinx-rtd-theme
 pytest
+pytest-cov
 ipython
 ipdb
 ruff

requirements.txt

@@ -3,7 +3,6 @@ PyYAML
 click
 flowcell_parser @ git+https://github.com/SciLifeLab/flowcell_parser
 pandas
-pytest
 python_crontab
 python_dateutil
 setuptools

taca/nanopore/ONT_run_classes.py

@@ -18,7 +18,7 @@
 logger = logging.getLogger(__name__)
 
 ONT_RUN_PATTERN = re.compile(
-    "^(\d{8})_(\d{4})_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)$"
+    r"^(\d{8})_(\d{4})_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)$"
 )
 
 
@@ -61,7 +61,7 @@ def __init__(self, run_abspath: str):
 
         # Get attributes from config
         self.minknow_reports_dir = CONFIG["nanopore_analysis"]["minknow_reports_dir"]
-        self.analysis_server = CONFIG["nanopore_analysis"]["analysis_server"]
+        self.analysis_server = CONFIG["nanopore_analysis"].get("analysis_server", None)
         self.rsync_options = CONFIG["nanopore_analysis"]["rsync_options"]
         for k, v in self.rsync_options.items():
             if v == "None":
@@ -115,7 +115,6 @@ def assert_contents(self):
         assert self.has_file("/report_*.html")
 
         # MinKNOW auxillary files
-        assert self.has_file("/final_summary*.txt")
         assert self.has_file("/pore_activity*.csv")
 
     def touch_db_entry(self):
@@ -450,12 +449,11 @@ def fetch_anglerfish_samplesheet(self) -> bool:
         """
 
         # Following line assumes run was started same year as samplesheet was generated
-        current_year = self.date[0:4]
         expected_file_pattern = f"Anglerfish_samplesheet_{self.experiment_name}_*.csv"
 
         # Finalize query pattern
         pattern_abspath = os.path.join(
-            self.anglerfish_samplesheets_dir, current_year, expected_file_pattern
+            self.anglerfish_samplesheets_dir, "*", expected_file_pattern
         )
 
         glob_results = glob.glob(pattern_abspath)
@@ -522,11 +520,14 @@ def run_anglerfish(self):
 
         # Create dir to trace TACA executing Anglerfish as a subprocess
         taca_anglerfish_run_dir = f"taca_anglerfish_run_{timestamp}"
-        os.mkdir(taca_anglerfish_run_dir)
+        taca_anglerfish_run_dir_abspath = (
+            f"{self.run_abspath}/{taca_anglerfish_run_dir}"
+        )
+        os.mkdir(taca_anglerfish_run_dir_abspath)
         # Copy samplesheet used for traceability
-        shutil.copy(self.anglerfish_samplesheet, f"{taca_anglerfish_run_dir}/")
+        shutil.copy(self.anglerfish_samplesheet, taca_anglerfish_run_dir_abspath)
         # Create files to dump subprocess std
-        stderr_abspath = f"{self.run_abspath}/{taca_anglerfish_run_dir}/stderr.txt"
+        stderr_abspath = f"{taca_anglerfish_run_dir_abspath}/stderr.txt"
 
         full_command = [
             # Dump subprocess PID into 'run-ongoing'-indicator file.
@@ -539,15 +540,15 @@ def run_anglerfish(self):
             #  1) Find the latest Anglerfish run dir (younger than the 'run-ongoing' file)
             f'find {self.run_abspath} -name "anglerfish_run*" -type d -newer {self.run_abspath}/.anglerfish_ongoing '
             #  2) Move the Anglerfish run dir into the TACA Anglerfish run dir
-            + "-exec mv \{\} "
-            + f"{self.run_abspath}/{taca_anglerfish_run_dir}/ \; "
+            + r"-exec mv \{\} "
+            + rf"{self.run_abspath}/{taca_anglerfish_run_dir}/ \; "
             #  3) Only do this once
             + "-quit",
             # Remove 'run-ongoing' file.
             f"rm {self.anglerfish_ongoing_abspath}",
         ]
 
-        with open(f"{taca_anglerfish_run_dir}/command.sh", "w") as stream:
+        with open(f"{taca_anglerfish_run_dir_abspath}/command.sh", "w") as stream:
             stream.write("\n".join(full_command))
 
         # Start Anglerfish subprocess

taca/nanopore/instrument_transfer.py

@@ -1,6 +1,6 @@
 """ This is a stand-alone script run on ONT instrument computers. It transfers new ONT runs to NAS using rsync.
 """
-__version__ = "1.0.13"
+__version__ = "1.0.14"
 
 import argparse
 import logging
@@ -91,8 +91,10 @@ def dump_path(run_path: str):
 def write_finished_indicator(run_path):
     """Write a hidden file to indicate
     when the finial rsync is finished."""
-    new_file = os.path.join(run_path, ".sync_finished")
-    open(new_file, "w").close()
+    finished_indicator = ".sync_finished"
+    new_file_path = os.path.join(run_path, finished_indicator)
+    open(new_file_path, "w").close()
+    return new_file_path
 
 
 def sync_to_storage(run_dir: str, destination: str, rsync_log: str):
@@ -134,10 +136,10 @@ def final_sync_to_storage(
     p = subprocess.run(command)
 
     if p.returncode == 0:
-        finished_indicator = write_finished_indicator(run_dir)
+        finished_indicator_path = write_finished_indicator(run_dir)
         dest = os.path.join(destination, os.path.basename(run_dir))
-        sync_finished_indicator = ["rsync", finished_indicator, dest]
-        p = subprocess.run(sync_finished_indicator)
+        sync_finished_indicator_command = ["rsync", finished_indicator_path, dest]
+        p = subprocess.run(sync_finished_indicator_command)
         archive_finished_run(run_dir, archive_dir)
     else:
         logging.info(
@@ -330,9 +332,7 @@ def dump_pore_count_history(run: str, pore_counts: list) -> str:
     return new_file_path
 
 
-# BEGIN_EXCLUDE
-if __name__ == "__main__":
-    # This is clunky but should be fine since it will only ever run as a cronjob
+if __name__ == "__main__":  # pragma: no cover
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument(
         "--source",
@@ -368,4 +368,3 @@ def dump_pore_count_history(run: str, pore_counts: list) -> str:
     args = parser.parse_args()
 
     main(args)
-# END_EXCLUDE

taca/utils/filesystem.py

@@ -3,8 +3,8 @@
 import os
 import shutil
 
-RUN_RE = "^\d{6,8}_[a-zA-Z\d\-]+_\d{2,}_[AB0][A-Z\d\-]+$"
-RUN_RE_ONT = "^(\d{8})_(\d{4})_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)$"
+RUN_RE = r"^\d{6,8}_[a-zA-Z\d\-]+_\d{2,}_[AB0][A-Z\d\-]+$"
+RUN_RE_ONT = r"^(\d{8})_(\d{4})_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)_([0-9a-zA-Z]+)$"
 
 
 @contextlib.contextmanager

taca/utils/transfer.py

@@ -55,11 +55,11 @@ def format_options(self):
         for param, val in self.cmdopts.items():
             if val is None:
                 cmdopts.append(param)
-            else:
-                if isinstance(val, str):
-                    val = [val]
+            elif isinstance(val, list):
                 for v in val:
                     cmdopts.append(f"{param}={v}")
+            else:
+                cmdopts.append(f"{param}={val}")
         return cmdopts
 
     def transfer(self):

tests/pytest/conftest.py

@@ -0,0 +1,63 @@
+import os
+import tempfile
+
+import pytest
+
+
+@pytest.fixture
+def create_dirs():
+    """Create the bottom-level file-tree to be used for all tests:
+
+    tmp
+    ├── log
+    │   ├── transfer_minion.tsv
+    │   └── transfer_promethion.tsv
+    ├── miarka
+    │   ├── minion
+    │   │   └── qc
+    │   └── promethion
+    ├── minknow_reports
+    ├── ngi-nas-ns
+    │   ├── minion_data
+    │   ├── promethion_data
+    │   └── samplesheets
+    │       └── anglerfish
+    └── sequencing
+        ├── minion
+        │   ├── nosync
+        │   └── qc
+        │       └── nosync
+        └── promethion
+            └── nosync
+
+    --> Return the the temporary directory object
+    """
+    tmp = tempfile.TemporaryDirectory()
+
+    # CREATE DIR STRUCTURE
+
+    # Sequencing data
+    os.makedirs(f"{tmp.name}/sequencing/promethion/nosync")
+    os.makedirs(f"{tmp.name}/sequencing/minion/nosync")
+    os.makedirs(f"{tmp.name}/sequencing/minion/qc/nosync")
+
+    # Non-sensitive metadata
+    os.makedirs(f"{tmp.name}/ngi-nas-ns/promethion_data")
+    os.makedirs(f"{tmp.name}/ngi-nas-ns/minion_data")
+    os.makedirs(f"{tmp.name}/ngi-nas-ns/samplesheets/anglerfish")
+
+    # Reports for GenStat
+    os.makedirs(f"{tmp.name}/minknow_reports")
+
+    # Logs
+    os.makedirs(f"{tmp.name}/log")
+    open(f"{tmp.name}/log/transfer_promethion.tsv", "w").close()
+    open(f"{tmp.name}/log/transfer_minion.tsv", "w").close
+
+    # Analysis server destination dirs
+    os.makedirs(f"{tmp.name}/miarka/promethion")
+    os.makedirs(f"{tmp.name}/miarka/minion/qc")
+
+    yield tmp
+
+    tmp.cleanup()