Integrate tool test validation into upgrade advisor.

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()

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,27 +55,30 @@ 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)
 
 
 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)
 

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"
     }
 }

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}")