Get test coverage close to 100%

CHANGES.md

@@ -10,7 +10,7 @@
 * Allow `xcengine image run` to pass arguments to the container (#57, #77)
 * Add option for `xcengine image run` to open a browser window (#58)
 * Add option to skip image build and just create Dockerfile and context (#60)
-* Refactor code and improve test coverage (#40)
+* Refactor code and improve test coverage (#40, #80)
 
 ## Changes in 0.1.1
 

test/test_cli.py

@@ -3,6 +3,7 @@
 import urllib
 from unittest.mock import patch, ANY, MagicMock
 import pytest
+import yaml
 from click.testing import CliRunner
 
 from xcengine.cli import cli
@@ -64,19 +65,26 @@ def test_make_script(
 
 @pytest.mark.parametrize("specify_dir", [False, True])
 @pytest.mark.parametrize("specify_env", [False, True])
+@pytest.mark.parametrize("specify_eoap", [False, True])
 @patch("xcengine.cli.ImageBuilder")
-def test_image_build(builder_mock, tmp_path, specify_dir, specify_env):
+def test_image_build(
+    builder_mock, tmp_path, specify_dir, specify_env, specify_eoap
+):
     (nb_path := tmp_path / "foo.ipynb").touch()
     (env_path := tmp_path / "environment.yml").touch()
     (build_dir := tmp_path / "build").mkdir()
+    eoap_path = tmp_path / "eoap.yaml"
     runner = CliRunner()
     tag = "foo"
     instance_mock = builder_mock.return_value = MagicMock()
+    cwl = {"foo": 42}
+    instance_mock.create_cwl.return_value = cwl
     result = runner.invoke(
         cli,
         ["image", "build", "--tag", tag]
         + (["--build-dir", str(build_dir)] if specify_dir else [])
         + (["--environment", str(env_path)] if specify_env else [])
+        + (["--eoap", str(eoap_path)] if specify_eoap else [])
         + [str(nb_path)],
     )
     assert result.output.startswith("Built image")
@@ -88,6 +96,8 @@ def test_image_build(builder_mock, tmp_path, specify_dir, specify_env):
         build_dir=(build_dir if specify_dir else ANY),
     )
     instance_mock.build.assert_called_once_with(skip_build=False)
+    if specify_eoap:
+        assert yaml.safe_load(eoap_path.read_text()) == cwl
 
 
 @patch("xcengine.cli.ContainerRunner")
@@ -182,6 +192,7 @@ def urlopen(url):
     assert passed_url == f"http://localhost:{port}"
     open_mock.assert_called_once_with(f"http://localhost:{port}/viewer")
 
+
 @patch("docker.from_env")
 def test_image_skip_build_save_dockerfile_and_env(from_env_mock, tmp_path):
     build_dir = tmp_path / "build"
@@ -202,4 +213,3 @@ def test_image_skip_build_save_dockerfile_and_env(from_env_mock, tmp_path):
     assert (build_dir / "Dockerfile").is_file()
     assert (build_dir / "environment.yml").is_file()
     from_env_mock.assert_not_called()
-

test/test_core.py

@@ -208,6 +208,25 @@ def interrupt_process():
     assert container.status == "stopped"
 
 
+def test_runner_extract_output_from_container(tmp_path):
+    runner = xcengine.core.ContainerRunner(
+        image := Mock(docker.models.images.Image),
+        tmp_path,
+        client := Mock(DockerClient),
+    )
+    image.tags = []
+
+    def byte_generator():
+        b = (pathlib.Path(__file__).parent / "data" / "data.tar").read_bytes()
+        yield b
+
+    container = MagicMock()
+    container.get_archive.return_value = byte_generator(), None
+    client.containers.run.return_value = container
+    runner.run(False, 8080, False, False)
+    assert (tmp_path / "foo").read_text() == "bar\n"
+
+
 @patch("xcengine.core.subprocess.run")
 def test_pip(mock_run):
     pip_output = {
@@ -351,18 +370,40 @@ def test_image_builder_write_dockerfile(tmp_path):
 
 
 @patch("docker.from_env")
-def test_image_builder_build_skip_build(from_env_mock, tmp_path):
+@pytest.mark.parametrize("set_env", [False, True])
+@pytest.mark.parametrize("skip_build", [False, True])
+def test_image_builder_build_dir(from_env_mock, tmp_path, set_env, skip_build):
+    client_mock = Mock(docker.client.DockerClient)
+    client_mock.images.build.return_value = None, None
+    from_env_mock.return_value = client_mock
+
     build_dir = tmp_path / "build"
+    env_path = tmp_path / "env2.yaml"
+    env_def = {
+        "name": "foo",
+        "channels": "bar",
+        "dependencies": ["python >=3.13", "baz >=42.0"],
+    }
+    env_path.write_text(yaml.safe_dump(env_def))
     image_builder = ImageBuilder(
         pathlib.Path(__file__).parent / "data" / "noparamtest.ipynb",
-        None,
+        env_path if set_env else None,
         build_dir,
         None,
     )
-    image_builder.build(skip_build=True)
-    from_env_mock.assert_not_called()
+    image_builder.build(skip_build=skip_build)
+    if skip_build:
+        from_env_mock.assert_not_called()
+    else:
+        client_mock.images.build.assert_called()
     env_path = build_dir / "environment.yml"
     assert env_path.is_file()
-    with open(env_path) as fh:
-        env_dict = yaml.safe_load(fh)
-        assert {"name", "channels", "dependencies"} <= set(env_dict)
+    output_env = yaml.safe_load(env_path.read_text())
+    assert {"name", "channels", "dependencies"} <= set(output_env)
+    if set_env:
+        assert output_env["name"] == env_def["name"]
+        assert output_env["channels"] == env_def["channels"]
+        assert set(output_env["dependencies"]) >= set(env_def["dependencies"])
+
+    cwl = image_builder.create_cwl()
+    assert "cwlVersion" in cwl

test/test_util.py

@@ -50,28 +50,39 @@ def test_clear_directory(tmp_path):
 
 
 @pytest.mark.parametrize("write_datasets", [False, True])
-def test_write_stac(tmp_path, dataset, write_datasets):
+@pytest.mark.parametrize("pre_existing_catalog", [False, True])
+def test_write_stac(tmp_path, dataset, write_datasets, pre_existing_catalog):
     datasets = {"ds1": dataset, "ds2": dataset.copy()}
     datasets["ds2"].attrs["xcengine_output_format"] = "netcdf"
     if write_datasets:
         output_path = tmp_path / "output"
         output_path.mkdir()
         datasets["ds1"].to_zarr(output_path / ("ds1.zarr"))
         datasets["ds2"].to_netcdf(output_path / ("ds2.nc"))
-
+    catalog_path = tmp_path / "catalog.json"
+    if pre_existing_catalog:
+        catalog_path.touch()
     write_stac(datasets, tmp_path)
-    catalog = pystac.Catalog.from_file(tmp_path / "catalog.json")
-    items = set(catalog.get_items(recursive=True))
-    assert {item.id for item in items} == datasets.keys()
-    catalog.make_all_asset_hrefs_absolute()
-    data_asset_hrefs = {
-        item.id: [a.href for a in item.assets.values() if "data" in a.roles]
-        for item in items
-    }
-    assert data_asset_hrefs == {
-        "ds1": [str((tmp_path / "ds1" / "ds1.zarr").resolve(strict=False))],
-        "ds2": [str((tmp_path / "ds2" / "ds2.nc").resolve(strict=False))],
-    }
+    if pre_existing_catalog:
+        # Check that our fake catalogue was not overwritten
+        assert catalog_path.stat().st_size == 0
+    else:
+        catalog = pystac.Catalog.from_file(catalog_path)
+        items = set(catalog.get_items(recursive=True))
+        assert {item.id for item in items} == datasets.keys()
+        catalog.make_all_asset_hrefs_absolute()
+        data_asset_hrefs = {
+            item.id: [
+                a.href for a in item.assets.values() if "data" in a.roles
+            ]
+            for item in items
+        }
+        assert data_asset_hrefs == {
+            "ds1": [
+                str((tmp_path / "ds1" / "ds1.zarr").resolve(strict=False))
+            ],
+            "ds2": [str((tmp_path / "ds2" / "ds2.nc").resolve(strict=False))],
+        }
 
 
 @pytest.mark.parametrize("eoap_mode", [False, True])