Skip to content

Replace if_needed with never path in tool testing framework - #23442

Open
guerler wants to merge 4 commits into
galaxyproject:devfrom
guerler:framework_tests_typed_path.000
Open

Replace if_needed with never path in tool testing framework#23442
guerler wants to merge 4 commits into
galaxyproject:devfrom
guerler:framework_tests_typed_path.000

Conversation

@guerler

@guerler guerler commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Requires #23431. Replaces if_needed with never path for tool testing.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

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

@guerler guerler added this to the 26.2 milestone Sep 3, 2026
@guerler
guerler requested a review from jmchilton September 3, 2026 13:25
@guerler guerler changed the title [WIP] Replace if_needed with never path in tool testing framework Replace if_needed with never path in tool testing framework Sep 3, 2026
@guerler
guerler marked this pull request as ready for review September 3, 2026 18:38
@jmchilton

Copy link
Copy Markdown
Member

My agent recommend migrated the excluded tools - but that solves nothing in terms of legacy tools already out there. We auto-promote some other legacy patterns I think - these seem doable also - so I convinced Codex it was a good idea. Let me know if we're wrong. -John

From Codex:

I do not think these five tools are inherently legacy-API-only. For old-profile XML tests, bare repeat-child parameters are supported syntax, and _process_raw_inputs / ParamContext already resolves them against the tool input tree into indexed paths such as queries_0|input2 before legacy submission.

Could we reuse that normalization while loading the test case, before calling case_state(), and build the typed request from the canonical indexed inputs? The normalization should preserve each original input dictionary and its attributes while replacing the matched legacy name with ParamContext.for_state(). That would let these tests run in the never lane without migrating their XML or skipping them.

I would avoid restoring the removed synthesis in _repeat_inputs_to_array verbatim: it independently guessed repeat membership and could diverge from the legacy loader for duplicate names, sibling repeats, and nesting. Sharing the existing consumptive, tool-tree-guided normalization keeps the modern and legacy APIs compatible.

@guerler
guerler force-pushed the framework_tests_typed_path.000 branch 2 times, most recently from 22a000b to 9e9ec4b Compare September 4, 2026 13:09
@guerler
guerler force-pushed the framework_tests_typed_path.000 branch from 9e9ec4b to 512775b Compare September 5, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants