MPI: move hints/reqs + DockerRequirement checking into Process._init_job()

Move cache tests to their own file and add a new test

Author: mr-c

cwltool/command_line_tool.py

@@ -431,28 +431,6 @@ def make_job_runner(self, runtimeContext: RuntimeContext) -> type[JobBase]:
                 return SingularityCommandLineJob
             elif runtimeContext.user_space_docker_cmd:
                 return UDockerCommandLineJob
-            if mpiReq is not None:
-                if mpiRequired:
-                    if dockerRequired:
-                        raise UnsupportedRequirement(
-                            "No support for Docker and MPIRequirement both being required"
-                        )
-                    else:
-                        _logger.warning(
-                            "MPI has been required while Docker is hinted, discarding Docker hint(s)"
-                        )
-                        self.hints = [h for h in self.hints if h["class"] != "DockerRequirement"]
-                        return CommandLineJob
-                else:
-                    if dockerRequired:
-                        _logger.warning(
-                            "Docker has been required while MPI is hinted, discarding MPI hint(s)"
-                        )
-                        self.hints = [h for h in self.hints if h["class"] != MPIRequirementName]
-                    else:
-                        raise UnsupportedRequirement(
-                            "Both Docker and MPI have been hinted - don't know what to do"
-                        )
             if runtimeContext.podman:
                 return PodmanCommandLineJob
             return DockerCommandLineJob

cwltool/process.py

@@ -810,12 +810,47 @@ def inc(d: list[int]) -> None:
         tmpdir = ""
         stagedir = ""
 
-        docker_req, _ = self.get_requirement("DockerRequirement")
+        docker_req, docker_required = self.get_requirement("DockerRequirement")
         default_docker = None
+        mpi_req, mpi_required = self.get_requirement(MPIRequirementName)
 
         if docker_req is None and runtime_context.default_container:
             default_docker = runtime_context.default_container
 
+        if (
+            docker_req is not None
+            and runtime_context.use_container
+            and not runtime_context.singularity
+            and not runtime_context.user_space_docker_cmd
+            and mpi_req is not None
+        ):
+            if mpi_required:
+                if docker_required:
+                    raise UnsupportedRequirement(
+                        "No support for DockerRequirement and MPIRequirement "
+                        "both being required, unless Singularity or uDocker is being used."
+                    )
+                else:
+                    _logger.warning(
+                        "MPI has been required while DockerRequirement is hinted "
+                        "and neither Singularity nor uDocker is being used, discarding Docker hint(s)."
+                    )
+                    self.hints = [h for h in self.hints if h["class"] != "DockerRequirement"]
+                    docker_req = None
+                    docker_required = False
+            else:
+                if docker_required:
+                    _logger.warning(
+                        "Docker has been required (and neither Singularity nor "
+                        "uDocker is being used) while MPI is hinted, discarding MPI hint(s)/"
+                    )
+                    self.hints = [h for h in self.hints if h["class"] != MPIRequirementName]
+                else:
+                    raise UnsupportedRequirement(
+                        "Both Docker and MPI have been hinted and neither "
+                        "Singularity nor uDocker are being used - don't know what to do."
+                    )
+
         if (docker_req or default_docker) and runtime_context.use_container:
             if docker_req is not None:
                 # Check if docker output directory is absolute

tests/test_caching.py

@@ -8,27 +8,6 @@
 test_factors = [(""), ("--parallel"), ("--debug"), ("--parallel --debug")]
 
 
-@needs_docker
-@pytest.mark.parametrize("factor", test_factors)
-def test_cid_file_non_existing_dir(tmp_path: Path, factor: str) -> None:
-    """Test that --cachedir with a bad path should produce a specific error."""
-    test_file = "cache_test_workflow.cwl"
-    bad_cidfile_dir = tmp_path / "cidfile-dir-badpath"
-    commands = factor.split()
-    commands.extend(
-        [
-            "--record-container-id",
-            "--cidfile-dir",
-            str(bad_cidfile_dir),
-            get_data("tests/wf/" + test_file),
-        ]
-    )
-    error_code, _, stderr = get_main_output(commands)
-    stderr = re.sub(r"\s\s+", " ", stderr)
-    assert "directory doesn't exist, please create it first" in stderr, stderr
-    assert error_code == 2 or error_code == 1, stderr
-
-
 @needs_docker
 @pytest.mark.parametrize("factor", test_factors)
 def test_wf_without_container(tmp_path: Path, factor: str) -> None:

tests/test_environment.py

@@ -204,7 +204,7 @@ def test_basic(crt_params: CheckHolder, tmp_path: Path, monkeypatch: pytest.Monk
         "USEDVAR": "VARVAL",
         "UNUSEDVAR": "VARVAL",
     }
-    args = crt_params.flags + [f"--tmpdir-prefix={tmp_prefix}"]
+    args = crt_params.flags + [f"--tmpdir-prefix={tmp_prefix}", "--debug"]
     env = get_tool_env(
         tmp_path,
         args,

tests/test_examples.py

@@ -1115,6 +1115,27 @@ def test_cid_file_dir_arg_is_file_instead_of_dir(tmp_path: Path, factor: str) ->
     assert error_code == 2 or error_code == 1, stderr
 
 
+@needs_docker
+@pytest.mark.parametrize("factor", test_factors)
+def test_cid_file_non_existing_dir(tmp_path: Path, factor: str) -> None:
+    """Test that --cidefile-dir with a bad path should produce a specific error."""
+    test_file = "cache_test_workflow.cwl"
+    bad_cidfile_dir = tmp_path / "cidfile-dir-badpath"
+    commands = factor.split()
+    commands.extend(
+        [
+            "--record-container-id",
+            "--cidfile-dir",
+            str(bad_cidfile_dir),
+            get_data("tests/wf/" + test_file),
+        ]
+    )
+    error_code, _, stderr = get_main_output(commands)
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "directory doesn't exist, please create it first" in stderr, stderr
+    assert error_code == 2 or error_code == 1, stderr
+
+
 @needs_docker
 @pytest.mark.parametrize("factor", test_factors)
 def test_cid_file_w_prefix(tmp_path: Path, factor: str) -> None:

tests/test_mpi.py

@@ -7,18 +7,19 @@
 from importlib.resources import files
 from io import StringIO
 from pathlib import Path
-from typing import Any, Optional
+from typing import Any, Optional, cast
 
 import pytest
 from ruamel.yaml.comments import CommentedMap, CommentedSeq
 from schema_salad.avro.schema import Names
+from schema_salad.ref_resolver import file_uri
 from schema_salad.utils import yaml_no_ts
 
 import cwltool.load_tool
 import cwltool.singularity
 import cwltool.udocker
 from cwltool.command_line_tool import CommandLineTool
-from cwltool.context import LoadingContext, RuntimeContext
+from cwltool.context import RuntimeContext
 from cwltool.main import main
 from cwltool.mpi import MpiConfig, MPIRequirementName
 
@@ -292,15 +293,22 @@ def schema_ext11() -> Generator[Names, None, None]:
 
 mpiReq = CommentedMap({"class": MPIRequirementName, "processes": 1})
 containerReq = CommentedMap({"class": "DockerRequirement"})
-basetool = CommentedMap({"cwlVersion": "v1.1", "inputs": CommentedSeq(), "outputs": CommentedSeq()})
+basetool = CommentedMap(
+    {
+        "cwlVersion": "v1.1",
+        "class": "CommandLineTool",
+        "inputs": CommentedSeq(),
+        "outputs": CommentedSeq(),
+    }
+)
 
 
 def mk_tool(
     schema: Names,
     opts: list[str],
     reqs: Optional[list[CommentedMap]] = None,
     hints: Optional[list[CommentedMap]] = None,
-) -> tuple[LoadingContext, RuntimeContext, CommentedMap]:
+) -> tuple[RuntimeContext, CommandLineTool]:
     tool = basetool.copy()
 
     if reqs is not None:
@@ -310,53 +318,57 @@ def mk_tool(
 
     args = cwltool.argparser.arg_parser().parse_args(opts)
     args.enable_ext = True
+    args.basedir = os.path.dirname(os.path.abspath("."))
     rc = RuntimeContext(vars(args))
     lc = cwltool.main.setup_loadingContext(None, rc, args)
     lc.avsc_names = schema
-    return lc, rc, tool
+    tool["id"] = file_uri(os.path.abspath("./mktool.cwl"))
+    assert lc.loader is not None
+    lc.loader.idx[tool["id"]] = tool
+    return rc, cast(CommandLineTool, cwltool.load_tool.load_tool(tool, lc))
 
 
 def test_singularity(schema_ext11: Names) -> None:
-    lc, rc, tool = mk_tool(schema_ext11, ["--singularity"], reqs=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, ["--singularity"], reqs=[mpiReq, containerReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.singularity.SingularityCommandLineJob
 
 
 def test_udocker(schema_ext11: Names) -> None:
-    lc, rc, tool = mk_tool(schema_ext11, ["--udocker"], reqs=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, ["--udocker"], reqs=[mpiReq, containerReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.udocker.UDockerCommandLineJob
 
 
 def test_docker_hint(schema_ext11: Names) -> None:
     # Docker hint, MPI required
-    lc, rc, tool = mk_tool(schema_ext11, [], hints=[containerReq], reqs=[mpiReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], hints=[containerReq], reqs=[mpiReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.job.CommandLineJob
 
 
 def test_docker_required(schema_ext11: Names) -> None:
     # Docker required, MPI hinted
-    lc, rc, tool = mk_tool(schema_ext11, [], reqs=[containerReq], hints=[mpiReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], reqs=[containerReq], hints=[mpiReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.docker.DockerCommandLineJob
 
 
 def test_docker_mpi_both_required(schema_ext11: Names) -> None:
     # Both required - error
-    lc, rc, tool = mk_tool(schema_ext11, [], reqs=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], reqs=[mpiReq, containerReq])
     with pytest.raises(cwltool.errors.UnsupportedRequirement):
-        clt.make_job_runner(rc)
+        clt._init_job({}, rc)
+    clt.make_job_runner(rc)
 
 
 def test_docker_mpi_both_hinted(schema_ext11: Names) -> None:
     # Both hinted - error
-    lc, rc, tool = mk_tool(schema_ext11, [], hints=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], hints=[mpiReq, containerReq])
     with pytest.raises(cwltool.errors.UnsupportedRequirement):
-        clt.make_job_runner(rc)
+        clt._init_job({}, rc)
+    clt.make_job_runner(rc)