required outputs with secondaryFiles fix for containers (#1870)

* required outputs with secondaryFiles fix for containers

* improve error message

* leave no files behind

Author: mr-c

cwltool/command_line_tool.py

@@ -1454,10 +1454,18 @@ def collect_output(
                                         continue
                                     if isinstance(sfitem, str):
                                         sfitem = {"path": pathprefix + sfitem}
-                                    if not fs_access.exists(sfitem["path"]) and sf_required:
+                                    original_sfitem = copy.deepcopy(sfitem)
+                                    if (
+                                        not fs_access.exists(
+                                            cast(
+                                                str, cast(CWLObjectType, revmap(sfitem))["location"]
+                                            )
+                                        )
+                                        and sf_required
+                                    ):
                                         raise WorkflowException(
                                             "Missing required secondary file '%s'"
-                                            % (sfitem["path"])
+                                            % (original_sfitem["path"])
                                         )
                                     if "path" in sfitem and "location" not in sfitem:
                                         revmap(sfitem)

tests/secondary-files-required-container.cwl

@@ -0,0 +1,20 @@
+#!/usr/bin/env cwl-runner
+cwlVersion: v1.2
+class: CommandLineTool
+
+hints:
+  DockerRequirement:
+    dockerPull: docker.io/alpine:latest
+
+inputs: []
+
+baseCommand: [ touch, file.ext1, file.ext2 ]
+
+outputs:
+  output:
+    type: File
+    secondaryFiles:
+      - pattern: ^.ext2
+        required: true
+    outputBinding:
+      glob: file.ext1

tests/secondary-files-required-missing-container.cwl

@@ -0,0 +1,20 @@
+#!/usr/bin/env cwl-runner
+cwlVersion: v1.2
+class: CommandLineTool
+
+hints:
+  DockerRequirement:
+    dockerPull: docker.io/alpine:latest
+
+inputs: []
+
+baseCommand: [ touch, file.ext1, file.ext2 ]
+
+outputs:
+  output:
+    type: File
+    secondaryFiles:
+      - pattern: ^.ext3
+        required: true
+    outputBinding:
+      glob: file.ext1

tests/test_docker.py

@@ -1,11 +1,20 @@
 """Tests for docker engine."""
+import json
 import re
 from pathlib import Path
 from shutil import which
 
+import pytest
+
 from cwltool.main import main
 
-from .util import get_data, get_main_output, needs_docker
+from .util import (
+    get_data,
+    get_main_output,
+    needs_docker,
+    needs_podman,
+    needs_singularity,
+)
 
 
 @needs_docker
@@ -136,3 +145,121 @@ def test_docker_strict_memory_limit_warning(tmp_path: Path) -> None:
     stderr = re.sub(r"\s\s+", " ", stderr)
     assert result_code == 0
     assert "Skipping Docker software container '--memory' limit" in stderr
+
+
+@needs_docker
+def test_docker_required_secfile(tmp_path: Path) -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--outdir",
+            str(tmp_path),
+            get_data("tests/secondary-files-required-container.cwl"),
+        ]
+    )
+    assert result_code == 0, stderr
+    assert (
+        json.loads(stdout)["output"]["secondaryFiles"][0]["checksum"]
+        == "sha1$da39a3ee5e6b4b0d3255bfef95601890afd80709"
+    )
+
+
+@needs_podman
+def test_podman_required_secfile(tmp_path: Path) -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--podman",
+            "--outdir",
+            str(tmp_path),
+            get_data("tests/secondary-files-required-container.cwl"),
+        ]
+    )
+    assert result_code == 0, stderr
+    assert (
+        json.loads(stdout)["output"]["secondaryFiles"][0]["checksum"]
+        == "sha1$da39a3ee5e6b4b0d3255bfef95601890afd80709"
+    )
+
+
+@needs_singularity
+def test_singularity_required_secfile(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    singularity_dir = tmp_path / "singularity"
+    singularity_dir.mkdir()
+    monkeypatch.setenv("CWL_SINGULARITY_CACHE", str(singularity_dir))
+
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--singularity",
+            "--outdir",
+            str(tmp_path / "out"),
+            get_data("tests/secondary-files-required-container.cwl"),
+        ]
+    )
+    assert result_code == 0, stderr
+    assert (
+        json.loads(stdout)["output"]["secondaryFiles"][0]["checksum"]
+        == "sha1$da39a3ee5e6b4b0d3255bfef95601890afd80709"
+    )
+
+
+@needs_docker
+def test_docker_required_missing_secfile(tmp_path: Path) -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--outdir",
+            str(tmp_path),
+            get_data("tests/secondary-files-required-missing-container.cwl"),
+        ]
+    )
+    assert result_code == 1, stderr
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "Job error:" in stderr
+    assert "Error collecting output for parameter 'output'" in stderr
+    assert (
+        "tests/secondary-files-required-missing-container.cwl:16:5: Missing required secondary file"
+    )
+    assert "file.ext3" in stderr
+
+
+@needs_podman
+def test_podman_required_missing_secfile(tmp_path: Path) -> None:
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--podman",
+            "--outdir",
+            str(tmp_path),
+            get_data("tests/secondary-files-required-missing-container.cwl"),
+        ]
+    )
+    assert result_code == 1, stderr
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "Job error:" in stderr
+    assert "Error collecting output for parameter 'output'" in stderr
+    assert (
+        "tests/secondary-files-required-missing-container.cwl:16:5: Missing required secondary file"
+    )
+    assert "file.ext3" in stderr
+
+
+@needs_singularity
+def test_singularity_required_missing_secfile(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    singularity_dir = tmp_path / "singularity"
+    singularity_dir.mkdir()
+    monkeypatch.setenv("CWL_SINGULARITY_CACHE", str(singularity_dir))
+    result_code, stdout, stderr = get_main_output(
+        [
+            "--singularity",
+            "--outdir",
+            str(tmp_path),
+            get_data("tests/secondary-files-required-missing-container.cwl"),
+        ]
+    )
+    assert result_code == 1, stderr
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "Job error:" in stderr
+    assert "Error collecting output for parameter 'output'" in stderr
+    assert (
+        "tests/secondary-files-required-missing-container.cwl:16:5: Missing required secondary file"
+    )
+    assert "file.ext3" in stderr