Merge pull request galaxyproject#16641 from mvdbeek/nested_subworkflo…

…w_skip [23.1] Fix nested conditional workflow steps
bgruening · Sep 5, 2023 · f4d52d7 · f4d52d7
2 parents 47628ef + 0df12cb
commit f4d52d7
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 8 deletions.
diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py
@@ -770,12 +770,15 @@ def execute(self, trans, progress, invocation_step, use_cached_job=False):
                 assert len(progress.when_values) == 1, "Got more than 1 when value, this shouldn't be possible"
             iteration_elements_iter = [(None, progress.when_values[0] if progress.when_values else None)]
 
-        when_values = []
-        if step.when_expression:
-            for iteration_elements, when_value in iteration_elements_iter:
-                if when_value is False:
-                    when_values.append(when_value)
-                    continue
+        when_values: List[Union[bool, None]] = []
+        for iteration_elements, when_value in iteration_elements_iter:
+            if when_value is False or not step.when_expression:
+                # We're skipping this step (when==False) or we keep
+                # the current when_value if there is no explicit when_expression on this step.
+                when_values.append(when_value)
+            else:
+                # Got a conditional step and we could potentially run it,
+                # so we have to build the step state and evaluate the expression
                 extra_step_state = {}
                 for step_input in step.inputs:
                     step_input_name = step_input.name
@@ -790,8 +793,8 @@ def execute(self, trans, progress, invocation_step, use_cached_job=False):
                         progress, step, execution_state={}, extra_step_state=extra_step_state
                     )
                 )
-            if collection_info:
-                collection_info.when_values = when_values
+        if collection_info:
+            collection_info.when_values = when_values
 
         subworkflow_invoker = progress.subworkflow_invoker(
             trans,

diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py
@@ -2157,6 +2157,84 @@ def test_run_workflow_subworkflow_conditional_step(self):
                 if step["workflow_step_label"] == "a_tool_step":
                     assert sum(1 for j in step["jobs"] if j["state"] == "skipped") == 1
 
+    def test_run_nested_conditional_workflow_steps(self):
+        with self.dataset_populator.test_history() as history_id:
+            summary = self._run_workflow(
+                """
+class: GalaxyWorkflow
+inputs:
+  dataset:
+    type: data
+  when:
+    type: boolean
+outputs:
+  output:
+    outputSource: outer_subworkflow/output
+steps:
+- label: outer_subworkflow
+  when: $(inputs.when)
+  in:
+    dataset:
+      source: dataset
+    when:
+      source: when
+  run:
+    class: GalaxyWorkflow
+    label: subworkflow cat1
+    inputs:
+      dataset:
+        type: data
+    outputs:
+      output:
+        outputSource: cat1_workflow/output
+    steps:
+    - label: cat1_workflow
+      in:
+        dataset:
+          source: dataset
+      run:
+        class: GalaxyWorkflow
+        label: cat1
+        inputs:
+          dataset:
+            type: data
+        outputs:
+          output:
+            outputSource: cat1/out_file1
+        steps:
+        - tool_id: cat1
+          label: cat1
+          in:
+            input1:
+              source: dataset
+""",
+                test_data="""
+dataset:
+  value: 1.bed
+  type: File
+when:
+  value: false
+  type: raw
+""",
+                history_id=history_id,
+                wait=True,
+                assert_ok=True,
+            )
+            invocation_details = self.workflow_populator.get_invocation(summary.invocation_id, step_details=True)
+            subworkflow_invocation_id = invocation_details["steps"][-1]["subworkflow_invocation_id"]
+            self.workflow_populator.wait_for_invocation_and_jobs(
+                history_id=history_id, workflow_id="whatever", invocation_id=subworkflow_invocation_id
+            )
+            invocation_details = self.workflow_populator.get_invocation(subworkflow_invocation_id, step_details=True)
+            subworkflow_invocation_id = invocation_details["steps"][-1]["subworkflow_invocation_id"]
+            self.workflow_populator.wait_for_invocation_and_jobs(
+                history_id=history_id, workflow_id="whatever", invocation_id=subworkflow_invocation_id
+            )
+            invocation_details = self.workflow_populator.get_invocation(subworkflow_invocation_id, step_details=True)
+            for step in invocation_details["steps"]:
+                if step["workflow_step_label"] == "cat1":
+                    assert sum(1 for j in step["jobs"] if j["state"] == "skipped") == 1
+
     def test_run_workflow_conditional_step_map_over_expression_tool(self):
         with self.dataset_populator.test_history() as history_id:
             summary = self._run_workflow(