diff --git a/lib/galaxy/tool_util/upgrade/__init__.py b/lib/galaxy/tool_util/upgrade/__init__.py index 2ff1aa6f0861..41be4988d0c4 100644 --- a/lib/galaxy/tool_util/upgrade/__init__.py +++ b/lib/galaxy/tool_util/upgrade/__init__.py @@ -10,6 +10,7 @@ # - We could try to walk parameters and give more specific advice on structured_like qualification. from json import loads from typing import ( + Any, cast, Dict, List, @@ -23,6 +24,7 @@ TypedDict, ) +from galaxy.tool_util.parameters.case import validate_test_cases_for_tool_source from galaxy.tool_util.parser.factory import get_tool_source from galaxy.tool_util.parser.xml import XmlToolSource from galaxy.util import Element @@ -33,10 +35,12 @@ "Target upgrade version is too new for this script, consider upgrading this script for the latest advice." ) +TYPE_LEVEL = Literal["must_fix", "consider", "ready", "info"] + class AdviceCode(TypedDict): name: str - level: Literal["must_fix", "consider", "ready", "info"] + level: TYPE_LEVEL message: str niche: NotRequired[bool] url: NotRequired[str] @@ -50,16 +54,47 @@ class AdviceCode(TypedDict): upgrade_codes_by_name[name] = cast(AdviceCode, upgrade_object) +class Advice: + advice_code: AdviceCode + message: Optional[str] + + def __init__(self, advice_code: AdviceCode, message: Optional[str]): + self.advice_code = advice_code + self.message = message + + @property + def url(self) -> Optional[str]: + return self.advice_code.get("url") + + @property + def level(self) -> TYPE_LEVEL: + return self.advice_code["level"] + + @property + def niche(self) -> bool: + return self.advice_code.get("niche", False) + + @property + def advice_code_message(self) -> str: + return self.advice_code["message"] + + def to_dict(self) -> Dict[str, Any]: + as_dict = cast(Dict[str, Any], self.advice_code.copy()) + as_dict["advice_code_message"] = self.advice_code_message + as_dict["message"] = self.message + return as_dict + + class AdviceCollection: - _advice: List[AdviceCode] + _advice: List[Advice] def __init__(self): self._advice = [] - def add(self, code: str): - self._advice.append(upgrade_codes_by_name[code]) + def add(self, code: str, message: Optional[str] = None): + self._advice.append(Advice(upgrade_codes_by_name[code], message)) - def to_list(self) -> List[AdviceCode]: + def to_list(self) -> List[Advice]: return self._advice @@ -218,6 +253,19 @@ def advise(cls, advice_collection: AdviceCollection, xml_file: str) -> None: advice_collection.add("24_0_request_cleaning") +class ProfileMigration24_2(ProfileMigration): + from_version = "24.0" + to_version = "24.2" + + @classmethod + def advise(cls, advice_collection: AdviceCollection, xml_file: str) -> None: + tool_source = _xml_tool_source(xml_file) + results = validate_test_cases_for_tool_source(tool_source, use_latest_profile=True) + for result in results: + if result.validation_error: + advice_collection.add("24_2_fix_test_case_validation", str(result.validation_error)) + + profile_migrations: List[Type[ProfileMigration]] = [ ProfileMigration16_04, ProfileMigration17_09, @@ -228,12 +276,13 @@ def advise(cls, advice_collection: AdviceCollection, xml_file: str) -> None: ProfileMigration21_09, ProfileMigration23_0, ProfileMigration24_0, + ProfileMigration24_2, ] -latest_supported_version = "24.0" +latest_supported_version = "24.2" -def advise_on_upgrade(xml_file: str, to_version: Optional[str] = None) -> List[AdviceCode]: +def advise_on_upgrade(xml_file: str, to_version: Optional[str] = None) -> List[Advice]: to_version = to_version or latest_supported_version tool_source = _xml_tool_source(xml_file) initial_version = tool_source.parse_profile() diff --git a/lib/galaxy/tool_util/upgrade/script.py b/lib/galaxy/tool_util/upgrade/script.py index 2ec4b2020ab3..2778fc40c484 100755 --- a/lib/galaxy/tool_util/upgrade/script.py +++ b/lib/galaxy/tool_util/upgrade/script.py @@ -3,11 +3,14 @@ import argparse import sys from json import dumps -from textwrap import wrap +from textwrap import ( + indent, + wrap, +) from typing import List from galaxy.tool_util.upgrade import ( - AdviceCode, + Advice, advise_on_upgrade, latest_supported_version, ) @@ -52,17 +55,20 @@ def arg_parser() -> argparse.ArgumentParser: return parser -def _print_advice(advice: AdviceCode): - message = "\n".join(wrap(advice["message"], initial_indent="", subsequent_indent=" ")) - level = advice["level"] +def _print_advice(advice: Advice): + message = "\n".join(wrap(advice.advice_code_message, initial_indent="", subsequent_indent=" ")) + level = advice.level level_str = LEVEL_TO_STRING[level] - url = advice.get("url") + url = advice.url print(f"- {level_str}{message}\n") + if advice.message: + print(indent(advice.message, " ")) + print("") if url: print(f" More information at {url}") -def _print_advice_list(advice_list: List[AdviceCode]): +def _print_advice_list(advice_list: List[Advice]): for advice in advice_list: _print_advice(advice) @@ -70,9 +76,9 @@ def _print_advice_list(advice_list: List[AdviceCode]): def advise(xml_file: str, version: str, json: bool, niche: bool): advice_list = advise_on_upgrade(xml_file, version) if not niche: - advice_list = [a for a in advice_list if not a.get("niche", False)] + advice_list = [a for a in advice_list if not a.niche] if json: - print(dumps(advice_list)) + print(dumps(a.to_dict() for a in advice_list)) else: _print_advice_list(advice_list) diff --git a/lib/galaxy/tool_util/upgrade/upgrade_codes.json b/lib/galaxy/tool_util/upgrade/upgrade_codes.json index 3116a1d73305..342ca16ee0ed 100644 --- a/lib/galaxy/tool_util/upgrade/upgrade_codes.json +++ b/lib/galaxy/tool_util/upgrade/upgrade_codes.json @@ -85,5 +85,10 @@ "24_0_request_cleaning": { "level": "consider", "message": "Starting with 24.0 data source tools, Galaxy requires explicit `request_param_translation` for each parameter sent to the tool. If this tools depends on unspecified parameters - new xml elements will need to be added for these parameters." + }, + "24_2_fix_test_case_validation": { + "level": "must_fix", + "message": "Starting with 24.2 tools, test cases must validate against a more stringent schema. Unknown parameters are disallowed (prevents misspellings), select parameters must be specified by value (to prevent ambiguity and match the API), column parameters must be specified as integers, and parameters must be full qualified ('|' separation to include parent repeat, cond, and sections).", + "url": "https://github.com/galaxyproject/galaxy/pull/18679" } } diff --git a/test/unit/tool_util/upgrade/test_upgrade_advice.py b/test/unit/tool_util/upgrade/test_upgrade_advice.py index 190f6c395107..3fbd06359288 100644 --- a/test/unit/tool_util/upgrade/test_upgrade_advice.py +++ b/test/unit/tool_util/upgrade/test_upgrade_advice.py @@ -8,7 +8,7 @@ from galaxy.tool_util.unittest_utils import functional_test_tool_path from galaxy.tool_util.upgrade import ( - AdviceCode, + Advice, advise_on_upgrade, TARGET_TOO_NEW, ) @@ -146,19 +146,29 @@ def test_24_0_request_cleaning(): assert_not_has_advice(advice, "24_0_request_cleaning") +def test_24_2_test_case_validation(): + test_data_source = _tool_path("column_param.xml") + advice = advise_on_upgrade(test_data_source) + assert_has_advice(advice, "24_2_fix_test_case_validation") + + int_param = _tool_path("parameters/gx_int.xml") + advice = advise_on_upgrade(int_param) + assert_not_has_advice(advice, "24_2_fix_test_case_validation") + + def _tool_path(tool_name: str): return os.path.join(functional_test_tool_path(tool_name)) -def assert_has_advice(advice_list: List[AdviceCode], advice_code: str): +def assert_has_advice(advice_list: List[Advice], advice_code: str): for advice in advice_list: - if advice["name"] == advice_code: + if advice.advice_code["name"] == advice_code: return raise AssertionError(f"Was expecting advice {advice_code} in list of upgrade advice {advice_list}") -def assert_not_has_advice(advice_list: List[AdviceCode], advice_code: str): +def assert_not_has_advice(advice_list: List[Advice], advice_code: str): for advice in advice_list: - if advice["name"] == advice_code: + if advice.advice_code["name"] == advice_code: raise AssertionError(f"Was not expecting advice {advice_code} in list of upgrade advice {advice_list}")