diff --git a/lib/galaxy/tool_util/parameters/case.py b/lib/galaxy/tool_util/parameters/case.py index 45d4a83308e..f8d541c25a2 100644 --- a/lib/galaxy/tool_util/parameters/case.py +++ b/lib/galaxy/tool_util/parameters/case.py @@ -324,15 +324,16 @@ def test_case_state( for test_input in inputs: input_name = test_input["name"] - if input_name not in handled_inputs: + if input_name not in handled_inputs and not _input_name_was_handled_by_legacy_fallback( + input_name, handled_inputs, profile + ): unhandled_inputs.append(input_name) tool_state = TestCaseToolState(state) if validate: tool_state.validate(tool_parameter_bundle, name=name) for input_name in unhandled_inputs: - if not _input_name_was_handled_by_legacy_fallback(input_name, handled_inputs, profile): - raise Exception(f"Invalid parameter name found {input_name}") + raise Exception(f"Invalid parameter name found {input_name}") return TestCaseStateAndWarnings(tool_state, warnings, unhandled_inputs) @@ -431,7 +432,8 @@ def _merge_into_state( if input_name not in state_at_level: state_at_level[input_name] = repeat_state_array - repeat_instance_inputs = _repeat_inputs_to_array(state_path, tool_input.parameters, context.inputs) + repeat_instance_inputs = _repeat_inputs_to_array(state_path, context.inputs) + supplied_instances = len(repeat_instance_inputs) if tool_input.min is not None: while len(repeat_instance_inputs) < tool_input.min: repeat_instance_inputs.append([]) @@ -440,14 +442,18 @@ def _merge_into_state( repeat_state_array.append({}) repeat_instance_prefix = f"{state_path}_{i}" - handled_inputs.update( - _merge_level_into_state( - tool_input.parameters, - context.for_inputs(repeat_instance_inputs[i]), - repeat_state_array[i], - repeat_instance_prefix, - ) + instance_handled_inputs = _merge_level_into_state( + tool_input.parameters, + context.for_inputs(repeat_instance_inputs[i]), + repeat_state_array[i], + repeat_instance_prefix, ) + # Instances past the ones the test actually supplied exist only to satisfy min. + # Nothing resolved into them, so their paths must not count as handling a raw + # input - the legacy suffix match would otherwise read queries_0|input2 as + # covering a bare input2 that was in fact dropped. + if i < supplied_instances: + handled_inputs.update(instance_handled_inputs) elif isinstance(tool_input, (SectionParameterModel,)): section_state = state_at_level.get(input_name, {}) if input_name not in state_at_level: @@ -504,9 +510,7 @@ def _merge_into_state( return handled_inputs -def _repeat_inputs_to_array( - state_path: str, parameters: list[ToolParameterT], inputs: ToolSourceTestInputs -) -> list[ToolSourceTestInputs]: +def _repeat_inputs_to_array(state_path: str, inputs: ToolSourceTestInputs) -> list[ToolSourceTestInputs]: inputs_as_dict = _inputs_as_dict(inputs) repeat_instance_input_dicts = repeat_inputs_to_array(state_path, inputs_as_dict) if not repeat_instance_input_dicts and "|" in state_path: @@ -520,21 +524,7 @@ def _repeat_inputs_to_array( repeat_instance_input_dicts = repeat_inputs_to_array(candidate_path, inputs_as_dict) if repeat_instance_input_dicts: break - repeat_instance_inputs = [list(instance_inputs.values()) for instance_inputs in repeat_instance_input_dicts] - if repeat_instance_inputs: - return repeat_instance_inputs - - legacy_repeat_inputs: list[ToolSourceTestInputs] = [] - for parameter in parameters: - parameter_name = parameter.name - matching_inputs = [input for input in inputs if input["name"] == parameter_name] - for i, input in enumerate(matching_inputs): - while len(legacy_repeat_inputs) <= i: - legacy_repeat_inputs.append([]) - synthetic_input = cast(ToolSourceTestInput, dict(input)) - synthetic_input["name"] = f"{state_path}_{i}|{parameter_name}" - legacy_repeat_inputs[i].append(synthetic_input) - return legacy_repeat_inputs + return [list(instance_inputs.values()) for instance_inputs in repeat_instance_input_dicts] def _select_which_when( diff --git a/lib/galaxy/tool_util/verify/parse.py b/lib/galaxy/tool_util/verify/parse.py index 43f2a7637c1..71990b05043 100644 --- a/lib/galaxy/tool_util/verify/parse.py +++ b/lib/galaxy/tool_util/verify/parse.py @@ -80,10 +80,17 @@ def parse_tool_test_descriptions( tool_parameter_bundle = input_models_for_tool_source(tool_source) parameters = tool_parameter_bundle.parameters validated_test_case = case_state(raw_test_dict, parameters, profile, validate=validate_on_load) - request_and_schema = TestRequestAndSchema( - validated_test_case.tool_state, - tool_parameter_bundle or ToolParameterBundleModel(parameters=parameters), - ) + if validated_test_case.unhandled_inputs: + # Inputs that map to no parameter (e.g. legacy unqualified repeats) can't be + # represented in a modern request; fall back to the legacy API. + validation_skipped_reason = ( + f"could not build request: unhandled inputs {validated_test_case.unhandled_inputs}" + ) + else: + request_and_schema = TestRequestAndSchema( + validated_test_case.tool_state, + tool_parameter_bundle or ToolParameterBundleModel(parameters=parameters), + ) except Exception as e: if validate_on_load: validation_exception = e diff --git a/test/functional/tools/async_min_repeat_unqualified.xml b/test/functional/tools/async_min_repeat_unqualified.xml new file mode 100644 index 00000000000..819218c1a87 --- /dev/null +++ b/test/functional/tools/async_min_repeat_unqualified.xml @@ -0,0 +1,38 @@ + + async regression: unqualified repeat param on a repeat with min set + + '$out_file1' + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/async_repeat_unqualified_no_min.xml b/test/functional/tools/async_repeat_unqualified_no_min.xml new file mode 100644 index 00000000000..8419cb7bc3c --- /dev/null +++ b/test/functional/tools/async_repeat_unqualified_no_min.xml @@ -0,0 +1,32 @@ + + async control: unqualified repeat param, no min on the repeat + + '$out_file1' + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/sample_tool_conf.xml b/test/functional/tools/sample_tool_conf.xml index 15bec683601..1aeae2e0263 100644 --- a/test/functional/tools/sample_tool_conf.xml +++ b/test/functional/tools/sample_tool_conf.xml @@ -125,6 +125,8 @@ + + diff --git a/test/unit/tool_util/test_parameter_test_cases.py b/test/unit/tool_util/test_parameter_test_cases.py index f9eb1152432..db40bf9ba36 100644 --- a/test/unit/tool_util/test_parameter_test_cases.py +++ b/test/unit/tool_util/test_parameter_test_cases.py @@ -47,6 +47,8 @@ TOOLS_THAT_USE_UNQUALIFIED_PARAMETER_ACCESS = [ "disambiguate_cond.xml", "multi_repeats.xml", "implicit_default_conds.xml", + "async_min_repeat_unqualified.xml", + "async_repeat_unqualified_no_min.xml", ] TOOLS_THAT_USE_SELECT_BY_VALUE = [ @@ -420,19 +422,22 @@ def test_legacy_partial_conditional_paths_are_resolved_for_request_state(): dict_verify_each(tool_state.input_state, expectations) -def test_legacy_unqualified_repeat_inputs_are_expanded_for_request_state(): +def test_legacy_unqualified_repeat_inputs_are_not_expanded(): + # Unqualified repeat params (the multi_repeats anti-pattern) are no longer synthesized into + # repeat instances; use explicit tags. The bare param is rejected on validation. tool_source = tool_source_for("multi_repeats") test_cases = tool_source.parse_tests_to_dict()["tests"] + with pytest.raises(Exception, match="Invalid parameter name found input2"): + case_state_for(tool_source, test_cases[2]) - test_case_state = case_state_for(tool_source, test_cases[2]).tool_state - expectations = [ - (["queries", 0, "input2", "path"], "simple_line.txt"), - (["queries", 1, "input2", "path"], "simple_line.txt"), - (["more_queries", 0, "more_queries_input", "path"], "simple_line.txt"), - (["more_queries", 1, "more_queries_input", "path"], "simple_line.txt"), - ] - dict_verify_each(test_case_state.input_state, expectations) +def test_unrepresentable_test_case_falls_back_to_legacy_request(): + # A test whose inputs can't be represented in a modern request (unqualified repeats) yields no + # request, so the interactor falls back to the legacy tool API rather than erroring. + tests = list(parse_tool_test_descriptions(tool_source_for("multi_repeats"))) + description = tests[2].to_dict() + assert description["error"] is False + assert description["request"] is None def test_legacy_unqualified_repeat_inside_conditional_is_resolved():