diff --git a/.github/workflows/ci-dev.yml b/.github/workflows/ci-dev.yml new file mode 100644 index 00000000..4ae02682 --- /dev/null +++ b/.github/workflows/ci-dev.yml @@ -0,0 +1,35 @@ +name: CI (develop) + +# Lightweight PR-time test suite that actually gates merges into develop. +# Runs the unit tests under tests/ inside the published shapepipe container +# — no conda bootstrap, no multi-OS matrix. Complements ci-release.yml +# (which is a heavier conda-based suite gating main/master). + +on: + pull_request: + branches: + - develop + push: + branches: + - develop + +jobs: + unit: + name: Unit tests + runs-on: ubuntu-latest + container: + image: ghcr.io/cosmostat/shapepipe:develop + options: --user root + + steps: + - uses: actions/checkout@v4 + + - name: Reinstall shapepipe from the checkout + # The container ships a snapshot of the repo under /app. Replace it + # with the PR's checkout so tests run against the proposed code. + run: | + pip install --no-deps --force-reinstall -e . + + - name: Run unit tests + run: | + pytest tests/unit/ -v --no-cov -p no:warnings diff --git a/.gitignore b/.gitignore index b08fc3c5..f40a4734 100644 --- a/.gitignore +++ b/.gitignore @@ -134,3 +134,4 @@ dmypy.json *shapepipe_run_* *shapepipe_runs* code +CLAUDE.md diff --git a/pyproject.toml b/pyproject.toml index 7e4e938a..2bbd3393 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,4 +55,4 @@ canfar_monitor_log = "shapepipe.canfar_run:run_monitor_log" [tool.pytest.ini_options] addopts = "--verbose --cov=shapepipe" -testpaths = ["shapepipe"] +testpaths = ["tests", "src/shapepipe/tests"] diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..638681fb --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,15 @@ +"""Shared pytest fixtures for the repository-level test suite.""" + +from pathlib import Path + +import pytest + + +@pytest.fixture(scope="session") +def repo_root() -> Path: + """Absolute path to the repository root. + + Tests live at ``/tests/``, so the root is two parents up + from this conftest file. + """ + return Path(__file__).resolve().parent.parent diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/test_config_parse.py b/tests/unit/test_config_parse.py new file mode 100644 index 00000000..76b7fe68 --- /dev/null +++ b/tests/unit/test_config_parse.py @@ -0,0 +1,34 @@ +"""Every example INI config must parse as a valid configparser file. + +Uses ``RawConfigParser`` so ``$SP_RUN``-style variable references don't +trigger interpolation errors. This catches malformed section headers, +duplicate keys within a section, and missing ``=``/``:`` separators. +""" + +import configparser +from pathlib import Path + +import pytest + + +def _config_files(root: Path) -> list[Path]: + return sorted((root / "example").rglob("*.ini")) + + +def pytest_generate_tests(metafunc): + if "config_path" in metafunc.fixturenames: + root = Path(__file__).resolve().parents[2] + configs = _config_files(root) + metafunc.parametrize( + "config_path", + configs, + ids=[str(p.relative_to(root)) for p in configs], + ) + + +def test_config_parses(config_path: Path) -> None: + parser = configparser.RawConfigParser(strict=True) + parser.read(config_path, encoding="utf-8") + assert parser.sections(), ( + f"{config_path} parsed but has no sections — likely malformed" + ) diff --git a/tests/unit/test_entrypoints.py b/tests/unit/test_entrypoints.py new file mode 100644 index 00000000..e201819f --- /dev/null +++ b/tests/unit/test_entrypoints.py @@ -0,0 +1,77 @@ +"""Every ``[project.scripts]`` entry must resolve and handle ``-h`` cleanly. + +The test skips entries whose command isn't on PATH — that's an install +concern, not a code concern. When a command *is* installed, ``-h`` +must exit 0 or 2 (argparse's help exit code) with non-empty output. +This catches entry points whose target function treats ``-h`` as a +positional argument, or whose argparser is initialised so late the +help flag never runs. +""" + +import shutil +import subprocess +import tomllib +from pathlib import Path + +import pytest + +# Entry points known to mishandle ``-h`` — tracked via ``xfail`` so the +# suite stays green and the issue is discoverable once fixed. +KNOWN_XFAIL = { + "summary_run": + "treats '-h' as the 'patch' positional arg, tries to mkdir '-h'", + # All three import shapepipe.canfar_run, which fails with + # IndentationError in canfar_monitor.py:55. + "canfar_submit_job": + "IndentationError in canfar_monitor.py:55 (transitive)", + "canfar_monitor": + "IndentationError in canfar_monitor.py:55 (transitive)", + "canfar_monitor_log": + "IndentationError in canfar_monitor.py:55 (transitive)", +} + + +def _scripts_from_pyproject() -> dict[str, str]: + root = Path(__file__).resolve().parents[2] + pyproject = tomllib.loads((root / "pyproject.toml").read_text()) + return pyproject.get("project", {}).get("scripts", {}) + + +def _params(): + for name in sorted(_scripts_from_pyproject().keys()): + if name in KNOWN_XFAIL: + yield pytest.param( + name, + marks=pytest.mark.xfail( + reason=KNOWN_XFAIL[name], strict=True, raises=Exception + ), + ) + else: + yield name + + +def pytest_generate_tests(metafunc): + if "script_name" in metafunc.fixturenames: + metafunc.parametrize("script_name", list(_params())) + + +def test_entrypoint_help(script_name: str) -> None: + if shutil.which(script_name) is None: + pytest.skip( + f"{script_name} not on PATH — not installed in this env" + ) + result = subprocess.run( + [script_name, "-h"], + capture_output=True, + text=True, + timeout=30, + ) + # argparse help exits 0; some parsers use 2. Either is fine so + # long as *something* was printed, meaning the help path ran. + assert result.returncode in (0, 2), ( + f"{script_name} -h exited {result.returncode}\n" + f"stdout: {result.stdout}\nstderr: {result.stderr}" + ) + assert result.stdout or result.stderr, ( + f"{script_name} -h produced no output" + ) diff --git a/tests/unit/test_imports.py b/tests/unit/test_imports.py new file mode 100644 index 00000000..80f9f9d0 --- /dev/null +++ b/tests/unit/test_imports.py @@ -0,0 +1,88 @@ +"""Every public ``shapepipe.*`` submodule must import cleanly. + +Catches module-level syntax errors, typos in import paths, circular +imports, and (crucially) the pattern where a module unconditionally +imports an optional dependency — a ``try/except ImportError`` guard +placed *after* a failing top-level import is unreachable. + +Assumes the shapepipe package and its declared dependencies are +installed (the official container, or ``pip install -e .``). +""" + +import importlib +import pkgutil + +import pytest + +import shapepipe + +# Modules with known-broken transitive imports — tracked so the suite +# lands green but the failure is discoverable and auto-notifies (via +# ``strict=True``) once the upstream fix lands. +KNOWN_XFAIL = { + # IndentationError at canfar_monitor.py:55 (docstring/body mismatch). + "shapepipe.canfar.canfar_monitor": + "IndentationError in canfar_monitor.py:55", + "shapepipe.canfar_run": + "IndentationError in canfar_monitor.py:55 (transitive)", + # stile v0.1 hard-imports treecorr.corr2, which newer treecorr removed. + "shapepipe.modules.mccd_package.mccd_plot_utilities": + "stile v0.1 imports removed treecorr.corr2", + "shapepipe.modules.mccd_plots_runner": + "stile v0.1 imports removed treecorr.corr2", + "shapepipe.modules.random_cat_package.random_cat": + "stile v0.1 imports removed treecorr.corr2", + "shapepipe.modules.random_cat_runner": + "stile v0.1 imports removed treecorr.corr2", + # Modules added in v1.x Dockerfile rewrites that import deps not in + # the develop image (astroquery, numba, fitsio) or removed from + # stdlib/setuptools (pkg_resources). + "shapepipe.modules.mask_package.mask": + "astroquery not in develop Docker image", + "shapepipe.modules.mask_runner": + "astroquery not in develop Docker image (transitive)", + "shapepipe.modules.ngmix_package.ngmix": + "numba not in develop Docker image", + "shapepipe.modules.ngmix_runner": + "numba not in develop Docker image (transitive)", + "shapepipe.modules.split_exp_package.split_exp": + "uses deprecated pkg_resources (setuptools>=68)", + "shapepipe.modules.split_exp_runner": + "uses deprecated pkg_resources (transitive)", + "shapepipe.modules.uncompress_fits_package.uncompress_fits": + "fitsio not installed (declared as optional extra)", + "shapepipe.modules.uncompress_fits_runner": + "fitsio not installed (transitive)", +} + + +def _iter_shapepipe_modules() -> list[str]: + return sorted( + m.name + for m in pkgutil.walk_packages( + shapepipe.__path__, prefix="shapepipe." + ) + if "tests" not in m.name.split(".") + ) + + +def _params(): + for name in _iter_shapepipe_modules(): + if name in KNOWN_XFAIL: + yield pytest.param( + name, + marks=pytest.mark.xfail( + reason=KNOWN_XFAIL[name], strict=True, raises=Exception + ), + ) + else: + yield name + + +def pytest_generate_tests(metafunc): + if "module_name" in metafunc.fixturenames: + metafunc.parametrize("module_name", list(_params())) + + +def test_module_imports(module_name: str) -> None: + importlib.import_module(module_name) diff --git a/tests/unit/test_runner_metadata.py b/tests/unit/test_runner_metadata.py new file mode 100644 index 00000000..8e9ae368 --- /dev/null +++ b/tests/unit/test_runner_metadata.py @@ -0,0 +1,93 @@ +"""Every ``*_runner.py`` module must export a function decorated with +``@module_runner``. + +The decorator attaches introspection metadata the pipeline needs +(``version``, ``input_module``, ``file_pattern``, ``file_ext``, +``depends``, ``executes``, ``numbering_scheme``, ``run_method``). A new +runner that forgets the decorator loads silently but fails at dispatch +time. This test catches the oversight at import time. + +We locate the runner by looking for any top-level function that carries +the metadata, since the function name doesn't always match the file +name (e.g. ``pastecat_runner.py`` exports ``paste_cat_runner``). +""" + +import importlib +import inspect +import pkgutil + +import pytest + +import shapepipe.modules + +REQUIRED_ATTRS = ( + "version", + "input_module", + "file_pattern", + "file_ext", + "depends", + "executes", + "numbering_scheme", + "run_method", +) + +# Modules imported here but broken upstream — tracked separately. +# Mapping: module name → xfail reason. +KNOWN_XFAIL = { + # Transitive failures: the runner's own import chain breaks before + # we can check metadata. Root causes tracked in test_imports.py. + "shapepipe.modules.mccd_plots_runner": + "stile v0.1 imports removed treecorr.corr2", + "shapepipe.modules.random_cat_runner": + "stile v0.1 imports removed treecorr.corr2", + "shapepipe.modules.mask_runner": + "astroquery not in develop Docker image", + "shapepipe.modules.ngmix_runner": + "numba not in develop Docker image", + "shapepipe.modules.split_exp_runner": + "uses deprecated pkg_resources (setuptools>=68)", + "shapepipe.modules.uncompress_fits_runner": + "fitsio not installed (declared as optional extra)", +} + + +def _runner_modules() -> list[str]: + return sorted( + m.name + for m in pkgutil.iter_modules( + shapepipe.modules.__path__, prefix="shapepipe.modules." + ) + if m.name.endswith("_runner") + ) + + +def _params(): + for name in _runner_modules(): + if name in KNOWN_XFAIL: + yield pytest.param( + name, + marks=pytest.mark.xfail( + reason=KNOWN_XFAIL[name], strict=True, raises=Exception + ), + ) + else: + yield name + + +def pytest_generate_tests(metafunc): + if "runner_module" in metafunc.fixturenames: + metafunc.parametrize("runner_module", list(_params())) + + +def test_runner_has_metadata(runner_module: str) -> None: + mod = importlib.import_module(runner_module) + decorated = [ + obj + for _, obj in inspect.getmembers(mod, inspect.isfunction) + if obj.__module__ == runner_module + and all(hasattr(obj, a) for a in REQUIRED_ATTRS) + ] + assert decorated, ( + f"{runner_module} exports no function decorated with @module_runner " + f"(required attrs: {REQUIRED_ATTRS})" + ) diff --git a/tests/unit/test_shell_syntax.py b/tests/unit/test_shell_syntax.py new file mode 100644 index 00000000..cb68714f --- /dev/null +++ b/tests/unit/test_shell_syntax.py @@ -0,0 +1,44 @@ +"""Every shell script under scripts/ must parse under ``bash -n``. + +Catches unclosed quotes, unbalanced brackets, stray parentheses, and the +whole family of errors that only surface the moment someone runs the +script. Parametrized one-test-per-file so CI output names the offender. +""" + +import subprocess +from pathlib import Path + +import pytest + + +def _shell_scripts(root: Path) -> list[Path]: + """Return every ``.sh``/``.bash`` under ``scripts/``.""" + scripts_dir = root / "scripts" + return sorted( + p for p in scripts_dir.rglob("*") + if p.is_file() and p.suffix in {".sh", ".bash"} + ) + + +def pytest_generate_tests(metafunc): + """Parametrize per-script at collection time so we can name each file.""" + if "script_path" in metafunc.fixturenames: + root = Path(__file__).resolve().parents[2] + scripts = _shell_scripts(root) + metafunc.parametrize( + "script_path", + scripts, + ids=[str(p.relative_to(root)) for p in scripts], + ) + + +def test_shell_script_parses(script_path: Path) -> None: + """bash -n must succeed.""" + result = subprocess.run( + ["bash", "-n", str(script_path)], + capture_output=True, + text=True, + ) + assert result.returncode == 0, ( + f"bash -n failed on {script_path}:\n{result.stderr}" + )