From 9f543845d5b511bf8cf76211a121de32cecd54b5 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 8 Jan 2025 12:18:55 +0200 Subject: [PATCH] Fix planemo timeout handling Planemo calls `verify_tool` with a maxseconds value, and then sets `tool_test_dict.setdefault("maxseconds", maxseconds)`, which fails, since we are serializing and already setting the default maxseconds while parsing the test dict. Fixes the IUC tool test timeouts not working when testing against 24.2. Broken in https://github.com/galaxyproject/galaxy/commit/9695ccc3c45a14b36beccfbaac69036a5ac65c21#diff-06835be1d1943ebce8373e2cd1f8e86c7b7813b720398850c978b734dd29e2daR1390 (maxseconds was prviously not serialized at all in `ToolTestDescription.to_dict`) --- lib/galaxy/tool_util/verify/interactor.py | 58 +++++++++---------- .../tool_util/test_test_definition_parsing.py | 15 +++++ 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index ca186d823ebb..d2e2358ef49a 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1688,7 +1688,7 @@ def adapt_tool_source_dict(processed_dict: ToolTestDict) -> ToolTestDescriptionD if not error_in_test_definition: processed_test_dict = cast(ValidToolTestDict, processed_dict) - maxseconds = _get_maxseconds(processed_test_dict) + maxseconds = processed_test_dict.get("maxseconds") 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"]) @@ -1750,10 +1750,6 @@ def _get_test_name(test_dict: Union[ToolTestDict, ToolTestDescriptionDict], test return name -def _get_maxseconds(test_dict: Union[ToolTestDict, ToolTestDescriptionDict]) -> int: - return int(cast(Union[str, int], test_dict.get("maxseconds") or DEFAULT_TOOL_TEST_WAIT or 86400)) - - def expanded_inputs_from_json(expanded_inputs_json: ExpandedToolInputsJsonified) -> ExpandedToolInputs: loaded_inputs: ExpandedToolInputs = {} for key, value in expanded_inputs_json.items(): @@ -1829,7 +1825,7 @@ def __init__(self, json_dict: ToolTestDescriptionDict): self.inputs = expanded_inputs_from_json(json_dict.get("inputs", {})) self.tool_id = json_dict["tool_id"] self.tool_version = json_dict.get("tool_version") - self.maxseconds = _get_maxseconds(json_dict) + self.maxseconds = json_dict.get("maxseconds") def test_data(self): """ @@ -1839,31 +1835,31 @@ def test_data(self): def to_dict(self) -> ToolTestDescriptionDict: inputs = expanded_inputs_to_json(self.inputs) - return ToolTestDescriptionDict( - { - "inputs": inputs, - "outputs": self.outputs, - "output_collections": [_.to_dict() for _ in self.output_collections], - "num_outputs": self.num_outputs, - "command_line": self.command_line, - "command_version": self.command_version, - "stdout": self.stdout, - "stderr": self.stderr, - "expect_exit_code": self.expect_exit_code, - "expect_failure": self.expect_failure, - "expect_test_failure": self.expect_test_failure, - "name": self.name, - "test_index": self.test_index, - "tool_id": self.tool_id, - "tool_version": self.tool_version, - "required_files": self.required_files, - "required_data_tables": self.required_data_tables, - "required_loc_files": self.required_loc_files, - "error": self.error, - "exception": self.exception, - "maxseconds": self.maxseconds, - } - ) + test_description_def: ToolTestDescriptionDict = { + "inputs": inputs, + "outputs": self.outputs, + "output_collections": [_.to_dict() for _ in self.output_collections], + "num_outputs": self.num_outputs, + "command_line": self.command_line, + "command_version": self.command_version, + "stdout": self.stdout, + "stderr": self.stderr, + "expect_exit_code": self.expect_exit_code, + "expect_failure": self.expect_failure, + "expect_test_failure": self.expect_test_failure, + "name": self.name, + "test_index": self.test_index, + "tool_id": self.tool_id, + "tool_version": self.tool_version, + "required_files": self.required_files, + "required_data_tables": self.required_data_tables, + "required_loc_files": self.required_loc_files, + "error": self.error, + "exception": self.exception, + } + if self.maxseconds is not None: + test_description_def["maxseconds"] = self.maxseconds + return ToolTestDescriptionDict(test_description_def) def test_data_iter(required_files): diff --git a/test/unit/tool_util/test_test_definition_parsing.py b/test/unit/tool_util/test_test_definition_parsing.py index 6576660c3b00..ed5fbdaaee91 100644 --- a/test/unit/tool_util/test_test_definition_parsing.py +++ b/test/unit/tool_util/test_test_definition_parsing.py @@ -68,6 +68,21 @@ def _init_tool_for_path(self, path): tool_source = get_tool_source(path) self.tool_source = tool_source + def test_maxseconds_not_filled_with_default(self): + self._init_tool_for_path(functional_test_tool_path("simple_constructs.xml")) + test_dicts = list(self._parse_tests()) + assert test_dicts[0].maxseconds is None + assert "maxseconds" not in test_dicts[0].to_dict() + + def test_maxseconds_parsed(self): + self._init_tool_for_path(functional_test_tool_path("maxseconds.xml")) + test_def = next(iter(self._parse_tests())) + assert test_def.maxseconds == 5 + test_dict = test_def.to_dict() + print(test_dict) + assert "maxseconds" in test_dict + assert test_dict["maxseconds"] == 5 + def test_simple_state_parsing(self): self._init_tool_for_path(functional_test_tool_path("simple_constructs.xml")) test_dicts = self._parse_tests()