Skip to content

Report tool-model / test-parse failures in test-case validation; fix Infinity in_range bound#23004

Open
richard-burhans wants to merge 1 commit into
galaxyproject:devfrom
richard-burhans:fix/test-case-validator-model-build-crashes
Open

Report tool-model / test-parse failures in test-case validation; fix Infinity in_range bound#23004
richard-burhans wants to merge 1 commit into
galaxyproject:devfrom
richard-burhans:fix/test-case-validator-model-build-crashes

Conversation

@richard-burhans

Copy link
Copy Markdown
Member

Fixes #23003.

validate_test_cases_for_tool_source() builds the parameter model (input_models_for_tool_source) and parses the test cases (parse_tests_to_dict) before its per-test loop, outside any try/except. A tool whose model cannot be built (e.g. an unknown parameter type) or whose <test> block is malformed therefore raised an unhandled exception out of the whole call, so a caller validating tools it does not control (the 24.2 upgrade advisor, planemo / lint, CI) aborted instead of reporting the tool.

This change has two parts:

  1. Report, don't raise (holistic). Wrap the model build + test parsing so a failure becomes a single reported TestCaseStateValidationResult with the error, rather than escaping. This addresses the model-construction / test-parsing layer — 31 tools across the public Galaxy tool corpus.

  2. _parse_number Infinity fix (targeted). An in_range validator bound written Infinity (or Inf, or scientific notation like 1E5) fell through the case-sensitive float check ("inf" in raw_value) to int(), which raised, even though float() parses it. The check is now case-insensitive, so such a tool validates instead of crashing.

This complements #23002, which fixes the same "report, don't raise" gap one layer in (per-test state building). With both, the validator goes from 159 crashing corpus tools to 15; the remaining 15 fail earlier in get_tool_source (malformed XML, or a macro file missing from the checkout), before this function is reached.

How to test the changes?

test/unit/tool_util/test_parameter_test_cases.py:

  • test_unmodelable_tool_is_reported_not_raised — a tool with an unknown parameter type is reported (validation_error), not raised.
  • test_infinity_in_range_validator_bound_does_not_crash — a tool with an in_range max="Infinity" validates cleanly.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

…Infinity bound

validate_test_cases_for_tool_source() builds the parameter model and parses the
test cases before its per-test loop, outside any try/except. A tool whose model
cannot be built (e.g. an unknown parameter type) or whose <test> block is
malformed therefore raised an unhandled exception out of the whole call, so a
caller validating tools it does not control (the 24.2 upgrade advisor,
planemo / lint, CI) aborted instead of reporting the tool.

Wrap the model build + test parsing so a failure becomes a single reported
validation result. This addresses the model-construction / test-parsing layer
(31 tools across the public Galaxy tool corpus); together with the per-test
state-building fix it takes the validator from 159 crashing tools to 15, and the
remaining 15 fail earlier in get_tool_source (malformed XML, or a macro file
missing from the checkout) before this function is reached.

Also fix _parse_number: an in_range validator bound written 'Infinity' (or 'Inf',
or scientific notation like '1E5') fell through the case-sensitive float check to
int(), which raised, even though float() parses it. Match those forms
case-insensitively so such a tool validates instead of crashing.

Add regression tests for an unmodelable tool and an Infinity in_range bound.
@richard-burhans

Copy link
Copy Markdown
Member Author

CI has settled with five failing jobs. All are in subsystems this PR does not touch — the change is confined to galaxy.tool_util.parameters.case and galaxy.tool_util.parser.parameter_validators — and this PR's own tests (tests/tool_util/test_parameter_test_cases.py::test_unmodelable_tool_is_reported_not_raised and …::test_infinity_in_range_validator_bound_does_not_crash) pass (they show green in the Test (3.8) packages run).

  • Test Galaxy packages (3.10 and 3.14): the job logs are no longer retrievable via the API (the check annotation reports only Process completed with exit code 1), but on the sibling PR Report test-case state-building errors instead of raising #23002 the identical Test Galaxy packages jobs fail at collection while importing fastmcp for packages/web_apps/tests/webapps/api/test_mcp.pyModuleNotFoundError: No module named 'uncalled_for' (a missing transitive dependency of the fastmcp version in the web_apps test environment). This PR's tool_util package tests pass.
  • Test (3.8): one failure, tests/tool_util/mulled/test_mulled_build.py::test_base_image_for_targets[qiime-1.9.1-…], preceded by CondaHTTPError: HTTP 000 CONNECTION FAILED reaching conda-forge — a network flake in a container-image-resolution test (1 failed, 1244 passed).
  • API tests (3.10, 1): two failures — lib/galaxy_test/api/test_tools.py::TestToolsApi::test_allow_uri_if_protocol_on_collection_with_deferred and lib/galaxy_test/api/test_workflows.py::TestWorkflowsApi::test_workflow_with_deleted_dataset_step_parameter (tools / workflow API tests).
  • Integration Selenium (3.10): test/integration_selenium/test_trs_import.py::TestTrsImport::test_auto_import_by_trs_url_workflowhubAssertionError: workflow … not imported, a TRS import from the external WorkflowHub (network-dependent).

None exercise tool test-case validation or validator-bound parsing. Happy to rebase once the fastmcp / uncalled_for issue on dev is resolved, if that helps CI go green.

@mvdbeek
mvdbeek requested a review from guerler July 7, 2026 14:35
try:
tool_parameter_bundle = input_models_for_tool_source(tool_source)
test_cases: List[ToolSourceTest] = tool_source.parse_tests_to_dict()["tests"]
except Exception as e:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is minor, but could you narrow this exception to ParameterDefinitionError and ValueError?

@guerler guerler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Please rebase to resolve conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

Tool-model construction and test parsing failures crash test-case validation instead of being reported

2 participants