From e64f0ef50321324992b5d47a1e1d5e336a81992f Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Fri, 22 May 2026 21:40:22 -0300 Subject: [PATCH 01/16] chore: configure pytest and move tests directory --- package/pyproject.toml | 35 +++++++++++++++++-- .../{src/pyaslreport => }/tests/__init__.py | 0 .../pyaslreport => }/tests/test_package.py | 2 +- 3 files changed, 34 insertions(+), 3 deletions(-) rename package/{src/pyaslreport => }/tests/__init__.py (100%) rename package/{src/pyaslreport => }/tests/test_package.py (97%) diff --git a/package/pyproject.toml b/package/pyproject.toml index ea50b37d2..51fdfeeb1 100644 --- a/package/pyproject.toml +++ b/package/pyproject.toml @@ -5,7 +5,7 @@ description = "A Python package for generating methods sections in for ASL param authors = [{ name="Ibrahim Abdelazim", email="ibrahim.abdelazim@fau.de" }] license = "MIT" readme = "README.md" -requires-python = ">=3.7" +requires-python = ">=3.11" dependencies = [ "PyYAML~=6.0.2", "numpy~=2.2.6", @@ -26,4 +26,35 @@ include-package-data = true where = ["src"] [tool.setuptools.package-data] -"pyaslreport" = ["*.yaml", "**/*.yaml"] \ No newline at end of file +"pyaslreport" = ["*.yaml", "**/*.yaml"] + +[project.optional-dependencies] +test = [ + "pytest>=7.0", + "pytest-cov>=4.0", +] + +[tool.pytest.ini_options] +minversion = "7.0" +testpaths = ["tests"] +python_files = ["test_*.py"] +python_classes = ["Test*"] +python_functions = ["test_*"] +addopts = [ + "-ra", + "--strict-markers", + "--strict-config", +] +markers = [ + "integration: integration tests requiring file fixtures (deselect with '-m \"not integration\"')", + "slow: tests that take noticeably longer to run", +] + +[tool.black] +line-length = 88 +target-version = ["py311", "py312"] + +[tool.isort] +profile = "black" +line_length = 88 +known_first_party = ["pyaslreport"] \ No newline at end of file diff --git a/package/src/pyaslreport/tests/__init__.py b/package/tests/__init__.py similarity index 100% rename from package/src/pyaslreport/tests/__init__.py rename to package/tests/__init__.py diff --git a/package/src/pyaslreport/tests/test_package.py b/package/tests/test_package.py similarity index 97% rename from package/src/pyaslreport/tests/test_package.py rename to package/tests/test_package.py index fda43d338..ccbd8b6e9 100644 --- a/package/src/pyaslreport/tests/test_package.py +++ b/package/tests/test_package.py @@ -1,6 +1,6 @@ import pytest from unittest.mock import patch, MagicMock -from .main import get_bids_metadata +from pyaslreport.main import get_bids_metadata # filepath: /home/ibrahim/MyPc/Projects/GSoC/ASL-Parameter-Generator/package/src/pyaslreport/test_main.py From 181f1412b03389eff00c1f1060ab70871cd6a62f Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Sun, 24 May 2026 01:18:27 -0300 Subject: [PATCH 02/16] test: add conftest with shared fixtures and CLI option --- package/tests/conftest.py | 198 ++++++++++++++++++++++++++++++++++ package/tests/test_package.py | 28 +++-- 2 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 package/tests/conftest.py diff --git a/package/tests/conftest.py b/package/tests/conftest.py new file mode 100644 index 000000000..4fcb386d3 --- /dev/null +++ b/package/tests/conftest.py @@ -0,0 +1,198 @@ +"""Shared fixtures and pytest configuration for pyaslreport tests. + +Fixtures provided: + minimal_nifti_path: Path to a tiny on-disk NIfTI file (auto-deleted after test). + make_context: Factory for building ProcessingContext with sensible defaults. + make_processor: Factory for building ASLProcessor without triggering validation. + examples_dir: Path to the integration examples directory (pytest-configurable). + minimal_asl_json: In-memory dict representing a minimal valid ASL JSON. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any, Callable + +import nibabel as nib +import numpy as np +import pytest + +from pyaslreport.modalities.asl.processor import ASLProcessor, ProcessingContext + + +# --------------------------------------------------------------------------- +# CLI option for integration test directory +# --------------------------------------------------------------------------- +def pytest_addoption(parser: pytest.Parser) -> None: + """Register the --examples-dir CLI option for the integration runner. + + Args: + parser: The pytest CLI parser, supplied by pytest at collection time. + + Notes: + Local usage: pytest --examples-dir=/path/to/examples + CI usage: omit the flag; defaults to the committed set in + tests/integration/examples. + """ + parser.addoption( + "--examples-dir", + action="store", + default=None, + help="Path to integration examples dir; falls back to the committed CI set.", + ) + + +# --------------------------------------------------------------------------- +# File-based fixtures +# --------------------------------------------------------------------------- +@pytest.fixture +def minimal_nifti_path(tmp_path: Path) -> Path: + """Create a tiny valid NIfTI file at tmp_path/asl.nii.gz. + + Args: + tmp_path: Pytest-provided per-test temporary directory. + + Returns: + Path to the written NIfTI file with shape (4, 4, 20, 2). The third + axis matches a sensible default for ProcessingContext.nifti_slice_number + so most tests do not need a custom shape. + """ + data = np.zeros((4, 4, 20, 2), dtype=np.float32) + img = nib.Nifti1Image(data, affine=np.eye(4)) + + path = tmp_path / "asl.nii.gz" + nib.save(img, str(path)) + + return path + + +@pytest.fixture +def examples_dir(request: pytest.FixtureRequest) -> Path: + """Return the path to the integration examples directory. + + Args: + request: Pytest fixture request, used to read the --examples-dir flag. + + Returns: + Resolved path to an existing examples directory. + + Raises: + pytest.skip.Exception: If the resolved path does not exist. Resolution + order is (1) the --examples-dir CLI flag, then (2) the committed + tests/integration/examples directory beside this conftest. + """ + cli_value = request.config.getoption("--examples-dir") + + if cli_value: + path = Path(cli_value).expanduser().resolve() + else: + path = Path(__file__).parent / "integration" / "examples" + + if not path.is_dir(): + pytest.skip(f"Examples directory not found: {path}") + + return path + + +# --------------------------------------------------------------------------- +# Factory fixtures +# --------------------------------------------------------------------------- +@pytest.fixture +def make_context() -> Callable[..., ProcessingContext]: + """Return a factory for building ProcessingContext with sensible defaults. + + Returns: + A callable that accepts keyword overrides and returns a fully-formed + ProcessingContext. All ten required fields receive defaults; pass + keyword arguments to override any of them. + + Example: + >>> def test_something(make_context): + ... ctx = make_context(m0_type="Separate", errors=["something"]) + ... assert ctx.m0_type == "Separate" + """ + + def _make(**overrides: Any) -> ProcessingContext: + defaults: dict[str, Any] = { + "asl_json_data": [], + "m0_prep_times_collection": [], + "errors": [], + "warnings": [], + "all_absent": True, + "bs_all_off": True, + "m0_type": None, + "global_pattern": None, + "total_acquired_pairs": None, + "nifti_slice_number": 20, + } + + defaults.update(overrides) + + return ProcessingContext(**defaults) + + return _make + + +@pytest.fixture +def make_processor(minimal_nifti_path: Path) -> Callable[..., ASLProcessor]: + """Return a factory for building ASLProcessor without input validation. + + Args: + minimal_nifti_path: Auto-injected fixture providing a real on-disk + NIfTI file. The factory uses it as the default nifti_file unless + the caller overrides it. + + Returns: + A callable that accepts keyword overrides and returns an ASLProcessor. + Useful for testing private methods like _group_files in isolation, + because BaseProcessor.__init__ stores self.data without validating. + + Example: + >>> def test_grouping(make_processor): + ... proc = make_processor(files=["/path/to/asl.json"]) + ... groups = proc._group_files("nifti") + """ + + def _make(**overrides: Any) -> ASLProcessor: + defaults: dict[str, Any] = { + "modality": "asl", + "files": [], + "dcm_files": [], + "nifti_file": str(minimal_nifti_path), + } + + defaults.update(overrides) + + return ASLProcessor(defaults) + + return _make + + +# --------------------------------------------------------------------------- +# Data fixtures (in-memory JSON dicts) +# --------------------------------------------------------------------------- +@pytest.fixture +def minimal_asl_json() -> dict[str, Any]: + """Return a minimal ASL JSON with all major-error fields valid. + + Returns: + Dictionary intended as a starting point for normalization and + validation tests. Spread it into a new dict and override individual + fields to introduce specific missing or invalid values. + """ + return { + "ArterialSpinLabelingType": "PCASL", + "MRAcquisitionType": "3D", + "PulseSequenceType": "GRASE", + "M0Type": "Separate", + "BackgroundSuppression": False, + "PostLabelingDelay": 1.8, + "LabelingDuration": 1.8, + "EchoTime": 0.012, + "RepetitionTimePreparation": 4.0, + "FlipAngle": 90, + "MagneticFieldStrength": 3, + "Manufacturer": "Siemens", + "ManufacturersModelName": "TrioTim", + "AcquisitionVoxelSize": [3, 3, 4], + } diff --git a/package/tests/test_package.py b/package/tests/test_package.py index ccbd8b6e9..ae89ca5aa 100644 --- a/package/tests/test_package.py +++ b/package/tests/test_package.py @@ -1,38 +1,50 @@ +from unittest.mock import MagicMock, patch + import pytest -from unittest.mock import patch, MagicMock + from pyaslreport.main import get_bids_metadata # filepath: /home/ibrahim/MyPc/Projects/GSoC/ASL-Parameter-Generator/package/src/pyaslreport/test_main.py + def test_get_bids_metadata_success(): data = {"modality": "asl", "dicom_dir": "/fake/dir"} fake_header = MagicMock() fake_sequence = MagicMock() fake_sequence.extract_bids_metadata.return_value = ("meta", "context") - with patch("pyaslreport.main.get_dicom_header", return_value=fake_header), \ - patch("pyaslreport.main.get_sequence", return_value=fake_sequence): + with ( + patch("pyaslreport.main.get_dicom_header", return_value=fake_header), + patch("pyaslreport.main.get_sequence", return_value=fake_sequence), + ): result = get_bids_metadata(data) assert result == ("meta", "context") fake_sequence.extract_bids_metadata.assert_called_once() + def test_get_bids_metadata_no_dicom_dir(): data = {"modality": "asl"} with pytest.raises(TypeError): get_bids_metadata(data) + def test_get_bids_metadata_no_sequence(): data = {"modality": "asl", "dicom_dir": "/fake/dir"} fake_header = MagicMock() - with patch("pyaslreport.main.get_dicom_header", return_value=fake_header), \ - patch("pyaslreport.main.get_sequence", return_value=None): + with ( + patch("pyaslreport.main.get_dicom_header", return_value=fake_header), + patch("pyaslreport.main.get_sequence", return_value=None), + ): with pytest.raises(ValueError) as exc: get_bids_metadata(data) assert "No matching sequence found" in str(exc.value) + def test_get_bids_metadata_invalid_modality(): data = {"modality": None, "dicom_dir": "/fake/dir"} fake_header = MagicMock() - with patch("pyaslreport.main.get_dicom_header", return_value=fake_header), \ - patch("pyaslreport.main.get_sequence", return_value=None): + with ( + patch("pyaslreport.main.get_dicom_header", return_value=fake_header), + patch("pyaslreport.main.get_sequence", return_value=None), + ): with pytest.raises(ValueError): - get_bids_metadata(data) \ No newline at end of file + get_bids_metadata(data) From 745e7870b29156b5f748c4320abd3ff389cf4594 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Wed, 27 May 2026 01:34:24 -0300 Subject: [PATCH 03/16] test: add test file stubs with sanity checks. --- package/tests/test_file_grouping.py | 11 +++++++++++ package/tests/test_m0_tsv_validation.py | 12 ++++++++++++ package/tests/test_normalization.py | 8 ++++++++ package/tests/test_validators.py | 24 ++++++++++++++++++++++++ 4 files changed, 55 insertions(+) create mode 100644 package/tests/test_file_grouping.py create mode 100644 package/tests/test_m0_tsv_validation.py create mode 100644 package/tests/test_normalization.py create mode 100644 package/tests/test_validators.py diff --git a/package/tests/test_file_grouping.py b/package/tests/test_file_grouping.py new file mode 100644 index 000000000..40ec4f330 --- /dev/null +++ b/package/tests/test_file_grouping.py @@ -0,0 +1,11 @@ +"""Tests for _group_files and FileReader behavior.""" + +from typing import Callable + +from pyaslreport.modalities.asl.processor import ASLProcessor + + +def test_make_processor_factory(make_processor: Callable[..., ASLProcessor]) -> None: + """The make_processor fixture produces an ASLProcessor with the given files.""" + proc = make_processor(files=[]) + assert proc.data["files"] == [] diff --git a/package/tests/test_m0_tsv_validation.py b/package/tests/test_m0_tsv_validation.py new file mode 100644 index 000000000..2caa4ab52 --- /dev/null +++ b/package/tests/test_m0_tsv_validation.py @@ -0,0 +1,12 @@ +"""Tests for M0 contradiction and TSV validation branches in ASLProcessor.""" + +from typing import Callable + +from pyaslreport.modalities.asl.processor import ProcessingContext + + +def test_make_context_factory(make_context: Callable[..., ProcessingContext]) -> None: + """The make_context fixture produces a ProcessingContext with overrides applied.""" + ctx = make_context(m0_type="Separate") + assert ctx.m0_type == "Separate" + assert ctx.errors == [] diff --git a/package/tests/test_normalization.py b/package/tests/test_normalization.py new file mode 100644 index 000000000..3144bfb7c --- /dev/null +++ b/package/tests/test_normalization.py @@ -0,0 +1,8 @@ +"""Tests for normalization logic: _rename_fields, _convert_units_to_milliseconds.""" + + +def test_module_imports() -> None: + """The processor module imports cleanly.""" + from pyaslreport.modalities.asl.processor import ASLProcessor + + assert ASLProcessor is not None diff --git a/package/tests/test_validators.py b/package/tests/test_validators.py new file mode 100644 index 000000000..973cb2b04 --- /dev/null +++ b/package/tests/test_validators.py @@ -0,0 +1,24 @@ +"""Tests for the six validator classes in modalities/asl/validators/.""" + + +def test_module_imports() -> None: + """All six validator classes import cleanly from the package.""" + from pyaslreport.modalities.asl.validators import ( + BooleanValidator, + ConsistencyValidator, + NumberArrayValidator, + NumberOrNumberArrayValidator, + NumberValidator, + StringValidator, + ) + + assert all( + [ + BooleanValidator, + ConsistencyValidator, + NumberArrayValidator, + NumberOrNumberArrayValidator, + NumberValidator, + StringValidator, + ] + ) From c7d119576acf44f5ea1672a3b25b76e46c965118 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Wed, 27 May 2026 02:16:56 -0300 Subject: [PATCH 04/16] ci: add GitHub Actions workflow for unit tests. --- .github/workflows/tests.yml | 54 +++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 000000000..19c1d1d4f --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,54 @@ +name: Tests + +# Runs on every PR and on pushes to main. +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +jobs: + unit-tests: + name: Unit tests (Python ${{ matrix.python-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false # run both Python versions even if one fails, so we see both + matrix: + # 3.11+ only: the package uses `match` and `int | float` unions (3.10+) + # and requires-python is >=3.11. Older versions fail on import. + python-version: ["3.11", "3.12"] + + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: "pip" + cache-dependency-path: package/pyproject.toml + + # The workflow lives at repo root, but the package is in ./package, + # so every step below runs from there (matches local dev from package/). + - name: Install package with test dependencies + working-directory: ./package + run: | + python -m pip install --upgrade pip + pip install -e ".[test]" + pip install black isort + + # --check / --check-only report only; they never edit. CI fails if code + # was committed unformatted. Mirrors the local pre-commit checks. + - name: Check formatting (Black) + working-directory: ./package + run: black --check tests/ + + - name: Check import order (isort) + working-directory: ./package + run: isort --check-only tests/ + + # -m "not integration": runs everything except tests marked @pytest.mark.integration. + # (no integration tests yet) + - name: Run unit tests + working-directory: ./package + run: pytest -v -m "not integration" \ No newline at end of file From 12506ca0d8f580f895f6472112d3b6a557ff570a Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Sat, 6 Jun 2026 19:37:06 -0300 Subject: [PATCH 05/16] test: cover normalization, unit conversion, and rounding --- package/tests/test_normalization.py | 162 +++++++++++++++++++++++++++- 1 file changed, 157 insertions(+), 5 deletions(-) diff --git a/package/tests/test_normalization.py b/package/tests/test_normalization.py index 3144bfb7c..13037604e 100644 --- a/package/tests/test_normalization.py +++ b/package/tests/test_normalization.py @@ -1,8 +1,160 @@ -"""Tests for normalization logic: _rename_fields, _convert_units_to_milliseconds.""" +"""Tests for normalization logic in ASLProcessor.""" +from typing import Callable -def test_module_imports() -> None: - """The processor module imports cleanly.""" - from pyaslreport.modalities.asl.processor import ASLProcessor +import pytest - assert ASLProcessor is not None +from pyaslreport.modalities.asl.processor import ASLProcessor +from pyaslreport.utils.math_utils import MathUtils +from pyaslreport.utils.unit_conversion_utils import UnitConverterUtils + +# ---------- _rename_fields ---------- + + +@pytest.mark.parametrize( + "old_key,new_key,value", + [ + ("RepetitionTime", "RepetitionTimePreparation", 4.5), + ("InversionTime", "PostLabelingDelay", 1.8), + ("BolusDuration", "BolusCutOffDelayTime", 0.7), + ("InitialPostLabelDelay", "PostLabelingDelay", 1.8), + ], +) +def test_rename_fields_renames_and_deletes_legacy( + make_processor: Callable[..., ASLProcessor], + old_key: str, + new_key: str, + value: float, +) -> None: + """Each of the four mappings copies old->new and deletes the old key.""" + proc = make_processor() + session = {old_key: value} + proc._rename_fields(session) + assert session[new_key] == value + assert old_key not in session + + +def test_rename_fields_numrfblocks_derives_labeling_duration( + make_processor: Callable[..., ASLProcessor], +) -> None: + """NumRFBlocks=100 derives LabelingDuration=1.84 (pins the numeric contract).""" + proc = make_processor() + session = {"NumRFBlocks": 100} + proc._rename_fields(session) + assert session["LabelingDuration"] == pytest.approx(1.84) + + +def test_rename_fields_numrfblocks_retains_source( + make_processor: Callable[..., ASLProcessor], +) -> None: + """CURRENT behavior: NumRFBlocks is NOT deleted after deriving LabelingDuration. + + Retention vs removal is an open contract question with mentors (provenance). + If they decide to remove it, change this to `assert "NumRFBlocks" not in session`. + """ + proc = make_processor() + session = {"NumRFBlocks": 100} + proc._rename_fields(session) + assert "NumRFBlocks" in session + + +def test_rename_fields_no_legacy_keys_is_noop( + make_processor: Callable[..., ASLProcessor], +) -> None: + """A modern session with no legacy keys is unchanged.""" + proc = make_processor() + session = {"PostLabelingDelay": 1.8, "RepetitionTimePreparation": 4.0} + original = dict(session) + proc._rename_fields(session) + assert session == original + + +# ---------- _convert_units_to_milliseconds ---------- + +TIME_FIELDS = [ + "EchoTime", + "RepetitionTimePreparation", + "LabelingDuration", + "BolusCutOffDelayTime", + "BackgroundSuppressionPulseTime", + "PostLabelingDelay", +] + + +@pytest.mark.parametrize("field", TIME_FIELDS) +def test_convert_scalar( + make_processor: Callable[..., ASLProcessor], field: str +) -> None: + """Each time field scalar is multiplied by 1000.""" + proc = make_processor() + session = {field: 1.5} + proc._convert_units_to_milliseconds(session) + assert session[field] == 1500 + + +@pytest.mark.parametrize("field", TIME_FIELDS) +def test_convert_list(make_processor: Callable[..., ASLProcessor], field: str) -> None: + """Each time field list maps element-wise.""" + proc = make_processor() + session = {field: [1.0, 2.0, 3.0]} + proc._convert_units_to_milliseconds(session) + assert session[field] == [1000, 2000, 3000] + + +def test_convert_leaves_non_time_fields( + make_processor: Callable[..., ASLProcessor], +) -> None: + """Non-time fields untouched; time field converted.""" + proc = make_processor() + session = {"FlipAngle": 90, "EchoTime": 0.012} + proc._convert_units_to_milliseconds(session) + assert session["FlipAngle"] == 90 + assert session["EchoTime"] == 12 + + +# ---------- MathUtils.round_if_close ---------- + + +@pytest.mark.parametrize( + "input_val,expected", + [ + (2000.0, 2000), + (2000.0000001, 2000), + (2000.5, 2000.5), + (1999.9999999, 2000), + (2000.123456, 2000.123), + (0.0, 0), + (-2000.0, -2000), + ], +) +def test_round_if_close(input_val: float, expected: int | float) -> None: + """Snaps to int within 1e-6 of an integer; else rounds to 3 decimals.""" + result = MathUtils.round_if_close(input_val) + assert result == expected + if isinstance(expected, int): + assert isinstance(result, int) + + +# ---------- UnitConverterUtils.convert_to_milliseconds ---------- + + +def test_convert_to_ms_scalar() -> None: + """A scalar in seconds becomes milliseconds (int when close to integer).""" + assert UnitConverterUtils.convert_to_milliseconds(2.0) == 2000 + + +def test_convert_to_ms_list() -> None: + """A list maps element-wise.""" + assert UnitConverterUtils.convert_to_milliseconds([1.0, 2.0]) == [1000, 2000] + + +def test_convert_to_ms_rejects_string() -> None: + """A non-numeric scalar raises TypeError.""" + with pytest.raises(TypeError): + UnitConverterUtils.convert_to_milliseconds("not a number") + + +def test_convert_to_ms_rejects_list_with_string() -> None: + """A list containing a non-number raises TypeError.""" + with pytest.raises(TypeError): + UnitConverterUtils.convert_to_milliseconds([1.0, "two"]) \ No newline at end of file From 96fafcfd733d5627c8cb30ab17f6c22a49ea2092 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Sun, 7 Jun 2026 19:01:00 -0300 Subject: [PATCH 06/16] style: fix format test_normalization with black --- package/tests/test_normalization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/tests/test_normalization.py b/package/tests/test_normalization.py index 13037604e..1c2bd7df6 100644 --- a/package/tests/test_normalization.py +++ b/package/tests/test_normalization.py @@ -157,4 +157,4 @@ def test_convert_to_ms_rejects_string() -> None: def test_convert_to_ms_rejects_list_with_string() -> None: """A list containing a non-number raises TypeError.""" with pytest.raises(TypeError): - UnitConverterUtils.convert_to_milliseconds([1.0, "two"]) \ No newline at end of file + UnitConverterUtils.convert_to_milliseconds([1.0, "two"]) From b1ce59d1c3175b4de218e2d1d2a30580767f715e Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Sun, 7 Jun 2026 19:13:43 -0300 Subject: [PATCH 07/16] chore: drop py312 from black target-version --- package/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/pyproject.toml b/package/pyproject.toml index 51fdfeeb1..28f2bf2e4 100644 --- a/package/pyproject.toml +++ b/package/pyproject.toml @@ -52,7 +52,7 @@ markers = [ [tool.black] line-length = 88 -target-version = ["py311", "py312"] +target-version = ["py311"] [tool.isort] profile = "black" From 7f628e5bd98b0720373118da2cce2331df1e376a Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Wed, 10 Jun 2026 10:35:23 -0300 Subject: [PATCH 08/16] test: cover all six validator classes plus schema loading --- package/tests/test_validators.py | 233 +++++++++++++++++++++++++++---- 1 file changed, 209 insertions(+), 24 deletions(-) diff --git a/package/tests/test_validators.py b/package/tests/test_validators.py index 973cb2b04..4a33d617f 100644 --- a/package/tests/test_validators.py +++ b/package/tests/test_validators.py @@ -1,24 +1,209 @@ -"""Tests for the six validator classes in modalities/asl/validators/.""" - - -def test_module_imports() -> None: - """All six validator classes import cleanly from the package.""" - from pyaslreport.modalities.asl.validators import ( - BooleanValidator, - ConsistencyValidator, - NumberArrayValidator, - NumberOrNumberArrayValidator, - NumberValidator, - StringValidator, - ) - - assert all( - [ - BooleanValidator, - ConsistencyValidator, - NumberArrayValidator, - NumberOrNumberArrayValidator, - NumberValidator, - StringValidator, - ] - ) +"""Tests for the six validator classes plus schema loading.""" + +from typing import Any + +from pyaslreport.core.config import config +from pyaslreport.modalities.asl.validators import ( + BooleanValidator, + ConsistencyValidator, + NumberArrayValidator, + NumberOrNumberArrayValidator, + NumberValidator, + StringValidator, +) + +MAJOR_SLOT, ERROR_SLOT, WARNING_SLOT = 0, 2, 4 + + +def fired_major(r: tuple[Any, ...]) -> bool: + """True if the major-error slot is populated.""" + return r[MAJOR_SLOT] is not None + + +def fired_error(r: tuple[Any, ...]) -> bool: + """True if the (non-major) error slot is populated.""" + return r[ERROR_SLOT] is not None + + +def fired_warning(r: tuple[Any, ...]) -> bool: + """True if the warning slot is populated.""" + return r[WARNING_SLOT] is not None + + +def all_clear(r: tuple[Any, ...]) -> bool: + """True if no slot is populated.""" + return all(x is None for x in r) + + +class TestNumberValidator: + def test_within_bounds(self) -> None: + assert all_clear(NumberValidator(min_error=0, max_error=10).validate(5)) + + def test_min_error_is_strict(self) -> None: + v = NumberValidator(min_error=0) + assert v.validate(0)[ERROR_SLOT] == "Value must be > 0" + assert all_clear(v.validate(0.0001)) + + def test_above_max_error(self) -> None: + assert fired_error(NumberValidator(max_error=10).validate(20)) + + def test_min_error_include_is_inclusive(self) -> None: + v = NumberValidator(min_error_include=0) + assert all_clear(v.validate(0)) + assert fired_error(v.validate(-1)) + + def test_max_error_include_boundary(self) -> None: + v = NumberValidator(max_error_include=360) + assert all_clear(v.validate(360)) + assert v.validate(360.001)[ERROR_SLOT] == "Value must be <= 360" + + def test_warning_threshold(self) -> None: + assert fired_warning(NumberValidator(min_warning=0).validate(-1)) + + def test_enforce_integer_alone(self) -> None: + v = NumberValidator(enforce_integer=True) + assert all_clear(v.validate(5)) + assert fired_error(v.validate(5.5)) + + def test_enforce_integer_rule_precedes_range(self) -> None: + """Integer rule is added first, so it fires before the range check.""" + v = NumberValidator(min_error=0, enforce_integer=True) + assert v.validate(2.5)[ERROR_SLOT] == "Value must be an integer" + assert v.validate(-1)[ERROR_SLOT] == "Value must be > 0" + assert all_clear(v.validate(3)) + + +class TestStringValidator: + def test_allowed_passes(self) -> None: + assert all_clear( + StringValidator(allowed_values=["PCASL", "PASL"]).validate("PCASL") + ) + + def test_case_insensitive(self) -> None: + assert all_clear(StringValidator(allowed_values=["PCASL"]).validate("pcasl")) + + def test_disallowed_routes_to_error(self) -> None: + r = StringValidator(allowed_values=["PCASL"]).validate("XYZ") + assert fired_error(r) and not fired_major(r) + + def test_major_flag_routes_to_major(self) -> None: + r = StringValidator(allowed_values=["PCASL"], major_error=True).validate("XYZ") + assert fired_major(r) and not fired_error(r) + + def test_no_allowed_values_accepts_anything(self) -> None: + assert all_clear(StringValidator().validate("anything")) + + +class TestBooleanValidator: + def test_true_false_pass(self) -> None: + assert all_clear(BooleanValidator().validate(True)) + assert all_clear(BooleanValidator().validate(False)) + + def test_string_errors(self) -> None: + assert fired_error(BooleanValidator().validate("true")) + + def test_int_errors(self) -> None: + """isinstance(1, bool) is False, so 1 is rejected.""" + assert fired_error(BooleanValidator().validate(1)) + + +class TestNumberArrayValidator: + def test_exact_size(self) -> None: + v = NumberArrayValidator(size_error=3) + assert all_clear(v.validate([1, 2, 3])) + assert fired_error(v.validate([1, 2])) + assert fired_error(v.validate([1, 2, "x"])) + + def test_min_error_skips_non_numbers(self) -> None: + """Range rule's guarded comprehension ignores non-numeric elements.""" + v = NumberArrayValidator(min_error=0) + assert fired_error(v.validate([1, -1, 2])) + assert all_clear(v.validate([1, 2, "x"])) + + def test_ascending_allows_equal_neighbors(self) -> None: + v = NumberArrayValidator(check_ascending=True) + assert all_clear(v.validate([1, 1, 2])) + assert fired_error(v.validate([3, 1, 2])) + + +class TestNumberOrNumberArrayValidator: + def test_scalar_dispatch(self) -> None: + v = NumberOrNumberArrayValidator(min_error=0) + assert fired_error(v.validate(-1)) + assert all_clear(v.validate(5)) + + def test_array_dispatch(self) -> None: + v = NumberOrNumberArrayValidator(min_error=0) + assert fired_error(v.validate([1, -1, 2])) + assert all_clear(v.validate([1, 2, 3])) + + def test_wrong_type_reports_in_major_slot(self) -> None: + """Type mismatch lands in slot 0; pins current (arguably odd) behavior.""" + assert NumberOrNumberArrayValidator().validate("x")[MAJOR_SLOT] is not None + + +class TestConsistencyValidator: + def test_string_same_passes(self) -> None: + v = ConsistencyValidator(validation_type="string") + assert all_clear(v.validate([("PCASL", "a"), ("PCASL", "b")])) + + def test_string_mismatch_error_tier(self) -> None: + v = ConsistencyValidator(validation_type="string", is_major=False) + assert fired_error(v.validate([("PCASL", "a"), ("PASL", "b")])) + + def test_string_mismatch_major_tier(self) -> None: + v = ConsistencyValidator(validation_type="string", is_major=True) + assert fired_major(v.validate([("PCASL", "a"), ("PASL", "b")])) + + def test_float_within_warning(self) -> None: + v = ConsistencyValidator( + "floatOrArray", error_variation=10, warning_variation=0.1 + ) + assert all_clear(v.validate([(2000, "a"), (2000.05, "b")])) + + def test_float_crosses_warning(self) -> None: + v = ConsistencyValidator( + "floatOrArray", error_variation=10, warning_variation=0.1 + ) + assert fired_warning(v.validate([(2000, "a"), (2000.5, "b")])) + + def test_float_crosses_error(self) -> None: + v = ConsistencyValidator( + "floatOrArray", error_variation=10, warning_variation=0.1 + ) + assert fired_error(v.validate([(2000, "a"), (2020, "b")])) + + def test_boolean_same_passes(self) -> None: + assert all_clear( + ConsistencyValidator("boolean").validate([(True, "a"), (True, "b")]) + ) + + def test_boolean_mixed_errors(self) -> None: + assert fired_error( + ConsistencyValidator("boolean").validate([(True, "a"), (False, "b")]) + ) + + +def test_all_expected_schemas_loaded() -> None: + """config['schemas'] contains every shipped schema.""" + expected = { + "major_error_schema", + "required_validator_schema", + "required_condition_schema", + "recommended_validator_schema", + "recommended_condition_schema", + "consistency_schema", + } + assert expected.issubset(config["schemas"].keys()) + + +def test_major_error_schema_fields() -> None: + """The four documented major-error fields are present.""" + schema = config["schemas"]["major_error_schema"] + for field in ( + "PLDType", + "ArterialSpinLabelingType", + "MRAcquisitionType", + "PulseSequenceType", + ): + assert field in schema From c73a149ea74f1a6d0f42cecce8744c6c80336390 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 15 Jun 2026 15:56:53 -0300 Subject: [PATCH 09/16] test: cover M0 contradiction paths, TSV validation, and BS warnings --- package/tests/test_m0_tsv_validation.py | 190 +++++++++++++++++++++++- 1 file changed, 183 insertions(+), 7 deletions(-) diff --git a/package/tests/test_m0_tsv_validation.py b/package/tests/test_m0_tsv_validation.py index 2caa4ab52..3cf1ed858 100644 --- a/package/tests/test_m0_tsv_validation.py +++ b/package/tests/test_m0_tsv_validation.py @@ -1,12 +1,188 @@ -"""Tests for M0 contradiction and TSV validation branches in ASLProcessor.""" +"""Tests for M0 contradiction paths, TSV validation, and BS warnings.""" from typing import Callable -from pyaslreport.modalities.asl.processor import ProcessingContext +from pyaslreport.modalities.asl.processor import ASLProcessor, ProcessingContext +# ---------- _validate_m0_data ---------- -def test_make_context_factory(make_context: Callable[..., ProcessingContext]) -> None: - """The make_context fixture produces a ProcessingContext with overrides applied.""" - ctx = make_context(m0_type="Separate") - assert ctx.m0_type == "Separate" - assert ctx.errors == [] + +class TestValidateM0Data: + def test_separate_with_no_m0_file_errors( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """M0Type=Separate but m0_json missing -> error appended.""" + proc = make_processor() + ctx = make_context(m0_type="Separate") + group = { + "asl_json": ("asl.json", {"M0Type": "Separate"}), + "m0_json": None, + "tsv": None, + } + proc._validate_m0_data(group, ctx, "asl.json", group["asl_json"][1]) + assert any("Separate" in e and "not provided" in e for e in ctx.errors) + + def test_absent_with_m0_file_errors( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """M0Type=Absent but m0_json present -> error appended.""" + proc = make_processor() + ctx = make_context(m0_type="Absent") + m0_data = {"EchoTime": 0.012} + group = { + "asl_json": ("asl.json", {"M0Type": "Absent"}), + "m0_json": ("m0.json", m0_data), + "tsv": None, + } + proc._validate_m0_data(group, ctx, "asl.json", group["asl_json"][1]) + assert any("Absent" in e and "is present" in e for e in ctx.errors) + + def test_included_with_separate_m0_file_errors( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """M0Type=Included but separate m0_json provided -> error.""" + proc = make_processor() + ctx = make_context(m0_type="Included") + m0_data = {"EchoTime": 0.012} + group = { + "asl_json": ("asl.json", {"M0Type": "Included"}), + "m0_json": ("m0.json", m0_data), + "tsv": None, + } + proc._validate_m0_data(group, ctx, "asl.json", group["asl_json"][1]) + assert any("Included" in e for e in ctx.errors) + + def test_separate_with_m0_file_no_error( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """M0Type=Separate with m0_json present -> no contradiction. + + Both ASL and M0 dicts agree on the five compare_params fields to avoid + spurious errors from that helper. + """ + proc = make_processor() + ctx = make_context(m0_type="Separate") + m0_data = { + "EchoTime": 0.012, + "FlipAngle": 90, + "MagneticFieldStrength": 3, + "MRAcquisitionType": "3D", + "PulseSequenceType": "GRASE", + } + asl_data = dict(m0_data, M0Type="Separate") + group = { + "asl_json": ("asl.json", asl_data), + "m0_json": ("m0.json", m0_data), + "tsv": None, + } + proc._validate_m0_data(group, ctx, "asl.json", asl_data) + m0_type_errors = [ + e for e in ctx.errors if "M0 type" in e or "specified as" in e + ] + assert m0_type_errors == [] + + +# ---------- TSV: _analyze_tsv_volume_types and _validate_m0scan_consistency ---------- + + +class TestTSVValidation: + def test_absent_with_m0scan_in_tsv_errors( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """M0Type=Absent but TSV contains 'm0scan' -> error.""" + proc = make_processor() + ctx = make_context(m0_type="Absent") + asl_data = {"M0Type": "Absent"} + tsv_data = ["m0scan", "control", "label"] + proc._analyze_tsv_volume_types( + tsv_data, ctx, "asl.json", asl_data, "context.tsv" + ) + assert any("Absent" in e and "m0scan" in e for e in ctx.errors) + + def test_separate_with_m0scan_in_tsv_errors( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """M0Type=Separate but TSV contains 'm0scan' -> error.""" + proc = make_processor() + ctx = make_context(m0_type="Separate") + asl_data = {"M0Type": "Separate"} + tsv_data = ["m0scan", "control", "label"] + proc._analyze_tsv_volume_types( + tsv_data, ctx, "asl.json", asl_data, "context.tsv" + ) + assert any("Separate" in e and "m0scan" in e for e in ctx.errors) + + def test_total_acquired_pairs_set( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """A clean TSV populates TotalAcquiredPairs.""" + proc = make_processor() + ctx = make_context(m0_type="Included") + asl_data = { + "M0Type": "Included", + "RepetitionTimePreparation": 4.0, + "BackgroundSuppression": False, + } + tsv_data = ["m0scan", "control", "label", "control", "label"] + proc._analyze_tsv_volume_types( + tsv_data, ctx, "asl.json", asl_data, "context.tsv" + ) + assert asl_data["TotalAcquiredPairs"] == 2 # two control-label pairs + + +# ---------- _handle_no_m0scan_warnings ---------- + + +class TestBackgroundSuppressionWarnings: + def test_bs_off_no_warning( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """BackgroundSuppression off -> no warnings.""" + proc = make_processor() + ctx = make_context() + asl_data = {"BackgroundSuppression": False} + proc._handle_no_m0scan_warnings(ctx, "asl.json", asl_data) + assert ctx.warnings == [] + + def test_bs_on_with_pulse_time_warns_about_efficiency( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """BS on with pulse times -> efficiency warning.""" + proc = make_processor() + ctx = make_context() + asl_data = { + "BackgroundSuppression": True, + "BackgroundSuppressionPulseTime": [0.15, 0.5], + } + proc._handle_no_m0scan_warnings(ctx, "asl.json", asl_data) + assert any("BS-pulse efficiency" in w for w in ctx.warnings) + + def test_bs_on_no_pulse_time_warns_about_relative_quantification( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """BS on without pulse times -> relative-quantification warning.""" + proc = make_processor() + ctx = make_context() + asl_data = {"BackgroundSuppression": True} + proc._handle_no_m0scan_warnings(ctx, "asl.json", asl_data) + assert any("relative quantification" in w for w in ctx.warnings) From db2f8ce689042b5c0234a65b95525b5d4e5bd90a Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 15 Jun 2026 18:21:13 -0300 Subject: [PATCH 10/16] test: cover file grouping for both modes plus TSV header --- package/tests/test_file_grouping.py | 153 +++++++++++++++++++++++++++- 1 file changed, 148 insertions(+), 5 deletions(-) diff --git a/package/tests/test_file_grouping.py b/package/tests/test_file_grouping.py index 40ec4f330..4f2cef745 100644 --- a/package/tests/test_file_grouping.py +++ b/package/tests/test_file_grouping.py @@ -1,11 +1,154 @@ """Tests for _group_files and FileReader behavior.""" +import json +from pathlib import Path from typing import Callable -from pyaslreport.modalities.asl.processor import ASLProcessor +import pytest +from pyaslreport.modalities.asl.processor import ASLProcessor, ProcessingContext -def test_make_processor_factory(make_processor: Callable[..., ASLProcessor]) -> None: - """The make_processor fixture produces an ASLProcessor with the given files.""" - proc = make_processor(files=[]) - assert proc.data["files"] == [] +# ---------- _group_files: NIfTI mode (exact suffix matching) ---------- + + +class TestGroupFilesNiftiMode: + def test_groups_asl_with_tsv_and_m0( + self, make_processor: Callable[..., ASLProcessor], tmp_path: Path + ) -> None: + """A canonical BIDS triple groups together correctly.""" + asl_json = tmp_path / "sub-01_asl.json" + asl_json.write_text(json.dumps({"M0Type": "Separate"})) + tsv = tmp_path / "sub-01_aslcontext.tsv" + tsv.write_text("volume_type\ncontrol\nlabel\n") + m0_json = tmp_path / "sub-01_m0scan.json" + m0_json.write_text(json.dumps({"EchoTime": 0.012})) + + proc = make_processor(files=[str(asl_json), str(tsv), str(m0_json)]) + groups = proc._group_files("nifti") + + assert len(groups) == 1 + g = groups[0] + assert g["asl_json"][0] == "sub-01_asl.json" + assert g["tsv"][0] == "sub-01_aslcontext.tsv" + assert g["m0_json"][0] == "sub-01_m0scan.json" + + def test_two_sessions_produce_two_groups( + self, make_processor: Callable[..., ASLProcessor], tmp_path: Path + ) -> None: + """Two BIDS sessions in the same directory produce two groups.""" + # Build the list explicitly. Do NOT scan tmp_path with iterdir(): the + # make_processor -> minimal_nifti_path fixture also writes asl.nii.gz + # into tmp_path, and _group_files rejects unknown extensions. + files: list[str] = [] + for i in [1, 2]: + asl_json = tmp_path / f"sub-0{i}_asl.json" + asl_json.write_text(json.dumps({"M0Type": "Separate"})) + tsv = tmp_path / f"sub-0{i}_aslcontext.tsv" + tsv.write_text("volume_type\ncontrol\nlabel\n") + files.extend([str(asl_json), str(tsv)]) + proc = make_processor(files=files) + groups = proc._group_files("nifti") + assert len(groups) == 2 + + def test_unsupported_extension_raises( + self, make_processor: Callable[..., ASLProcessor], tmp_path: Path + ) -> None: + """An unsupported extension raises ValueError during grouping.""" + bad = tmp_path / "weird.xml" + bad.write_text("") + proc = make_processor(files=[str(bad)]) + with pytest.raises(ValueError, match="Unsupported file format"): + proc._group_files("nifti") + + +# ---------- _group_files: DICOM mode (substring matching for m0) ---------- + + +class TestGroupFilesDicomMode: + def test_dicom_mode_uses_substring_for_m0( + self, make_processor: Callable[..., ASLProcessor], tmp_path: Path + ) -> None: + """In DICOM mode, any filename containing 'm0' counts as M0.""" + asl_json = tmp_path / "scan_dump.json" + asl_json.write_text(json.dumps({"M0Type": "Separate"})) + m0_json = tmp_path / "scan_m0_dump.json" + m0_json.write_text(json.dumps({"EchoTime": 0.012})) + + proc = make_processor(files=[str(asl_json), str(m0_json)]) + groups = proc._group_files("dicom") + assert len(groups) == 1 + assert groups[0]["asl_json"][0] == "scan_dump.json" + assert groups[0]["m0_json"][0] == "scan_m0_dump.json" + + +# ---------- _validate_tsv_data: missing TSV behavior ---------- + + +class TestMissingTSV: + def test_missing_tsv_in_nifti_mode_errors( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """Missing aslcontext.tsv in NIfTI mode produces a missing-file error.""" + proc = make_processor() + ctx = make_context() + group = { + "asl_json": ("asl.json", {"M0Type": "Absent"}), + "m0_json": None, + "tsv": None, + } + proc._validate_tsv_data(group, ctx, "asl.json", group["asl_json"][1], "nifti") + assert any("aslcontext.tsv" in e and "missing" in e for e in ctx.errors) + + def test_missing_tsv_in_dicom_mode_falls_through_to_dicom_repetitions( + self, + make_processor: Callable[..., ASLProcessor], + make_context: Callable[..., ProcessingContext], + ) -> None: + """Missing TSV in DICOM mode delegates to _analyze_dicom_repetitions.""" + proc = make_processor() + ctx = make_context() + asl_data = {"lRepetitions": 10} + group = {"asl_json": ("asl.json", asl_data), "m0_json": None, "tsv": None} + proc._validate_tsv_data(group, ctx, "asl.json", asl_data, "dicom") + # _analyze_dicom_repetitions sets total_acquired_pairs from lRepetitions/2 + assert ctx.total_acquired_pairs == 5 + # No TSV-missing error in DICOM mode + assert not any("aslcontext.tsv" in e for e in ctx.errors) + + +# ---------- FileReader: TSV header enforcement ---------- + + +class TestFileReaderTSVHeader: + def test_valid_header_returns_data(self, tmp_path: Path) -> None: + """A 'volume_type' header with rows returns the rows as a list.""" + from pyaslreport.io.readers.file_reader import FileReader + + f = tmp_path / "valid.tsv" + f.write_text("volume_type\ncontrol\nlabel\n") + result = FileReader.read(str(f)) + assert result == ["control", "label"] + + def test_invalid_header_raises(self, tmp_path: Path) -> None: + """A header that isn't exactly 'volume_type' raises RuntimeError. + + NOTE: FileReader.read re-wraps the inner error as + 'Error reading file: Invalid TSV header, ...'. The substring match below + still matches; do NOT anchor this regex with '^'. + """ + from pyaslreport.io.readers.file_reader import FileReader + + f = tmp_path / "bad.tsv" + f.write_text("volume_types\ncontrol\nlabel\n") # plural, wrong + with pytest.raises(RuntimeError, match="Invalid TSV header"): + FileReader.read(str(f)) + + def test_empty_file_returns_none(self, tmp_path: Path) -> None: + """A truly empty TSV returns None rather than raising.""" + from pyaslreport.io.readers.file_reader import FileReader + + f = tmp_path / "empty.tsv" + f.write_text("") + assert FileReader.read(str(f)) is None From b4357e12066854b42e2b3fa2f5f62f50a52c9194 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 22 Jun 2026 12:46:03 -0300 Subject: [PATCH 11/16] test: clarify NumRFBlocks retention for provenance --- package/tests/test_normalization.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/package/tests/test_normalization.py b/package/tests/test_normalization.py index 1c2bd7df6..cb9a5f252 100644 --- a/package/tests/test_normalization.py +++ b/package/tests/test_normalization.py @@ -47,11 +47,7 @@ def test_rename_fields_numrfblocks_derives_labeling_duration( def test_rename_fields_numrfblocks_retains_source( make_processor: Callable[..., ASLProcessor], ) -> None: - """CURRENT behavior: NumRFBlocks is NOT deleted after deriving LabelingDuration. - - Retention vs removal is an open contract question with mentors (provenance). - If they decide to remove it, change this to `assert "NumRFBlocks" not in session`. - """ + """NumRFBlocks is retained after deriving LabelingDuration for provenance.""" proc = make_processor() session = {"NumRFBlocks": 100} proc._rename_fields(session) From 3ac661e6a39df90d55040eac4851689d0f07d4af Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 22 Jun 2026 15:16:06 -0300 Subject: [PATCH 12/16] test: assert NumberOrNumberArray routing as intended behavior --- package/tests/test_validators.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/package/tests/test_validators.py b/package/tests/test_validators.py index 4a33d617f..953c4b484 100644 --- a/package/tests/test_validators.py +++ b/package/tests/test_validators.py @@ -137,9 +137,10 @@ def test_array_dispatch(self) -> None: assert fired_error(v.validate([1, -1, 2])) assert all_clear(v.validate([1, 2, 3])) - def test_wrong_type_reports_in_major_slot(self) -> None: - """Type mismatch lands in slot 0; pins current (arguably odd) behavior.""" - assert NumberOrNumberArrayValidator().validate("x")[MAJOR_SLOT] is not None + def test_wrong_type_is_reported_as_major_error(self) -> None: + """A non-numeric, non-list value is reported as a major error by design.""" + r = NumberOrNumberArrayValidator().validate("x") + assert fired_major(r) and not fired_error(r) class TestConsistencyValidator: From 5aeb357db6d65bb26d5176a01994050efb12918a Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 22 Jun 2026 18:42:42 -0300 Subject: [PATCH 13/16] fix: compare int-valued fields in compare_params --- package/src/pyaslreport/modalities/asl/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/src/pyaslreport/modalities/asl/utils.py b/package/src/pyaslreport/modalities/asl/utils.py index f23dccd43..1c55d1f9a 100644 --- a/package/src/pyaslreport/modalities/asl/utils.py +++ b/package/src/pyaslreport/modalities/asl/utils.py @@ -48,7 +48,7 @@ def compare_params(params_asl, params_m0, asl_filename, m0_filename): f"Discrepancy in '{param}' for ASL file '{asl_filename}' and M0 file '{m0_filename}': " f"ASL value = {asl_value}, M0 value = {m0_value}") elif validation_type == "floatOrArray": - if isinstance(asl_value, float) and isinstance(m0_value, float): + if isinstance(asl_value, (int, float)) and isinstance(m0_value, (int, float)): difference = abs(asl_value - m0_value) difference_formatted = f"{difference:.2f}" if difference > error_variation: From 1399adda849965846a566b317852b69f62042b3a Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 22 Jun 2026 19:46:12 -0300 Subject: [PATCH 14/16] chore: pin black and isort in test extra, install via .[test] in CI --- .github/workflows/tests.yml | 1 - package/pyproject.toml | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 19c1d1d4f..c788940e7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -35,7 +35,6 @@ jobs: run: | python -m pip install --upgrade pip pip install -e ".[test]" - pip install black isort # --check / --check-only report only; they never edit. CI fails if code # was committed unformatted. Mirrors the local pre-commit checks. diff --git a/package/pyproject.toml b/package/pyproject.toml index 28f2bf2e4..6efe5ce13 100644 --- a/package/pyproject.toml +++ b/package/pyproject.toml @@ -32,6 +32,8 @@ where = ["src"] test = [ "pytest>=7.0", "pytest-cov>=4.0", + "black==26.5.1", + "isort==8.0.1", ] [tool.pytest.ini_options] From 9854f19f9fdacb9bff6d2cb0abf32a9c16f5d738 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 13 Jul 2026 09:16:59 -0300 Subject: [PATCH 15/16] test: add example-based integration runner and harness --- package/tests/conftest.py | 32 +++ package/tests/integration/__init__.py | 0 package/tests/integration/runner.py | 286 ++++++++++++++++++++++++++ package/tests/test_integration.py | 263 +++++++++++++++++++++++ 4 files changed, 581 insertions(+) create mode 100644 package/tests/integration/__init__.py create mode 100644 package/tests/integration/runner.py create mode 100644 package/tests/test_integration.py diff --git a/package/tests/conftest.py b/package/tests/conftest.py index 4fcb386d3..107509306 100644 --- a/package/tests/conftest.py +++ b/package/tests/conftest.py @@ -40,6 +40,12 @@ def pytest_addoption(parser: pytest.Parser) -> None: default=None, help="Path to integration examples dir; falls back to the committed CI set.", ) + parser.addoption( + "--update-expected", + action="store_true", + default=False, + help="Regenerate each example's expected_output.json from current tool output.", + ) # --------------------------------------------------------------------------- @@ -196,3 +202,29 @@ def minimal_asl_json() -> dict[str, Any]: "ManufacturersModelName": "TrioTim", "AcquisitionVoxelSize": [3, 3, 4], } + + +# --------------------------------------------------------------------------- +# Integration example discovery (parametrizes test_integration over cases) +# --------------------------------------------------------------------------- +def pytest_generate_tests(metafunc: pytest.Metafunc) -> None: + """Parametrize integration tests over discovered example case folders. + + Reads --examples-dir (registered above): when absent, falls back to the + committed set at tests/integration/examples. An empty or missing dir yields + zero cases, which pytest reports as a single skip rather than an error. + + Args: + metafunc: The pytest metafunc for the test being collected. + """ + if "integration_case" in metafunc.fixturenames: + from tests.integration.runner import discover_examples + + cli = metafunc.config.getoption("--examples-dir") + base = ( + Path(cli).expanduser().resolve() + if cli + else Path(__file__).parent / "integration" / "examples" + ) + cases = discover_examples(base) + metafunc.parametrize("integration_case", cases, ids=[p.name for p in cases]) diff --git a/package/tests/integration/__init__.py b/package/tests/integration/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/package/tests/integration/runner.py b/package/tests/integration/runner.py new file mode 100644 index 000000000..bd63c245a --- /dev/null +++ b/package/tests/integration/runner.py @@ -0,0 +1,286 @@ +"""Shared logic for the example-based integration runner. + +Single source of truth imported by: + - conftest.pytest_generate_tests (case discovery at collection time) + - tests/test_integration.py (running + comparing, plus harness self-tests) + +Example folder structure (fixed): + + examples/ + / ← one "case" == one generate_report call + expected_output.json ← generated by --update-expected (see below) + sub-/ + [ses-/] ← optional session level + perf/ + *_asl.json (required: defines an acquisition) + *_asl.nii[.gz] (required: >= 1 per case; only its slice count is read) + *_m0scan.json (optional) + *_aslcontext.tsv(optional) + +A CASE is any immediate child of examples/ with at least one *_asl.json inside a +folder named ``perf``. Discovery keys on inputs, NOT on expected_output.json, so +``--update-expected`` can bootstrap a case that has no golden yet. A case may hold +several acquisitions (multiple subjects/sessions/runs); the tool consolidates them +into one consistency report. Only files inside ``perf`` are considered; anat/, +fmap/, derivatives/ and sourcedata/ are ignored. + +ASL <-> M0/TSV pairing (per perf folder), following the mentors' rules. The tool +pairs sidecars to an ASL purely by ORDER in the flat file list, so the runner emits +each acquisition as asl -> m0 -> tsv and decides the pairing itself: + + * exactly matching prefixes (e.g. sub-01_run-1) -> pair directly + * an ASL with no prefix-matched sidecar -> reuse a FALLBACK sidecar + (the one with the latest creation time, tie-broken by filename), which covers + both "more ASL than M0: reuse the shared M0 for all unmatched" and + "more M0 than ASL: use the latest M0". A repeated M0 path in the list is fine + (verified: the tool re-reads it and pairs it to each ASL). + +Every fallback emits a ``PairingFallbackWarning`` (visible in pytest's warnings +summary) so the person running --update-expected can review the decision before +committing the golden. + +NOTE ON "latest creation time": Linux has no true birth time (st_birthtime is +absent; st_ctime is metadata-change time), and git does not preserve timestamps, so +in a fresh clone all files share ~one time. The filename tie-break therefore +dominates for committed examples (deterministic in CI), while real creation time is +honoured locally when it is meaningful. The fallback only fires when prefixes don't +match, which is rare. +""" + +from __future__ import annotations + +import json +import warnings +from pathlib import Path +from typing import Any, Optional + +from pyaslreport import generate_report +from pyaslreport.enums import ModalityTypeValues + +EXPECTED_FILENAME = "expected_output.json" + +_ASL_JSON = "_asl.json" +_M0_JSON = "_m0scan.json" +_TSV = "_aslcontext.tsv" +_NII_SUFFIXES = ("_asl.nii.gz", "_asl.nii") +_SKIP_DIRS = {"derivatives", "sourcedata"} + + +class ExampleStructureError(ValueError): + """Raised when a case folder can't be turned into a valid call at all.""" + + +class PairingFallbackWarning(UserWarning): + """Emitted when an ASL is paired to an M0/TSV by fallback, not exact prefix.""" + + +def _creation_time(path: Path) -> float: + """Best-available creation time (birth time if present, else mtime). + + Linux lacks st_birthtime, and git does not preserve timestamps, so callers + must tie-break by filename for determinism across clones. + """ + st = path.stat() + return float(getattr(st, "st_birthtime", st.st_mtime)) + + +def _perf_dirs(case: Path) -> list[Path]: + """Return raw-BIDS ``perf`` directories under a case, sorted by path.""" + out = [] + for d in case.rglob("perf"): + if d.is_dir() and not _SKIP_DIRS.intersection(p.name for p in d.parents): + out.append(d) + return sorted(out) + + +def _prefix(name: str, suffix: str) -> str: + """Strip a known modality suffix to get the shared BIDS entity prefix.""" + return name[: -len(suffix)] + + +def _categorize( + perf: Path, +) -> tuple[dict[str, Path], dict[str, Path], dict[str, Path], dict[str, Path]]: + """Index a perf dir's files by entity prefix, per modality. + + Returns: + (asls, m0s, tsvs, niis) dicts mapping prefix -> path. + + Raises: + ExampleStructureError: If two files in one modality share a prefix. + """ + asls: dict[str, Path] = {} + m0s: dict[str, Path] = {} + tsvs: dict[str, Path] = {} + niis: dict[str, Path] = {} + + def _put(bucket: dict[str, Path], key: str, path: Path, what: str) -> None: + if key in bucket: + raise ExampleStructureError( + f"{perf}: two {what} files share prefix '{key}' " + f"({bucket[key].name}, {path.name})" + ) + bucket[key] = path + + for p in sorted(perf.iterdir()): + n = p.name + if n.endswith(_M0_JSON): + _put(m0s, _prefix(n, _M0_JSON), p, "m0scan") + elif n.endswith(_ASL_JSON): + _put(asls, _prefix(n, _ASL_JSON), p, "asl.json") + elif n.endswith(_TSV): + _put(tsvs, _prefix(n, _TSV), p, "aslcontext.tsv") + else: + for suf in _NII_SUFFIXES: + if n.endswith(suf): + _put(niis, _prefix(n, suf), p, "asl nifti") + break + return asls, m0s, tsvs, niis + + +def _pair_sidecar( + asl_prefixes: list[str], sidecars: dict[str, Path], kind: str, perf: Path +) -> dict[str, Optional[Path]]: + """Map each ASL prefix to a sidecar, prefix-first then fallback. + + Fallback (used when an ASL has no exact-prefix sidecar) is the sidecar with the + latest creation time, tie-broken by filename. This realises both mentor rules: + reuse the shared M0 when there are more ASLs than M0s, and use the latest M0 + when there are more M0s than ASLs. Each fallback emits a warning. + + Args: + asl_prefixes: Sorted ASL entity prefixes in this perf dir. + sidecars: prefix -> path for one modality (m0scan or aslcontext.tsv). + kind: Human label for messages ("m0scan" / "aslcontext.tsv"). + perf: The perf directory (for messages). + + Returns: + prefix -> chosen sidecar path (or None if no sidecar of this kind exists). + """ + if not sidecars: + return {pre: None for pre in asl_prefixes} + fallback = max(sidecars.values(), key=lambda p: (_creation_time(p), p.name)) + result: dict[str, Optional[Path]] = {} + for pre in asl_prefixes: + if pre in sidecars: + result[pre] = sidecars[pre] + else: + result[pre] = fallback + warnings.warn( + f"{perf.name}: ASL '{pre}' has no prefix-matched {kind}; " + f"reusing '{fallback.name}' (latest by creation time, then filename).", + PairingFallbackWarning, + stacklevel=2, + ) + return result + + +# One acquisition: (asl, m0|None, tsv|None, nifti|None) +_Acq = tuple[Path, Optional[Path], Optional[Path], Optional[Path]] + + +def _collect_acquisitions(case: Path) -> list[_Acq]: + """Gather ordered acquisitions across all perf dirs of a case. + + Raises: + ExampleStructureError: If the case has no *_asl.json in any perf dir. + """ + acquisitions: list[_Acq] = [] + for perf in _perf_dirs(case): + asls, m0s, tsvs, niis = _categorize(perf) + if not asls: + continue + prefixes = sorted(asls) + m0_for = _pair_sidecar(prefixes, m0s, "m0scan", perf) + tsv_for = _pair_sidecar(prefixes, tsvs, "aslcontext.tsv", perf) + for pre in prefixes: + acquisitions.append((asls[pre], m0_for[pre], tsv_for[pre], niis.get(pre))) + if not acquisitions: + raise ExampleStructureError( + f"{case}: no *_asl.json found in any perf/ directory" + ) + return acquisitions + + +def build_inputs(case: Path) -> tuple[list[str], str]: + """Build the ordered (files, nifti_file) for a case's generate_report call. + + Files are emitted per acquisition as asl -> m0 -> tsv (load-bearing order). A + single representative NIfTI is used (the tool records one slice count): the + first *_asl.nii* in sorted order. Acquisitions may differ in geometry, so + differing slice counts are allowed, not an error. + + Raises: + ExampleStructureError: If the case has no ASL input or no ASL NIfTI. + """ + acquisitions = _collect_acquisitions(case) + + files: list[str] = [] + for asl_path, m0_path, tsv_path, _ in acquisitions: + files.append(str(asl_path)) + if m0_path is not None: + files.append(str(m0_path)) + if tsv_path is not None: + files.append(str(tsv_path)) + + niftis = sorted( + p + for p in case.rglob("*asl.nii*") + if "m0scan" not in p.name and not any(part in _SKIP_DIRS for part in p.parts) + ) + if not niftis: + raise ExampleStructureError( + f"{case}: no *_asl.nii[.gz] found in any perf/ directory" + ) + return files, str(niftis[0]) + + +def discover_examples(base: Path) -> list[Path]: + """Return case dirs under `base` (immediate children with a perf/*_asl.json). + + Keys on inputs, not on expected_output.json, so --update-expected can + bootstrap a brand-new case. Missing/empty base yields []. + """ + if not base.is_dir(): + return [] + cases = [] + for child in base.iterdir(): + if not child.is_dir(): + continue + if any( + p.name.endswith(_ASL_JSON) + for perf in _perf_dirs(child) + for p in perf.iterdir() + ): + cases.append(child) + return sorted(cases) + + +def run_example(case: Path) -> dict[str, Any]: + """Run the BIDS report path on one example case directory.""" + files, nifti = build_inputs(case) + return generate_report( + { + "modality": ModalityTypeValues.ASL, + "files": files, + "nifti_file": nifti, + "dcm_files": [], + } + ) + + +def normalize(obj: Any) -> Any: + """Round-trip through JSON so tuples (e.g. m0_parameters) compare as lists.""" + return json.loads(json.dumps(obj, default=str)) + + +def load_expected(case: Path) -> dict[str, Any]: + """Load and JSON-normalize a case's expected_output.json.""" + return normalize(json.loads((case / EXPECTED_FILENAME).read_text())) + + +def write_expected(case: Path, report: dict[str, Any]) -> None: + """Overwrite a case's expected_output.json with `report` (golden update).""" + (case / EXPECTED_FILENAME).write_text( + json.dumps(report, indent=2, default=str) + "\n" + ) diff --git a/package/tests/test_integration.py b/package/tests/test_integration.py new file mode 100644 index 000000000..79a4fa152 --- /dev/null +++ b/package/tests/test_integration.py @@ -0,0 +1,263 @@ +"""Example-based integration runner for the BIDS report path. + +Two groups of tests live here: + +1. ``test_example_matches_expected`` — parametrized over committed example + folders (discovered in conftest). It is a golden / characterization test: + the FULL 21-key output, including generated report text, must equal the + committed expected_output.json. With ``--update-expected`` it rewrites the + expected file instead of asserting. Until real examples are committed it + collects zero cases and skips. + +2. ``TestRunnerHarness`` — self-tests that synthesize a throwaway example in + tmp_path and exercise discovery, input ordering, comparison, and the + golden-update path. They need no committed fixtures, are NOT marked + integration, and run in the unit job so the machinery is verified on every + push regardless of whether any real example exists yet. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import nibabel as nib +import numpy as np +import pytest + +from tests.integration.runner import ( + ExampleStructureError, + PairingFallbackWarning, + build_inputs, + discover_examples, + load_expected, + normalize, + run_example, + write_expected, +) + + +# --------------------------------------------------------------------------- +# Golden test over committed examples +# --------------------------------------------------------------------------- +@pytest.mark.integration +def test_example_matches_expected( + integration_case: Path, request: pytest.FixtureRequest +) -> None: + """Full output (text included) equals expected_output.json, key by key. + + Args: + integration_case: A discovered example directory. + request: Used to read the --update-expected flag. + """ + report = run_example(integration_case) + + if request.config.getoption("--update-expected"): + write_expected(integration_case, report) + pytest.skip(f"updated expected_output.json for {integration_case.name}") + + if not (integration_case / "expected_output.json").exists(): + pytest.fail( + f"{integration_case.name}: no expected_output.json. " + f"Generate it with: pytest -m integration --update-expected" + ) + + expected = load_expected(integration_case) + actual = normalize(report) + assert set(actual.keys()) == set( + expected.keys() + ), f"{integration_case.name}: key set drift" + for key in sorted(expected.keys()): + assert actual[key] == expected[key], f"{integration_case.name}: '{key}' differs" + + +# --------------------------------------------------------------------------- +# Harness self-tests (no committed fixtures required) +# --------------------------------------------------------------------------- +def _make_example(root: Path, *, slices: int = 18, with_m0: bool = True) -> Path: + """Build a minimal single-acquisition BIDS example under root/case/sub-X/perf. + + Returns: + The case directory (root/case). + """ + case = root / "case" + _write_acq(case / "sub-X" / "perf", "sub-X", slices=slices, with_m0=with_m0) + return case + + +def _write_acq( + perf: Path, prefix: str, *, slices: int = 18, with_m0: bool = True +) -> None: + """Write one acquisition's asl/m0/tsv/nii into a perf dir under a prefix.""" + perf.mkdir(parents=True, exist_ok=True) + asl = { + "ArterialSpinLabelingType": "PCASL", + "MRAcquisitionType": "3D", + "PulseSequenceType": "GRASE", + "M0Type": "Separate", + "BackgroundSuppression": False, + "EchoTime": 0.012, + "RepetitionTimePreparation": 4.0, + "FlipAngle": 90, + "MagneticFieldStrength": 3, + "Manufacturer": "Siemens", + "PostLabelingDelay": 1.8, + "LabelingDuration": 1.8, + } + (perf / f"{prefix}_asl.json").write_text(json.dumps(asl)) + if with_m0: + (perf / f"{prefix}_m0scan.json").write_text( + json.dumps( + { + "EchoTime": 0.012, + "RepetitionTimePreparation": 4.0, + "M0Type": "Separate", + } + ) + ) + (perf / f"{prefix}_aslcontext.tsv").write_text( + "volume_type\n" + "\n".join(["label", "control"] * 3) + "\n" + ) + nib.save( + nib.Nifti1Image(np.zeros((4, 4, slices, 2), dtype=np.float32), np.eye(4)), + str(perf / f"{prefix}_asl.nii.gz"), + ) + + +class TestRunnerHarness: + """Verify discovery, ordering, comparison, and golden-update end to end.""" + + def test_discovery_keys_on_inputs_not_golden(self, tmp_path: Path) -> None: + """A folder is a case as soon as it has inputs, before any golden exists. + + This is what lets --update-expected bootstrap a brand-new case. + """ + case = _make_example(tmp_path) + assert discover_examples(tmp_path) == [case] # found with no expected yet + assert not (case / "expected_output.json").exists() + write_expected(case, run_example(case)) + assert discover_examples(tmp_path) == [case] # still found after golden + + def test_discovery_handles_missing_dir(self, tmp_path: Path) -> None: + """A non-existent base yields no cases (drives the empty-skip path).""" + assert discover_examples(tmp_path / "nope") == [] + + def test_build_inputs_orders_asl_before_m0(self, tmp_path: Path) -> None: + """asl.json precedes m0scan.json precedes tsv (load-bearing order).""" + case = _make_example(tmp_path) + files, nifti = build_inputs(case) + names = [Path(f).name for f in files] + assert names == ["sub-X_asl.json", "sub-X_m0scan.json", "sub-X_aslcontext.tsv"] + assert nifti.endswith("sub-X_asl.nii.gz") + + def test_multi_run_pairs_per_acquisition(self, tmp_path: Path) -> None: + """Two runs in one perf dir interleave asl->m0->tsv per run, not by type.""" + case = tmp_path / "case" + perf = case / "sub-X" / "perf" + _write_acq(perf, "sub-X_run-1") + _write_acq(perf, "sub-X_run-2") + files, _ = build_inputs(case) + names = [Path(f).name for f in files] + assert names == [ + "sub-X_run-1_asl.json", + "sub-X_run-1_m0scan.json", + "sub-X_run-1_aslcontext.tsv", + "sub-X_run-2_asl.json", + "sub-X_run-2_m0scan.json", + "sub-X_run-2_aslcontext.tsv", + ] + + def test_multi_session_and_subject_supported(self, tmp_path: Path) -> None: + """Sessions and subjects each contribute their perf acquisition, in order.""" + case = tmp_path / "case" + _write_acq(case / "sub-A" / "ses-01" / "perf", "sub-A_ses-01") + _write_acq(case / "sub-A" / "ses-02" / "perf", "sub-A_ses-02") + _write_acq(case / "sub-B" / "perf", "sub-B") + files, _ = build_inputs(case) + asls = [Path(f).name for f in files if f.endswith("_asl.json")] + assert asls == [ + "sub-A_ses-01_asl.json", + "sub-A_ses-02_asl.json", + "sub-B_asl.json", + ] + + def test_more_asl_than_m0_reuses_shared(self, tmp_path: Path) -> None: + """Two runs, one shared M0 with no run prefix: both reuse it (+ warning).""" + case = tmp_path / "case" + perf = case / "sub-X" / "perf" + _write_acq(perf, "sub-X_run-1") + _write_acq(perf, "sub-X_run-2") + (perf / "sub-X_run-1_m0scan.json").unlink() + (perf / "sub-X_run-2_m0scan.json").unlink() + (perf / "sub-X_m0scan.json").write_text( + json.dumps( + { + "EchoTime": 0.012, + "RepetitionTimePreparation": 4.0, + "M0Type": "Separate", + } + ) + ) # shared M0, no run- entity + with pytest.warns(PairingFallbackWarning): + files, _ = build_inputs(case) + m0s = [Path(f).name for f in files if f.endswith("_m0scan.json")] + assert m0s == ["sub-X_m0scan.json", "sub-X_m0scan.json"] # reused for both + assert run_example(case)["nifti_slice_number"] == 18 # still runs + + def test_more_m0_than_asl_uses_latest(self, tmp_path: Path) -> None: + """One ASL, two non-matching M0s: the latest (by time, then name) is used.""" + case = tmp_path / "case" + perf = case / "sub-X" / "perf" + _write_acq(perf, "sub-X") + (perf / "sub-X_m0scan.json").unlink() # drop the prefix-matching M0 + (perf / "aaa_m0scan.json").write_text(json.dumps({"M0Type": "Separate"})) + (perf / "zzz_m0scan.json").write_text(json.dumps({"M0Type": "Separate"})) + with pytest.warns(PairingFallbackWarning): + files, _ = build_inputs(case) + m0s = [Path(f).name for f in files if f.endswith("_m0scan.json")] + # equal mtimes in tmp -> filename tie-break -> 'zzz' wins + assert m0s == ["zzz_m0scan.json"] + + def test_differing_slice_counts_use_first_deterministically( + self, tmp_path: Path + ) -> None: + """Acquisitions may differ in geometry (e.g. All_five); first is representative.""" + case = tmp_path / "case" + _write_acq(case / "sub-A" / "perf", "sub-A", slices=18) + _write_acq(case / "sub-B" / "perf", "sub-B", slices=32) + _, nifti = build_inputs(case) + assert nifti.endswith("sub-A_asl.nii.gz") # sub-A sorts first + assert run_example(case)["nifti_slice_number"] == 18 + + def test_missing_nifti_fails_loudly(self, tmp_path: Path) -> None: + """No ASL NIfTI anywhere is a hard, clear failure.""" + case = _make_example(tmp_path) + for n in (case).rglob("*asl.nii.gz"): + n.unlink() + with pytest.raises(ExampleStructureError, match="no .*asl.nii"): + build_inputs(case) + + def test_run_returns_21_key_contract(self, tmp_path: Path) -> None: + """The tool returns exactly 21 keys for a valid example.""" + case = _make_example(tmp_path) + report = run_example(case) + assert len(report) == 21 + + def test_roundtrip_matches_and_drift_fails(self, tmp_path: Path) -> None: + """A freshly-written golden matches; a corrupted one is detected.""" + case = _make_example(tmp_path) + write_expected(case, run_example(case)) + expected = load_expected(case) + actual = normalize(run_example(case)) + assert actual == expected # golden round-trips + + corrupted = dict(expected) + corrupted["nifti_slice_number"] = expected["nifti_slice_number"] + 1 + write_expected(case, corrupted) + assert normalize(run_example(case)) != load_expected(case) # drift caught + + def test_slice_count_tracks_nifti(self, tmp_path: Path) -> None: + """nifti_slice_number reflects the synthesized slice axis.""" + case = _make_example(tmp_path, slices=27) + assert run_example(case)["nifti_slice_number"] == 27 From 48df7c460910874ebd5289ccc637977876ac3d68 Mon Sep 17 00:00:00 2001 From: Vitor Lima dos Santos Date: Mon, 13 Jul 2026 09:24:15 -0300 Subject: [PATCH 16/16] ci: add integration job to tests workflow --- .github/workflows/tests.yml | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c788940e7..91b06fcdc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -50,4 +50,34 @@ jobs: # (no integration tests yet) - name: Run unit tests working-directory: ./package - run: pytest -v -m "not integration" \ No newline at end of file + run: pytest -v -m "not integration" + + integration-tests: + name: Integration tests (Python ${{ matrix.python-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.12"] + + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: "pip" + cache-dependency-path: package/pyproject.toml + + - name: Install package with test dependencies + working-directory: ./package + run: | + python -m pip install --upgrade pip + pip install -e ".[test]" + + # Runs only @pytest.mark.integration tests against the committed example set + # (tests/integration/examples). With no examples yet this is a clean skip. + - name: Run integration tests + working-directory: ./package + run: pytest -v -m integration \ No newline at end of file