-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix test syntax issues. #530
base: main
Are you sure you want to change the base?
Conversation
Thanks John very useful. I will change the workflow I am responsible of. |
He did it in #317 but unfortunately few have integrated the changes.... |
If it is ok for you. We can split this PR into 2. |
Is there a way to integrate this into planemo lint? |
It will make its way there - I'm going to open the PR to Galaxy's tool_util package today that is the first step. |
Thanks. |
I put here the output of the tests: Test Summary
|
Here are the failed tests one by one:
|
|
|
|
|
|
For
|
@@ -23,8 +23,8 @@ | |||
bin_size: 50 | |||
outputs: | |||
average_bigwigs: | |||
class: Collection | |||
collection_type: list | |||
attributes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the model behind this ? attributes seems kind of odd, shouldn't this nest directly under the output ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should and I will implement that in follow ups but that is an enhancement - this version of the models just reflects what actually works with current planemo. I don't think anyone ever thought this through and so it is a pretty low-level mapping to the XML parsing outputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't ask my why I put this test... I cannot remember.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I think I am the only one to use this type of test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests passing have been integrated into #531 .
|
asserts: | ||
has_text: | ||
text: "SuCOS_Score" | ||
n: 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will only test the last one, do you have a check for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's really checkable as that's a yaml language issue, we don't go that deep.
has_n_columns: | ||
n: 8 | ||
tooldistillator_summarize: | ||
assert: | ||
asserts: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
below there are multiple 'has_text' and only the last one will be tested, right?
asserts: | ||
has_line: | ||
line: "$$$$" | ||
n: 4 | ||
asserts: | ||
has_text: | ||
text: "SuCOS_Score" | ||
n: 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asserts: | |
has_line: | |
line: "$$$$" | |
n: 4 | |
asserts: | |
has_text: | |
text: "SuCOS_Score" | |
n: 4 | |
asserts: | |
has_line: | |
line: "$$$$" | |
n: 4 | |
has_text: | |
text: "SuCOS_Score" | |
n: 4 |
@jmchilton or @mvdbeek would you mind to rebase with main and check if there are other issues to solve, please? |
I guess @mvdbeek already did this with an alternative way of automatically verifying the syntax with https://github.com/galaxyproject/iwc/pull/317/files. This is what it took to get some pydantic models I'm working on to pass - though I still am skipping a few assembly workflows that use a syntax for checking output parameter values that I need to figure out how to model.