Skip to content

Commit

Permalink
Only load validating tool tests for modern tool versions.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmchilton committed Aug 10, 2024
1 parent 4eddefc commit 3a4e4b4
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 11 deletions.
2 changes: 2 additions & 0 deletions lib/galaxy/tool_util/parameters/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from .case import test_case_state
from .convert import (
decode,
encode,
Expand Down Expand Up @@ -104,6 +105,7 @@
"TestCaseToolState",
"ToolParameterT",
"to_json_schema_string",
"test_case_state",
"RequestToolState",
"RequestInternalToolState",
"flat_state_path",
Expand Down
5 changes: 4 additions & 1 deletion lib/galaxy/tool_util/parameters/case.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
Set,
)

from packaging.version import Version

from galaxy.tool_util.parser.interface import (
ToolSourceTest,
ToolSourceTestInput,
Expand Down Expand Up @@ -76,7 +78,8 @@ def legacy_from_string(parameter: ToolParameterT, value: str, warnings: List[str
f"Implicitly converted {parameter.name} to a column index integer from a string value, please use 'value_json' to define this test input parameter value instead."
)
result_value = int(value)
else:
elif Version(profile) < Version("24.2"):
# allow this for older tools but new tools will just require the integer index
warnings.append(
f"Using column names as test case values is deprecated, please adjust {parameter.name} to just use an integer column index."
)
Expand Down
10 changes: 10 additions & 0 deletions lib/galaxy/tool_util/verify/parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
ToolTestDescription,
ValidToolTestDict,
)
from galaxy.tool_util.parameters import (
input_models_for_tool_source,
test_case_state as case_state,
)
from galaxy.util import (
string_as_bool,
string_as_bool_or_none,
Expand All @@ -52,9 +56,15 @@ def parse_tool_test_descriptions(
"""
Build ToolTestDescription objects for each test description.
"""
validate_on_load = Version(tool_source.parse_profile()) >= Version("24.2")
raw_tests_dict: ToolSourceTests = tool_source.parse_tests_to_dict()
tests: List[ToolTestDescription] = []
tool_parameter_bundle = input_models_for_tool_source(tool_source)
profile = tool_source.parse_profile()
for i, raw_test_dict in enumerate(raw_tests_dict.get("tests", [])):
if validate_on_load:
test_case_state_and_warnings = case_state(raw_test_dict, tool_parameter_bundle.input_models, profile)
# todo catch and append InvalidToolTestDict on validation failure
test = _description_from_tool_source(tool_source, raw_test_dict, i, tool_guid)
tests.append(test)
return tests
Expand Down
41 changes: 32 additions & 9 deletions test/unit/tool_util/test_parameter_test_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from galaxy.tool_util.parser.factory import get_tool_source
from galaxy.tool_util.unittest_utils import functional_test_tool_directory
from galaxy.tool_util.models import parse_tool
from galaxy.tool_util.verify.parse import parse_tool_test_descriptions


# legacy tools allows specifying parameter and repeat parameters without
Expand Down Expand Up @@ -58,15 +59,37 @@ def test_parameter_test_cases_validate():
assert len(warnings[1]) == 1


VALIDATING_TOOL_NAMES = [
"checksum.xml",
"all_output_types.xml",
"cheetah_casting.xml",
"collection_creates_dynamic_list_of_pairs.xml",
"collection_creates_dynamic_nested.xml",
"collection_mixed_param.xml",
"collection_type_source.xml",
]
def test_legacy_features_fail_validation_with_24_2(tmp_path):
for filename in TOOLS_THAT_USE_UNQUALIFIED_PARAMETER_ACCESS + TOOLS_THAT_USE_TRUE_FALSE_VALUE_BOOLEAN_SPECIFICATION:
_assert_tool_test_parsing_only_fails_with_newer_profile(tmp_path, filename)

# column parameters need to be indexes
_assert_tool_test_parsing_only_fails_with_newer_profile(tmp_path, "column_param.xml")

# selection by value only
_assert_tool_test_parsing_only_fails_with_newer_profile(tmp_path, "multi_select.xml")


def _assert_tool_test_parsing_only_fails_with_newer_profile(tmp_path, filename: str):
test_tool_directory = functional_test_tool_directory()
original_path = os.path.join(test_tool_directory, filename)
new_path = tmp_path / filename
with open(original_path, "r") as rf:
tool_contents = rf.read()
import re
tool_contents = re.sub(f"profile=\".*\"", r"", tool_contents)
print(tool_contents)
new_profile_contents = tool_contents.replace("<tool ", "<tool profile=\"24.2\" ", 1)
with open(new_path, "w") as wf:
wf.write(new_profile_contents)
parse_tool_test_descriptions(get_tool_source(original_path))
exception = None
try:
parse_tool_test_descriptions(get_tool_source(new_path))
except Exception as e:
print(e)
exception = e
assert exception, f"expected {filename} to have validation failure preventing loading of tools"


def test_validate_framework_test_tools():
Expand Down
1 change: 0 additions & 1 deletion test/unit/tool_util/test_test_definition_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def test_unqualified_access_disabled_in_24_2(self):
self._init_tool_for_path(functional_test_tool_path("deprecated/simple_constructs_24_2.xml"))
test_dicts = self._parse_tests()
test_0 = test_dicts[0].to_dict()
assert "p1|p1use" not in test_0["inputs"]

def test_bigwigtowig_converter(self):
# defines
Expand Down

0 comments on commit 3a4e4b4

Please sign in to comment.