Add framework test for profile behavior of format="input"
#15482
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
|
Is making that a linting error enough ? |
Yes, lets do this. |
bernt-matthias
force-pushed
the
test/profile-format-input
branch
from
001200a
to
e2ec896
Compare
bernt-matthias
force-pushed
the
test/profile-format-input
branch
2 times, most recently
from
e17310b
to
0b05080
Compare
|
There's a conflict here, otherwise this looks fine to me. |
mvdbeek
approved these changes
Jun 21, 2023
bernt-matthias
force-pushed
the
test/profile-format-input
branch
from
1fa6ab4
to
0b05080
Compare
bernt-matthias
force-pushed
the
test/profile-format-input
branch
from
bbc14e2
to
cf3cac2
Compare
stumbled over this here: galaxyproteomics/tools-galaxyp#679 could not find a test
|
Good to go 😁 |
mvdbeek
reviewed
Jul 31, 2023
| ) | ||
| if profile <= "16.01": | ||
| lint_ctx.warn( | ||
| f"Using format='input' on {node.tag}: format_source attribute is less ambiguous and should be used instead (with a non-legacy tool profile).", |
There was a problem hiding this comment.
Why not use the same message ? It'll work with or without profile version, and that's another linter.
There was a problem hiding this comment.
You mean: I should use this warning irrespective of the profile?
There was a problem hiding this comment.
But use the one from below. No need to refer to the profile version, that is handled by another linter.
bernt-matthias
force-pushed
the
test/profile-format-input
branch
from
62f3138
to
0d7d7f3
Compare
mvdbeek
reviewed
Jul 31, 2023
bernt-matthias
force-pushed
the
test/profile-format-input
branch
from
0d7d7f3
to
f2fdb2c
Compare
irrespective of the profile
mvdbeek
reviewed
Jul 31, 2023
mvdbeek
approved these changes
Jul 31, 2023
mvdbeek
reviewed
Jul 31, 2023
bernt-matthias
force-pushed
the
test/profile-format-input
branch
from
51b5943
to
c563387
Compare
mvdbeek
approved these changes
Jul 31, 2023
|
This PR was merged without a "kind/" label, please correct. |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
stumbled over this here: galaxyproteomics/tools-galaxyp#679 could not find a test.
Wondering if we can/should be a bit more strict .. in the sense that some of the tool testing / linting would lead to an error that is easily recognized, e.g. by adding a linter rule or fail the tests if a unknown format is used (
"input"in this case)In the original implementation of the profile behavior (b92074e) it is stated "Disable format="input" - require explicit metadata targets (metadata_source, format_source).". Currently "disable" means that just the format is set to the invalid datatype "input". Wondering if we want to change this to really disable this, e.g. by just failing tool execution.
How to test the changes?
(Select all options that apply)
License