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 diff --git a/test/data/data.tar b/test/data/data.tar new file mode 100644 index 0000000..d8b02a9 Binary files /dev/null and b/test/data/data.tar differ 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() - diff --git a/test/test_core.py b/test/test_core.py index 3cf3169..d6a6255 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 = { @@ -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 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])