20933: Fix tests

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py
index 11ae66b1398a12ea0879f001fce811617dd06331..ea7c9f7a33eee04849ae338388e1f998f5a1918b 100644 (file)
--- a/sdk/cwl/arvados_cwl/arvcontainer.py
+++ b/sdk/cwl/arvados_cwl/arvcontainer.py
@@ -560,7 +560,7 @@ class RunnerContainer(Runner):
                 }
                 self.job_order[param] = {"$include": mnt}
 
-        container_image = arvados_jobs_image(self.arvrunner, self.jobs_image, runtimeContext),
+        container_image = arvados_jobs_image(self.arvrunner, self.jobs_image, runtimeContext)
 
         workflow_runner_req, _ = self.embedded_tool.get_requirement("http://arvados.org/cwl#WorkflowRunnerResources")
         if workflow_runner_req and workflow_runner_req.get("acrContainerImage"):

diff --git a/sdk/cwl/arvados_cwl/arvworkflow.py b/sdk/cwl/arvados_cwl/arvworkflow.py
index cdce3d643a337bb28f43c7124233fbcb094853ea..3ad2c6419a2b9ce39c11b6471e3dac6555c317ef 100644 (file)
--- a/sdk/cwl/arvados_cwl/arvworkflow.py
+++ b/sdk/cwl/arvados_cwl/arvworkflow.py
@@ -42,6 +42,7 @@ from .pathmapper import ArvPathMapper, trim_listing
 from .arvtool import ArvadosCommandTool, set_cluster_target
 from ._version import __version__
 from .util import common_prefix
+from .arvdocker import arv_docker_get_image
 
 from .perf import Perf
 
@@ -178,14 +179,14 @@ def rel_ref(s, baseuri, urlexpander, merged_map, jobmapper):
 def is_basetype(tp):
     return _basetype_re.match(tp) is not None
 
-def update_refs(d, baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix):
+def update_refs(api, d, baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix):
     if isinstance(d, MutableSequence):
         for i, s in enumerate(d):
             if prefix and isinstance(s, str):
                 if s.startswith(prefix):
                     d[i] = replacePrefix+s[len(prefix):]
             else:
-                update_refs(s, baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix)
+                update_refs(api, s, baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix)
     elif isinstance(d, MutableMapping):
         for field in ("id", "name"):
             if isinstance(d.get(field), str) and d[field].startswith("_:"):
@@ -198,8 +199,8 @@ def update_refs(d, baseuri, urlexpander, merged_map, jobmapper, runtimeContext,
             baseuri = urlexpander(d["name"], baseuri, scoped_id=True)
 
         if d.get("class") == "DockerRequirement":
-            dockerImageId = d.get("dockerImageId") or d.get("dockerPull")
-            d["http://arvados.org/cwl#dockerCollectionPDH"] = runtimeContext.cached_docker_lookups.get(dockerImageId)
+            d["http://arvados.org/cwl#dockerCollectionPDH"] = arv_docker_get_image(api, d, False,
+                                                                                   runtimeContext)
 
         for field in d:
             if field in ("location", "run", "name") and isinstance(d[field], str):
@@ -222,15 +223,21 @@ def update_refs(d, baseuri, urlexpander, merged_map, jobmapper, runtimeContext,
                     if isinstance(d["inputs"][inp], str) and not is_basetype(d["inputs"][inp]):
                         d["inputs"][inp] = rel_ref(d["inputs"][inp], baseuri, urlexpander, merged_map, jobmapper)
                     if isinstance(d["inputs"][inp], MutableMapping):
-                        update_refs(d["inputs"][inp], baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix)
+                        update_refs(api, d["inputs"][inp], baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix)
                 continue
 
+            if field in ("requirements", "hints") and isinstance(d[field], MutableMapping):
+                dr = d[field].get("DockerRequirement")
+                if dr:
+                    dr["http://arvados.org/cwl#dockerCollectionPDH"] = arv_docker_get_image(api, dr, False,
+                                                                                            runtimeContext)
+
             if field == "$schemas":
                 for n, s in enumerate(d["$schemas"]):
                     d["$schemas"][n] = rel_ref(d["$schemas"][n], baseuri, urlexpander, merged_map, jobmapper)
                 continue
 
-            update_refs(d[field], baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix)
+            update_refs(api, d[field], baseuri, urlexpander, merged_map, jobmapper, runtimeContext, prefix, replacePrefix)
 
 
 def fix_schemadef(req, baseuri, urlexpander, merged_map, jobmapper, pdh):
@@ -327,7 +334,7 @@ def upload_workflow(arvRunner, tool, job_order, project_uuid,
 
         # 2. find $import, $include, $schema, run, location
         # 3. update field value
-        update_refs(result, w, tool.doc_loader.expand_url, merged_map, jobmapper, runtimeContext, "", "")
+        update_refs(arvRunner.api, result, w, tool.doc_loader.expand_url, merged_map, jobmapper, runtimeContext, "", "")
 
         # Write the updated file to the collection.
         with col.open(w[len(prefix):], "wt") as f:
@@ -485,7 +492,7 @@ def upload_workflow(arvRunner, tool, job_order, project_uuid,
         if r["class"] == "SchemaDefRequirement":
             wrapper["requirements"][i] = fix_schemadef(r, main["id"], tool.doc_loader.expand_url, merged_map, jobmapper, col.portable_data_hash())
 
-    update_refs(wrapper, main["id"], tool.doc_loader.expand_url, merged_map, jobmapper, runtimeContext, main["id"]+"#", "#main/")
+    update_refs(arvRunner.api, wrapper, main["id"], tool.doc_loader.expand_url, merged_map, jobmapper, runtimeContext, main["id"]+"#", "#main/")
 
     doc = {"cwlVersion": "v1.2", "$graph": [wrapper]}
 

diff --git a/sdk/cwl/setup.py b/sdk/cwl/setup.py
index 92f0952af22c7bd214990a92d90e938bdcef9b0a..1be2c119c2ff4c455a27e093597af206ee2c24e6 100644 (file)
--- a/sdk/cwl/setup.py
+++ b/sdk/cwl/setup.py
@@ -43,7 +43,8 @@ setup(name='arvados-cwl-runner',
           'networkx < 2.6',
           'msgpack==1.0.3',
           'importlib-metadata<5',
-          'setuptools>=40.3.0'
+          'setuptools>=40.3.0',
+          'zipp<3.16.0'
       ],
       data_files=[
           ('share/doc/arvados-cwl-runner', ['LICENSE-2.0.txt', 'README.rst']),

diff --git a/sdk/cwl/tests/test_container.py b/sdk/cwl/tests/test_container.py
index a2f404d7ebe9b24b3d726d6057be14848b088cdd..8e3a8ab85e66e70ff76f4a8357e262ec543b084c 100644 (file)
--- a/sdk/cwl/tests/test_container.py
+++ b/sdk/cwl/tests/test_container.py
@@ -85,7 +85,8 @@ class TestContainer(unittest.TestCase):
              "construct_tool_object": runner.arv_make_tool,
              "fetcher_constructor": functools.partial(arvados_cwl.CollectionFetcher, api_client=runner.api, fs_access=fs_access),
              "loader": Loader({}),
-             "metadata": cmap({"cwlVersion": INTERNAL_VERSION, "http://commonwl.org/cwltool#original_cwlVersion": "v1.0"})
+             "metadata": cmap({"cwlVersion": INTERNAL_VERSION, "http://commonwl.org/cwltool#original_cwlVersion": "v1.0"}),
+             "default_docker_image": "arvados/jobs:"+arvados_cwl.__version__
              })
         runtimeContext = arvados_cwl.context.ArvRuntimeContext(
             {"work_api": "containers",
@@ -1463,7 +1464,8 @@ class TestWorkflow(unittest.TestCase):
              "make_fs_access": make_fs_access,
              "loader": document_loader,
              "metadata": {"cwlVersion": INTERNAL_VERSION, "http://commonwl.org/cwltool#original_cwlVersion": "v1.0"},
-             "construct_tool_object": runner.arv_make_tool})
+             "construct_tool_object": runner.arv_make_tool,
+             "default_docker_image": "arvados/jobs:"+arvados_cwl.__version__})
         runtimeContext = arvados_cwl.context.ArvRuntimeContext(
             {"work_api": "containers",
              "basedir": "",

diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py
index 9dad245254c50cfac4df2f2734bd41fe59f1ab61..b10d1159f99d96c4b97d90586437c9946a932b56 100644 (file)
--- a/sdk/cwl/tests/test_submit.py
+++ b/sdk/cwl/tests/test_submit.py
@@ -1180,7 +1180,7 @@ class TestSubmit(unittest.TestCase):
                                         "out": [
                                             {"id": "#main/step/out"}
                                         ],
-                                        "run": "keep:7628e49da34b93de9f4baf08a6212817+247/secret_wf.cwl"
+                                        "run": "keep:991302581d01db470345a131480e623b+247/secret_wf.cwl"
                                     }
                                 ]
                             }
@@ -1737,3 +1737,40 @@ class TestCreateWorkflow(unittest.TestCase):
         self.assertEqual(stubs.capture_stdout.getvalue(),
                          stubs.expect_workflow_uuid + '\n')
         self.assertEqual(exited, 0)
+
+    @stubs()
+    def test_create_map(self, stubs):
+        # test uploading a document that uses objects instead of arrays
+        # for certain fields like inputs and requirements.
+
+        project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
+
+        exited = arvados_cwl.main(
+            ["--create-workflow", "--debug",
+             "--api=containers",
+             "--project-uuid", project_uuid,
+             "--disable-git",
+             "tests/wf/submit_wf_map.cwl", "tests/submit_test_job.json"],
+            stubs.capture_stdout, sys.stderr, api_client=stubs.api)
+
+        stubs.api.pipeline_templates().create.refute_called()
+        stubs.api.container_requests().create.refute_called()
+
+        expect_workflow = StripYAMLComments(
+            open("tests/wf/expect_upload_wrapper_map.cwl").read().rstrip())
+
+        body = {
+            "workflow": {
+                "owner_uuid": project_uuid,
+                "name": "submit_wf_map.cwl",
+                "description": "",
+                "definition": expect_workflow,
+            }
+        }
+        stubs.api.workflows().create.assert_called_with(
+            body=JsonDiffMatcher(body))
+
+        self.assertEqual(stubs.capture_stdout.getvalue(),
+                         stubs.expect_workflow_uuid + '\n')
+        self.assertEqual(exited, 0)

diff --git a/sdk/cwl/tests/tool/submit_tool_map.cwl b/sdk/cwl/tests/tool/submit_tool_map.cwl
new file mode 100644 (file)
index 0000000..7a833d4
--- /dev/null
+++ b/sdk/cwl/tests/tool/submit_tool_map.cwl
@@ -0,0 +1,24 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Test case for arvados-cwl-runner
+#
+# Used to test whether scanning a tool file for dependencies (e.g. default
+# value blub.txt) and uploading to Keep works as intended.
+
+class: CommandLineTool
+cwlVersion: v1.0
+requirements:
+  DockerRequirement:
+    dockerPull: debian:buster-slim
+inputs:
+  x:
+    type: File
+    default:
+      class: File
+      location: blub.txt
+    inputBinding:
+      position: 1
+outputs: []
+baseCommand: cat

diff --git a/sdk/cwl/tests/wf/expect_upload_wrapper_map.cwl b/sdk/cwl/tests/wf/expect_upload_wrapper_map.cwl
new file mode 100644 (file)
index 0000000..8f98f47
--- /dev/null
+++ b/sdk/cwl/tests/wf/expect_upload_wrapper_map.cwl
@@ -0,0 +1,88 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+{
+    "$graph": [
+        {
+            "class": "Workflow",
+            "hints": [
+                {
+                    "acrContainerImage": "999999999999999999999999999999d3+99",
+                    "class": "http://arvados.org/cwl#WorkflowRunnerResources"
+                }
+            ],
+            "id": "#main",
+            "inputs": [
+                {
+                    "default": {
+                        "basename": "blorp.txt",
+                        "class": "File",
+                        "location": "keep:169f39d466a5438ac4a90e779bf750c7+53/blorp.txt",
+                        "nameext": ".txt",
+                        "nameroot": "blorp",
+                        "size": 16
+                    },
+                    "id": "#main/x",
+                    "type": "File"
+                },
+                {
+                    "default": {
+                        "basename": "99999999999999999999999999999998+99",
+                        "class": "Directory",
+                        "location": "keep:99999999999999999999999999999998+99"
+                    },
+                    "id": "#main/y",
+                    "type": "Directory"
+                },
+                {
+                    "default": {
+                        "basename": "anonymous",
+                        "class": "Directory",
+                        "listing": [
+                            {
+                                "basename": "renamed.txt",
+                                "class": "File",
+                                "location": "keep:99999999999999999999999999999998+99/file1.txt",
+                                "nameext": ".txt",
+                                "nameroot": "renamed",
+                                "size": 0
+                            }
+                        ]
+                    },
+                    "id": "#main/z",
+                    "type": "Directory"
+                }
+            ],
+            "outputs": [],
+            "requirements": [
+                {
+                    "class": "SubworkflowFeatureRequirement"
+                }
+            ],
+            "steps": [
+                {
+                    "id": "#main/submit_wf_map.cwl",
+                    "in": [
+                        {
+                            "id": "#main/step/x",
+                            "source": "#main/x"
+                        },
+                        {
+                            "id": "#main/step/y",
+                            "source": "#main/y"
+                        },
+                        {
+                            "id": "#main/step/z",
+                            "source": "#main/z"
+                        }
+                    ],
+                    "label": "submit_wf_map.cwl",
+                    "out": [],
+                    "run": "keep:2b94b65162db72023301a582e085646f+290/wf/submit_wf_map.cwl"
+                }
+            ]
+        }
+    ],
+    "cwlVersion": "v1.2"
+}

diff --git a/sdk/cwl/tests/wf/submit_wf_map.cwl b/sdk/cwl/tests/wf/submit_wf_map.cwl
new file mode 100644 (file)
index 0000000..e8bb9cf
--- /dev/null
+++ b/sdk/cwl/tests/wf/submit_wf_map.cwl
@@ -0,0 +1,25 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Test case for arvados-cwl-runner
+#
+# Used to test whether scanning a workflow file for dependencies
+# (e.g. submit_tool.cwl) and uploading to Keep works as intended.
+
+class: Workflow
+cwlVersion: v1.2
+inputs:
+  x:
+    type: File
+  y:
+    type: Directory
+  z:
+    type: Directory
+outputs: []
+steps:
+  step1:
+    in:
+      x: x
+    out: []
+    run: ../tool/submit_tool_map.cwl