.gitignore

@@ -21,6 +21,7 @@
 LPD/results
 Test
 build
+coverage.*
 docs/build
 docs/source/_notebooks
 docs/source/_static/reports

.gitlab-ci.yml

@@ -44,6 +44,11 @@ pytest:
   script:
     - python3 -m pip install ".[test]"
     - python3 -m pytest --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

src/xfel_calibrate/calibrate.py

@@ -15,13 +15,13 @@ 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 (
@@ -701,12 +701,7 @@ def concurrent_run(
         os.path.basename(base_name), cparm, suffix)
 
     nbpath = os.path.join(temp_path, new_name)
-    with open(nbpath, "wb") as f:
-        f.write(
-            nbconvert.exporters.export(
-                nbconvert.NotebookExporter, new_nb
-            )[0].encode('utf-8')
-        )
+    nbformat.write(new_nb, nbpath)
 
     # add finalization to the last job
     if final_job:

tests/test_xfel_calibrate/conftest.py

+# pylint: disable=missing-module-docstring
 import sys
 from pathlib import Path
-from typing import Callable, Dict, List, Optional, Union
+from typing import Callable, Dict, List, NamedTuple, Optional, Union
 from unittest import mock
 
 import extra_data.tests.make_examples as make_examples
@@ -8,9 +9,11 @@ import pytest
 
 from xfel_calibrate import calibrate, settings
 
+from pytest_subprocess.core import FakeProcess
+
 
 @pytest.fixture(autouse=True)
-def fake_process_xfel_calibrate(fake_process):
+def fake_process_xfel_calibrate(fake_process: FakeProcess) -> FakeProcess:
     """Sets up fake process for use with xfel calibrate
 
     Fakes:
@@ -85,7 +88,9 @@ class MockProposal:
         self.instrument = instrument
         self.cycle = cycle
         self.proposal = proposal
-        self.sequences = list(range(sequences))
+        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"
@@ -212,3 +217,20 @@ class CalibrateCall:
 
                 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],
+        )

tests/test_xfel_calibrate/test_cli.py

+# 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 ...`
@@ -16,10 +17,17 @@ import pytest
 from nbparameterise import extract_parameters
 
 import xfel_calibrate.calibrate as calibrate
-from tests.test_xfel_calibrate.conftest import CalibrateCall, MockProposal
+from tests.test_xfel_calibrate.conftest import (
+    CalibrateCall,
+    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):
@@ -87,6 +95,10 @@ class TestBasicCalls:
 
 
 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")
     def mock_proposal(self, tmpdir_factory):
         return MockProposal(
@@ -124,14 +136,14 @@ class TestTutorialNotebook:
         pass
 
     def test_output_ipynb(self, calibrate_call: CalibrateCall):
-        nb_path = list(calibrate_call.tmp_path.glob("**/*/*.ipynb"))
+        notebook_path = calibrate_call.paths.notebooks
 
-        assert len(nb_path) == 1
+        assert len(notebook_path) == 1
 
-        with nb_path[0].open() as f:
-            nb = nbformat.read(f, as_version=4)
+        with notebook_path[0].open() as file:
+            notebook = nbformat.read(file, as_version=4)
 
-        parameters = {p.name: p.value for p in extract_parameters(nb)}
+        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]
@@ -141,14 +153,13 @@ class TestTutorialNotebook:
     def test_output_finalize(
         self, mock_proposal: MockProposal, calibrate_call: CalibrateCall
     ):
-        finalize_path = list(calibrate_call.tmp_path.glob("**/*/finalize.py"))[0]
         # TODO: Specify `feature_version` once we run on python 3.8+
-        finalize_ast = ast.parse(finalize_path.read_text())
+        finalize_ast = ast.parse(calibrate_call.paths.finalize.read_text())
 
         finalize_kwargs = {
             k.arg: ast.literal_eval(k.value)
             for k in ast.walk(finalize_ast)
-            if type(k) is ast.keyword and k.arg != "finaljob"
+            if isinstance(k, ast.keyword) and k.arg != "finaljob"
         }
 
         today = date.today()
@@ -168,19 +179,16 @@ class TestTutorialNotebook:
     @pytest.mark.skip(reason="not implemented")
     def test_output_rst(self, calibrate_call: CalibrateCall):
         # TODO: Finish this test later, not a priority
-        rst_path = list(calibrate_call.tmp_path.glob("**/*/InputParameters.rst"))[
-            0
-        ]  # noqa
+        # rst_path = calibrate_call.paths.InputParameters
+        pass
 
     def test_output_sh(self, calibrate_call: CalibrateCall):
-        sh_path = list(calibrate_call.tmp_path.glob("**/*/run_calibrate.sh"))[0]
-
         sh_target = (
             "# pycalibration version: 0.0.0\nxfel-calibrate Tutorial TEST "
             + f"--out-folder {str(calibrate_call.out_folder)} --runs 1000"
         )
 
-        assert sh_path.read_text() == sh_target
+        assert calibrate_call.paths.run_calibrate.read_text() == sh_target
 
 
 class TestIntelliList:
@@ -200,18 +208,39 @@ class TestIntelliList:
         return CalibrateCall(
             tmp_path,
             capsys,
+            in_folder=mock_proposal.path_raw,
             out_folder=mock_proposal.path_proc,
-            detector="Tutorial",
-            cal_type="TEST",
-            extra_args=["--random-seed", "1,2,3-10"],
+            detector="TEST",
+            cal_type="TEST-CLI",
+            extra_args=[
+                "--number",
+                "10",
+                "--list-normal",
+                "1,2,10",
+                "--list-intellilist",
+                "1,2,5-8",
+                "--concurrency-parameter",
+                "1,2",
+            ],
         )
 
     def test_intellilist(self, calibrate_call: CalibrateCall):
-        assert "--random-seed" in calibrate_call.args
-        assert "1,2,3-10" in calibrate_call.args
-        assert (
-            "Parsed input 1,2,3-10 to [1, 2, 3, 4, 5, 6, 7, 8, 9]" in calibrate_call.out
-        )
+        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 + 1
 
 
 class TestAgipdNotebook:
@@ -235,8 +264,68 @@ class TestAgipdNotebook:
             out_folder=mock_proposal.path_proc,
             detector="AGIPD",
             cal_type="CORRECT",
-            extra_args=["--run", "0", "--sequences", "1-3"],
+            extra_args=[
+                "--run",
+                "0",
+                "--sequences",
+                "1-3",
+                #  TODO: enable this when notebook execution tests are ready to be ran
+                # "--no-cluster-job",
+            ],
+        )
+
+    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"
         )
 
-    def test_agipd_correct(self, calibrate_call: CalibrateCall):
-        print(calibrate_call.out)
+        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)