galaxy
galaxy copied to clipboard
galaxyproject
tool verification: move no output assertion
from planemo the verify_tool function is called from within an try-except block which silently catches any exception.
Thus any exception raised from within verify_tool will not be detected, i.e. the assertion needs to be moved into _verify_outputs (which also seems to make sense by name) in order to make verify_tool record the problem properly.
This only affects tool tests. Guess we do not want to actually forbid tool runs with no outputs.. Even if I don't know what they are used for.
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:
- [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]
License
- [x] I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.
A minor remark on the PR title: In general it is a good idea for the title to document what the PR does, instead of how you did something (which is pretty clear from the code anyway). So in this case I'd say something like "Make sure that missing outputs are recorded as test errors" helps a lot, both for the docs and the reviewer.
Guess I should backport this to 24.0 since this suddenly became a bug fix :)
Fixes https://github.com/galaxyproject/galaxy/issues/17829
@bernt-matthias I'm merging this into dev - I don't think it's a bug (as per discussion above)? I've also edited the title - please feel free to change it if it's not a good summary of the PR. As per this comment, we'd have to manually edit this for the 24.1 release notes. EDIT: i'm rerunning the failed tests first as a sanity check.
Not sure how to retarget without adding loads of commits?
You should be able to edit the target: click the 'edit' button at the top of the page - it'll let you edit the title AND the base branch: you'll see a drop down list. Select '24.0 release' - it'll ask you to confirm and that's it. There may or may not be conflicts - is that what you were referring to?
If I do this I get 215 commits.
Oh no! Would you like to do #17873 instead of this (adding the extra commit to it)? i.e., merge it into 24.0 and then, if there are conflicts, manually forward-merge it into dev?
All this is now in https://github.com/galaxyproject/galaxy/pull/17874 (had to create a new PR since I force pushed before reopening .. and apparently one cannot reopen in this case).