Merge pull request #15177 from jmchilton/fixup_refactoring_types

Fixup types surrounding workflow refactoring testing.

mypy.ini

@@ -620,8 +620,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-integration.test_workflow_sync]
 check_untyped_defs = False
-[mypy-integration.test_workflow_refactoring]
-check_untyped_defs = False
 [mypy-integration.test_tool_data_delete]
 check_untyped_defs = False
 [mypy-integration.test_shed_tool_tests]
@@ -681,8 +679,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-*.workflows.test_modules]
 check_untyped_defs = False
-[mypy-*.workflows.test_refactor_models]
-check_untyped_defs = False
 [mypy-*.managers.test_HDAManager]
 check_untyped_defs = False
 [mypy-*.managers.test_DatasetManager]

test/integration/test_workflow_refactoring.py

@@ -1,5 +1,10 @@
 import contextlib
 import json
+from typing import (
+    Any,
+    Dict,
+    List,
+)
 
 from galaxy.managers.context import ProvidesAppContext
 from galaxy.managers.workflows import RefactorRequest
@@ -38,6 +43,9 @@
       input1: test_input
 """
 
+ActionJson = Dict[str, Any]
+ActionsJson = List[ActionJson]
+
 
 class TestWorkflowRefactoringIntegration(integration_util.IntegrationTestCase, UsesShedApi):
 
@@ -50,7 +58,7 @@ def setUp(self):
     def test_basic_refactoring_types(self):
         self.workflow_populator.upload_yaml_workflow(REFACTORING_SIMPLE_TEST)
 
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "update_name", "name": "my cool new name"},
         ]
         self._refactor(actions)
@@ -187,7 +195,7 @@ def test_basic_refactoring_types(self):
     def test_basic_refactoring_types_dry_run(self):
         self.workflow_populator.upload_yaml_workflow(REFACTORING_SIMPLE_TEST)
 
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "update_name", "name": "my cool new name"},
         ]
         response = self._dry_run(actions)
@@ -247,7 +255,7 @@ def test_basic_refactoring_types_dry_run(self):
     def test_refactoring_legacy_parameters(self):
         wf = self.workflow_populator.load_workflow_from_resource("test_workflow_randomlines_legacy_params")
         self.workflow_populator.create_workflow(wf)
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "extract_untyped_parameter", "name": "seed"},
             {"action_type": "extract_untyped_parameter", "name": "num", "label": "renamed_num"},
         ]
@@ -302,7 +310,7 @@ def test_refactoring_legacy_parameters_without_tool_state(self):
         rename: "${pja_only_param} name"
 """
         )
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "extract_untyped_parameter", "name": "pja_only_param"},
         ]
         self._refactor(actions)
@@ -325,7 +333,7 @@ def test_refactoring_legacy_parameters_without_tool_state_dry_run(self):
         rename: "${pja_only_param} name"
 """
         )
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "extract_untyped_parameter", "name": "pja_only_param"},
         ]
         response = self._dry_run(actions)
@@ -351,7 +359,7 @@ def test_refactoring_legacy_parameters_without_tool_state_relabel(self):
         rename: "${pja_only_param} name"
 """
         )
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "extract_untyped_parameter", "name": "pja_only_param", "label": "new_label"},
         ]
         self._refactor(actions)
@@ -367,7 +375,7 @@ def test_removing_unlabeled_workflow_outputs(self):
         self.workflow_populator.create_workflow(wf)
         only_step = self._latest_workflow.step_by_index(0)
         assert len(only_step.workflow_outputs) == 1
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "remove_unlabeled_workflow_outputs"},
         ]
         self._refactor(actions)
@@ -396,7 +404,7 @@ def test_fill_defaults_option(self):
 
     def test_refactor_works_with_subworkflows(self):
         self.workflow_populator.upload_yaml_workflow(WORKFLOW_NESTED_SIMPLE)
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "update_step_label", "step": {"label": "nested_workflow"}, "label": "new_nested_workflow"},
         ]
         self._refactor(actions)
@@ -429,7 +437,7 @@ def test_refactor_works_with_missing_tools(self):
         assert self._latest_workflow.step_by_index(1).tool_id == "random-missing"
         assert "num_lines" in self._latest_workflow.step_by_index(1).tool_inputs
 
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "update_step_label", "step": {"order_index": 1}, "label": "random2_new"},
         ]
         self._refactor(actions)
@@ -438,7 +446,7 @@ def test_refactor_works_with_missing_tools(self):
 
     def test_refactor_fill_step_defaults(self):
         self._load_two_random_lines_wf_with_missing_state()
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "fill_step_defaults", "step": {"order_index": 0}},
         ]
         action_executions = self._refactor(actions).action_executions
@@ -458,7 +466,7 @@ def test_refactor_fill_step_defaults(self):
 
     def test_refactor_fill_step_defaults_dry_run(self):
         self._load_two_random_lines_wf_with_missing_state()
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "fill_step_defaults", "step": {"order_index": 0}},
         ]
         response = self._dry_run(actions)
@@ -481,7 +489,7 @@ def test_refactor_fill_step_defaults_dry_run(self):
 
     def test_refactor_fill_defaults(self):
         self._load_two_random_lines_wf_with_missing_state()
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "fill_defaults"},
         ]
         action_executions = self._refactor(actions).action_executions
@@ -516,7 +524,7 @@ def test_tool_version_upgrade_no_state_change(self):
 """
         )
         assert self._latest_workflow.step_by_label("the_step").tool_version == "0.1"
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "upgrade_tool", "step": {"label": "the_step"}},
         ]
         # t = self._app.toolbox.get_tool("multiple_versions", tool_version="0.1")
@@ -540,7 +548,7 @@ def test_tool_version_upgrade_state_added(self):
 """
         )
         assert self._latest_workflow.step_by_label("the_step").tool_version == "0.1"
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "upgrade_tool", "step": {"label": "the_step"}, "tool_version": "0.2"},
         ]
         action_executions = self._refactor(actions).action_executions
@@ -575,7 +583,7 @@ def test_subworkflow_upgrade_simplest(self):
         pre_upgrade_native = self._download_native(self._most_recent_stored_workflow)
         self._assert_nested_workflow_num_lines_is(pre_upgrade_native, "1")
 
-        actions = [
+        actions: ActionsJson = [
             {"action_type": "upgrade_subworkflow", "step": {"label": "nested_workflow"}},
         ]
         response = self._dry_run(actions)
@@ -603,7 +611,7 @@ def test_subworkflow_upgrade_specified(self):
         nested_stored_workflow = self._recent_stored_workflow(2)
         assert len(nested_stored_workflow.workflows) == 3
         middle_workflow_id = self._app.security.encode_id(nested_stored_workflow.workflows[1].id)
-        actions = [
+        actions: ActionsJson = [
             {
                 "action_type": "upgrade_subworkflow",
                 "step": {"label": "nested_workflow"},
@@ -758,7 +766,7 @@ def _export_for_update(self, workflow):
         with self.workflow_populator.export_for_update(workflow_id) as workflow_object:
             yield workflow_object
 
-    def _refactor(self, actions, stored_workflow=None, dry_run=False, style="ga"):
+    def _refactor(self, actions: List[Dict[str, Any]], stored_workflow=None, dry_run=False, style="ga"):
         user = self._app.model.session.query(User).order_by(User.id.desc()).limit(1).one()
         mock_trans = MockTrans(self._app, user)
 
@@ -776,9 +784,9 @@ def url_for(*args, **kwd):
                 RefactorRequest(actions=actions, dry_run=dry_run, style=style),
             )
         finally:
-            app = url_for = original_url_for
+            app.url_for = original_url_for
 
-    def _dry_run(self, actions, stored_workflow=None):
+    def _dry_run(self, actions: ActionsJson, stored_workflow=None):
         # Do a bunch of checks to ensure nothing workflow related was written to the database
         # or even added to the sa_session.
         sa_session = self._app.model.session
@@ -887,3 +895,7 @@ def __init__(self, app, user):
     @property
     def app(self):
         return self._app
+
+    @property
+    def url_builder(self) -> None:
+        return None

test/unit/workflows/test_refactor_models.py

@@ -1,6 +1,17 @@
 from galaxy.workflow.refactor.schema import (
+    AddInputAction,
+    AddStepAction,
+    ConnectAction,
+    DisconnectAction,
+    ExtractInputAction,
+    ExtractUntypedParameter,
+    InputReferenceByLabel,
+    InputReferenceByOrderIndex,
     RefactorActionExecution,
     RefactorActions,
+    StepReferenceByLabel,
+    StepReferenceByOrderIndex,
+    UpdateStepLabelAction,
 )
 
 
@@ -27,41 +38,75 @@ def test_root_list():
     actions = ar.actions
 
     a0 = actions[0]
+    assert a0.action_type == "add_step"
+    assert isinstance(a0, AddStepAction)
+    assert a0.tool_state
     assert a0.tool_state["a"] == 6
     assert a0.label == "foobar"
 
     a1 = actions[1]
+    assert a1.action_type == "update_step_label"
+    assert isinstance(a1, UpdateStepLabelAction)
+    assert isinstance(a1.step, StepReferenceByOrderIndex)
     assert a1.step.order_index == 5
+
     a2 = actions[2]
+    assert isinstance(a2, UpdateStepLabelAction)
+    assert isinstance(a2.step, StepReferenceByLabel)
     assert a2.step.label == "cool_label"
 
     a3 = actions[3]
+    assert a3.action_type == "connect"
+    assert isinstance(a3, ConnectAction)
     # Verify it sets default output_name
     assert a3.output.output_name == "output"
     assert a3.input.input_name == "foobar"
 
     a4 = actions[4]
+    assert isinstance(a4, DisconnectAction)
     assert a4.output.output_name == "o_name"
+    assert isinstance(a4.input, InputReferenceByLabel)
     assert a4.input.input_name == "foobar2"
     assert a4.input.label == "foolabel"
 
     a5 = actions[5]
+    assert isinstance(
+        a5,
+        AddInputAction,
+    )
     assert a5.type == "data"
     assert a5.optional is False
 
     a6 = actions[6]
+    assert isinstance(
+        a6,
+        AddInputAction,
+    )
     assert a6.optional is True
     assert a6.default == 5
 
     a7 = actions[7]
+    assert isinstance(
+        a7,
+        ExtractInputAction,
+    )
+    assert isinstance(a7.input, InputReferenceByOrderIndex)
     assert a7.input.order_index == 5
     assert a7.input.input_name == "foobar"
 
     a8 = actions[8]
+    assert isinstance(
+        a8,
+        ExtractUntypedParameter,
+    )
     assert a8.name == "foo"
     assert a8.label is None
 
     a9 = actions[9]
+    assert isinstance(
+        a9,
+        ExtractUntypedParameter,
+    )
     assert a9.name == "foo"
     assert a9.label == "new_foo"