Build docker containers before executing 'viash run' (#22)

* Build docker containers before executing 'viash run'

* Undo unnecessary change

* Fix 3.12 tests

* Update CHANGELOG

CHANGELOG.rst

@@ -2,9 +2,28 @@
 Changelog
 *********
 
-*.*.* (TBD)
+0.7.0 (TBD)
 ===========
 
+Breaking Changes
+----------------
+
+* `run_component` now executes `viash run` with `--platform docker` when the test 
+  is executed inline (and not as a result of using 'viash test') instead of the first
+  platform that is defined in the config. Another platform can be specified by using
+  the `platform` keyword argument (#22).
+
+Minor Changes
+-------------
+
+* `run_component` now uses `cachedbuild` setup strategy when tests are executed inline
+  with platform `docker`. This makes sure that the docker image is update when it already
+  exists on the system (#22).
+
+* Added debugging information about the calls that are made to `viash`. If you want to enable
+  debugging information, use `--log-cli-level=DEBUG` from `pytest` (#22).
+
+
 0.6.0 (22/01/2024)
 ==================
 

tests/integration/test_run_component/dummy_config.vsh.yaml

@@ -27,7 +27,7 @@ functionality:
 platforms:
   - type: native
   - type: docker
-    image: python:3.10
+    image: python:3.12
     test_setup:
       - type: docker
         copy: ["filiberke /viashpy"]

tests/integration/test_run_component/test_script.py

@@ -54,7 +54,7 @@ def viash_source_config_path(self, random_config_path):
         return result_path
 
     def test_run_component_check_correct_memory_syntax(self, run_component):
-        captured_output = run_component(["--output", "bar.txt"])
+        captured_output = run_component(["--output", "bar.txt"], platform="native")
         with open("bar.txt", "r") as open_output_file:
             contents = open_output_file.read()
             assert contents == "foo!"

tests/unittests/fixtures/test_run_component.py

@@ -149,10 +149,20 @@ def test_run_component_different_memory_specification_warnings(
         specifier for specifier in memory_specifiers if specifier != "None"
     ]
     if any(memory_specifiers):
-        expected_memory_args = f', "---memory", "{expected_bytes}B", '
-    expected = (
-        '["viash", "run", Path(meta["config"]), "--", "bar"%s]' % expected_memory_args
+        expected_memory_args = f', "--memory", "{expected_bytes}B", '
+    expected_base_command = (
+        '["viash", "run", Path(meta["config"]), "-p", "docker", "-c",'
+        "\".platforms[.type == 'docker'].target_tag := 'test'\""
     )
+    expected_build_call = (
+        f"mocker.call({expected_base_command}, "
+        '"--", "---setup", "cachedbuild"], stderr=subprocess.STDOUT)'
+    )
+    expected_run_call = (
+        f'mocker.call({expected_base_command}{expected_memory_args}"--", "bar"], '
+        "stderr=subprocess.STDOUT)"
+    )
+
     makepyfile_and_add_meta(
         f"""
         import subprocess
@@ -163,8 +173,8 @@ def test_loading_run_component(mocker, run_component):
                                                return_value=b"Some dummy output")
             mocked_path = mocker.patch('viashpy.testing.Path.is_file', return_value=True)
             stdout = run_component(["bar"])
-            mocked_check_output.assert_called_once_with({expected},
-                                                        stderr=subprocess.STDOUT)
+            mocked_check_output.assert_has_calls([{expected_build_call},
+                                                  {expected_run_call}])
             assert stdout == b"Some dummy output"
         """,
         dummy_config,
@@ -190,13 +200,17 @@ def test_loading_run_component(mocker, run_component):
 @pytest.mark.parametrize("memory, expected_bytes", [(None, None), (6, 6442450944)])
 @pytest.mark.parametrize("cpu", [None, 2])
 @pytest.mark.parametrize(
-    "config_fixture, expected",
+    "config_fixture, expected_build_cmd, expected_run_cmd, expected_prefix",
     [
         (
             "dummy_config",
-            '["viash", "run", Path(meta["config"]), "--", "bar"%s%s]',
+            '["viash", "run", Path(meta["config"]), "-p", "docker", "-c", '
+            '".platforms[.type == \'docker\'].target_tag := \'test\'", "--", "---setup", "cachedbuild"]',
+            '["viash", "run", Path(meta["config"]), "-p", "docker", '
+            '"-c", ".platforms[.type == \'docker\'].target_tag := \'test\'"%s%s, "--", "bar"]',
+            "--",
         ),
-        ("dummy_config_with_info", '[Path("foo"), "bar"%s%s]'),
+        ("dummy_config_with_info", "", '[Path("foo"), "bar"%s%s]', "---"),
     ],
 )
 def test_run_component_executes_subprocess(
@@ -207,13 +221,23 @@ def test_run_component_executes_subprocess(
     expected_bytes,
     cpu,
     config_fixture,
-    expected,
+    expected_build_cmd,
+    expected_run_cmd,
+    expected_prefix,
 ):
     format_string = (
-        f', "---cpus", "{cpu}"' if cpu else "",
-        f', "---memory", "{expected_bytes}B"' if memory else "",
+        f', "{expected_prefix}cpus", "{cpu}"' if cpu else "",
+        f', "{expected_prefix}memory", "{expected_bytes}B"' if memory else "",
+    )
+    expected_run_cmd = expected_run_cmd % format_string
+    expected_build_cmd_call, excpected_run_cmd_call = "", ""
+    if expected_build_cmd:
+        expected_build_cmd_call = f"mocker.call({expected_build_cmd}, stderr=-2)"
+    if expected_run_cmd:
+        excpected_run_cmd_call = f"mocker.call({expected_run_cmd}, stderr=-2)"
+    expected = ", ".join(
+        filter(None, [expected_build_cmd_call, excpected_run_cmd_call])
     )
-    expected = expected % format_string
     makepyfile_and_add_meta(
         f"""
         import subprocess
@@ -224,8 +248,7 @@ def test_loading_run_component(mocker, run_component):
                                                return_value=b"Some dummy output")
             mocked_path = mocker.patch('viashpy.testing.Path.is_file', return_value=True)
             stdout = run_component(["bar"])
-            mocked_check_output.assert_called_once_with({expected},
-                                                        stderr=subprocess.STDOUT)
+            mocked_check_output.assert_has_calls([{expected}])
             assert stdout == b"Some dummy output"
         """,
         request.getfixturevalue(config_fixture),

tox.ini

@@ -2,7 +2,7 @@
 [tox]
 envlist = 
     py{39}-pytest{62}
-    py{39,310,311,312}-pytest{70,71,72,73,74}
+    py{39,310,311,312}-pytest{70,71,72,73,74,80}
     flake8
 isolated_build = True
 
@@ -14,6 +14,7 @@ deps =
     pytest72: pytest>=7.2,<7.3
     pytest73: pytest>=7.3,<7.4
     pytest74: pytest>=7.4,<7.5
+    pytest80: pytest>=8.0,<8.1
     coverage
     pytest-mock
 commands = 

viashpy/_run.py

@@ -2,6 +2,10 @@
 from subprocess import check_output, STDOUT, DEVNULL, PIPE
 from pathlib import Path
 from typing import Any
+from .types import Platform
+import logging
+
+logger = logging.getLogger(__name__)
 
 
 class ToBytesConverter:
@@ -59,11 +63,9 @@ def run_build_component(
         raise FileNotFoundError(
             f"{executable_location} does not exist or is not a file."
         )
-    return check_output(
-        [executable_location] + args + _format_cpu_and_memory(cpus, memory),
-        stderr=stderr,
-        **popen_kwargs,
-    )
+    full_command = [executable_location] + args + _format_cpu_and_memory(cpus, memory)
+    logger.debug("Running '%s'", " ".join(map(str, full_command)))
+    return check_output(full_command, stderr=stderr, **popen_kwargs)
 
 
 def viash_run(
@@ -72,17 +74,44 @@ def viash_run(
     *,
     cpus: int | None = None,
     memory: str | None = None,
+    platform: Platform = "docker",
     viash_location: str | Path = "viash",
     stderr: STDOUT | PIPE | DEVNULL | -1 | -2 | -3 = STDOUT,
     **popen_kwargs,
 ):
     config = Path(config)
     if not config.is_file():
         raise FileNotFoundError(f"{config} does not exist or is not a file.")
-    return check_output(
-        [viash_location, "run", config, "--"]
+    args = ["--"] + args
+    if platform == "docker":
+        build_args = [
+            viash_location,
+            "run",
+            config,
+            "-p",
+            "docker",
+            "-c",
+            ".platforms[.type == 'docker'].target_tag := 'test'",
+            "--",
+            "---setup",
+            "cachedbuild",
+        ]
+        logger.debug("Building docker image: %s", " ".join(map(str, build_args)))
+        check_output(
+            build_args, stderr=stderr, **popen_kwargs
+        )  # CalledProcessError should be handled by caller
+    full_command = (
+        [
+            viash_location,
+            "run",
+            config,
+            "-p",
+            platform,
+            "-c",
+            ".platforms[.type == 'docker'].target_tag := 'test'",
+        ]
+        + _format_cpu_and_memory(cpus, memory, "--")
         + args
-        + _format_cpu_and_memory(cpus, memory),
-        stderr=stderr,
-        **popen_kwargs,
     )
+    logger.debug("Running '%s'", " ".join(map(str, full_command)))
+    return check_output(full_command, stderr=stderr, **popen_kwargs)

viashpy/testing.py

@@ -1,6 +1,7 @@
 import pytest
 import logging
 from ._run import run_build_component, viash_run, tobytesconverter
+from .types import Platform
 from .config import read_viash_config
 from pathlib import Path
 from functools import wraps
@@ -183,13 +184,14 @@ def wrapper(*args, **kwargs):
     if viash_source_config_path.is_file():
 
         @run_and_handle_errors
-        def wrapper(args_as_list):
+        def wrapper(args_as_list, platform: Platform = "docker"):
             return viash_run(
                 viash_source_config_path,
                 args_as_list,
                 viash_location=viash_executable,
                 cpus=cpus,
                 memory=memory_bytes,
+                platform=platform,
             )
 
         return wrapper

viashpy/types.py

@@ -0,0 +1,3 @@
+from typing import Literal
+
+Platform = Literal["native", "docker", "nextflow"]

viashpy/utils.py

@@ -20,8 +20,7 @@ def extract_tar(pathname: Path | str, output_dir: Path | str):
     if not pathname.is_file():
         raise FileNotFoundError(f"{pathname} does not exist or is not a file.")
 
-    # TODO: is_tarfile supports a path from python >= 3.9
-    if not tarfile.is_tarfile(str(pathname)):
+    if not tarfile.is_tarfile(pathname):
         raise ValueError(f"{pathname} is not a tarfile.")
 
     if not output_dir.is_dir():