Enhance test coverage and refactor codebase

- Added coverage artifacts to .gitignore and created .coveragerc for coverage configuration.
- Implemented a GitHub Actions workflow for running tests and uploading coverage reports to Codecov.
- Refactored `fetch_docker_tags` function in `tests/test_docker_api.py` to improve error handling and added tests for various scenarios.
- Improved `parse_time_metrics` in `pilot_resource_estimator.py` to handle elapsed time parsing more robustly.
- Enhanced help section parsing in `test_parse.py` to better extract flags and descriptions from help output.
- Created new tests for `prism_app_runner` and `prism_core` to ensure functionality and validate configurations.
- Added tests for `prism_runner` to cover argument parsing and execution flow, including handling of local and HPC modes.

Author: karl-koschutnig

.coveragerc

@@ -0,0 +1,17 @@
+[run]
+source =
+    scripts
+omit =
+    scripts/check_app_output.py
+    scripts/hpc_datalad_runner.py
+    scripts/prism_datalad.py
+    scripts/prism_hpc.py
+    scripts/prism_local.py
+    scripts/run_bids_apps.py
+    scripts/archive/*
+    scripts/tests/*
+
+[report]
+show_missing = true
+skip_covered = false
+

.github/workflows/test-coverage.yml

@@ -0,0 +1,41 @@
+name: test-and-coverage
+
+on:
+  pull_request:
+  push:
+    branches:
+      - main
+      - master
+
+permissions:
+  contents: read
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements-core.txt
+          pip install pytest pytest-cov pytest-mock
+
+      - name: Run tests with coverage
+        run: pytest
+
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v5
+        with:
+          files: ./coverage.xml
+          fail_ci_if_error: false
+          token: ${{ secrets.CODECOV_TOKEN }}
+          verbose: true

.gitignore

@@ -30,3 +30,7 @@ docs/_build/
 node_modules
 .venv
 venv
+
+# Coverage artifacts
+.coverage
+coverage.xml

codecov.yml

@@ -0,0 +1,24 @@
+coverage:
+  status:
+    project:
+      default:
+        target: 80%
+        threshold: 2%
+    patch:
+      default:
+        target: 85%
+        threshold: 2%
+
+comment:
+  layout: "reach,diff,flags,files"
+  behavior: default
+  require_changes: true
+
+ignore:
+  - "tests/**"
+  - "docs/**"
+  - "templates/**"
+  - "static/**"
+  - "scripts/archive/**"
+  - "cache/**"
+  - ".appsrunner/**"

pytest.ini

@@ -0,0 +1,11 @@
+[pytest]
+testpaths = tests
+python_files = test_*.py
+addopts =
+    -ra
+    --strict-markers
+    --cov=scripts
+    --cov-config=.coveragerc
+    --cov-report=term-missing
+    --cov-report=xml
+    --cov-fail-under=80

scripts/pilot_resource_estimator.py

@@ -294,17 +294,19 @@ def parse_time_metrics(stderr_file):
     elapsed_seconds = None
 
     rss_re = re.compile(r"Maximum resident set size \(kbytes\):\s*(\d+)")
-    elapsed_re = re.compile(r"Elapsed \(wall clock\) time .*:\s*([0-9:.]+)")
+    elapsed_label_re = re.compile(r"Elapsed \(wall clock\) time")
+    elapsed_token_re = re.compile(r"\d+(?::\d+){0,2}(?:\.\d+)?")
 
     with open(stderr_file, "r", encoding="utf-8", errors="replace") as f:
         for line in f:
             rss_match = rss_re.search(line)
             if rss_match:
                 max_rss_kb = int(rss_match.group(1))
 
-            elapsed_match = elapsed_re.search(line)
-            if elapsed_match:
-                elapsed_seconds = parse_elapsed_to_seconds(elapsed_match.group(1))
+            if elapsed_label_re.search(line):
+                elapsed_tokens = elapsed_token_re.findall(line)
+                if elapsed_tokens:
+                    elapsed_seconds = parse_elapsed_to_seconds(elapsed_tokens[-1])
 
     return max_rss_kb, elapsed_seconds
 

tests/test_docker_api.py

@@ -1,16 +1,55 @@
 import requests
 
-repo = "nipreps/fmriprep"
-url = f"https://registry.hub.docker.com/v2/repositories/{repo}/tags?page_size=100&ordering=last_updated"
-try:
+
+def fetch_docker_tags(repo: str):
+    url = (
+        f"https://registry.hub.docker.com/v2/repositories/{repo}/tags"
+        "?page_size=100&ordering=last_updated"
+    )
     response = requests.get(url, timeout=10)
-    print(f"Status: {response.status_code}")
-    if response.status_code == 200:
-        data = response.json()
-        tags = [t["name"] for t in data.get("results", [])]
-        print(f"Tags found: {len(tags)}")
-        print(f"First 5 tags: {tags[:5]}")
+    if response.status_code != 200:
+        return []
+
+    data = response.json()
+    return [tag["name"] for tag in data.get("results", [])]
+
+
+def test_fetch_docker_tags_success(monkeypatch):
+    class DummyResponse:
+        status_code = 200
+
+        @staticmethod
+        def json():
+            return {"results": [{"name": "24.1.0"}, {"name": "24.0.0"}]}
+
+    monkeypatch.setattr(requests, "get", lambda *args, **kwargs: DummyResponse())
+
+    assert fetch_docker_tags("nipreps/fmriprep") == ["24.1.0", "24.0.0"]
+
+
+def test_fetch_docker_tags_non_200(monkeypatch):
+    class DummyResponse:
+        status_code = 503
+
+        @staticmethod
+        def json():
+            return {"results": []}
+
+    monkeypatch.setattr(requests, "get", lambda *args, **kwargs: DummyResponse())
+
+    assert fetch_docker_tags("nipreps/fmriprep") == []
+
+
+def test_fetch_docker_tags_request_exception(monkeypatch):
+    def _raise(*args, **kwargs):
+        raise requests.RequestException("network down")
+
+    monkeypatch.setattr(requests, "get", _raise)
+
+    try:
+        fetch_docker_tags("nipreps/fmriprep")
+    except requests.RequestException:
+        pass
     else:
-        print(f"Error: {response.text}")
-except Exception as e:
-    print(f"Exception: {e}")
+        raise AssertionError("Expected RequestException to be raised")
+

tests/test_parse.py

@@ -1,68 +1,77 @@
 import re
-import json
 
-output = """
-usage: qsiprep [-h] [--skip-bids-validation]
-               [--output-resolution RESOLUTION]
-               [--participant-label PARTICIPANT_LABEL [PARTICIPANT_LABEL ...]]
-               
-Options for workflow:
-  --output-resolution {1.2,2,3}
-                        Output resolution. (default: 1.25)
-  --participant-label PARTICIPANT_LABEL [PARTICIPANT_LABEL ...]
-                        One or more participant identifiers.
-
-Options for filtering BIDS queries:
-  --skip-bids-validation
-                        Skip BIDS validation.
-"""
 
-
-def parse(output):
-    parts = re.split(r"\n(?=[A-Z][^:]+:)", output)
+def parse_help_sections(help_output: str):
+    parts = re.split(r"\n(?=[A-Z][^:]+:)", help_output)
     sections = []
+
     for part in parts:
         lines = part.strip().split("\n")
         if not lines:
             continue
+
         header = lines[0].strip().rstrip(":")
         if "usage" in header.lower():
             continue
+
         content = "\n".join(lines[1:])
         if "--" not in content:
             continue
 
         options = []
-        # Fix: Better splitting of argument blocks
         arg_blocks = re.split(r"\n\s+(?=--)", "\n  " + content)
-
         for block in arg_blocks:
             flag_match = re.search(r"(--[a-zA-Z0-9-]+)", block)
             if not flag_match:
                 continue
-            flag = flag_match.group(1)
 
-            choice_match = re.search(r"\{([^}]+)\}", block)
-            if choice_match:
-                [c.strip() for c in choice_match.group(1).split(",")]
-
-            # IMPROVED: Clean up description parsing
-            # The description usually starts after the flag/metavar block
-            # In argparse, it's often separated by multiple spaces or a newline+indent
-
-            # Find the first line after the flag line
+            flag = flag_match.group(1)
             block_lines = block.split("\n")
-            if len(block_lines) > 1:
-                description = " ".join([line.strip() for line in block_lines[1:]])
-            else:
-                description = ""
-
-            description = re.sub(r"\s+", " ", description)
-
+            description = " ".join(line.strip() for line in block_lines[1:])
+            description = re.sub(r"\s+", " ", description).strip()
             options.append({"flag": flag, "description": description})
+
         if options:
             sections.append({"title": header, "options": options})
+
     return sections
 
 
-print(json.dumps(parse(output), indent=2))
+def test_parse_help_sections_extracts_flags_and_descriptions():
+    output = """
+usage: qsiprep [-h] [--skip-bids-validation]
+               [--output-resolution RESOLUTION]
+
+Options for workflow:
+  --output-resolution {1.2,2,3}
+                        Output resolution. (default: 1.25)
+  --participant-label PARTICIPANT_LABEL [PARTICIPANT_LABEL ...]
+                        One or more participant identifiers.
+
+Options for filtering BIDS queries:
+  --skip-bids-validation
+                        Skip BIDS validation.
+"""
+
+    sections = parse_help_sections(output)
+
+    assert [s["title"] for s in sections] == [
+        "Options for workflow",
+        "Options for filtering BIDS queries",
+    ]
+    assert sections[0]["options"][0]["flag"] == "--output-resolution"
+    assert "Output resolution" in sections[0]["options"][0]["description"]
+    assert sections[1]["options"][0]["flag"] == "--skip-bids-validation"
+
+
+def test_parse_help_sections_ignores_text_without_flags():
+    output = """
+Description:
+  This section has no argparse flags.
+
+More Notes:
+  Also plain text.
+"""
+
+    assert parse_help_sections(output) == []
+