Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for optional select with default and fix xsd docs #13565

Draft
wants to merge 13 commits into
base: dev
Choose a base branch
from

Conversation

bernt-matthias
Copy link
Contributor

@bernt-matthias bernt-matthias commented Mar 17, 2022

Add test for optional select with default testing if it can be unselected in a test

was just wondering if this works and was happy to see that is does. Maybe we should document that omitting value in TestParam has this effect? And fix documentation.

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 contributions under Galaxy's current license.
  • I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

@github-actions github-actions bot added this to the 22.05 milestone Mar 17, 2022
@bernt-matthias bernt-matthias changed the title Add test for optional select with default Add test for optional select with default and fix xsd docs Mar 17, 2022
@bernt-matthias bernt-matthias force-pushed the topic/test-optional-select-with-default branch 3 times, most recently from 4e294db to 1e3dae0 Compare March 24, 2022 11:05
@bernt-matthias bernt-matthias force-pushed the topic/test-optional-select-with-default branch from 7bb6417 to b750fab Compare March 24, 2022 16:17
@bernt-matthias
Copy link
Contributor Author

bernt-matthias commented Mar 25, 2022

With the new test I run into

Traceback (most recent call last):
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/web/framework/decorators.py", line 336, in decorator
    rval = func(self, trans, *args, **kwargs)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/api/tools.py", line 590, in create
    return self._create(trans, payload, **kwd)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/api/tools.py", line 666, in _create
    trans, incoming, history=target_history, use_cached_job=use_cached_job, input_format=input_format
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/tools/__init__.py", line 1787, in handle_input
    trans=trans, incoming=incoming, request_context=request_context, input_format=input_format
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/tools/__init__.py", line 1764, in expand_incoming
    input_format=input_format,
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/tools/parameters/__init__.py", line 412, in populate_state
    simple_errors=simple_errors,
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/tools/parameters/__init__.py", line 616, in _populate_state_legacy
    if check
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/tools/parameters/__init__.py", line 240, in check_param
    value = param.from_json(value, trans, param_values)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/tools/parameters/basic.py", line 1012, in from_json
    if value in legal_values:
TypeError: unhashable type: 'dict'

Added a "bit" of logging code which revealed that an upload with empty name overwrites the content of the select with value="". Here an excerpt of the logs:

run_tool self.uploads {... '': {'src': 'hda', 'id': 'dd93bb8edc42a50e'}, ...}
...
expand_incoming expanded_incomings [{'select_opt_wdefault': {'src': 'hda', 'id': 'dd93bb8edc42a50e'}}]

Remembered that we also had this here #9885 and @mvdbeek suggested this as fix 94588be . So I cherry picked this over to here.

@bernt-matthias bernt-matthias force-pushed the topic/test-optional-select-with-default branch from 97f4011 to ed4e250 Compare April 20, 2022 16:59
testing if it can be unselected in a test
and add test for the formerly wrong docs
by using quotes (otherwise space gets lost)
@bernt-matthias bernt-matthias force-pushed the topic/test-optional-select-with-default branch from 1f373ab to 74d9354 Compare April 21, 2022 08:37
and keep processed composite file inputs.
@mvdbeek mvdbeek removed this from the 22.05 milestone May 25, 2022
@nsoranzo nsoranzo marked this pull request as draft September 16, 2022 00:04
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.

3 participants