From d9b2d8e4591684d8beda3dd46a9b1dad17961d43 Mon Sep 17 00:00:00 2001
From: Robert Rosca <robert.rosca@xfel.eu>
Date: Sat, 24 Apr 2021 10:15:15 +0200
Subject: [PATCH] Add todo note for using `feature_version` kwarg
Addresses https://git.xfel.eu/gitlab/detectors/pycalibration/merge_requests/464#note_228864
---
.gitignore | 1 +
.gitlab-ci.yml | 9 +-
notebooks/test/test-cli.ipynb | 70 ++++
pyproject.toml | 14 +-
setup.py | 3 +-
src/xfel_calibrate/calibrate.py | 35 +-
src/xfel_calibrate/notebooks.py | 10 +
src/xfel_calibrate/settings.py | 2 +-
tests/conftest.py | 25 ++
tests/test_cal_tools.py | 2 +
tests/test_calibrate.py | 35 --
tests/test_webservice.py | 1 +
tests/test_xfel_calibrate/__init__.py | 0
tests/test_xfel_calibrate/conftest.py | 240 ++++++++++++
tests/test_xfel_calibrate/test_calibrate.py | 52 +++
tests/test_xfel_calibrate/test_cli.py | 408 ++++++++++++++++++++
16 files changed, 855 insertions(+), 52 deletions(-)
create mode 100644 notebooks/test/test-cli.ipynb
create mode 100644 tests/conftest.py
delete mode 100644 tests/test_calibrate.py
create mode 100644 tests/test_xfel_calibrate/__init__.py
create mode 100644 tests/test_xfel_calibrate/conftest.py
create mode 100644 tests/test_xfel_calibrate/test_calibrate.py
create mode 100644 tests/test_xfel_calibrate/test_cli.py
diff --git a/.gitignore b/.gitignore
index 3ea4aabcd..478939103 100644
--- a/.gitignore
+++ b/.gitignore
@@ -21,6 +21,7 @@
LPD/results
Test
build
+coverage.*
docs/build
docs/source/_notebooks
docs/source/_static/reports
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2b56d9b29..0b4409016 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -43,7 +43,12 @@ pytest:
<<: *before_script
script:
- python3 -m pip install ".[test]"
- - python3 -m pytest --verbose --cov=cal_tools --cov=xfel_calibrate
+ - python3 -m pytest --color yes --verbose --cov=cal_tools --cov=xfel_calibrate
+# Nope... https://docs.gitlab.com/12.10/ee/user/project/merge_requests/test_coverage_visualization.html#enabling-the-feature
+# - coverage xml
+# artifacts:
+# reports:
+# cobertura: coverage.xml
cython-editable-install-test:
stage: test
@@ -51,4 +56,4 @@ cython-editable-install-test:
<<: *before_script
script:
- python3 -m pip install -e ".[test]"
- - python3 -m pytest ./tests/test_agipdalgs.py
+ - python3 -m pytest --color yes --verbose ./tests/test_agipdalgs.py
diff --git a/notebooks/test/test-cli.ipynb b/notebooks/test/test-cli.ipynb
new file mode 100644
index 000000000..ef866040a
--- /dev/null
+++ b/notebooks/test/test-cli.ipynb
@@ -0,0 +1,70 @@
+{
+ "cells": [
+ {
+ "cell_type": "markdown",
+ "metadata": {},
+ "source": [
+ "# Test Notebook - CLI\n",
+ "\n",
+ "Author: Robert Rosca\n",
+ "\n",
+ "Version: 0.1\n",
+ "\n",
+ "Notebook for use with the unit and continuous integration tests."
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "in_folder = \"/dev/null\" # input folder\n",
+ "out_folder = \"/dev/null\" # output folder\n",
+ "list_normal = [10] # parameterized list, range allowed\n",
+ "list_intellilist = [2345] # parameterized list with ranges, range allowed\n",
+ "concurrency_parameter = [1] # concurrency parameter, range allowed\n",
+ "number = 0 # parameterized number"
+ ]
+ },
+ {
+ "cell_type": "markdown",
+ "metadata": {},
+ "source": [
+ "Tests notebook execution by just creating an empty file in the output directory."
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "from pathlib import Path"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "in_folder = Path(in_folder)\n",
+ "\n",
+ "(in_folder / \"touch\").touch()"
+ ]
+ }
+ ],
+ "metadata": {
+ "kernelspec": {
+ "display_name": "Python 3.6.8 64-bit",
+ "name": "python3"
+ },
+ "language_info": {
+ "name": "python",
+ "version": ""
+ }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 0
+}
diff --git a/pyproject.toml b/pyproject.toml
index 070d67401..bddced5c9 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -4,6 +4,12 @@ requires = ["cython==0.29.21", "numpy==1.19.1", "setuptools>=40.8.0", "wheel"]
[tool.isort]
profile = "black"
+[tool.pylint.messages_control]
+disable = "C0330, C0326"
+
+[tool.pylint.format]
+max-line-length = "88"
+
[tool.pytest.ini_options]
norecursedirs = [
"legacy",
@@ -15,5 +21,11 @@ norecursedirs = [
"dist",
"node_modules",
"venv",
- "{arch}"
+ "{arch}",
+]
+
+required_plugins = [
+ "pytest-asyncio",
+ "pytest-cov",
+ "pytest-subprocess",
]
diff --git a/setup.py b/setup.py
index 6a66076cb..6acd8a39c 100644
--- a/setup.py
+++ b/setup.py
@@ -81,7 +81,7 @@ setup(
"astcheck==0.2.5",
"astsearch==0.1.3",
"dill==0.3.0",
- "extra_data==1.2.0",
+ "extra_data==1.4.1",
"extra_geom==1.1.1",
"fabio==0.9.0",
"gitpython==3.1.0",
@@ -125,6 +125,7 @@ setup(
"nbval",
"pytest-asyncio",
"pytest-cov",
+ "pytest-subprocess",
"pytest>=5.4.0",
"testpath",
"unittest-xml-reporting==3.0.2",
diff --git a/src/xfel_calibrate/calibrate.py b/src/xfel_calibrate/calibrate.py
index e0e43e1f7..000dddea5 100755
--- a/src/xfel_calibrate/calibrate.py
+++ b/src/xfel_calibrate/calibrate.py
@@ -15,16 +15,28 @@ from pathlib import Path
from subprocess import DEVNULL, check_output
from typing import List, Union
-import cal_tools.tools
-import nbconvert
import nbformat
import numpy as np
from jinja2 import Template
from nbparameterise import extract_parameters, parameter_values, replace_definitions
+import cal_tools.tools
+
from .finalize import tex_escape
from .notebooks import notebooks
-from .settings import *
+from .settings import (
+ default_report_path,
+ free_nodes_cmd,
+ launcher_command,
+ max_reserved,
+ preempt_nodes_cmd,
+ python_path,
+ reservation,
+ reservation_char,
+ sprof,
+ temp_path,
+ try_report_to_output,
+)
PKG_DIR = os.path.dirname(os.path.abspath(__file__))
@@ -115,7 +127,6 @@ def make_intelli_list(ltype):
super(IntelliListAction, self).__init__(*args, **kwargs)
def __call__(self, parser, namespace, values, option_string=None):
-
parsed_values = []
values = ",".join(values)
if isinstance(values, str):
@@ -171,7 +182,7 @@ def extract_title_author_version(nb):
# In case of a standard installation a version is stored
# in the _version.py file
try:
- git_dir = os.path.join(PKG_DIR, '..', '.git')
+ git_dir = os.path.join(PKG_DIR, '..', '..', '.git')
version = check_output([
'git', f'--git-dir={git_dir}', 'describe', '--tag',
], stderr=DEVNULL).decode('utf8')
@@ -284,8 +295,7 @@ def balance_sequences(in_folder: str, run: int, sequences: List[int],
if isinstance(karabo_da, str):
karabo_da = [karabo_da]
elif not isinstance(karabo_da, list):
- raise ValueError("Balance sequences expects "
- "karabo_da as a string or list.")
+ raise TypeError("Balance sequences expects `karabo_da` as a string or list.")
in_path = Path(in_folder, f"r{run:04d}")
@@ -305,8 +315,10 @@ def balance_sequences(in_folder: str, run: int, sequences: List[int],
if sequences != [-1]:
seq_nums = sorted(seq_nums.intersection(sequences))
if len(seq_nums) == 0:
- raise ValueError(f"Selected sequences {sequences} are not "
- f"available in {in_path}")
+ raise ValueError(
+ f"Selected sequences {sequences} are not "
+ f"available in {in_path}"
+ )
# Validate required nodes with max_nodes
nsplits = len(seq_nums) // sequences_per_node
@@ -332,6 +344,7 @@ def make_extended_parser() -> argparse.ArgumentParser:
try:
det_notebooks = notebooks[detector]
except KeyError:
+ # TODO: This should really go to stderr not stdout
print("Not one of the known detectors: {}".format(notebooks.keys()))
sys.exit(1)
@@ -682,9 +695,7 @@ def concurrent_run(temp_path, nb, nbname, args, cparm=None, cval=None,
os.path.basename(base_name), cparm, suffix)
nbpath = os.path.join(temp_path, new_name)
- with open(nbpath, "w") as f:
- f.write(nbconvert.exporters.export(
- nbconvert.NotebookExporter, new_nb)[0])
+ nbformat.write(new_nb, nbpath)
# add finalization to the last job
if final_job:
diff --git a/src/xfel_calibrate/notebooks.py b/src/xfel_calibrate/notebooks.py
index c584b6657..1b4e4cd96 100644
--- a/src/xfel_calibrate/notebooks.py
+++ b/src/xfel_calibrate/notebooks.py
@@ -233,4 +233,14 @@ notebooks = {
"cluster cores": 16},
},
},
+ "TEST": {
+ "TEST-CLI": {
+ "notebook": "notebooks/test/test-cli.ipynb",
+ "concurrency": {
+ "parameter": "concurrency_parameter",
+ "default concurrency": None,
+ "cluster cores": 1,
+ },
+ },
+ },
}
diff --git a/src/xfel_calibrate/settings.py b/src/xfel_calibrate/settings.py
index 709fdd5ec..43e79c9a1 100644
--- a/src/xfel_calibrate/settings.py
+++ b/src/xfel_calibrate/settings.py
@@ -17,7 +17,7 @@ try_report_to_output = True
logo_path = "xfel.pdf"
# the command to run this concurrently. It is prepended to the actual call
-sprof = os.environ.get("XFELCALSLURM", "exfel")
+sprof = os.environ.get("XFELCALSLURM", "exfel") # TODO: https://git.xfel.eu/gitlab/calibration/planning/issues/3
launcher_command = "sbatch -t 24:00:00 --requeue --output {temp_path}/slurm-%j.out"
free_nodes_cmd = "sinfo -p exfel -t idle -N --noheader | wc -l"
preempt_nodes_cmd = "squeue -p all,grid --noheader | grep max-exfl | egrep -v 'max-exfl18[3-8]|max-exfl100|max-exflg' | wc -l"
diff --git a/tests/conftest.py b/tests/conftest.py
new file mode 100644
index 000000000..d0e461a4c
--- /dev/null
+++ b/tests/conftest.py
@@ -0,0 +1,25 @@
+from pathlib import Path
+
+import pytest
+
+
+def pytest_addoption(parser):
+ parser.addoption(
+ "--no-gpfs",
+ action="store_true",
+ default="false",
+ help="Skips tests marked as requiring GPFS access",
+ )
+
+
+def pytest_configure(config):
+ config.addinivalue_line(
+ "markers", "requires_gpfs(): marks skips for tests that require GPFS access"
+ )
+
+
+def pytest_runtest_setup(item):
+ if list(item.iter_markers(name="requires_gpfs")) and (
+ not Path("/gpfs").is_dir() or item.config.getoption("--no-gpfs")
+ ):
+ pytest.skip("gpfs not available")
diff --git a/tests/test_cal_tools.py b/tests/test_cal_tools.py
index 7ee13b577..6bc8ae1cd 100644
--- a/tests/test_cal_tools.py
+++ b/tests/test_cal_tools.py
@@ -21,6 +21,7 @@ def test_show_processed_modules():
assert 'LDP' in err.value()
+@pytest.mark.requires_gpfs
def test_dir_creation_date():
folder = '/gpfs/exfel/exp/CALLAB/202031/p900113/raw'
@@ -94,6 +95,7 @@ def test_get_pdu_from_db():
'PHYSICAL_DETECTOR_UNIT-3_DO_NOT_DELETE']
+@pytest.mark.requires_gpfs
def test_initialize_from_db():
creation_time = datetime.strptime("2020-01-07 13:26:48.00",
"%Y-%m-%d %H:%M:%S.%f")
diff --git a/tests/test_calibrate.py b/tests/test_calibrate.py
deleted file mode 100644
index 7cc48d61b..000000000
--- a/tests/test_calibrate.py
+++ /dev/null
@@ -1,35 +0,0 @@
-import pytest
-
-from xfel_calibrate.calibrate import balance_sequences
-
-
-def test_balance_sequences():
-
- ret = balance_sequences(in_folder="/gpfs/exfel/exp/CALLAB/202031/p900113/raw", # noqa
- run=9992, sequences=[0, 2, 5, 10, 20, 50, 100],
- sequences_per_node=1, karabo_da=["all"],
- max_nodes=8)
-
- expected = [[0], [2]]
- assert expected == ret
-
- ret = balance_sequences(in_folder="/gpfs/exfel/exp/CALLAB/202031/p900113/raw", # noqa
- run=9992, sequences=[-1],
- sequences_per_node=1, karabo_da=["JNGFR01"],
- max_nodes=3)
- expected = []
- assert expected == ret
-
- with pytest.raises(ValueError) as e:
- balance_sequences(in_folder="/gpfs/exfel/exp/CALLAB/202031/p900113/raw", # noqa
- run=9992, sequences=[1991, 2021],
- sequences_per_node=1, karabo_da=["all"],
- max_nodes=3)
- assert 'Selected sequences [1991, 2021]]' in e.value()
-
- with pytest.raises(ValueError) as e:
- balance_sequences(in_folder="/gpfs/exfel/exp/CALLAB/202031/p900113/raw", # noqa
- run=9992, sequences=[1991, 2021],
- sequences_per_node=1, karabo_da=-1,
- max_nodes=3)
- assert 'karabo_da as a string or list' in e.value()
diff --git a/tests/test_webservice.py b/tests/test_webservice.py
index 604668a93..f26ea60d9 100644
--- a/tests/test_webservice.py
+++ b/tests/test_webservice.py
@@ -9,6 +9,7 @@ sys.path.insert(0, Path(__file__).parent / 'webservice')
from webservice.webservice import check_files, merge, parse_config, wait_on_transfer
+@pytest.mark.requires_gpfs
def test_check_files():
in_folder = '/gpfs/exfel/exp/CALLAB/202031/p900113/raw'
runs = [9985, 9984]
diff --git a/tests/test_xfel_calibrate/__init__.py b/tests/test_xfel_calibrate/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/test_xfel_calibrate/conftest.py b/tests/test_xfel_calibrate/conftest.py
new file mode 100644
index 000000000..c00cfda97
--- /dev/null
+++ b/tests/test_xfel_calibrate/conftest.py
@@ -0,0 +1,240 @@
+# pylint: disable=missing-module-docstring
+import sys
+from pathlib import Path
+from typing import Callable, Dict, List, NamedTuple, Optional, Union
+from unittest import mock
+
+import extra_data.tests.make_examples as make_examples
+from pytest_subprocess import FakeProcess
+
+from xfel_calibrate import calibrate, settings
+
+
+class FakeProcessCalibrate(FakeProcess):
+ """Class used to create a fake process which will mock calls to processes
+ expected to be called by pycalibration
+ """
+ def __init__(self):
+ """Sets up fake process for use with xfel calibrate
+
+ Fakes:
+ - Slurm free nodes call
+ - Slurm preempt nodes call
+ - Slurm sbatch call
+ - Git describe calls (for pulling repo version)
+
+ Expected use is something like:
+
+ ```
+ @pytest.fixture(scope="class", autouse=True)
+ def fake_process_calibrate(self):
+ with FakeProcessCalibrate() as fake_process:
+ yield fake_process
+ ```
+ """
+ super().__init__()
+ # Fake calls to slurm
+ self.register_subprocess(settings.free_nodes_cmd, stdout=["1"])
+ self.register_subprocess(settings.preempt_nodes_cmd, stdout=["1"])
+ self.register_subprocess(
+ ["sbatch", self.any()], stdout=["Submitted batch job 000000"]
+ )
+
+ # For the version insertion...
+ self.register_subprocess(
+ ["git", self.any(), "describe", "--tag"], stdout=["0.0.0"]
+ )
+
+ self.keep_last_process(True)
+
+
+class MockProposal:
+ """Class used with pytest to create proposal directories with placeholder
+ files
+
+ Use this to generate a skeleton of the proposal structure for tests which
+ only rely on the files being present and not their contents, e.g. balancing
+ by sequences done in some notebooks
+ """
+
+ def __init__(
+ self,
+ *,
+ tmp_path: Path,
+ runs: Union[int, list, slice],
+ instrument: str,
+ cycle: str,
+ proposal: str,
+ sequences: int = 2,
+ ):
+ """Create a MockProposal object, this should be used to create a fixture
+
+ Expected use is something like:
+
+ ```python3
+ @pytest.fixture
+ def mock_proposal(tmp_path):
+ return MockProposal(tmp_path, runs=4)
+ ```
+
+ Then `mock_proposal` can be passed as a fixture (or set to autouse) to
+ make the object available for individual or class tests
+
+ Args:
+ tmp_path (Path): Temporary path, should come from pytest fixture
+ runs (Union[int, list, slice]): Defines what run directories to make
+ instrument (str, optional): Instrument name, e.g. "AGIPD".
+ cycle (str, optional): Cycle, e.g. "202031".
+ proposal (str, optional): Proposal number, e.g. "p900113".
+ sequences (int, optional): Number of sequence files. Defaults to 2.
+ """
+ self.tmp_path = tmp_path
+ self.instrument = instrument
+ self.cycle = cycle
+ self.proposal = proposal
+ self.sequences = list(
+ range(sequences)
+ ) # TODO: Implement once it's in extra-data
+
+ self.path = tmp_path / instrument / cycle / proposal
+ self.path_raw = self.path / "raw"
+ self.path_proc = self.path / "proc"
+
+ self.runs = self.create_runs(runs)
+
+ def create_runs(self, runs: Union[int, List[int], slice]) -> Dict[int, Path]:
+ """Create run directories with skeleton files for the proposal
+
+ Args:
+ runs (Union[int, list, slice]): Defines what run directories to make
+
+ Returns:
+ [Dict[int, Path]]: Dictionary of the run number and run directory
+ """
+ if isinstance(runs, int):
+ runs = list(range(runs))
+ elif isinstance(runs, list):
+ if not all(isinstance(r, int) for r in runs):
+ raise TypeError("lists of runs must contain only integers")
+ elif isinstance(runs, slice):
+ if runs.stop:
+ runs = list(range(runs.stop)[runs])
+ else:
+ raise ValueError("runs slice must have a stop value")
+ else:
+ raise TypeError("runs must be an int, list of integers, or a slice")
+
+ run_directories = {run: self.path_raw / f"r{run:04d}" for run in runs}
+
+ for run_path in run_directories.values():
+ run_path.mkdir(parents=True)
+
+ self._create_run(run_path)
+
+ return run_directories
+
+ @property
+ def _create_run(self) -> Callable:
+ """Return the number of modules for a detector type
+
+ Returns:
+ modules [Iterable]: Number of modules for a detector type
+ """
+ return {
+ "AGIPD": make_examples.make_spb_run,
+ "DSSC": make_examples.make_scs_run,
+ "JGFR": make_examples.make_jungfrau_run,
+ "LPD": make_examples.make_fxe_run,
+ "None": lambda x: None,
+ }[self.instrument]
+
+
+class CalibrateCall:
+ """Class used with pytest to create call `xfel-calibrate` and log its output
+
+ Use this to manage calls to `xfel-calibrate` via pytest fixtures
+ """
+
+ def __init__(
+ self,
+ tmp_path,
+ capsys,
+ *,
+ detector: str,
+ cal_type: str,
+ command: str = "xfel-calibrate",
+ in_folder: Optional[Path] = None,
+ out_folder: Optional[Path] = None,
+ extra_args: List[str] = None,
+ ):
+ """Create a CallibrateCall object, this should be used to create a fixture
+
+ Expected use is something like:
+
+ ```python3
+ @pytest.fixture(scope="function")
+ def calibrate_call(self, mock_proposal: MockProposal, capsys, tmp_path):
+ return CalibrateCall(
+ tmp_path,
+ capsys,
+ in_folder=mock_proposal.path_raw,
+ out_folder=mock_proposal.path_proc,
+ command="xfel-calibrate",
+ detector="AGIPD",
+ cal_type="CORRECT",
+ extra_args=["--run", "0"],
+ )
+ ```
+
+ Args:
+ tmp_path ([type]): Temporary path, should come from pytest fixture
+ capsys ([type]): capsys path, should come from pytest fixture
+ detector (str): Detector passed to the command, e.g. AGIPD
+ cal_type (str): Calibration type passed to the command, e.g. CORRECT
+ command (str): Main entrypoint called. Defaults to "xfel-calibrate".
+ in_folder (Optional[Path], optional): Path to the input folder, usually
+ raw. Defaults to None.
+ out_folder (Optional[Path], optional): Path to the output folder, usually
+ proc. Defaults to None.
+ extra_args (List[str], optional): Additional arguments to pass to the
+ command. Defaults to None.
+ """
+ self.tmp_path = tmp_path
+ self.in_folder = in_folder
+ self.out_folder = out_folder
+
+ self.args = []
+ self.args.extend([command, detector, cal_type])
+ if in_folder:
+ self.args.extend(["--in-folder", str(self.in_folder)])
+ if out_folder:
+ self.args.extend(["--out-folder", str(self.out_folder)])
+
+ if extra_args:
+ self.args.extend(extra_args)
+
+ with mock.patch.object(sys, "argv", self.args):
+ with mock.patch.object(calibrate, "temp_path", str(tmp_path)):
+ calibrate.run()
+
+ out, err = capsys.readouterr()
+
+ self.out: str = out
+ self.err: str = err
+
+ Paths = NamedTuple(
+ "Paths",
+ [
+ ("notebooks", List[Path]),
+ ("run_calibrate", Path),
+ ("finalize", Path),
+ ("InputParameters", Path),
+ ],
+ )
+
+ self.paths = Paths(
+ notebooks=list(self.tmp_path.glob("**/*/*.ipynb")),
+ run_calibrate=list(self.tmp_path.glob("**/*/run_calibrate.sh"))[0],
+ finalize=list(self.tmp_path.glob("**/*/finalize.py"))[0],
+ InputParameters=list(self.tmp_path.glob("**/*/InputParameters.rst"))[0],
+ )
diff --git a/tests/test_xfel_calibrate/test_calibrate.py b/tests/test_xfel_calibrate/test_calibrate.py
new file mode 100644
index 000000000..b290cf12a
--- /dev/null
+++ b/tests/test_xfel_calibrate/test_calibrate.py
@@ -0,0 +1,52 @@
+# TODO: These are unit tests, `test_cli.py` contains integration tests, may be
+# worth splitting these up in the future so that it's easier to track
+# what's-what, track the coverage of both approaches individually, and run them
+# independently from each other
+
+import pytest
+
+from xfel_calibrate.calibrate import balance_sequences
+
+
+@pytest.mark.parametrize(
+ "karabo_da,sequences,expected",
+ [
+ pytest.param(
+ "all",
+ [0, 2, 5, 10, 20, 50, 100],
+ [[0], [2]],
+ marks=pytest.mark.requires_gpfs(),
+ ),
+ ("JNGFR01", [-1], []),
+ ],
+)
+def test_balance_sequences(karabo_da, sequences, expected):
+ ret = balance_sequences(
+ in_folder="/gpfs/exfel/exp/CALLAB/202031/p900113/raw",
+ run=9992,
+ sequences=sequences,
+ sequences_per_node=1,
+ karabo_da=karabo_da,
+ max_nodes=8,
+ )
+
+ assert ret == expected
+
+
+@pytest.mark.parametrize(
+ "karabo_da,sequences,expected",
+ [
+ pytest.param("all", [1991, 2021], ValueError, marks=pytest.mark.requires_gpfs()),
+ (-1, [], TypeError),
+ ],
+)
+def test_balance_sequences_raises(karabo_da, sequences, expected):
+ with pytest.raises(expected):
+ balance_sequences(
+ in_folder="/gpfs/exfel/exp/CALLAB/202031/p900113/raw",
+ run=9992,
+ sequences=sequences,
+ sequences_per_node=1,
+ karabo_da=karabo_da,
+ max_nodes=8,
+ )
diff --git a/tests/test_xfel_calibrate/test_cli.py b/tests/test_xfel_calibrate/test_cli.py
new file mode 100644
index 000000000..f0ffeb4af
--- /dev/null
+++ b/tests/test_xfel_calibrate/test_cli.py
@@ -0,0 +1,408 @@
+# pylint: disable=missing-class-docstring, missing-function-docstring, no-self-use
+"""Tests for the CLI portion of `xfel_calibrate`
+
+These tests cover the CLI interface which is called by the `xfel-calibrate ...`
+entrypoint. Some sections of the `calibrate.py` file are still not covered by
+the current test cases, this should be improved later on.
+"""
+
+import ast
+import shlex
+import sys
+from datetime import date
+from pathlib import Path
+from unittest import mock
+
+import nbformat
+import pytest
+from nbparameterise import extract_parameters
+
+import xfel_calibrate.calibrate as calibrate
+from tests.test_xfel_calibrate.conftest import (
+ CalibrateCall,
+ FakeProcessCalibrate,
+ MockProposal,
+)
+
+
+class TestBasicCalls:
+ """Tests which only call the command line utility `xfel-calibrate` and check
+ that the expected output is present in stdout
+ """
+
+ @mock.patch.object(sys, "argv", ["xfel-calibrate", "--help"])
+ def test_help(self, capsys):
+ with pytest.raises(SystemExit):
+ calibrate.run()
+
+ out, err = capsys.readouterr()
+
+ # Should always be present in these help outputs
+ assert "positional arguments:" in out
+ assert "optional arguments:" in out
+
+ assert err == ""
+
+ @mock.patch.object(sys, "argv", ["xfel-calibrate", "TEST", "-h"])
+ def test_help_detector(self, capsys):
+ with pytest.raises(SystemExit):
+ calibrate.run()
+
+ out, err = capsys.readouterr()
+
+ assert "Notebook for use with the unit and continuous integration" in out
+ assert "tests." in out
+
+ assert err == ""
+
+ @mock.patch.object(sys, "argv", ["xfel-calibrate", "NotADetector", "beep", "-h"])
+ def test_unknown_detector(self, capsys):
+ with pytest.raises(SystemExit) as exit_exception:
+ calibrate.run()
+
+ out, err = capsys.readouterr()
+
+ assert exit_exception.value.code == 1
+
+ assert "Not one of the known calibrations or detectors" in out
+
+ assert err == ""
+
+ @mock.patch.object(sys, "argv", ["xfel-calibrate", "NotADetector", "-h"])
+ def test_unknown_detector_h(self, capsys):
+ with pytest.raises(SystemExit) as exit_exception:
+ calibrate.run()
+
+ out, err = capsys.readouterr()
+
+ assert exit_exception.value.code == 1
+
+ assert "Not one of the known detectors" in out
+
+ assert err == ""
+
+ @mock.patch.object(sys, "argv", ["xfel-calibrate", "Tutorial", "TEST", "--help"])
+ def test_help_nb(self, capsys):
+ with pytest.raises(SystemExit):
+ calibrate.run()
+
+ out, err = capsys.readouterr()
+
+ # Should always be present in these help outputs
+ assert "positional arguments:" in out
+ assert "optional arguments:" in out
+
+ # Defined in the test notebook, should be propagated to help output
+ assert "sensor-size" in out
+ assert "random-seed" in out
+
+ assert err == ""
+
+
+class TestTutorialNotebook:
+ """Checks calling `xfel-calibrate` on the `Tutorial TEST` notebook, looks
+ at the stdout as well as files generated by the call
+ """
+
+ @pytest.fixture(scope="class", autouse=True)
+ def fake_process_calibrate(self):
+ with FakeProcessCalibrate() as fake_process:
+ yield fake_process
+
+ @pytest.fixture(scope="class")
+ def mock_proposal(self, tmp_path_factory):
+ return MockProposal(
+ tmp_path=tmp_path_factory.mktemp("exp"),
+ instrument="None",
+ cycle="000000",
+ proposal="p000000",
+ runs=0,
+ sequences=0,
+ )
+
+ @pytest.fixture(scope="function")
+ def calibrate_call(
+ self,
+ mock_proposal: MockProposal,
+ capsys,
+ tmp_path,
+ ):
+ return CalibrateCall(
+ tmp_path,
+ capsys,
+ out_folder=mock_proposal.path_proc,
+ detector="Tutorial",
+ cal_type="TEST",
+ extra_args=["--runs", "1000"],
+ )
+
+ def test_call(
+ self,
+ calibrate_call: CalibrateCall,
+ ):
+ assert "sbatch" in calibrate_call.out
+ assert "--job-name xfel_calibrate" in calibrate_call.out
+ assert str(calibrate_call.tmp_path) in calibrate_call.out
+ assert "Submitted job: " in calibrate_call.out
+
+ assert calibrate_call.err == ""
+
+ def test_expected_processes_called(
+ self,
+ fake_process_calibrate: FakeProcessCalibrate,
+ ):
+ process_calls = [
+ list(shlex.shlex(p, posix=True, punctuation_chars=True))
+ if isinstance(p, str)
+ else p
+ for p in fake_process_calibrate.calls
+ ]
+
+ processes_called = [p[0] for p in process_calls] # List of the process names
+
+ assert "sbatch" in processes_called
+
+ @pytest.mark.skip(reason="not implemented")
+ def test_output_metadata_yml(self):
+ # TODO: Finish this test later, not a priority
+ # metadata_yml_path = list(self.tmp_path.glob("**/calibration_metadata.yml"))
+ pass
+
+ def test_output_ipynb(self, calibrate_call: CalibrateCall):
+ notebook_path = calibrate_call.paths.notebooks
+
+ assert len(notebook_path) == 1
+
+ with notebook_path[0].open() as file:
+ notebook = nbformat.read(file, as_version=4)
+
+ parameters = {p.name: p.value for p in extract_parameters(notebook)}
+
+ assert parameters["out_folder"] == str(calibrate_call.out_folder)
+ assert parameters["sensor_size"] == [10, 30]
+ assert parameters["random_seed"] == [2345]
+ assert parameters["runs"] == 1000
+
+ def test_output_finalize(
+ self, mock_proposal: MockProposal, calibrate_call: CalibrateCall
+ ):
+ # TODO: Specify `feature_version` once we run on python 3.8+
+ finalize_ast = ast.parse(calibrate_call.paths.finalize.read_text())
+
+ today = date.today()
+
+ expected_equals = {
+ "joblist": [],
+ "project": "Tutorial Calculation",
+ "calibration": "Tutorial Calculation",
+ "author": "Astrid Muennich",
+ "version": "0.0.0",
+ "data_path": "",
+ }
+
+ expected_contains = {
+ "request_time": str(today),
+ "submission_time": str(today),
+ "run_path": str(calibrate_call.tmp_path),
+ # TODO: add a test checking that the out folder is correct
+ # reffer to: https://git.xfel.eu/gitlab/detectors/pycalibration/issues/52
+ "out_path": str(mock_proposal.path_proc),
+ "report_to": str(mock_proposal.path_proc),
+ }
+
+ # Pull the keyword arguments out of the finalize function call via the AST,
+ # here we use the keys in `expected_...` to filter which kwargs are parsed
+ # as some cannot be read
+ finalize_kwargs = {
+ k.arg: ast.literal_eval(k.value)
+ for k in ast.walk(finalize_ast)
+ if isinstance(k, ast.keyword)
+ and (k.arg in expected_equals or k.arg in expected_contains)
+ }
+
+ for k, v in expected_equals.items():
+ assert v == finalize_kwargs[k]
+
+ for k, v in expected_contains.items():
+ assert v in finalize_kwargs[k]
+
+ @pytest.mark.skip(reason="not implemented")
+ def test_output_rst(self, calibrate_call: CalibrateCall):
+ # TODO: Finish this test later, not a priority
+ # rst_path = calibrate_call.paths.InputParameters
+ pass
+
+ def test_output_sh(self, calibrate_call: CalibrateCall):
+ cmd = list(
+ shlex.shlex(
+ calibrate_call.paths.run_calibrate.read_text(),
+ posix=True,
+ punctuation_chars=True,
+ )
+ )
+
+ assert (
+ cmd[0] == "xfel-calibrate"
+ ), f"{calibrate_call.paths.run_calibrate} does not call `xfel-calibrate`"
+
+ assert cmd[1:3] == ["Tutorial", "TEST"]
+ assert {"--out-folder", str(calibrate_call.out_folder)}.issubset(cmd)
+ assert {"--runs", "1000"}.issubset(cmd)
+
+
+class TestIntelliList:
+ @pytest.fixture(scope="class", autouse=True)
+ def fake_process_calibrate(self):
+ with FakeProcessCalibrate() as fake_process:
+ yield fake_process
+
+ @pytest.fixture(scope="class")
+ def mock_proposal(self, tmpdir_factory):
+ return MockProposal(
+ tmp_path=Path(tmpdir_factory.mktemp("exp")),
+ instrument="AGIPD",
+ cycle="202031",
+ proposal="p900113",
+ runs=1,
+ sequences=1,
+ )
+
+ @pytest.fixture(scope="function")
+ def calibrate_call(self, mock_proposal: MockProposal, capsys, tmp_path):
+ return CalibrateCall(
+ tmp_path,
+ capsys,
+ in_folder=mock_proposal.path_raw,
+ out_folder=mock_proposal.path_proc,
+ detector="TEST",
+ cal_type="TEST-CLI",
+ extra_args=[
+ "--number",
+ "10",
+ "--list-normal",
+ "1,2,10",
+ "--list-intellilist",
+ "1,2,5-8",
+ "--concurrency-parameter",
+ "0,1",
+ ],
+ )
+
+ def test_intellilist(self, calibrate_call: CalibrateCall):
+ assert "--number" in calibrate_call.args
+ assert "--list-intellilist" in calibrate_call.args
+ assert "1,2,5-8" in calibrate_call.args
+
+ assert len(calibrate_call.paths.notebooks) == 2
+
+ for i, notebook_path in enumerate(calibrate_call.paths.notebooks):
+ with notebook_path.open() as file:
+ notebook = nbformat.read(file, as_version=4)
+
+ parameters = {p.name: p.value for p in extract_parameters(notebook)}
+
+ assert parameters["number"] == 10
+ assert parameters["list_normal"] == [1, 2, 10]
+ assert parameters["list_intellilist"] == [1, 2, 5, 6, 7]
+ assert parameters["concurrency_parameter"][0] == i
+
+
+class TestAgipdNotebook:
+ @pytest.fixture(scope="class", autouse=True)
+ def fake_process_calibrate(self):
+ with FakeProcessCalibrate() as fake_process:
+ yield fake_process
+
+ @pytest.fixture(scope="function")
+ def mock_proposal(self, tmpdir_factory):
+ return MockProposal(
+ tmp_path=Path(tmpdir_factory.mktemp("exp")),
+ instrument="AGIPD",
+ cycle="202031",
+ proposal="p900113",
+ runs=2,
+ # TODO: update this once extra-data tests can have variable sequences
+ # sequences=5,
+ )
+
+ @pytest.fixture(scope="function")
+ def calibrate_call(self, mock_proposal: MockProposal, capsys, tmp_path):
+ return CalibrateCall(
+ tmp_path,
+ capsys,
+ in_folder=mock_proposal.path_raw,
+ out_folder=mock_proposal.path_proc / "r0000",
+ detector="AGIPD",
+ cal_type="CORRECT",
+ extra_args=[
+ "--run",
+ "0",
+ "--sequences",
+ "1-3",
+ # TODO: enable this when notebook execution tests are ready to be ran
+ # "--no-cluster-job",
+ ],
+ )
+
+ @pytest.mark.skip(reason="not implemented")
+ def test_out_folder_correct(self):
+ # TODO: add a test checking that the out folder is correct
+ # reffer to: https://git.xfel.eu/gitlab/detectors/pycalibration/issues/52
+ pass
+
+ @pytest.mark.skip(reason="requires extra-data test file sequence options")
+ def test_files_present(self, calibrate_call: CalibrateCall):
+ # There should be three notebooks: one pre, then the main, then one post
+ assert len(calibrate_call.paths.notebooks) == 3
+ # This is pretty fragile, but the name of notebooks should not change
+ # (too) often
+ root_nb_path = calibrate_call.paths.notebooks[0].parent
+ notebooks = [
+ root_nb_path / "AGIPD_Correct_and_Verify__sequences__1.ipynb",
+ root_nb_path / "AGIPD_Correct_and_Verify_Summary_NBC__None__None.ipynb",
+ root_nb_path / "AGIPD_Retrieve_Constants_Precorrection__None__None.ipynb",
+ ]
+
+ assert all(nb in calibrate_call.paths.notebooks for nb in notebooks)
+
+ @pytest.mark.skip(reason="not implemented")
+ def test_nb_sequences(self, calibrate_call: CalibrateCall):
+ notebook_path = (
+ calibrate_call.paths.notebooks[0].parent
+ / "AGIPD_Correct_and_Verify__sequences__1.ipynb"
+ )
+
+ with notebook_path.open() as file:
+ notebook = nbformat.read(file, as_version=4)
+
+ parameters = {p.name: p.value for p in extract_parameters(notebook)}
+ # TODO: add test cases for this notebook
+ print(parameters)
+
+ @pytest.mark.skip(reason="not implemented")
+ def test_nb_summary(self, calibrate_call: CalibrateCall):
+ notebook_path = (
+ calibrate_call.paths.notebooks[0].parent
+ / "AGIPD_Correct_and_Verify_Summary_NBC__None__None.ipynb"
+ )
+
+ with notebook_path.open() as file:
+ notebook = nbformat.read(file, as_version=4)
+
+ parameters = {p.name: p.value for p in extract_parameters(notebook)}
+ # TODO: add test cases for this notebook
+ print(parameters)
+
+ @pytest.mark.skip(reason="not implemented")
+ def test_nb_precorrection(self, calibrate_call: CalibrateCall):
+ notebook_path = (
+ calibrate_call.paths.notebooks[0].parent
+ / "AGIPD_Retrieve_Constants_Precorrection__None__None.ipynb"
+ )
+
+ with notebook_path.open() as file:
+ notebook = nbformat.read(file, as_version=4)
+ # TODO: add test cases for this notebook
+ parameters = {p.name: p.value for p in extract_parameters(notebook)}
+
+ print(parameters)
--
GitLab