From 01b9a4242fc8c41a105c6e052ad78aa1fbfe9699 Mon Sep 17 00:00:00 2001 From: Stella-Maria Renucci Date: Fri, 14 Nov 2025 13:19:33 +0100 Subject: [PATCH 1/4] dev: issue#2163, added minmax resourceReq validation --- cwltool/checker.py | 13 ++++++++++++ cwltool/command_line_tool.py | 5 +++++ cwltool/process.py | 2 ++ cwltool/workflow.py | 11 +++++++++- tests/test_examples.py | 14 +++++++++++++ tests/test_validate.py | 16 +++++++++++++++ tests/wf/bad_resreq_mnmx_clt.cwl | 20 ++++++++++++++++++ tests/wf/bad_resreq_mnmx_wf.cwl | 35 ++++++++++++++++++++++++++++++++ 8 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/wf/bad_resreq_mnmx_clt.cwl create mode 100755 tests/wf/bad_resreq_mnmx_wf.cwl diff --git a/cwltool/checker.py b/cwltool/checker.py index d81642e1d..a57d484d8 100644 --- a/cwltool/checker.py +++ b/cwltool/checker.py @@ -557,3 +557,16 @@ def loop_checker(steps: Iterator[MutableMapping[str, Any]]) -> None: ) if exceptions: raise ValidationException("\n".join(exceptions)) + + +def resreq_minmax_checker(requirement: CWLObjectType): + """ + Check if the minResource is lesser than the maxResource in resource requirements. + + :raises ValidationException: If the minResource is greater than the maxResource. + """ + for a in ("cores", "ram", "tmpdir", "outdir"): + mn: int | float | None = requirement.get(a + "Min") + mx: int | float | None = requirement.get(a + "Max") + if mn is not None and mx is not None and mx < mn: + raise ValidationException(f"{a}Min cannot be greater than {a}Max.") diff --git a/cwltool/command_line_tool.py b/cwltool/command_line_tool.py index 54913b47f..3c6aeb2c8 100644 --- a/cwltool/command_line_tool.py +++ b/cwltool/command_line_tool.py @@ -39,6 +39,7 @@ content_limit_respected_read_bytes, substitute, ) +from .checker import resreq_minmax_checker from .context import LoadingContext, RuntimeContext, getdefault from .docker import DockerCommandLineJob, PodmanCommandLineJob from .errors import UnsupportedRequirement, WorkflowException @@ -410,6 +411,10 @@ def __init__(self, toolpath_object: CommentedMap, loadingContext: LoadingContext else PathCheckingMode.STRICT ) + resource_req = self.get_requirement("ResourceRequirement")[0] + if getdefault(loadingContext.do_validate, True) and resource_req: + resreq_minmax_checker(resource_req) + def make_job_runner(self, runtimeContext: RuntimeContext) -> type[JobBase]: """Return the correct CommandLineJob class given the container settings.""" dockerReq, dockerRequired = self.get_requirement("DockerRequirement") diff --git a/cwltool/process.py b/cwltool/process.py index 75069b859..2594385f6 100644 --- a/cwltool/process.py +++ b/cwltool/process.py @@ -1028,6 +1028,8 @@ def evalResources( mx = mn if mn is not None: + if mx < mn: + raise ValidationException(f"{a}Min cannot be greater than {a}Max.") request[a + "Min"] = mn request[a + "Max"] = cast(Union[int, float], mx) diff --git a/cwltool/workflow.py b/cwltool/workflow.py index fe34c75ce..e28b58319 100644 --- a/cwltool/workflow.py +++ b/cwltool/workflow.py @@ -13,7 +13,12 @@ from schema_salad.sourceline import SourceLine, indent from . import command_line_tool, context, procgenerator -from .checker import circular_dependency_checker, loop_checker, static_checker +from .checker import ( + circular_dependency_checker, + loop_checker, + resreq_minmax_checker, + static_checker, +) from .context import LoadingContext, RuntimeContext, getdefault from .cwlprov.provenance_profile import ProvenanceProfile from .cwlprov.writablebagfile import create_job @@ -133,6 +138,10 @@ def __init__( circular_dependency_checker(step_inputs) loop_checker(step.tool for step in self.steps) + resource_req = self.get_requirement("ResourceRequirement")[0] + if resource_req: + resreq_minmax_checker(resource_req) + def make_workflow_step( self, toolpath_object: CommentedMap, diff --git a/tests/test_examples.py b/tests/test_examples.py index f371f45ae..ed0a1660a 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -1841,3 +1841,17 @@ def test_make_template() -> None: ] ) assert exit_code == 0, stderr + + +@pytest.mark.parametrize( + "file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"] +) +def test_invalid_resource_requirements(file): + """Ensure an error with an invalid resource requirement.""" + exit_code, stdout, stderr = get_main_output( + [ + get_data(file), + ] + ) + assert exit_code == 1 + assert "cannot be greater than" in stderr diff --git a/tests/test_validate.py b/tests/test_validate.py index 8e664d9aa..1b337e82f 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -3,6 +3,7 @@ import io import logging import re +import pytest from .util import get_data, get_main_output @@ -125,3 +126,18 @@ def test_validate_custom_logger() -> None: assert "tests/CometAdapter.cwl#out' previously defined" not in stdout assert "tests/CometAdapter.cwl#out' previously defined" not in stderr assert "tests/CometAdapter.cwl#out' previously defined" in custom_log_text + + +@pytest.mark.parametrize( + "file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"] +) +def test_validate_with_invalid_requirements(file) -> None: + """Ensure that --validate returns an error with an invalid resource requirement.""" + exit_code, stdout, stderr = get_main_output( + [ + "--validate", + get_data(file), + ] + ) + assert exit_code == 1 + assert "cannot be greater than" in stdout diff --git a/tests/wf/bad_resreq_mnmx_clt.cwl b/tests/wf/bad_resreq_mnmx_clt.cwl new file mode 100644 index 000000000..ec4379314 --- /dev/null +++ b/tests/wf/bad_resreq_mnmx_clt.cwl @@ -0,0 +1,20 @@ +#!/usr/bin/env cwl-runner +class: CommandLineTool +cwlVersion: v1.2 + +requirements: + ResourceRequirement: + coresMin: 4 + coresMax: 2 + +inputs: [] + +outputs: + output: + type: stdout + +baseCommand: echo + +stdout: cores.txt + +arguments: [ $(runtime.cores) ] diff --git a/tests/wf/bad_resreq_mnmx_wf.cwl b/tests/wf/bad_resreq_mnmx_wf.cwl new file mode 100755 index 000000000..76feb598f --- /dev/null +++ b/tests/wf/bad_resreq_mnmx_wf.cwl @@ -0,0 +1,35 @@ +#!/usr/bin/env cwl-runner + +cwlVersion: v1.0 +class: Workflow +inputs: + inp: File + ex: string + +requirements: + ResourceRequirement: + ramMin: 128 + ramMax: 64 + +outputs: + classout: + type: File + outputSource: argument/classfile + +steps: + untar: + run: tar-param.cwl + in: + tarfile: inp + extractfile: ex + out: [example_out] + requirements: + ResourceRequirement: + ramMin: 64 + ramMax: 128 + + argument: + run: arguments.cwl + in: + src: untar/example_out + out: [classfile] From a0a9dc31e8aea011babfc4b4bff6c9fa9cab92e1 Mon Sep 17 00:00:00 2001 From: Stella-Maria Renucci Date: Fri, 14 Nov 2025 13:34:27 +0100 Subject: [PATCH 2/4] fix: mypy --- cwltool/checker.py | 6 +++--- cwltool/process.py | 4 ++-- tests/test_examples.py | 2 +- tests/test_validate.py | 3 ++- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cwltool/checker.py b/cwltool/checker.py index a57d484d8..00e763047 100644 --- a/cwltool/checker.py +++ b/cwltool/checker.py @@ -559,14 +559,14 @@ def loop_checker(steps: Iterator[MutableMapping[str, Any]]) -> None: raise ValidationException("\n".join(exceptions)) -def resreq_minmax_checker(requirement: CWLObjectType): +def resreq_minmax_checker(requirement: CWLObjectType) -> None: """ Check if the minResource is lesser than the maxResource in resource requirements. :raises ValidationException: If the minResource is greater than the maxResource. """ for a in ("cores", "ram", "tmpdir", "outdir"): - mn: int | float | None = requirement.get(a + "Min") - mx: int | float | None = requirement.get(a + "Max") + mn: int | float | None = cast(Union[int, float], requirement.get(a + "Min")) + mx: int | float | None = cast(Union[int, float], requirement.get(a + "Max")) if mn is not None and mx is not None and mx < mn: raise ValidationException(f"{a}Min cannot be greater than {a}Max.") diff --git a/cwltool/process.py b/cwltool/process.py index 2594385f6..cf35a149a 100644 --- a/cwltool/process.py +++ b/cwltool/process.py @@ -1027,11 +1027,11 @@ def evalResources( elif mx is None: mx = mn - if mn is not None: + if mn is not None and mx is not None: if mx < mn: raise ValidationException(f"{a}Min cannot be greater than {a}Max.") request[a + "Min"] = mn - request[a + "Max"] = cast(Union[int, float], mx) + request[a + "Max"] = mx request_evaluated = cast(dict[str, Union[int, float]], request) if runtimeContext.select_resources is not None: diff --git a/tests/test_examples.py b/tests/test_examples.py index ed0a1660a..0b9ca3452 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -1846,7 +1846,7 @@ def test_make_template() -> None: @pytest.mark.parametrize( "file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"] ) -def test_invalid_resource_requirements(file): +def test_invalid_resource_requirements(file: str) -> None: """Ensure an error with an invalid resource requirement.""" exit_code, stdout, stderr = get_main_output( [ diff --git a/tests/test_validate.py b/tests/test_validate.py index 1b337e82f..6d9b20fba 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -3,6 +3,7 @@ import io import logging import re + import pytest from .util import get_data, get_main_output @@ -131,7 +132,7 @@ def test_validate_custom_logger() -> None: @pytest.mark.parametrize( "file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"] ) -def test_validate_with_invalid_requirements(file) -> None: +def test_validate_with_invalid_requirements(file: str) -> None: """Ensure that --validate returns an error with an invalid resource requirement.""" exit_code, stdout, stderr = get_main_output( [ From 5b7dd55ef4373572a55b93602938042018c0b703 Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" <1330696+mr-c@users.noreply.github.com> Date: Mon, 17 Nov 2025 14:44:17 +0100 Subject: [PATCH 3/4] Apply suggestions from code review --- cwltool/checker.py | 4 ++-- tests/test_examples.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cwltool/checker.py b/cwltool/checker.py index 00e763047..6e5822ce8 100644 --- a/cwltool/checker.py +++ b/cwltool/checker.py @@ -566,7 +566,7 @@ def resreq_minmax_checker(requirement: CWLObjectType) -> None: :raises ValidationException: If the minResource is greater than the maxResource. """ for a in ("cores", "ram", "tmpdir", "outdir"): - mn: int | float | None = cast(Union[int, float], requirement.get(a + "Min")) - mx: int | float | None = cast(Union[int, float], requirement.get(a + "Max")) + mn = cast(Union[int, float, None], requirement.get(a + "Min")) + mx = cast(Union[int, float, None], requirement.get(a + "Max")) if mn is not None and mx is not None and mx < mn: raise ValidationException(f"{a}Min cannot be greater than {a}Max.") diff --git a/tests/test_examples.py b/tests/test_examples.py index 0b9ca3452..a0f1ad08a 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -1850,6 +1850,7 @@ def test_invalid_resource_requirements(file: str) -> None: """Ensure an error with an invalid resource requirement.""" exit_code, stdout, stderr = get_main_output( [ + "--disable-validate", get_data(file), ] ) From 877d538acee956d46d3beb305e7598b99223fc07 Mon Sep 17 00:00:00 2001 From: Stella-Maria Renucci Date: Wed, 19 Nov 2025 09:45:12 +0100 Subject: [PATCH 4/4] fix: removed inputs/outputs from CWL files to simplify them, focusing only on requirements --- tests/wf/bad_resreq_mnmx_clt.cwl | 10 ++------- tests/wf/bad_resreq_mnmx_wf.cwl | 36 ++++++++++++-------------------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/tests/wf/bad_resreq_mnmx_clt.cwl b/tests/wf/bad_resreq_mnmx_clt.cwl index ec4379314..50ca836e8 100644 --- a/tests/wf/bad_resreq_mnmx_clt.cwl +++ b/tests/wf/bad_resreq_mnmx_clt.cwl @@ -8,13 +8,7 @@ requirements: coresMax: 2 inputs: [] +outputs: [] -outputs: - output: - type: stdout +baseCommand: ["echo", "Hello World"] -baseCommand: echo - -stdout: cores.txt - -arguments: [ $(runtime.cores) ] diff --git a/tests/wf/bad_resreq_mnmx_wf.cwl b/tests/wf/bad_resreq_mnmx_wf.cwl index 76feb598f..64b1e1c9a 100755 --- a/tests/wf/bad_resreq_mnmx_wf.cwl +++ b/tests/wf/bad_resreq_mnmx_wf.cwl @@ -1,35 +1,25 @@ #!/usr/bin/env cwl-runner - -cwlVersion: v1.0 class: Workflow -inputs: - inp: File - ex: string +cwlVersion: v1.2 requirements: ResourceRequirement: ramMin: 128 ramMax: 64 -outputs: - classout: - type: File - outputSource: argument/classfile +inputs: [] +outputs: [] steps: - untar: - run: tar-param.cwl - in: - tarfile: inp - extractfile: ex - out: [example_out] + hello_world: requirements: ResourceRequirement: - ramMin: 64 - ramMax: 128 - - argument: - run: arguments.cwl - in: - src: untar/example_out - out: [classfile] + ramMin: 64 + ramMax: 128 + run: + class: CommandLineTool + baseCommand: [ "echo", "Hello World" ] + inputs: [ ] + outputs: [ ] + out: [ ] + in: [ ]