From 05ede4ab0e11048a542e1ec8dd5fa4daaad40b91 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..91e7de7483ba 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()