Report test-case state-building errors instead of raising#23002
Report test-case state-building errors instead of raising#23002richard-burhans wants to merge 1 commit into
Conversation
validate_test_cases_for_tool_source() is meant to return a per-test validation result, but only model validation and the unhandled-name check ran inside the try/except that records validation_error. Building the test-case state (test_case_state) ran before it, so any failure there -- an unresolvable conditional 'when' value, a non-numeric integer/float value, a malformed repeat -- raised an unhandled exception out of the whole call. A caller validating tools it does not control (the 24.2 upgrade advisor, planemo/lint, CI) then aborts instead of reporting the test case as invalid. Move test_case_state() inside the try so a state-building failure is captured as that test's validation_error, exactly like the existing 'Invalid parameter name found' check; fall back to an empty tool state for the result when state-building fails. Across the public Galaxy tool corpus this converts 113 of 159 tools that previously crashed the validator into reported validation errors (the remaining 46 fail earlier, during model construction or XML parsing, and are out of scope here). Add a regression test covering a non-numeric numeric value and an unresolvable conditional test value.
b80375e to
786712d
Compare
richard-burhans
changed the title
|
CI has now fully settled with five failing jobs. All are in subsystems this PR does not touch — the change here is confined to 1 & 2 — A missing transitive dependency ( 3 — 4 — 5 — None of these exercise tool test-case validation: #1–#2 are a |
|
Will you please merge richard-burhans#1 into this work? I think there was some concern about the broad Exception hiding actual parsing errors and I think that PR addresses some if not all of those. |
|
Putting this in draft while waiting for comments to be addressed. |
3 participants
Fixes #23001.
validate_test_cases_for_tool_source()is meant to return a per-test validation result, but intest_case_validation()only the model validation and the unhandled-parameter-name check ran inside thetry/exceptthat recordsvalidation_error. Building the test-case state (test_case_state()) ran before it, so any failure there escaped the whole call:whenbranch (Invalid conditional test value ...),integer/floatparameter (legacy_from_stringint()/float()),A caller validating tools it does not control — the 24.2 upgrade advisor, planemo / lint, CI — then aborts instead of reporting the test case as invalid.
This moves
test_case_state()inside thetry, so a state-building failure is captured as that test'svalidation_error, exactly like the existingtool_state.validate(...)failure and the "Invalid parameter name found" check. When state building fails the result falls back to an empty tool state.Across the public Galaxy tool corpus this converts 113 of 159 tools that previously crashed the validator into reported validation errors. The remaining 46 fail earlier (during parameter-model construction or XML parsing) and are out of scope for this change.
How to test the changes?
test/unit/tool_util/test_parameter_test_cases.py::test_malformed_test_case_is_reported_not_raisedbuilds tools with (a) a non-numeric value for an integer/float parameter and (b) a conditional test value that selects nowhen, and asserts the validator returns avalidation_errorfor each instead of raising.License