From f571311c98f9b24bd41a9421bb5c8c8c3212183d Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 14:36:21 +0100 Subject: [PATCH 1/8] Add test coverage for CLI --eoap option --- test/test_cli.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/test_cli.py b/test/test_cli.py index 599111d..ad2170a 100644 --- a/test/test_cli.py +++ b/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() - From 9c4ca0082d394d72eebc4aee1bbc8ec0cfbee27c Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 14:52:12 +0100 Subject: [PATCH 2/8] Improve test coverage of util.write_stac --- test/test_util.py | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index 8965a61..b50fedc 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -50,7 +50,8 @@ 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: @@ -58,20 +59,30 @@ def test_write_stac(tmp_path, dataset, write_datasets): 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]) From 69971c08f8e8c3b8fccc7e8b9cce2d592ec2ac69 Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 15:39:24 +0100 Subject: [PATCH 3/8] Improve test coverage of ImageBuilder.build --- test/test_core.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/test/test_core.py b/test/test_core.py index 3cf3169..0e951b6 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -351,11 +351,19 @@ 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]) +def test_image_builder_build_skip_build(from_env_mock, tmp_path, set_env): 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, ) @@ -363,6 +371,9 @@ def test_image_builder_build_skip_build(from_env_mock, tmp_path): from_env_mock.assert_not_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"]) From 0797e04fbd9f891477994e4462c7e45ff4bc8f3c Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 16:36:41 +0100 Subject: [PATCH 4/8] Add unit test for extracting data from container --- test/test_core.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/test_core.py b/test/test_core.py index 0e951b6..53ee3f9 100644 --- a/test/test_core.py +++ b/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 = { From 7b8af28bc05e39d290d6789411cf6c7bbadb545e Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 16:46:15 +0100 Subject: [PATCH 5/8] Add unit test for image building --- test/test_core.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/test_core.py b/test/test_core.py index 53ee3f9..cca67d9 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -371,7 +371,12 @@ def test_image_builder_write_dockerfile(tmp_path): @patch("docker.from_env") @pytest.mark.parametrize("set_env", [False, True]) -def test_image_builder_build_skip_build(from_env_mock, tmp_path, set_env): +@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 = { @@ -386,8 +391,11 @@ def test_image_builder_build_skip_build(from_env_mock, tmp_path, set_env): 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() output_env = yaml.safe_load(env_path.read_text()) From 830812b64f6ce6cdbb85850ddfc58cd962426ae8 Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 16:49:01 +0100 Subject: [PATCH 6/8] Add missing test data file --- test/data/data.tar | Bin 0 -> 2560 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/data/data.tar diff --git a/test/data/data.tar b/test/data/data.tar new file mode 100644 index 0000000000000000000000000000000000000000..d8b02a9d171635d81bfc16dce81d0f16dfbdd8a2 GIT binary patch literal 2560 zcmeH{Jr2Vl4279<3g4h6V0<1bwPUG>d;b6x63x=dQl#L;{0Q^>#D~Nzk44x|PuYY6%+aem04@NPDN3jO)kozx{6`wYeH_Jbu>s zJ?7=C+NLgj2g>7b{A=D+cwnZH`~D9!RvItA{I68`Crwek_Qm<{Hq1O7m=0XefgMkd BE(`zw literal 0 HcmV?d00001 From f81ef26e0d44b46e6cd2c3c1da590232f5c83789 Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 17:02:37 +0100 Subject: [PATCH 7/8] Add test for ImageBuilder.create_cwl() --- test/test_core.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_core.py b/test/test_core.py index cca67d9..d6a6255 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -404,3 +404,6 @@ def test_image_builder_build_dir(from_env_mock, tmp_path, set_env, skip_build): 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 From d75f6831b490babb770a976e2d54107cb61041a6 Mon Sep 17 00:00:00 2001 From: Pontus Lurcock Date: Tue, 24 Feb 2026 17:07:12 +0100 Subject: [PATCH 8/8] Update changelog --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 97a441b..6fb7306 100644 --- a/CHANGES.md +++ b/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