From 22709b416c7024f7c9a25e70bcd7351b9a1a5152 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 12:26:04 -0400 Subject: [PATCH 1/8] Abstraction for finding functional test tools. --- lib/galaxy/app_unittest_utils/tools_support.py | 8 ++++++-- lib/galaxy/tool_util/unittest_utils/__init__.py | 7 +++++++ test/unit/tool_util/test_parsing.py | 3 ++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/app_unittest_utils/tools_support.py b/lib/galaxy/app_unittest_utils/tools_support.py index e29b96dc0299..9af461601a83 100644 --- a/lib/galaxy/app_unittest_utils/tools_support.py +++ b/lib/galaxy/app_unittest_utils/tools_support.py @@ -106,9 +106,13 @@ def setup_app(self): self.app.config.tool_secret = "testsecret" self.app.config.track_jobs_in_database = False - def __setup_tool(self): + @property + def tool_source(self): tool_source = get_tool_source(self.tool_file) - self.tool = create_tool_from_source(self.app, tool_source, config_file=self.tool_file) + return tool_source + + def __setup_tool(self): + self.tool = create_tool_from_source(self.app, self.tool_source, config_file=self.tool_file) if getattr(self, "tool_action", None): self.tool.tool_action = self.tool_action return self.tool diff --git a/lib/galaxy/tool_util/unittest_utils/__init__.py b/lib/galaxy/tool_util/unittest_utils/__init__.py index d9814f09d578..d179aff465bc 100644 --- a/lib/galaxy/tool_util/unittest_utils/__init__.py +++ b/lib/galaxy/tool_util/unittest_utils/__init__.py @@ -1,3 +1,4 @@ +import os from typing import ( Callable, Dict, @@ -6,6 +7,8 @@ ) from unittest.mock import Mock +from galaxy.util import galaxy_directory + def mock_trans(has_user=True, is_admin=False): """A mock ``trans`` object for exposing user info to toolbox filter unit tests.""" @@ -26,3 +29,7 @@ def get_content(filename: Optional[str]) -> bytes: return content return get_content + + +def functional_test_tool_path(test_path: str) -> str: + return os.path.join(galaxy_directory(), "test/functional/tools", test_path) diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index 4832c7b73d9b..af630961287c 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -18,6 +18,7 @@ ToolOutputCollection, ToolOutputDataset, ) +from galaxy.tool_util.unittest_utils import functional_test_tool_path from galaxy.util import galaxy_directory from galaxy.util.unittest import TestCase @@ -719,7 +720,7 @@ class FunctionalTestToolTestCase(BaseLoaderTestCase): source_contents: None def _get_source_file_name(self) -> str: - return os.path.join(galaxy_directory(), "test/functional/tools", self.test_path) + return functional_test_tool_path(self.test_path) class TestExpressionTestToolLoader(FunctionalTestToolTestCase): From ebb5026c4cc017698ff634186d025952ea8bcf0b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 12:43:30 -0400 Subject: [PATCH 2/8] Test cases for tool test parsing. --- test/unit/app/tools/test_test_parsing.py | 51 ++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 test/unit/app/tools/test_test_parsing.py diff --git a/test/unit/app/tools/test_test_parsing.py b/test/unit/app/tools/test_test_parsing.py new file mode 100644 index 000000000000..00391e647176 --- /dev/null +++ b/test/unit/app/tools/test_test_parsing.py @@ -0,0 +1,51 @@ +""" Test Tool testing logic. + +I am going to migrate from using galaxy.tools.parameters and Galaxy Tool internals to +tool sources and I want to ensure the results do not change. +""" + +from typing import ( + Any, + List, +) + +from galaxy.app_unittest_utils import tools_support +from galaxy.tool_util.unittest_utils import functional_test_tool_path +from galaxy.tools.test import parse_tests +from galaxy.util.unittest import TestCase + +# Not the whole response, just some keys and such to test... +SIMPLE_CONSTRUCTS_EXPECTATIONS = [ + (["inputs", "p1|p1use"], [True]), + (["inputs", "booltest"], [True]), + (["inputs", "inttest"], ["12456"]), + (["inputs", "files_0|file"], ["simple_line.txt"]), + (["outputs", 0, "name"], "out_file1"), +] + + +class TestTestParsing(TestCase, tools_support.UsesTools): + tool_action: "MockAction" + + def setUp(self): + self.setup_app() + + def tearDown(self): + self.tear_down_app() + + def test_state_parsing(self): + self._init_tool_for_path(functional_test_tool_path("simple_constructs.xml")) + test_dicts = parse_tests(self.tool, self.tool_source) + self._verify_each(test_dicts[0].to_dict(), SIMPLE_CONSTRUCTS_EXPECTATIONS) + print(test_dicts[0].to_dict()) + assert False + + def _verify_each(self, target_dict: dict, expectations: List[Any]): + for path, expectation in expectations: + self._verify(target_dict, path, expectation) + + def _verify(self, target_dict: dict, expectation_path: List[str], expectation: Any): + rest = target_dict + for path_part in expectation_path: + rest = rest[path_part] + assert rest == expectation From 9bbce88b39485fbc5d73cdf754a8b4ad8730539f Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 13:45:12 -0400 Subject: [PATCH 3/8] Migrates some tool parsing to galaxy.tool_util.parser... ... small functions but they ensure we do the same things in and out of Galaxy. --- lib/galaxy/tool_util/parser/util.py | 64 ++++++++++++++++++++++++++++ lib/galaxy/tools/__init__.py | 15 +++---- lib/galaxy/tools/parameters/basic.py | 30 +++++-------- 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/lib/galaxy/tool_util/parser/util.py b/lib/galaxy/tool_util/parser/util.py index 46cd012809a0..f9d09e457af7 100644 --- a/lib/galaxy/tool_util/parser/util.py +++ b/lib/galaxy/tool_util/parser/util.py @@ -1,4 +1,18 @@ from collections import OrderedDict +from typing import ( + Optional, + Tuple, + TYPE_CHECKING, + Union, +) + +from packaging.version import Version + +if TYPE_CHECKING: + from .interface import ( + InputSource, + ToolSource, + ) DEFAULT_DELTA = 10000 DEFAULT_DELTA_FRAC = None @@ -23,3 +37,53 @@ def _parse_name(name, argument): raise ValueError("parameter must specify a 'name' or 'argument'.") name = argument.lstrip("-").replace("-", "_") return name + + +def parse_profile_version(tool_source: "ToolSource") -> float: + return float(tool_source.parse_profile()) + + +def parse_tool_version_with_defaults( + id: Optional[str], tool_source: "ToolSource", profile: Optional[Version] = None +) -> str: + if profile is None: + profile = Version(tool_source.parse_profile()) + + version = tool_source.parse_version() + if not version: + if profile < Version("16.04"): + # For backward compatibility, some tools may not have versions yet. + version = "1.0.0" + else: + raise Exception(f"Missing tool 'version' for tool with id '{id}' at '{tool_source}'") + return version + + +def boolean_is_checked(input_source: "InputSource"): + nullable = input_source.get_bool("optional", False) + return input_source.get_bool("checked", None if nullable else False) + + +def boolean_true_and_false_values(input_source, profile: Optional[Union[float, str]] = None) -> Tuple[str, str]: + truevalue = input_source.get("truevalue", "true") + falsevalue = input_source.get("falsevalue", "false") + if profile and Version(str(profile)) >= Version("23.1"): + if truevalue == falsevalue: + raise ParameterParseException("Cannot set true and false to the same value") + if truevalue.lower() == "false": + raise ParameterParseException( + f"Cannot set truevalue to [{truevalue}], Galaxy state may encounter issues distinguishing booleans and strings in this case." + ) + if falsevalue.lower() == "true": + raise ParameterParseException( + f"Cannot set falsevalue to [{falsevalue}], Galaxy state may encounter issues distinguishing booleans and strings in this case." + ) + return (truevalue, falsevalue) + + +class ParameterParseException(Exception): + message: str + + def __init__(self, message): + super().__init__(message) + self.message = message diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 554275aaf6c4..91bf85374dc2 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -81,6 +81,10 @@ PageSource, ToolSource, ) +from galaxy.tool_util.parser.util import ( + parse_profile_version, + parse_tool_version_with_defaults, +) from galaxy.tool_util.parser.xml import ( XmlPageSource, XmlToolSource, @@ -1006,7 +1010,7 @@ def parse(self, tool_source: ToolSource, guid=None, dynamic=False): """ Read tool configuration from the element `root` and fill in `self`. """ - self.profile = float(tool_source.parse_profile()) + self.profile = parse_profile_version(tool_source) # Get the UNIQUE id for the tool self.old_id = tool_source.parse_id() if guid is None: @@ -1037,14 +1041,7 @@ def parse(self, tool_source: ToolSource, guid=None, dynamic=False): if not dynamic and not self.name: raise Exception(f"Missing tool 'name' for tool with id '{self.id}' at '{tool_source}'") - version = tool_source.parse_version() - if not version: - if profile < Version("16.04"): - # For backward compatibility, some tools may not have versions yet. - version = "1.0.0" - else: - raise Exception(f"Missing tool 'version' for tool with id '{self.id}' at '{tool_source}'") - + version = parse_tool_version_with_defaults(self.id, tool_source, profile) self.version = version # Legacy feature, ignored by UI. diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 9b5a8f5fde9c..d05b2a099399 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -42,6 +42,11 @@ from galaxy.model.dataset_collections import builder from galaxy.schema.fetch_data import FilesPayload from galaxy.tool_util.parser import get_input_source as ensure_input_source +from galaxy.tool_util.parser.util import ( + boolean_is_checked, + boolean_true_and_false_values, + ParameterParseException, +) from galaxy.tools.parameters.workflow_utils import workflow_building_modes from galaxy.util import ( sanitize_param, @@ -593,27 +598,14 @@ class BooleanToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) - truevalue = input_source.get("truevalue", "true") - falsevalue = input_source.get("falsevalue", "false") - if tool and Version(str(tool.profile)) >= Version("23.1"): - if truevalue == falsevalue: - raise ParameterValueError("Cannot set true and false to the same value", self.name) - if truevalue.lower() == "false": - raise ParameterValueError( - f"Cannot set truevalue to [{truevalue}], Galaxy state may encounter issues distinguishing booleans and strings in this case.", - self.name, - ) - if falsevalue.lower() == "true": - raise ParameterValueError( - f"Cannot set falsevalue to [{falsevalue}], Galaxy state may encounter issues distinguishing booleans and strings in this case.", - self.name, - ) - + try: + truevalue, falsevalue = boolean_true_and_false_values(input_source, tool and tool.profile) + except ParameterParseException as ppe: + raise ParameterValueError(ppe.message, self.name) self.truevalue = truevalue self.falsevalue = falsevalue - nullable = input_source.get_bool("optional", False) - self.optional = nullable - self.checked = input_source.get_bool("checked", None if nullable else False) + self.optional = input_source.get_bool("optional", False) + self.checked = boolean_is_checked(input_source) def from_json(self, value, trans=None, other_values=None): return self.to_python(value) From 3ba28f2da7e1c44a4809aeb0ae6b0aff205bcc23 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 14:28:08 -0400 Subject: [PATCH 4/8] Improved tool source typing... --- lib/galaxy/tool_util/parser/cwl.py | 2 +- lib/galaxy/tool_util/parser/interface.py | 9 ++++++--- lib/galaxy/tool_util/parser/xml.py | 2 +- lib/galaxy/tool_util/parser/yaml.py | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tool_util/parser/cwl.py b/lib/galaxy/tool_util/parser/cwl.py index 45a4634ae82f..571a7faa4172 100644 --- a/lib/galaxy/tool_util/parser/cwl.py +++ b/lib/galaxy/tool_util/parser/cwl.py @@ -113,7 +113,7 @@ def parse_description(self): def parse_interactivetool(self): return [] - def parse_input_pages(self): + def parse_input_pages(self) -> PagesSource: page_source = CwlPageSource(self.tool_proxy) return PagesSource([page_source]) diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index f9d7f6440968..e049855d185d 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -42,10 +42,13 @@ class AssertionDict(TypedDict): AssertionList = Optional[List[AssertionDict]] XmlInt = Union[str, int] +ToolSourceTestInputs = Any +ToolSourceTestOutputs = Any + class ToolSourceTest(TypedDict): - inputs: Any - outputs: Any + inputs: ToolSourceTestInputs + outputs: ToolSourceTestOutputs output_collections: List[Any] stdout: AssertionList stderr: AssertionList @@ -245,7 +248,7 @@ def parse_requirements_and_containers( """Return triple of ToolRequirement, ContainerDescription and ResourceRequirement lists.""" @abstractmethod - def parse_input_pages(self): + def parse_input_pages(self) -> "PagesSource": """Return a PagesSource representing inputs by page for tool.""" def parse_provided_metadata_style(self): diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 00a8be56df77..d6ef4ac62d87 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -391,7 +391,7 @@ def parse_include_exclude_list(tag_name): def parse_requirements_and_containers(self): return requirements.parse_requirements_from_xml(self.root, parse_resources=True) - def parse_input_pages(self): + def parse_input_pages(self) -> "XmlPagesSource": return XmlPagesSource(self.root) def parse_provided_metadata_style(self): diff --git a/lib/galaxy/tool_util/parser/yaml.py b/lib/galaxy/tool_util/parser/yaml.py index 8e7eaffecb7c..5393b05b3953 100644 --- a/lib/galaxy/tool_util/parser/yaml.py +++ b/lib/galaxy/tool_util/parser/yaml.py @@ -110,7 +110,7 @@ def parse_requirements_and_containers(self): resource_requirements=[r for r in mixed_requirements if r.get("type") == "resource"], ) - def parse_input_pages(self): + def parse_input_pages(self) -> PagesSource: # All YAML tools have only one page (feature is deprecated) page_source = YamlPageSource(self.root_dict.get("inputs", {})) return PagesSource([page_source]) From 5e1b7ab52e4b7d3d61a889066b6d1a6a2c6b8af2 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 15:39:58 -0400 Subject: [PATCH 5/8] Bring tool source one iteration closer to XML decouple dynamic options. --- lib/galaxy/tool_util/parameters/factory.py | 6 ++++- lib/galaxy/tool_util/parser/interface.py | 26 ++++++++++++++++++++-- lib/galaxy/tool_util/parser/xml.py | 25 ++++++++++++++++----- lib/galaxy/tools/parameters/basic.py | 8 ++++--- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py index 0b28e7da3288..ea149e4b7668 100644 --- a/lib/galaxy/tool_util/parameters/factory.py +++ b/lib/galaxy/tool_util/parameters/factory.py @@ -130,7 +130,11 @@ def _from_input_source_galaxy(input_source: InputSource) -> ToolParameterT: # Function... example in devteam cummeRbund. optional = input_source.parse_optional() dynamic_options = input_source.get("dynamic_options", None) - dynamic_options_elem = input_source.parse_dynamic_options_elem() + dynamic_options_config = input_source.parse_dynamic_options() + if dynamic_options_config: + dynamic_options_elem = dynamic_options.elem() + else: + dynamic_options_elem = None multiple = input_source.get_bool("multiple", False) is_static = dynamic_options is None and dynamic_options_elem is None options: Optional[List[LabelValue]] = None diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index e049855d185d..809fe5f0c387 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -20,6 +20,7 @@ from pydantic import BaseModel from typing_extensions import TypedDict +from galaxy.util import Element from galaxy.util.path import safe_walk from .util import _parse_name @@ -362,6 +363,23 @@ def inputs_defined(self): return True +class DynamicOptions(metaclass=ABCMeta): + + def elem(self) -> Element: + # For things in transition that still depend on XML - provide a way + # to grab it and just throw an error if feature is attempted to be + # used with other tool sources. + raise NotImplementedError(NOT_IMPLEMENTED_MESSAGE) + + @abstractmethod + def get_data_table_name(self) -> Optional[str]: + """If dynamic options are loaded from a data table, return the name.""" + + @abstractmethod + def get_index_file_name(self) -> Optional[str]: + """If dynamic options are loaded from an index file, return the name.""" + + class InputSource(metaclass=ABCMeta): default_optional = False @@ -421,8 +439,12 @@ def parse_optional(self, default=None): default = self.default_optional return self.get_bool("optional", default) - def parse_dynamic_options_elem(self): - """Return an XML element describing dynamic options.""" + def parse_dynamic_options(self) -> Optional[DynamicOptions]: + """Return an optional element describing dynamic options. + + These options are still very XML based but as they are adapted to the infrastructure, the return + type here will evolve. + """ return None def parse_static_options(self) -> List[Tuple[str, str, bool]]: diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index d6ef4ac62d87..a1e0e86d9c19 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -34,6 +34,7 @@ from .interface import ( AssertionList, Citation, + DynamicOptions, InputSource, PageSource, PagesSource, @@ -1217,6 +1218,22 @@ def parse_input_sources(self): return list(map(XmlInputSource, self.parent_elem)) +class XmlDynamicOptions(DynamicOptions): + + def __init__(self, options_elem: Element): + self._options_elem = options_elem + + def elem(self) -> Element: + return self._options_elem + + def get_data_table_name(self) -> Optional[str]: + """If dynamic options are loaded from a data table, return the name.""" + return self._options_elem.get("from_data_table") + + def get_index_file_name(self) -> Optional[str]: + return self._options_elem.get("from_file") + + class XmlInputSource(InputSource): def __init__(self, input_elem): self.input_elem = input_elem @@ -1246,12 +1263,10 @@ def parse_sanitizer_elem(self): def parse_validator_elems(self): return self.input_elem.findall("validator") - def parse_dynamic_options_elem(self): - """Return a galaxy.tools.parameters.dynamic_options.DynamicOptions - if appropriate. - """ + def parse_dynamic_options(self) -> Optional[XmlDynamicOptions]: + """Return a XmlDynamicOptions to describe dynamic options if options elem is available.""" options_elem = self.input_elem.find("options") - return options_elem + return XmlDynamicOptions(options_elem) if options_elem is not None else None def parse_static_options(self) -> List[Tuple[str, str, bool]]: """ diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index d05b2a099399..c4053ae01bbc 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -118,9 +118,11 @@ def is_runtime_context(trans, other_values): def parse_dynamic_options(param, input_source): - if (options_elem := input_source.parse_dynamic_options_elem()) is not None: - return dynamic_options.DynamicOptions(options_elem, param) - return None + dynamic_options_config = input_source.parse_dynamic_options() + if not dynamic_options_config: + return None + options_elem = dynamic_options_config.elem() + return dynamic_options.DynamicOptions(options_elem, param) # Describe a parameter value error where there is no actual supplied From 90c6dcc590de81e91332c6c44bd3a02d412a868f Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 15:00:03 -0400 Subject: [PATCH 6/8] Option to parse tool test information from ToolSource instead of Tool internals. --- lib/galaxy/tool_util/verify/interactor.py | 6 +- lib/galaxy/tool_util/verify/parse.py | 496 ++++++++++++++++++++++ lib/galaxy/tools/test.py | 104 +---- test/unit/app/tools/test_test_parsing.py | 103 ++++- 4 files changed, 610 insertions(+), 99 deletions(-) create mode 100644 lib/galaxy/tool_util/verify/parse.py diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index b51e2a33ad5a..c31dba70175e 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -95,11 +95,11 @@ class ValidToolTestDict(TypedDict): output_collections: List[Dict[str, Any]] stdout: NotRequired[AssertionList] stderr: NotRequired[AssertionList] - expect_exit_code: NotRequired[int] + expect_exit_code: NotRequired[Optional[Union[str, int]]] expect_failure: NotRequired[bool] expect_test_failure: NotRequired[bool] - maxseconds: NotRequired[int] - num_outputs: NotRequired[int] + maxseconds: NotRequired[Optional[int]] + num_outputs: NotRequired[Optional[Union[str, int]]] command_line: NotRequired[AssertionList] command_version: NotRequired[AssertionList] required_files: NotRequired[List[Any]] diff --git a/lib/galaxy/tool_util/verify/parse.py b/lib/galaxy/tool_util/verify/parse.py new file mode 100644 index 000000000000..74ae8f3a1196 --- /dev/null +++ b/lib/galaxy/tool_util/verify/parse.py @@ -0,0 +1,496 @@ +import logging +import os +from typing import ( + Any, + Iterable, + List, + Optional, + Tuple, + Union, +) + +from galaxy.tool_util.parser.interface import ( + InputSource, + ToolSource, + ToolSourceTest, + ToolSourceTestInputs, + ToolSourceTests, +) +from galaxy.tool_util.parser.util import ( + boolean_is_checked, + boolean_true_and_false_values, + parse_tool_version_with_defaults, +) +from galaxy.tool_util.verify.interactor import ( + InvalidToolTestDict, + ToolTestDescription, + ValidToolTestDict, +) +from galaxy.util import ( + string_as_bool, + string_as_bool_or_none, + unicodify, +) + +log = logging.getLogger(__name__) + +RequiredFilesT = List[Tuple[str, dict]] +RequiredDataTablesT = List[str] +RequiredLocFileT = List[str] + + +def parse_tool_test_descriptions( + tool_source: ToolSource, tool_guid: Optional[str] = None +) -> Iterable[ToolTestDescription]: + """ + Build ToolTestDescription objects for each test description. + """ + raw_tests_dict: ToolSourceTests = tool_source.parse_tests_to_dict() + tests: List[ToolTestDescription] = [] + for i, raw_test_dict in enumerate(raw_tests_dict.get("tests", [])): + test = _description_from_tool_source(tool_source, raw_test_dict, i, tool_guid) + tests.append(test) + return tests + + +def _description_from_tool_source( + tool_source: ToolSource, raw_test_dict: ToolSourceTest, test_index: int, tool_guid: Optional[str] +) -> ToolTestDescription: + required_files: RequiredFilesT = [] + required_data_tables: RequiredDataTablesT = [] + required_loc_files: RequiredLocFileT = [] + + num_outputs = raw_test_dict.get("expect_num_outputs", None) + if num_outputs: + num_outputs = int(num_outputs) + maxseconds = raw_test_dict.get("maxseconds", None) + if maxseconds is not None: + maxseconds = int(maxseconds) + + tool_id = tool_guid or tool_source.parse_id() + assert tool_id + tool_version = parse_tool_version_with_defaults(tool_id, tool_source) + + processed_test_dict: Union[ValidToolTestDict, InvalidToolTestDict] + try: + processed_inputs = _process_raw_inputs( + tool_source, + input_sources(tool_source), + raw_test_dict["inputs"], + required_files, + required_data_tables, + required_loc_files, + ) + processed_test_dict = ValidToolTestDict( + { + "inputs": processed_inputs, + "outputs": raw_test_dict["outputs"], + "output_collections": raw_test_dict["output_collections"], + "num_outputs": num_outputs, + "command_line": raw_test_dict.get("command", None), + "command_version": raw_test_dict.get("command_version", None), + "stdout": raw_test_dict.get("stdout", None), + "stderr": raw_test_dict.get("stderr", None), + "expect_exit_code": raw_test_dict.get("expect_exit_code", None), + "expect_failure": raw_test_dict.get("expect_failure", False), + "expect_test_failure": raw_test_dict.get("expect_test_failure", False), + "required_files": required_files, + "required_data_tables": required_data_tables, + "required_loc_files": required_loc_files, + "tool_id": tool_id, + "tool_version": tool_version, + "test_index": test_index, + "maxseconds": maxseconds, + "error": False, + } + ) + except Exception as e: + processed_test_dict = InvalidToolTestDict( + { + "tool_id": tool_id, + "tool_version": tool_version, + "test_index": test_index, + "inputs": {}, + "error": True, + "exception": unicodify(e), + "maxseconds": maxseconds, + } + ) + + return ToolTestDescription(processed_test_dict) + + +def _process_raw_inputs( + tool_source: ToolSource, + input_sources: List[InputSource], + raw_inputs: ToolSourceTestInputs, + required_files: RequiredFilesT, + required_data_tables: RequiredDataTablesT, + required_loc_files: RequiredLocFileT, + parent_context: Optional[Union["ParamContext", "RootParamContext"]] = None, +): + """ + Recursively expand flat list of inputs into "tree" form of flat list + (| using to nest to new levels) structure and expand dataset + information as proceeding to populate self.required_files. + """ + parent_context = parent_context or RootParamContext() + expanded_inputs = {} + for input_source in input_sources: + input_type = input_source.parse_input_type() + name = input_source.parse_name() + if input_type == "conditional": + cond_context = ParamContext(name=name, parent_context=parent_context) + test_param_input_source = input_source.parse_test_input_source() + case_name = test_param_input_source.parse_name() + case_context = ParamContext(name=case_name, parent_context=cond_context) + raw_input_dict = case_context.extract_value(raw_inputs) + case_value = raw_input_dict["value"] if raw_input_dict else None + case_when, case_input_sources = _matching_case_for_value( + tool_source, input_source, test_param_input_source, case_value + ) + if case_input_sources: + for case_input_source in case_input_sources.parse_input_sources(): + case_inputs = _process_raw_inputs( + tool_source, + [case_input_source], + raw_inputs, + required_files, + required_data_tables, + required_loc_files, + parent_context=cond_context, + ) + expanded_inputs.update(case_inputs) + expanded_case_value = split_if_str(case_when) + if case_value is not None: + # A bit tricky here - we are growing inputs with value + # that may be implicit (i.e. not defined by user just + # a default defined in tool). So we do not want to grow + # expanded_inputs and risk repeat block viewing this + # as a new instance with value defined and hence enter + # an infinite loop - hence the "case_value is not None" + # check. + processed_value = _process_simple_value( + test_param_input_source, expanded_case_value, required_data_tables, required_loc_files + ) + expanded_inputs[case_context.for_state()] = processed_value + elif input_type == "section": + context = ParamContext(name=name, parent_context=parent_context) + page_source = input_source.parse_nested_inputs_source() + for section_input_source in page_source.parse_input_sources(): + expanded_input = _process_raw_inputs( + tool_source, + [section_input_source], + raw_inputs, + required_files, + required_data_tables, + required_loc_files, + parent_context=context, + ) + if expanded_input: + expanded_inputs.update(expanded_input) + elif input_type == "repeat": + repeat_index = 0 + while True: + context = ParamContext(name=name, index=repeat_index, parent_context=parent_context) + updated = False + page_source = input_source.parse_nested_inputs_source() + for r_value in page_source.parse_input_sources(): + expanded_input = _process_raw_inputs( + tool_source, + [r_value], + raw_inputs, + required_files, + required_data_tables, + required_loc_files, + parent_context=context, + ) + if expanded_input: + expanded_inputs.update(expanded_input) + updated = True + if not updated: + break + repeat_index += 1 + else: + context = ParamContext(name=name, parent_context=parent_context) + raw_input_dict = context.extract_value(raw_inputs) + param_type = input_source.get("type") + if raw_input_dict: + name = raw_input_dict["name"] + param_value = raw_input_dict["value"] + param_extra = raw_input_dict["attributes"] + location = param_extra.get("location") + if param_type != "text": + param_value = split_if_str(param_value) + if param_type == "data": + if location and input_source.get_bool("multiple", False): + # We get the input/s from the location which can be a list of urls separated by commas + locations = split_if_str(location) + param_value = [] + for location in locations: + v = os.path.basename(location) + param_value.append(v) + # param_extra should contain only the corresponding location + extra = dict(param_extra) + extra["location"] = location + _add_uploaded_dataset(context.for_state(), v, extra, input_source, required_files) + else: + if not isinstance(param_value, list): + param_value = [param_value] + for v in param_value: + _add_uploaded_dataset(context.for_state(), v, param_extra, input_source, required_files) + processed_value = param_value + elif param_type == "data_collection": + assert "collection" in param_extra + collection_def = param_extra["collection"] + for input_dict in collection_def.collect_inputs(): + name = input_dict["name"] + value = input_dict["value"] + attributes = input_dict["attributes"] + require_file(name, value, attributes, required_files) + processed_value = collection_def + else: + processed_value = _process_simple_value( + input_source, param_value, required_data_tables, required_loc_files + ) + expanded_inputs[context.for_state()] = processed_value + return expanded_inputs + + +def input_sources(tool_source: ToolSource) -> List[InputSource]: + input_sources = [] + pages_source = tool_source.parse_input_pages() + if pages_source.inputs_defined: + for page_source in pages_source.page_sources: + for input_source in page_source.parse_input_sources(): + input_sources.append(input_source) + return input_sources + + +class ParamContext: + def __init__(self, name, index=None, parent_context=None): + self.parent_context = parent_context + self.name = name + self.index = None if index is None else int(index) + + def for_state(self): + name = self.name if self.index is None else "%s_%d" % (self.name, self.index) + parent_for_state = self.parent_context.for_state() + if parent_for_state: + return f"{parent_for_state}|{name}" + else: + return name + + def __str__(self): + return f"Context[for_state={self.for_state()}]" + + def param_names(self): + for parent_context_param in self.parent_context.param_names(): + if self.index is not None: + yield "%s|%s_%d" % (parent_context_param, self.name, self.index) + else: + yield f"{parent_context_param}|{self.name}" + if self.index is not None: + yield "%s_%d" % (self.name, self.index) + else: + yield self.name + + def extract_value(self, raw_inputs): + for param_name in self.param_names(): + value = self.__raw_param_found(param_name, raw_inputs) + if value: + return value + return None + + def __raw_param_found(self, param_name, raw_inputs): + index = None + for i, raw_input_dict in enumerate(raw_inputs): + if raw_input_dict["name"] == param_name: + index = i + if index is not None: + raw_input_dict = raw_inputs[index] + del raw_inputs[index] + return raw_input_dict + else: + return None + + +class RootParamContext: + def __init__(self): + pass + + def for_state(self): + return "" + + def param_names(self): + return [] + + def get_index(self): + return 0 + + +def _process_simple_value( + param: InputSource, + param_value: Any, + required_data_tables: RequiredDataTablesT, + required_loc_files: RequiredLocFileT, +): + input_type = param.get("type") + if input_type == "select": + # Tests may specify values as either raw value or the value + # as they appear in the list - the API doesn't and shouldn't + # accept the text value - so we need to convert the text + # into the form value. + def process_param_value(param_value): + found_value = False + value_for_text = None + static_options = param.parse_static_options() + for text, opt_value, _ in static_options: + if param_value == opt_value: + found_value = True + if value_for_text is None and param_value == text: + value_for_text = opt_value + dynamic_options = param.parse_dynamic_options() + if dynamic_options and not input_type == "drill_down": + data_table_name = dynamic_options.get_data_table_name() + index_file_name = dynamic_options.get_index_file_name() + if data_table_name: + required_data_tables.append(data_table_name) + elif index_file_name: + required_loc_files.append(index_file_name) + if not found_value and value_for_text is not None: + processed_value = value_for_text + else: + processed_value = param_value + return processed_value + + # Do replacement described above for lists or singleton + # values. + if isinstance(param_value, list): + processed_value = list(map(process_param_value, param_value)) + else: + processed_value = process_param_value(param_value) + elif input_type == "boolean": + # Like above, tests may use the tool define values of simply + # true/false. + processed_value = _process_bool_param_value(param, param_value) + else: + processed_value = param_value + return processed_value + + +def _matching_case_for_value(tool_source: ToolSource, cond: InputSource, test_param: InputSource, declared_value: Any): + tool_id = tool_source.parse_id() + cond_name = cond.parse_name() + + assert test_param.parse_input_type() == "param" + test_param_type = test_param.get("type") + + if test_param_type == "boolean": + if declared_value is None: + # No explicit value for param in test case, determine from default + query_value = boolean_is_checked(test_param) + else: + query_value = _process_bool_param_value(test_param, declared_value) + + def matches_declared_value(case_value): + return _process_bool_param_value(test_param, case_value) == query_value + + elif test_param_type == "select": + static_options = test_param.parse_static_options() + if declared_value is not None: + # Test case supplied explicit value to check against. + + def matches_declared_value(case_value): + return case_value == declared_value + + elif static_options: + # No explicit value in test case, not much to do if options are dynamic but + # if static options are available can find the one specified as default or + # fallback on top most option (like GUI). + + for name, _, selected in static_options: + if selected: + default_option = name + else: + first_option = static_options[0] + first_option_value = first_option[1] + default_option = first_option_value + + def matches_declared_value(case_value): + return case_value == default_option + + else: + # No explicit value for this param and cannot determine a + # default - give up. Previously this would just result in a key + # error exception. + msg = f"Failed to find test parameter value specification required for conditional {cond_name}" + raise Exception(msg) + else: + msg = f"Invalid conditional test type found {test_param_type}" + raise Exception(msg) + + # Check the tool's defined cases against predicate to determine + # selected or default. + for case_when, case_input_sources in cond.parse_when_input_sources(): + if matches_declared_value(case_when): + return case_when, case_input_sources + else: + msg_template = "%s - Failed to find case matching value (%s) for test parameter specification for conditional %s. Remainder of test behavior is unspecified." + msg = msg_template % (tool_id, declared_value, cond_name) + log.info(msg) + return None + + +def _add_uploaded_dataset(name: str, value: Any, extra, input_parameter: InputSource, required_files: RequiredFilesT): + if value is None: + assert input_parameter.parse_optional(), f"{name} is not optional. You must provide a valid filename." + return value + return require_file(name, value, extra, required_files) + + +def require_file(name, value, extra, required_files): + if (value, extra) not in required_files: + required_files.append((value, extra)) # these files will be uploaded + name_change = [att for att in extra.get("edit_attributes", []) if att.get("type") == "name"] + if name_change: + name_change = name_change[-1].get("value") # only the last name change really matters + value = name_change # change value for select to renamed uploaded file for e.g. composite dataset + else: + for end in [".zip", ".gz"]: + if value.endswith(end): + value = value[: -len(end)] + break + value = os.path.basename(value) # if uploading a file in a path other than root of test-data + return value + + +def _process_bool_param_value(input_source: InputSource, param_value: Any) -> Any: + truevalue, falsevalue = boolean_true_and_false_values(input_source) + optional = input_source.parse_optional() + return process_bool_param_value(truevalue, falsevalue, optional, param_value) + + +def process_bool_param_value(truevalue: str, falsevalue: str, optional: bool, param_value: Any) -> Any: + was_list = False + if isinstance(param_value, list): + was_list = True + param_value = param_value[0] + + if truevalue == param_value: + processed_value = True + elif falsevalue == param_value: + processed_value = False + else: + if optional: + processed_value = string_as_bool_or_none(param_value) + else: + processed_value = string_as_bool(param_value) + return [processed_value] if was_list else processed_value + + +def split_if_str(value): + split = isinstance(value, str) + if split: + value = value.split(",") + return value diff --git a/lib/galaxy/tools/test.py b/lib/galaxy/tools/test.py index 41921dd4eaea..7447c17bd1d9 100644 --- a/lib/galaxy/tools/test.py +++ b/lib/galaxy/tools/test.py @@ -15,6 +15,15 @@ ToolTestDescription, ValidToolTestDict, ) +from galaxy.tool_util.verify.parse import ( + ParamContext, + process_bool_param_value, + RequiredDataTablesT, + RequiredFilesT, + RequiredLocFileT, + RootParamContext, + split_if_str, +) from galaxy.util import ( string_as_bool, string_as_bool_or_none, @@ -38,9 +47,9 @@ def parse_tests(tool, tests_source) -> Iterable[ToolTestDescription]: def description_from_tool_object(tool, test_index, raw_test_dict) -> ToolTestDescription: - required_files: List[Tuple[str, dict]] = [] - required_data_tables: List[str] = [] - required_loc_files: List[str] = [] + required_files: RequiredFilesT = [] + required_data_tables: RequiredDataTablesT = [] + required_loc_files: RequiredLocFileT = [] num_outputs = raw_test_dict.get("expect_num_outputs", None) if num_outputs: @@ -126,7 +135,7 @@ def _process_raw_inputs( ) expanded_inputs.update(case_inputs) if not value.type == "text": - expanded_case_value = _split_if_str(case.value) + expanded_case_value = split_if_str(case.value) if case_value is not None: # A bit tricky here - we are growing inputs with value # that may be implicit (i.e. not defined by user just @@ -185,11 +194,11 @@ def _process_raw_inputs( param_extra = raw_input_dict["attributes"] location = param_extra.get("location") if not value.type == "text": - param_value = _split_if_str(param_value) + param_value = split_if_str(param_value) if isinstance(value, galaxy.tools.parameters.basic.DataToolParameter): if location and value.multiple: # We get the input/s from the location which can be a list of urls separated by commas - locations = _split_if_str(location) + locations = split_if_str(location) param_value = [] for location in locations: v = os.path.basename(location) @@ -320,29 +329,9 @@ def _add_uploaded_dataset(name, value, extra, input_parameter, required_files): return require_file(name, value, extra, required_files) -def _split_if_str(value): - split = isinstance(value, str) - if split: - value = value.split(",") - return value - - def _process_bool_param_value(param, param_value): assert isinstance(param, galaxy.tools.parameters.basic.BooleanToolParameter) - was_list = False - if isinstance(param_value, list): - was_list = True - param_value = param_value[0] - if param.truevalue == param_value: - processed_value = True - elif param.falsevalue == param_value: - processed_value = False - else: - if param.optional: - processed_value = string_as_bool_or_none(param_value) - else: - processed_value = string_as_bool(param_value) - return [processed_value] if was_list else processed_value + return process_bool_param_value(param.truevalue, param.falsevalue, param.optional, param_value) def require_file(name, value, extra, required_files): @@ -359,64 +348,3 @@ def require_file(name, value, extra, required_files): break value = os.path.basename(value) # if uploading a file in a path other than root of test-data return value - - -class ParamContext: - def __init__(self, name, index=None, parent_context=None): - self.parent_context = parent_context - self.name = name - self.index = None if index is None else int(index) - - def for_state(self): - name = self.name if self.index is None else "%s_%d" % (self.name, self.index) - if parent_for_state := self.parent_context.for_state(): - return f"{parent_for_state}|{name}" - else: - return name - - def __str__(self): - return f"Context[for_state={self.for_state()}]" - - def param_names(self): - for parent_context_param in self.parent_context.param_names(): - if self.index is not None: - yield "%s|%s_%d" % (parent_context_param, self.name, self.index) - else: - yield f"{parent_context_param}|{self.name}" - if self.index is not None: - yield "%s_%d" % (self.name, self.index) - else: - yield self.name - - def extract_value(self, raw_inputs): - for param_name in self.param_names(): - value = self.__raw_param_found(param_name, raw_inputs) - if value: - return value - return None - - def __raw_param_found(self, param_name, raw_inputs): - index = None - for i, raw_input_dict in enumerate(raw_inputs): - if raw_input_dict["name"] == param_name: - index = i - if index is not None: - raw_input_dict = raw_inputs[index] - del raw_inputs[index] - return raw_input_dict - else: - return None - - -class RootParamContext: - def __init__(self): - pass - - def for_state(self): - return "" - - def param_names(self): - return [] - - def get_index(self): - return 0 diff --git a/test/unit/app/tools/test_test_parsing.py b/test/unit/app/tools/test_test_parsing.py index 00391e647176..cb986252577c 100644 --- a/test/unit/app/tools/test_test_parsing.py +++ b/test/unit/app/tools/test_test_parsing.py @@ -4,24 +4,65 @@ tool sources and I want to ensure the results do not change. """ +import os from typing import ( Any, List, ) +from pytest import skip + from galaxy.app_unittest_utils import tools_support from galaxy.tool_util.unittest_utils import functional_test_tool_path +from galaxy.tool_util.verify.parse import parse_tool_test_descriptions from galaxy.tools.test import parse_tests +from galaxy.util import ( + in_packages, + galaxy_directory, +) from galaxy.util.unittest import TestCase + # Not the whole response, just some keys and such to test... -SIMPLE_CONSTRUCTS_EXPECTATIONS = [ - (["inputs", "p1|p1use"], [True]), +SIMPLE_CONSTRUCTS_EXPECTATIONS_0 = [ (["inputs", "booltest"], [True]), (["inputs", "inttest"], ["12456"]), + (["inputs", "floattest"], ["6.789"]), + (["inputs", "p1|p1use"], [True]), + (["inputs", "files_0|file"], ["simple_line.txt"]), + (["outputs", 0, "name"], "out_file1"), + (["required_files", 0, 0], "simple_line.txt"), + (["required_files", 0, 1, "value"], "simple_line.txt"), +] +# this test case covers specifying boolean parameters by string truevalue/falsevalue +SIMPLE_CONSTRUCTS_EXPECTATIONS_1 = [ + (["inputs", "booltest"], [True]), + (["inputs", "p1|p1use"], [True]), (["inputs", "files_0|file"], ["simple_line.txt"]), (["outputs", 0, "name"], "out_file1"), ] +SECTION_EXPECTATIONS = [ + (["inputs", "int|inttest"], ["12456"]), + (["inputs", "float|floattest"], ["6.789"]), +] +MIN_REPEAT_EXPECTATIONS = [ + (["inputs", "queries_0|input"], ["simple_line.txt"]), + (["inputs", "queries_1|input"], ["simple_line.txt"]), + (["inputs", "queries2_0|input2"], ["simple_line_alternative.txt"]), +] +DBKEY_FILTER_INPUT_EXPECTATIONS = [ + (["inputs", "inputs"], ["simple_line.txt"]), + (["inputs", "index"], ["hg19_value"]), + (["required_files", 0, 1, "dbkey"], "hg19"), + (["required_data_tables", 0], "test_fasta_indexes"), +] +COLLECTION_TYPE_SOURCE_EXPECTATIONS = [ + (["inputs", "input_collect", "model_class"], "TestCollectionDef"), + (["inputs", "input_collect", "collection_type"], "list"), +] +BIGWIG_TO_WIG_EXPECTATIONS = [ + (["inputs", "chrom"], "chr21"), +] class TestTestParsing(TestCase, tools_support.UsesTools): @@ -33,19 +74,65 @@ def setUp(self): def tearDown(self): self.tear_down_app() - def test_state_parsing(self): + def _parse_tests(self): + return parse_tests(self.tool, self.tool_source) + + def test_simple_state_parsing(self): self._init_tool_for_path(functional_test_tool_path("simple_constructs.xml")) - test_dicts = parse_tests(self.tool, self.tool_source) - self._verify_each(test_dicts[0].to_dict(), SIMPLE_CONSTRUCTS_EXPECTATIONS) - print(test_dicts[0].to_dict()) - assert False + test_dicts = self._parse_tests() + self._verify_each(test_dicts[0].to_dict(), SIMPLE_CONSTRUCTS_EXPECTATIONS_0) + self._verify_each(test_dicts[1].to_dict(), SIMPLE_CONSTRUCTS_EXPECTATIONS_1) + + def test_section_state_parsing(self): + self._init_tool_for_path(functional_test_tool_path("section.xml")) + test_dicts = self._parse_tests() + # without and with section tags in the tests... + self._verify_each(test_dicts[0].to_dict(), SECTION_EXPECTATIONS) + self._verify_each(test_dicts[1].to_dict(), SECTION_EXPECTATIONS) + + def test_repeat_state_parsing(self): + self._init_tool_for_path(functional_test_tool_path("min_repeat.xml")) + test_dicts = self._parse_tests() + # without and with section tags in the tests... + self._verify_each(test_dicts[0].to_dict(), MIN_REPEAT_EXPECTATIONS) + + def test_dynamic_options_data_table_parsing(self): + self._init_tool_for_path(functional_test_tool_path("dbkey_filter_input.xml")) + test_dicts = self._parse_tests() + self._verify_each(test_dicts[0].to_dict(), DBKEY_FILTER_INPUT_EXPECTATIONS) + + def test_collection_type_source_parsing(self): + self._init_tool_for_path(functional_test_tool_path("collection_type_source.xml")) + test_dicts = self._parse_tests() + self._verify_each(test_dicts[0].to_dict(), COLLECTION_TYPE_SOURCE_EXPECTATIONS) + + def test_bigwigtowig_converter(self): + # defines + if in_packages(): + skip( + "skipping this mode for now - we need a framework test tool that skips names and just specifies arguments" + ) + tool_path = os.path.join( + galaxy_directory(), "lib", "galaxy", "datatypes", "converters", "bigwig_to_wig_converter.xml" + ) + self._init_tool_for_path(tool_path) + test_dicts = self._parse_tests() + self._verify_each(test_dicts[1].to_dict(), BIGWIG_TO_WIG_EXPECTATIONS) def _verify_each(self, target_dict: dict, expectations: List[Any]): for path, expectation in expectations: + exception = target_dict.get("exception") + assert not exception, f"Test failed to generate with exception {exception}" self._verify(target_dict, path, expectation) def _verify(self, target_dict: dict, expectation_path: List[str], expectation: Any): rest = target_dict for path_part in expectation_path: rest = rest[path_part] - assert rest == expectation + assert rest == expectation, f"{rest} != {expectation} for {expectation_path}" + + +class TestToolSourceTestParsing(TestTestParsing): + + def _parse_tests(self): + return parse_tool_test_descriptions(self.tool_source) From 071c3767735fab35b28655ea882c18df9eef8303 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 17 Jul 2024 19:27:43 -0400 Subject: [PATCH 7/8] Drop old Tool test parsing logic in favor of ToolSource logic. Add to tool_util, drop all the Galaxy mocking required using the older tool based logic. --- lib/galaxy/tools/__init__.py | 7 +- lib/galaxy/tools/test.py | 350 ------------------ .../test_test_definition_parsing.py} | 31 +- 3 files changed, 12 insertions(+), 376 deletions(-) delete mode 100644 lib/galaxy/tools/test.py rename test/unit/{app/tools/test_test_parsing.py => tool_util/test_test_definition_parsing.py} (88%) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 91bf85374dc2..3979eb569510 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -97,6 +97,7 @@ ) from galaxy.tool_util.toolbox.views.sources import StaticToolBoxViewSources from galaxy.tool_util.verify.interactor import ToolTestDescription +from galaxy.tool_util.verify.parse import parse_tool_test_descriptions from galaxy.tool_util.verify.test_data import TestDataNotFoundError from galaxy.tool_util.version import ( LegacyVersion, @@ -149,7 +150,6 @@ from galaxy.tools.parameters.meta import expand_meta_parameters from galaxy.tools.parameters.workflow_utils import workflow_building_modes from galaxy.tools.parameters.wrapped_json import json_wrap -from galaxy.tools.test import parse_tests from galaxy.util import ( in_directory, listify, @@ -1306,9 +1306,10 @@ def __parse_trackster_conf(self, tool_source): self.trackster_conf = TracksterConfig.parse(trackster_conf) def parse_tests(self): - if tests_source := self.tool_source: + if self.tool_source: + test_descriptions = parse_tool_test_descriptions(self.tool_source, self.id) try: - self.__tests = json.dumps([t.to_dict() for t in parse_tests(self, tests_source)], indent=None) + self.__tests = json.dumps([t.to_dict() for t in test_descriptions], indent=None) except Exception: self.__tests = None log.exception("Failed to parse tool tests for tool '%s'", self.id) diff --git a/lib/galaxy/tools/test.py b/lib/galaxy/tools/test.py deleted file mode 100644 index 7447c17bd1d9..000000000000 --- a/lib/galaxy/tools/test.py +++ /dev/null @@ -1,350 +0,0 @@ -import logging -import os -import os.path -from typing import ( - Iterable, - List, - Tuple, - Union, -) - -import galaxy.tools.parameters.basic -import galaxy.tools.parameters.grouping -from galaxy.tool_util.verify.interactor import ( - InvalidToolTestDict, - ToolTestDescription, - ValidToolTestDict, -) -from galaxy.tool_util.verify.parse import ( - ParamContext, - process_bool_param_value, - RequiredDataTablesT, - RequiredFilesT, - RequiredLocFileT, - RootParamContext, - split_if_str, -) -from galaxy.util import ( - string_as_bool, - string_as_bool_or_none, - unicodify, -) - -log = logging.getLogger(__name__) - - -def parse_tests(tool, tests_source) -> Iterable[ToolTestDescription]: - """ - Build ToolTestDescription objects for each "" elements and - return default interactor (if any). - """ - raw_tests_dict = tests_source.parse_tests_to_dict() - tests: List[ToolTestDescription] = [] - for i, raw_test_dict in enumerate(raw_tests_dict.get("tests", [])): - test = description_from_tool_object(tool, i, raw_test_dict) - tests.append(test) - return tests - - -def description_from_tool_object(tool, test_index, raw_test_dict) -> ToolTestDescription: - required_files: RequiredFilesT = [] - required_data_tables: RequiredDataTablesT = [] - required_loc_files: RequiredLocFileT = [] - - num_outputs = raw_test_dict.get("expect_num_outputs", None) - if num_outputs: - num_outputs = int(num_outputs) - maxseconds = raw_test_dict.get("maxseconds", None) - if maxseconds is not None: - maxseconds = int(maxseconds) - - processed_test_dict: Union[ValidToolTestDict, InvalidToolTestDict] - try: - processed_inputs = _process_raw_inputs( - tool, tool.inputs, raw_test_dict["inputs"], required_files, required_data_tables, required_loc_files - ) - - processed_test_dict = ValidToolTestDict( - { - "inputs": processed_inputs, - "outputs": raw_test_dict["outputs"], - "output_collections": raw_test_dict["output_collections"], - "num_outputs": num_outputs, - "command_line": raw_test_dict.get("command", None), - "command_version": raw_test_dict.get("command_version", None), - "stdout": raw_test_dict.get("stdout", None), - "stderr": raw_test_dict.get("stderr", None), - "expect_exit_code": raw_test_dict.get("expect_exit_code", None), - "expect_failure": raw_test_dict.get("expect_failure", False), - "expect_test_failure": raw_test_dict.get("expect_test_failure", False), - "required_files": required_files, - "required_data_tables": required_data_tables, - "required_loc_files": required_loc_files, - "tool_id": tool.id, - "tool_version": tool.version, - "test_index": test_index, - "maxseconds": maxseconds, - "error": False, - } - ) - except Exception as e: - log.exception("Failed to load tool test number [%d] for %s" % (test_index, tool.id)) - processed_test_dict = InvalidToolTestDict( - { - "tool_id": tool.id, - "tool_version": tool.version, - "test_index": test_index, - "inputs": {}, - "error": True, - "exception": unicodify(e), - "maxseconds": maxseconds, - } - ) - - return ToolTestDescription(processed_test_dict) - - -def _process_raw_inputs( - tool, tool_inputs, raw_inputs, required_files, required_data_tables, required_loc_files, parent_context=None -): - """ - Recursively expand flat list of inputs into "tree" form of flat list - (| using to nest to new levels) structure and expand dataset - information as proceeding to populate self.required_files. - """ - parent_context = parent_context or RootParamContext() - expanded_inputs = {} - for value in tool_inputs.values(): - if isinstance(value, galaxy.tools.parameters.grouping.Conditional): - cond_context = ParamContext(name=value.name, parent_context=parent_context) - assert value.test_param - case_context = ParamContext(name=value.test_param.name, parent_context=cond_context) - raw_input_dict = case_context.extract_value(raw_inputs) - case_value = raw_input_dict["value"] if raw_input_dict else None - case = _matching_case_for_value(tool, value, case_value) - if case: - for input_name, input_value in case.inputs.items(): - case_inputs = _process_raw_inputs( - tool, - {input_name: input_value}, - raw_inputs, - required_files, - required_data_tables, - required_loc_files, - parent_context=cond_context, - ) - expanded_inputs.update(case_inputs) - if not value.type == "text": - expanded_case_value = split_if_str(case.value) - if case_value is not None: - # A bit tricky here - we are growing inputs with value - # that may be implicit (i.e. not defined by user just - # a default defined in tool). So we do not want to grow - # expanded_inputs and risk repeat block viewing this - # as a new instance with value defined and hence enter - # an infinite loop - hence the "case_value is not None" - # check. - processed_value = _process_simple_value( - value.test_param, expanded_case_value, required_data_tables, required_loc_files - ) - expanded_inputs[case_context.for_state()] = processed_value - elif isinstance(value, galaxy.tools.parameters.grouping.Section): - context = ParamContext(name=value.name, parent_context=parent_context) - assert value.inputs - for r_value in value.inputs.values(): - expanded_input = _process_raw_inputs( - tool, - {context.for_state(): r_value}, - raw_inputs, - required_files, - required_data_tables, - required_loc_files, - parent_context=context, - ) - if expanded_input: - expanded_inputs.update(expanded_input) - elif isinstance(value, galaxy.tools.parameters.grouping.Repeat): - repeat_index = 0 - while True: - context = ParamContext(name=value.name, index=repeat_index, parent_context=parent_context) - updated = False - assert value.inputs - for r_value in value.inputs.values(): - expanded_input = _process_raw_inputs( - tool, - {context.for_state(): r_value}, - raw_inputs, - required_files, - required_data_tables, - required_loc_files, - parent_context=context, - ) - if expanded_input: - expanded_inputs.update(expanded_input) - updated = True - if not updated: - break - repeat_index += 1 - else: - context = ParamContext(name=value.name, parent_context=parent_context) - raw_input_dict = context.extract_value(raw_inputs) - if raw_input_dict: - name = raw_input_dict["name"] - param_value = raw_input_dict["value"] - param_extra = raw_input_dict["attributes"] - location = param_extra.get("location") - if not value.type == "text": - param_value = split_if_str(param_value) - if isinstance(value, galaxy.tools.parameters.basic.DataToolParameter): - if location and value.multiple: - # We get the input/s from the location which can be a list of urls separated by commas - locations = split_if_str(location) - param_value = [] - for location in locations: - v = os.path.basename(location) - param_value.append(v) - # param_extra should contain only the corresponding location - extra = dict(param_extra) - extra["location"] = location - _add_uploaded_dataset(context.for_state(), v, extra, value, required_files) - else: - if not isinstance(param_value, list): - param_value = [param_value] - for v in param_value: - _add_uploaded_dataset(context.for_state(), v, param_extra, value, required_files) - processed_value = param_value - elif isinstance(value, galaxy.tools.parameters.basic.DataCollectionToolParameter): - assert "collection" in param_extra - collection_def = param_extra["collection"] - for input_dict in collection_def.collect_inputs(): - name = input_dict["name"] - value = input_dict["value"] - attributes = input_dict["attributes"] - require_file(name, value, attributes, required_files) - processed_value = collection_def - else: - processed_value = _process_simple_value( - value, param_value, required_data_tables, required_loc_files - ) - expanded_inputs[context.for_state()] = processed_value - return expanded_inputs - - -def _process_simple_value(param, param_value, required_data_tables, required_loc_files): - if isinstance(param, galaxy.tools.parameters.basic.SelectToolParameter): - # Tests may specify values as either raw value or the value - # as they appear in the list - the API doesn't and shouldn't - # accept the text value - so we need to convert the text - # into the form value. - def process_param_value(param_value): - found_value = False - value_for_text = None - for text, opt_value, _ in getattr(param, "static_options", []): - if param_value == opt_value: - found_value = True - if value_for_text is None and param_value == text: - value_for_text = opt_value - if param.options and not isinstance(param, galaxy.tools.parameters.basic.DrillDownSelectToolParameter): - if param.options.tool_data_table_name: - required_data_tables.append(param.options.tool_data_table_name) - elif param.options.index_file: - required_loc_files.append(param.options.index_file) - if not found_value and value_for_text is not None: - processed_value = value_for_text - else: - processed_value = param_value - return processed_value - - # Do replacement described above for lists or singleton - # values. - if isinstance(param_value, list): - processed_value = list(map(process_param_value, param_value)) - else: - processed_value = process_param_value(param_value) - elif isinstance(param, galaxy.tools.parameters.basic.BooleanToolParameter): - # Like above, tests may use the tool define values of simply - # true/false. - processed_value = _process_bool_param_value(param, param_value) - else: - processed_value = param_value - return processed_value - - -def _matching_case_for_value(tool, cond, declared_value): - test_param = cond.test_param - if isinstance(test_param, galaxy.tools.parameters.basic.BooleanToolParameter): - if declared_value is None: - # No explicit value for param in test case, determine from default - query_value = test_param.checked - else: - query_value = _process_bool_param_value(test_param, declared_value) - - def matches_declared_value(case_value): - return _process_bool_param_value(test_param, case_value) == query_value - - elif isinstance(test_param, galaxy.tools.parameters.basic.SelectToolParameter): - if declared_value is not None: - # Test case supplied explicit value to check against. - - def matches_declared_value(case_value): - return case_value == declared_value - - elif test_param.static_options: - # No explicit value in test case, not much to do if options are dynamic but - # if static options are available can find the one specified as default or - # fallback on top most option (like GUI). - for name, _, selected in test_param.static_options: - if selected: - default_option = name - else: - first_option = test_param.static_options[0] - first_option_value = first_option[1] - default_option = first_option_value - - def matches_declared_value(case_value): - return case_value == default_option - - else: - # No explicit value for this param and cannot determine a - # default - give up. Previously this would just result in a key - # error exception. - msg = f"Failed to find test parameter value specification required for conditional {cond.name}" - raise Exception(msg) - - # Check the tool's defined cases against predicate to determine - # selected or default. - for case in cond.cases: - if matches_declared_value(case.value): - return case - else: - msg_template = "%s - Failed to find case matching value (%s) for test parameter specification for conditional %s. Remainder of test behavior is unspecified." - msg = msg_template % (tool.id, declared_value, cond.name) - log.info(msg) - - -def _add_uploaded_dataset(name, value, extra, input_parameter, required_files): - if value is None: - assert input_parameter.optional, f"{name} is not optional. You must provide a valid filename." - return value - return require_file(name, value, extra, required_files) - - -def _process_bool_param_value(param, param_value): - assert isinstance(param, galaxy.tools.parameters.basic.BooleanToolParameter) - return process_bool_param_value(param.truevalue, param.falsevalue, param.optional, param_value) - - -def require_file(name, value, extra, required_files): - if (value, extra) not in required_files: - required_files.append((value, extra)) # these files will be uploaded - name_change = [att for att in extra.get("edit_attributes", []) if att.get("type") == "name"] - if name_change: - name_change = name_change[-1].get("value") # only the last name change really matters - value = name_change # change value for select to renamed uploaded file for e.g. composite dataset - else: - for end in [".zip", ".gz"]: - if value.endswith(end): - value = value[: -len(end)] - break - value = os.path.basename(value) # if uploading a file in a path other than root of test-data - return value diff --git a/test/unit/app/tools/test_test_parsing.py b/test/unit/tool_util/test_test_definition_parsing.py similarity index 88% rename from test/unit/app/tools/test_test_parsing.py rename to test/unit/tool_util/test_test_definition_parsing.py index cb986252577c..74ae00b194cf 100644 --- a/test/unit/app/tools/test_test_parsing.py +++ b/test/unit/tool_util/test_test_definition_parsing.py @@ -1,8 +1,4 @@ -""" Test Tool testing logic. - -I am going to migrate from using galaxy.tools.parameters and Galaxy Tool internals to -tool sources and I want to ensure the results do not change. -""" +"""Tool test parsing to dicts logic.""" import os from typing import ( @@ -12,10 +8,9 @@ from pytest import skip -from galaxy.app_unittest_utils import tools_support +from galaxy.tool_util.parser.factory import get_tool_source from galaxy.tool_util.unittest_utils import functional_test_tool_path from galaxy.tool_util.verify.parse import parse_tool_test_descriptions -from galaxy.tools.test import parse_tests from galaxy.util import ( in_packages, galaxy_directory, @@ -65,17 +60,13 @@ ] -class TestTestParsing(TestCase, tools_support.UsesTools): - tool_action: "MockAction" - - def setUp(self): - self.setup_app() - - def tearDown(self): - self.tear_down_app() - +class TestTestParsing(TestCase): def _parse_tests(self): - return parse_tests(self.tool, self.tool_source) + return parse_tool_test_descriptions(self.tool_source) + + def _init_tool_for_path(self, path): + tool_source = get_tool_source(path) + self.tool_source = tool_source def test_simple_state_parsing(self): self._init_tool_for_path(functional_test_tool_path("simple_constructs.xml")) @@ -130,9 +121,3 @@ def _verify(self, target_dict: dict, expectation_path: List[str], expectation: A for path_part in expectation_path: rest = rest[path_part] assert rest == expectation, f"{rest} != {expectation} for {expectation_path}" - - -class TestToolSourceTestParsing(TestTestParsing): - - def _parse_tests(self): - return parse_tool_test_descriptions(self.tool_source) From 7d4c9f43f31a5e6a967fcc0921e33dabe7d576fd Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 18 Jul 2024 11:59:31 -0400 Subject: [PATCH 8/8] More typing of test case parsing. --- lib/galaxy/tool_util/parser/interface.py | 3 +- lib/galaxy/tool_util/verify/_types.py | 14 ++++++ lib/galaxy/tool_util/verify/interactor.py | 49 +++++++++++++------ lib/galaxy/tool_util/verify/parse.py | 43 +++++++++------- .../tool_util/test_test_definition_parsing.py | 3 +- 5 files changed, 76 insertions(+), 36 deletions(-) create mode 100644 lib/galaxy/tool_util/verify/_types.py diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index 809fe5f0c387..092938836a6c 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -45,12 +45,13 @@ class AssertionDict(TypedDict): ToolSourceTestInputs = Any ToolSourceTestOutputs = Any +TestSourceTestOutputColllection = Any class ToolSourceTest(TypedDict): inputs: ToolSourceTestInputs outputs: ToolSourceTestOutputs - output_collections: List[Any] + output_collections: List[TestSourceTestOutputColllection] stdout: AssertionList stderr: AssertionList expect_exit_code: Optional[XmlInt] diff --git a/lib/galaxy/tool_util/verify/_types.py b/lib/galaxy/tool_util/verify/_types.py new file mode 100644 index 000000000000..e7362fafd7b8 --- /dev/null +++ b/lib/galaxy/tool_util/verify/_types.py @@ -0,0 +1,14 @@ +"""Types used by interactor and test case processor.""" + +from typing import ( + Any, + Dict, + List, + Tuple, +) + +ExtraFileInfoDictT = Dict[str, Any] +RequiredFileTuple = Tuple[str, ExtraFileInfoDictT] +RequiredFilesT = List[RequiredFileTuple] +RequiredDataTablesT = List[str] +RequiredLocFileT = List[str] diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index c31dba70175e..52b0218cedfb 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -39,6 +39,7 @@ AssertionList, TestCollectionDef, TestCollectionOutputDef, + TestSourceTestOutputColllection, ) from galaxy.util import requests from galaxy.util.bunch import Bunch @@ -47,6 +48,11 @@ parse_checksum_hash, ) from . import verify +from ._types import ( + RequiredDataTablesT, + RequiredFilesT, + RequiredLocFileT, +) from .asserts import verify_assertions from .wait import wait_on @@ -92,7 +98,7 @@ def __getitem__(self, item): class ValidToolTestDict(TypedDict): inputs: Any outputs: Any - output_collections: List[Dict[str, Any]] + output_collections: List[TestSourceTestOutputColllection] stdout: NotRequired[AssertionList] stderr: NotRequired[AssertionList] expect_exit_code: NotRequired[Optional[Union[str, int]]] @@ -102,9 +108,9 @@ class ValidToolTestDict(TypedDict): num_outputs: NotRequired[Optional[Union[str, int]]] command_line: NotRequired[AssertionList] command_version: NotRequired[AssertionList] - required_files: NotRequired[List[Any]] - required_data_tables: NotRequired[List[Any]] - required_loc_files: NotRequired[List[str]] + required_files: NotRequired[RequiredFilesT] + required_data_tables: NotRequired[RequiredDataTablesT] + required_loc_files: NotRequired[RequiredLocFileT] error: Literal[False] tool_id: str tool_version: str @@ -1661,7 +1667,7 @@ class ToolTestDescriptionDict(TypedDict): name: str inputs: Any outputs: Any - output_collections: List[Dict[str, Any]] + output_collections: List[TestSourceTestOutputColllection] stdout: Optional[AssertionList] stderr: Optional[AssertionList] expect_exit_code: Optional[int] @@ -1693,13 +1699,14 @@ class ToolTestDescription: stderr: Optional[AssertionList] command_line: Optional[AssertionList] command_version: Optional[AssertionList] - required_files: List[Any] - required_data_tables: List[Any] - required_loc_files: List[str] + required_files: RequiredFilesT + required_data_tables: RequiredDataTablesT + required_loc_files: RequiredLocFileT expect_exit_code: Optional[int] expect_failure: bool expect_test_failure: bool exception: Optional[str] + output_collections: List[TestCollectionOutputDef] def __init__(self, processed_test_dict: ToolTestDict): assert ( @@ -1708,14 +1715,31 @@ def __init__(self, processed_test_dict: ToolTestDict): test_index = processed_test_dict["test_index"] name = cast(str, processed_test_dict.get("name", f"Test-{test_index + 1}")) error_in_test_definition = processed_test_dict["error"] + num_outputs: Optional[int] = None if not error_in_test_definition: processed_test_dict = cast(ValidToolTestDict, processed_test_dict) maxseconds = int(processed_test_dict.get("maxseconds") or DEFAULT_TOOL_TEST_WAIT or 86400) output_collections = processed_test_dict.get("output_collections", []) + if "num_outputs" in processed_test_dict and processed_test_dict["num_outputs"]: + num_outputs = int(processed_test_dict["num_outputs"]) + self.required_files = processed_test_dict.get("required_files", []) + self.required_data_tables = processed_test_dict.get("required_data_tables", []) + self.required_loc_files = processed_test_dict.get("required_loc_files", []) + self.command_line = processed_test_dict.get("command_line", None) + self.command_version = processed_test_dict.get("command_version", None) + self.stdout = processed_test_dict.get("stdout", None) + self.stderr = processed_test_dict.get("stderr", None) else: processed_test_dict = cast(InvalidToolTestDict, processed_test_dict) maxseconds = DEFAULT_TOOL_TEST_WAIT output_collections = [] + self.required_files = [] + self.required_data_tables = [] + self.required_loc_files = [] + self.command_line = None + self.command_version = None + self.stdout = None + self.stderr = None self.test_index = test_index assert ( @@ -1725,9 +1749,6 @@ def __init__(self, processed_test_dict: ToolTestDict): self.tool_version = processed_test_dict.get("tool_version") self.name = name self.maxseconds = maxseconds - self.required_files = cast(List[Any], processed_test_dict.get("required_files", [])) - self.required_data_tables = cast(List[Any], processed_test_dict.get("required_data_tables", [])) - self.required_loc_files = cast(List[str], processed_test_dict.get("required_loc_files", [])) inputs = processed_test_dict.get("inputs", {}) loaded_inputs = {} @@ -1739,16 +1760,12 @@ def __init__(self, processed_test_dict: ToolTestDict): self.inputs = loaded_inputs self.outputs = processed_test_dict.get("outputs", []) - self.num_outputs = cast(Optional[int], processed_test_dict.get("num_outputs", None)) + self.num_outputs = num_outputs self.error = processed_test_dict.get("error", False) self.exception = cast(Optional[str], processed_test_dict.get("exception", None)) self.output_collections = [TestCollectionOutputDef.from_dict(d) for d in output_collections] - self.command_line = cast(Optional[AssertionList], processed_test_dict.get("command_line", None)) - self.command_version = cast(Optional[AssertionList], processed_test_dict.get("command_version", None)) - self.stdout = cast(Optional[AssertionList], processed_test_dict.get("stdout", None)) - self.stderr = cast(Optional[AssertionList], processed_test_dict.get("stderr", None)) self.expect_exit_code = cast(Optional[int], processed_test_dict.get("expect_exit_code", None)) self.expect_failure = cast(bool, processed_test_dict.get("expect_failure", False)) self.expect_test_failure = cast(bool, processed_test_dict.get("expect_test_failure", False)) diff --git a/lib/galaxy/tool_util/verify/parse.py b/lib/galaxy/tool_util/verify/parse.py index 74ae8f3a1196..00415cc76535 100644 --- a/lib/galaxy/tool_util/verify/parse.py +++ b/lib/galaxy/tool_util/verify/parse.py @@ -5,7 +5,6 @@ Iterable, List, Optional, - Tuple, Union, ) @@ -31,12 +30,16 @@ string_as_bool_or_none, unicodify, ) +from ._types import ( + ExtraFileInfoDictT, + RequiredDataTablesT, + RequiredFilesT, + RequiredLocFileT, +) log = logging.getLogger(__name__) -RequiredFilesT = List[Tuple[str, dict]] -RequiredDataTablesT = List[str] -RequiredLocFileT = List[str] +AnyParamContext = Union["ParamContext", "RootParamContext"] def parse_tool_test_descriptions( @@ -127,7 +130,7 @@ def _process_raw_inputs( required_files: RequiredFilesT, required_data_tables: RequiredDataTablesT, required_loc_files: RequiredLocFileT, - parent_context: Optional[Union["ParamContext", "RootParamContext"]] = None, + parent_context: Optional[AnyParamContext] = None, ): """ Recursively expand flat list of inputs into "tree" form of flat list @@ -192,7 +195,7 @@ def _process_raw_inputs( elif input_type == "repeat": repeat_index = 0 while True: - context = ParamContext(name=name, index=repeat_index, parent_context=parent_context) + context = ParamContext(name=name, parent_context=parent_context, index=repeat_index) updated = False page_source = input_source.parse_nested_inputs_source() for r_value in page_source.parse_input_sources(): @@ -268,12 +271,12 @@ def input_sources(tool_source: ToolSource) -> List[InputSource]: class ParamContext: - def __init__(self, name, index=None, parent_context=None): + def __init__(self, name: str, parent_context: AnyParamContext, index: Optional[int] = None): self.parent_context = parent_context self.name = name self.index = None if index is None else int(index) - def for_state(self): + def for_state(self) -> str: name = self.name if self.index is None else "%s_%d" % (self.name, self.index) parent_for_state = self.parent_context.for_state() if parent_for_state: @@ -281,7 +284,7 @@ def for_state(self): else: return name - def __str__(self): + def __str__(self) -> str: return f"Context[for_state={self.for_state()}]" def param_names(self): @@ -295,14 +298,14 @@ def param_names(self): else: yield self.name - def extract_value(self, raw_inputs): + def extract_value(self, raw_inputs: ToolSourceTestInputs): for param_name in self.param_names(): value = self.__raw_param_found(param_name, raw_inputs) if value: return value return None - def __raw_param_found(self, param_name, raw_inputs): + def __raw_param_found(self, param_name: str, raw_inputs: ToolSourceTestInputs): index = None for i, raw_input_dict in enumerate(raw_inputs): if raw_input_dict["name"] == param_name: @@ -442,20 +445,26 @@ def matches_declared_value(case_value): return None -def _add_uploaded_dataset(name: str, value: Any, extra, input_parameter: InputSource, required_files: RequiredFilesT): +def _add_uploaded_dataset( + name: str, + value: Optional[str], + extra: ExtraFileInfoDictT, + input_parameter: InputSource, + required_files: RequiredFilesT, +) -> Optional[str]: if value is None: assert input_parameter.parse_optional(), f"{name} is not optional. You must provide a valid filename." return value return require_file(name, value, extra, required_files) -def require_file(name, value, extra, required_files): +def require_file(name: str, value: str, extra: ExtraFileInfoDictT, required_files: RequiredFilesT) -> str: if (value, extra) not in required_files: required_files.append((value, extra)) # these files will be uploaded - name_change = [att for att in extra.get("edit_attributes", []) if att.get("type") == "name"] - if name_change: - name_change = name_change[-1].get("value") # only the last name change really matters - value = name_change # change value for select to renamed uploaded file for e.g. composite dataset + name_changes = [att for att in extra.get("edit_attributes", []) if att.get("type") == "name"] + if name_changes: + name_change = name_changes[-1].get("value") # only the last name change really matters + value = str(name_change) # change value for select to renamed uploaded file for e.g. composite dataset else: for end in [".zip", ".gz"]: if value.endswith(end): diff --git a/test/unit/tool_util/test_test_definition_parsing.py b/test/unit/tool_util/test_test_definition_parsing.py index 74ae00b194cf..7ff73523f3c2 100644 --- a/test/unit/tool_util/test_test_definition_parsing.py +++ b/test/unit/tool_util/test_test_definition_parsing.py @@ -12,12 +12,11 @@ from galaxy.tool_util.unittest_utils import functional_test_tool_path from galaxy.tool_util.verify.parse import parse_tool_test_descriptions from galaxy.util import ( - in_packages, galaxy_directory, + in_packages, ) from galaxy.util.unittest import TestCase - # Not the whole response, just some keys and such to test... SIMPLE_CONSTRUCTS_EXPECTATIONS_0 = [ (["inputs", "booltest"], [True]),